simplify gating logic

This commit is contained in:
Brad Rydzewski 2017-05-05 20:05:42 +02:00
parent 4aac0bc4d6
commit 3a64aa4cf2
15 changed files with 168 additions and 105 deletions

View file

@ -400,7 +400,7 @@ func setupEvilGlobals(c *cli.Context, v store.Store) {
droneserver.Config.Services.Pubsub.Create(context.Background(), "topic/events")
droneserver.Config.Services.Registries = registry.New(v)
droneserver.Config.Services.Secrets = secrets.New(v)
droneserver.Config.Services.Senders = sender.New(v)
droneserver.Config.Services.Senders = sender.New(v, v)
if endpoint := c.String("registry-service"); endpoint != "" {
droneserver.Config.Services.Registries = registry.NewRemote(endpoint)
}

View file

@ -4,8 +4,8 @@ package model
type ConfigStore interface {
ConfigLoad(int64) (*Config, error)
ConfigFind(*Repo, string) (*Config, error)
ConfigUpdate(*Config) error
ConfigInsert(*Config) error
ConfigFindApproved(*Config) (bool, error)
ConfigCreate(*Config) error
}
// Config represents a pipeline configuration.
@ -14,5 +14,4 @@ type Config struct {
RepoID int64 `json:"-" meddler:"config_repo_id"`
Data string `json:"data" meddler:"config_data"`
Hash string `json:"hash" meddler:"config_hash"`
Approved bool `json:"approved" meddler:"config_approved"`
}

View file

@ -6,15 +6,23 @@ import (
type builtin struct {
store model.SenderStore
conf model.ConfigStore
}
// New returns a new local gating service.
func New(store model.SenderStore) model.SenderService {
return &builtin{store}
func New(store model.SenderStore, conf model.ConfigStore) model.SenderService {
return &builtin{store, conf}
}
func (b *builtin) SenderAllowed(user *model.User, repo *model.Repo, build *model.Build, conf *model.Config) (bool, error) {
if !conf.Approved {
if build.Event == model.EventPull && build.Sender != user.Login {
// check to see if the configuration has already been used in an
// existing build. If yes it is considered approved.
if ok, _ := b.conf.ConfigFindApproved(conf); ok {
return true, nil
}
// else check to see if the configuration is sent from a user
// account that is a repositroy approver themselves.
sender, err := b.store.SenderFind(repo, build.Sender)
if err != nil || sender.Block {
return false, nil

View file

@ -180,10 +180,6 @@ func PostApproval(c *gin.Context) {
c.AbortWithError(404, err)
return
}
if !conf.Approved {
conf.Approved = true
Config.Storage.Config.ConfigUpdate(conf)
}
netrc, err := remote_.Netrc(user, repo)
if err != nil {
@ -404,10 +400,6 @@ func PostBuild(c *gin.Context) {
c.AbortWithError(404, err)
return
}
if !conf.Approved {
conf.Approved = true
Config.Storage.Config.ConfigUpdate(conf)
}
netrc, err := remote_.Netrc(user, repo)
if err != nil {

View file

@ -145,24 +145,14 @@ func PostHook(c *gin.Context) {
RepoID: repo.ID,
Data: string(confb),
Hash: sha,
Approved: false,
}
if user.Login == repo.Owner || build.Event != model.EventPull || repo.IsGated == false {
conf.Approved = true
}
err = Config.Storage.Config.ConfigInsert(conf)
err = Config.Storage.Config.ConfigCreate(conf)
if err != nil {
logrus.Errorf("failure to persist config for %s. %s", repo.FullName, err)
c.AbortWithError(500, err)
return
}
}
if !conf.Approved {
if user.Login == repo.Owner || build.Event != model.EventPull || repo.IsGated == false {
conf.Approved = true
Config.Storage.Config.ConfigUpdate(conf)
}
}
build.ConfigID = conf.ID
netrc, err := remote_.Netrc(user, repo)

View file

@ -1,13 +1,15 @@
package datastore
import (
gosql "database/sql"
"github.com/drone/drone/model"
"github.com/drone/drone/store/datastore/sql"
"github.com/russross/meddler"
)
func (db *datastore) ConfigLoad(id int64) (*model.Config, error) {
stmt := sql.Lookup(db.driver, "config-find-repo-id")
stmt := sql.Lookup(db.driver, "config-find-id")
conf := new(model.Config)
err := meddler.QueryRow(db, conf, stmt, id)
return conf, err
@ -20,10 +22,18 @@ func (db *datastore) ConfigFind(repo *model.Repo, hash string) (*model.Config, e
return conf, err
}
func (db *datastore) ConfigUpdate(config *model.Config) error {
return meddler.Update(db, "config", config)
func (db *datastore) ConfigFindApproved(config *model.Config) (bool, error) {
var dest int64
stmt := sql.Lookup(db.driver, "config-find-approved")
err := db.DB.QueryRow(stmt, config.RepoID, config.ID).Scan(&dest)
if err == gosql.ErrNoRows {
return false, nil
} else if err != nil {
return false, err
}
return true, nil
}
func (db *datastore) ConfigInsert(config *model.Config) error {
func (db *datastore) ConfigCreate(config *model.Config) error {
return meddler.Insert(db, "config", config)
}

View file

@ -18,12 +18,11 @@ func TestConfig(t *testing.T) {
hash = "8d8647c9aa90d893bfb79dddbe901f03e258588121e5202632f8ae5738590b26"
)
if err := s.ConfigInsert(
if err := s.ConfigCreate(
&model.Config{
RepoID: 2,
Data: data,
Hash: hash,
Approved: false,
},
); err != nil {
t.Errorf("Unexpected error: insert config: %s", err)
@ -47,60 +46,102 @@ func TestConfig(t *testing.T) {
if got, want := config.Hash, hash; got != want {
t.Errorf("Want config hash %s, got %s", want, got)
}
if got, want := config.Approved, false; got != want {
t.Errorf("Want config approved %v, got %v", want, got)
}
config.Approved = true
err = s.ConfigUpdate(config)
loaded, err := s.ConfigLoad(config.ID)
if err != nil {
t.Errorf("Want config updated, got error %q", err)
t.Errorf("Want config by id, got error %q", err)
return
}
updated, err := s.ConfigFind(&model.Repo{ID: 2}, hash)
if err != nil {
t.Errorf("Want config find, got error %q", err)
return
}
if got, want := updated.Approved, true; got != want {
t.Errorf("Want config approved updated %v, got %v", want, got)
if got, want := loaded.ID, config.ID; got != want {
t.Errorf("Want config by id %d, got %d", want, got)
}
}
//
// func TestConfigIndexes(t *testing.T) {
// s := newTest()
// defer func() {
// s.Exec("delete from config")
// s.Close()
// }()
//
// if err := s.FileCreate(
// &model.File{
// BuildID: 1,
// ProcID: 1,
// Name: "hello.txt",
// Size: 11,
// Mime: "text/plain",
// },
// bytes.NewBufferString("hello world"),
// ); err != nil {
// t.Errorf("Unexpected error: insert file: %s", err)
// return
// }
//
// // fail due to duplicate file name
// if err := s.FileCreate(
// &model.File{
// BuildID: 1,
// ProcID: 1,
// Name: "hello.txt",
// Mime: "text/plain",
// Size: 11,
// },
// bytes.NewBufferString("hello world"),
// ); err == nil {
// t.Errorf("Unexpected error: dupliate pid")
// }
// }
func TestConfigApproved(t *testing.T) {
s := newTest()
defer func() {
s.Exec("delete from config")
s.Exec("delete from builds")
s.Close()
}()
var (
data = "pipeline: [ { image: golang, commands: [ go build, go test ] } ]"
hash = "8d8647c9aa90d893bfb79dddbe901f03e258588121e5202632f8ae5738590b26"
conf = &model.Config{
RepoID: 1,
Data: data,
Hash: hash,
}
)
if err := s.ConfigCreate(conf); err != nil {
t.Errorf("Unexpected error: insert config: %s", err)
return
}
s.CreateBuild(&model.Build{
RepoID: 1,
ConfigID: conf.ID,
Status: model.StatusBlocked,
Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac",
})
s.CreateBuild(&model.Build{
RepoID: 1,
ConfigID: conf.ID,
Status: model.StatusPending,
Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac",
})
if ok, _ := s.ConfigFindApproved(conf); ok == true {
t.Errorf("Want config not approved, when blocked or pending")
return
}
s.CreateBuild(&model.Build{
RepoID: 1,
ConfigID: conf.ID,
Status: model.StatusRunning,
Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac",
})
if ok, _ := s.ConfigFindApproved(conf); ok == false {
t.Errorf("Want config approved, when running.")
return
}
}
func TestConfigIndexes(t *testing.T) {
s := newTest()
defer func() {
s.Exec("delete from config")
s.Close()
}()
var (
data = "pipeline: [ { image: golang, commands: [ go build, go test ] } ]"
hash = "8d8647c9aa90d893bfb79dddbe901f03e258588121e5202632f8ae5738590b26"
)
if err := s.ConfigCreate(
&model.Config{
RepoID: 2,
Data: data,
Hash: hash,
},
); err != nil {
t.Errorf("Unexpected error: insert config: %s", err)
return
}
// fail due to duplicate sha
if err := s.ConfigCreate(
&model.Config{
RepoID: 2,
Data: data,
Hash: hash,
},
); err == nil {
t.Errorf("Unexpected error: dupliate sha")
}
}

View file

@ -5,7 +5,6 @@ CREATE TABLE config (
,config_repo_id INTEGER
,config_hash VARCHAR(250)
,config_data MEDIUMBLOB
,config_approved BOOLEAN
,UNIQUE(config_hash, config_repo_id)
);

View file

@ -5,7 +5,6 @@ CREATE TABLE config (
,config_repo_id INTEGER
,config_hash VARCHAR(250)
,config_data BYTEA
,config_approved BOOLEAN
,UNIQUE(config_hash, config_repo_id)
);

View file

@ -5,7 +5,6 @@ CREATE TABLE config (
,config_repo_id INTEGER
,config_hash TEXT
,config_data BLOB
,config_approved BOOLEAN
,UNIQUE(config_hash, config_repo_id)
);

View file

@ -5,7 +5,6 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_id = $1
@ -16,7 +15,14 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_repo_id = $1
AND config_hash = $2
-- name: config-find-approved
SELECT build_id FROM builds
WHERE build_repo_id = $1
AND build_config_id = $2
AND build_status NOT IN ('blocked', 'pending')
LIMIT 1

View file

@ -8,6 +8,7 @@ func Lookup(name string) string {
var index = map[string]string{
"config-find-id": configFindId,
"config-find-repo-hash": configFindRepoHash,
"config-find-approved": configFindApproved,
"count-users": countUsers,
"count-repos": countRepos,
"count-builds": countBuilds,
@ -41,7 +42,6 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_id = $1
`
@ -52,12 +52,19 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_repo_id = $1
AND config_hash = $2
`
var configFindApproved = `
SELECT build_id FROM builds
WHERE build_repo_id = $1
AND build_config_id = $2
AND build_status NOT IN ('blocked', 'pending')
LIMIT 1
`
var countUsers = `
SELECT reltuples
FROM pg_class WHERE relname = 'users';

View file

@ -5,7 +5,6 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_id = ?
@ -16,7 +15,14 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_repo_id = ?
AND config_hash = ?
-- name: config-find-approved
SELECT build_id FROM builds
WHERE build_repo_id = ?
AND build_config_id = ?
AND build_status NOT IN ('blocked', 'pending')
LIMIT 1

View file

@ -8,6 +8,7 @@ func Lookup(name string) string {
var index = map[string]string{
"config-find-id": configFindId,
"config-find-repo-hash": configFindRepoHash,
"config-find-approved": configFindApproved,
"count-users": countUsers,
"count-repos": countRepos,
"count-builds": countBuilds,
@ -41,7 +42,6 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_id = ?
`
@ -52,12 +52,19 @@ SELECT
,config_repo_id
,config_hash
,config_data
,config_approved
FROM config
WHERE config_repo_id = ?
AND config_hash = ?
`
var configFindApproved = `
SELECT build_id FROM builds
WHERE build_repo_id = ?
AND build_config_id = ?
AND build_status NOT IN ('blocked', 'pending')
LIMIT 1
`
var countUsers = `
SELECT count(1)
FROM users

View file

@ -94,8 +94,8 @@ type Store interface {
ConfigLoad(int64) (*model.Config, error)
ConfigFind(*model.Repo, string) (*model.Config, error)
ConfigUpdate(*model.Config) error
ConfigInsert(*model.Config) error
ConfigFindApproved(*model.Config) (bool, error)
ConfigCreate(*model.Config) error
SenderFind(*model.Repo, string) (*model.Sender, error)
SenderList(*model.Repo) ([]*model.Sender, error)