From 4569b60f09ce1c436d4e0f42b2d47a4a2ecebdd5 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Fri, 5 May 2017 18:59:37 +0200 Subject: [PATCH] persist and compare yaml for gating --- drone/server/server.go | 1 + model/build.go | 1 + model/config.go | 34 +++--- model/repo.go | 1 + model/settings.go | 24 ++++ router/middleware/config.go | 4 +- router/middleware/session/user.go | 2 +- server/build.go | 16 ++- server/hook.go | 42 ++++++- server/login.go | 4 +- server/rpc.go | 5 +- store/datastore/config.go | 29 +++++ store/datastore/config_test.go | 106 ++++++++++++++++++ store/datastore/ddl/mysql/16.sql | 21 ++++ store/datastore/ddl/postgres/16.sql | 21 ++++ store/datastore/ddl/sqlite3/16.sql | 21 ++++ store/datastore/sql/postgres/files/config.sql | 22 ++++ store/datastore/sql/postgres/sql_gen.go | 25 +++++ store/datastore/sql/sqlite/files/config.sql | 22 ++++ store/datastore/sql/sqlite/sql_gen.go | 25 +++++ store/store.go | 5 + 21 files changed, 395 insertions(+), 36 deletions(-) create mode 100644 model/settings.go create mode 100644 store/datastore/config.go create mode 100644 store/datastore/config_test.go create mode 100644 store/datastore/ddl/mysql/16.sql create mode 100644 store/datastore/ddl/postgres/16.sql create mode 100644 store/datastore/ddl/sqlite3/16.sql create mode 100644 store/datastore/sql/postgres/files/config.sql create mode 100644 store/datastore/sql/sqlite/files/config.sql diff --git a/drone/server/server.go b/drone/server/server.go index a2eb5fd4..9993e1a2 100644 --- a/drone/server/server.go +++ b/drone/server/server.go @@ -391,6 +391,7 @@ func setupEvilGlobals(c *cli.Context, v store.Store) { // storage droneserver.Config.Storage.Files = v + droneserver.Config.Storage.Config = v // services droneserver.Config.Services.Queue = setupQueue(c, v) diff --git a/model/build.go b/model/build.go index f718598e..3ed1e3f9 100644 --- a/model/build.go +++ b/model/build.go @@ -4,6 +4,7 @@ package model type Build struct { ID int64 `json:"id" meddler:"build_id,pk"` RepoID int64 `json:"-" meddler:"build_repo_id"` + ConfigID int64 `json:"-" meddler:"build_config_id"` Number int `json:"number" meddler:"build_number"` Parent int `json:"parent" meddler:"build_parent"` Event string `json:"event" meddler:"build_event"` diff --git a/model/config.go b/model/config.go index 76c3f7d4..29134178 100644 --- a/model/config.go +++ b/model/config.go @@ -1,24 +1,18 @@ package model -// Config defines system configuration parameters. +// ConfigStore persists pipeline configuration to storage. +type ConfigStore interface { + ConfigLoad(int64) (*Config, error) + ConfigFind(*Repo, string) (*Config, error) + ConfigUpdate(*Config) error + ConfigInsert(*Config) error +} + +// Config represents a pipeline configuration. type Config struct { - Open bool // Enables open registration - Secret string // Secret token used to authenticate agents - Admins map[string]bool // Administrative users - Orgs map[string]bool // Organization whitelist -} - -// IsAdmin returns true if the user is a member of the administrator list. -func (c *Config) IsAdmin(user *User) bool { - return c.Admins[user.Login] -} - -// IsMember returns true if the user is a member of the whitelisted teams. -func (c *Config) IsMember(teams []*Team) bool { - for _, team := range teams { - if c.Orgs[team.Login] { - return true - } - } - return false + ID int64 `json:"-" meddler:"config_id,pk"` + 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"` } diff --git a/model/repo.go b/model/repo.go index c650e5dd..2ced3d59 100644 --- a/model/repo.go +++ b/model/repo.go @@ -26,6 +26,7 @@ type Repo struct { IsTrusted bool `json:"trusted" meddler:"repo_trusted"` IsStarred bool `json:"starred,omitempty" meddler:"-"` IsGated bool `json:"gated" meddler:"repo_gated"` + IsGatedConf bool `json:"gated_conf" meddler:"repo_gated_conf"` AllowPull bool `json:"allow_pr" meddler:"repo_allow_pr"` AllowPush bool `json:"allow_push" meddler:"repo_allow_push"` AllowDeploy bool `json:"allow_deploys" meddler:"repo_allow_deploys"` diff --git a/model/settings.go b/model/settings.go new file mode 100644 index 00000000..b3722655 --- /dev/null +++ b/model/settings.go @@ -0,0 +1,24 @@ +package model + +// Settings defines system configuration parameters. +type Settings struct { + Open bool // Enables open registration + Secret string // Secret token used to authenticate agents + Admins map[string]bool // Administrative users + Orgs map[string]bool // Organization whitelist +} + +// IsAdmin returns true if the user is a member of the administrator list. +func (c *Settings) IsAdmin(user *User) bool { + return c.Admins[user.Login] +} + +// IsMember returns true if the user is a member of the whitelisted teams. +func (c *Settings) IsMember(teams []*Team) bool { + for _, team := range teams { + if c.Orgs[team.Login] { + return true + } + } + return false +} diff --git a/router/middleware/config.go b/router/middleware/config.go index 77a0217f..67f798ec 100644 --- a/router/middleware/config.go +++ b/router/middleware/config.go @@ -19,8 +19,8 @@ func Config(cli *cli.Context) gin.HandlerFunc { } // helper function to create the configuration from the CLI context. -func setupConfig(c *cli.Context) *model.Config { - return &model.Config{ +func setupConfig(c *cli.Context) *model.Settings { + return &model.Settings{ Open: c.Bool("open"), Secret: c.String("agent-secret"), Admins: sliceToMap2(c.StringSlice("admin")), diff --git a/router/middleware/session/user.go b/router/middleware/session/user.go index c1c0e09b..f965014a 100644 --- a/router/middleware/session/user.go +++ b/router/middleware/session/user.go @@ -45,7 +45,7 @@ func SetUser() gin.HandlerFunc { }) if err == nil { confv := c.MustGet("config") - if conf, ok := confv.(*model.Config); ok { + if conf, ok := confv.(*model.Settings); ok { user.Admin = conf.IsAdmin(user) } c.Set("user", user) diff --git a/server/build.go b/server/build.go index 7aaf95d5..b362f2bb 100644 --- a/server/build.go +++ b/server/build.go @@ -174,12 +174,16 @@ func PostApproval(c *gin.Context) { // // fetch the build file from the database - raw, err := remote_.File(user, repo, build, repo.Config) + conf, err := Config.Storage.Config.ConfigLoad(build.ConfigID) if err != nil { logrus.Errorf("failure to get build config for %s. %s", repo.FullName, err) 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 { @@ -222,7 +226,7 @@ func PostApproval(c *gin.Context) { Secs: secs, Regs: regs, Link: httputil.GetURL(c.Request), - Yaml: string(raw), + Yaml: conf.Data, } items, err := b.Build() if err != nil { @@ -394,12 +398,16 @@ func PostBuild(c *gin.Context) { } // fetch the .drone.yml file from the database - raw, err := remote_.File(user, repo, build, repo.Config) + conf, err := Config.Storage.Config.ConfigLoad(build.ConfigID) if err != nil { logrus.Errorf("failure to get build config for %s. %s", repo.FullName, err) 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 { @@ -493,7 +501,7 @@ func PostBuild(c *gin.Context) { Secs: secs, Regs: regs, Link: httputil.GetURL(c.Request), - Yaml: string(raw), + Yaml: conf.Data, } // TODO inject environment varibles !!!!!! buildParams items, err := b.Build() diff --git a/server/hook.go b/server/hook.go index d065a742..7d5f8a94 100644 --- a/server/hook.go +++ b/server/hook.go @@ -2,6 +2,7 @@ package server import ( "context" + "crypto/sha256" "encoding/json" "fmt" "regexp" @@ -131,12 +132,38 @@ func PostHook(c *gin.Context) { } // fetch the build file from the database - raw, err := remote_.File(user, repo, build, repo.Config) + confb, err := remote_.File(user, repo, build, repo.Config) if err != nil { logrus.Errorf("failure to get build config for %s. %s", repo.FullName, err) c.AbortWithError(404, err) return } + sha := shasum(confb) + conf, err := Config.Storage.Config.ConfigFind(repo, sha) + if err != nil { + conf = &model.Config{ + RepoID: repo.ID, + Data: string(confb), + Hash: sha, + Approved: false, + } + if user.Login == repo.Owner || build.Event != model.EventPull { + conf.Approved = true + } + err = Config.Storage.Config.ConfigInsert(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.IsGatedConf { + conf.Approved = true + Config.Storage.Config.ConfigUpdate(conf) + } + } + build.ConfigID = conf.ID netrc, err := remote_.Netrc(user, repo) if err != nil { @@ -145,7 +172,7 @@ func PostHook(c *gin.Context) { } // verify the branches can be built vs skipped - branches, err := yaml.ParseBytes(raw) + branches, err := yaml.ParseString(conf.Data) if err == nil { if !branches.Branches.Match(build.Branch) && build.Event != model.EventTag && build.Event != model.EventDeploy { c.String(200, "Branch does not match restrictions defined in yaml") @@ -168,7 +195,7 @@ func PostHook(c *gin.Context) { build.Verified = true build.Status = model.StatusPending - if repo.IsGated { + if repo.IsGated || repo.IsGatedConf { allowed, _ := Config.Services.Senders.SenderAllowed(user, repo, build) if !allowed { build.Status = model.StatusBlocked @@ -212,7 +239,7 @@ func PostHook(c *gin.Context) { Secs: secs, Regs: regs, Link: httputil.GetURL(c.Request), - Yaml: string(raw), + Yaml: conf.Data, } items, err := b.Build() if err != nil { @@ -442,7 +469,7 @@ func (b *builder) Build() ([]*buildItem, error) { linter.WithTrusted(b.Repo.IsTrusted), ).Lint(parsed) if lerr != nil { - return nil, err + return nil, lerr } var registries []compiler.Registry @@ -511,3 +538,8 @@ func (b *builder) Build() ([]*buildItem, error) { return items, nil } + +func shasum(raw []byte) string { + sum := sha256.Sum256(raw) + return fmt.Sprintf("%x", sum) +} diff --git a/server/login.go b/server/login.go index 9e951d78..fcc654ed 100644 --- a/server/login.go +++ b/server/login.go @@ -161,7 +161,7 @@ type tokenPayload struct { } // ToConfig returns the config from the Context -func ToConfig(c *gin.Context) *model.Config { +func ToConfig(c *gin.Context) *model.Settings { v := c.MustGet("config") - return v.(*model.Config) + return v.(*model.Settings) } diff --git a/server/rpc.go b/server/rpc.go index 0e5feed3..702d8eca 100644 --- a/server/rpc.go +++ b/server/rpc.go @@ -41,8 +41,9 @@ var Config = struct { // Repos model.RepoStore // Builds model.BuildStore // Logs model.LogStore - Files model.FileStore - Procs model.ProcStore + Config model.ConfigStore + Files model.FileStore + Procs model.ProcStore // Registries model.RegistryStore // Secrets model.SecretStore } diff --git a/store/datastore/config.go b/store/datastore/config.go new file mode 100644 index 00000000..ad430924 --- /dev/null +++ b/store/datastore/config.go @@ -0,0 +1,29 @@ +package datastore + +import ( + "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") + conf := new(model.Config) + err := meddler.QueryRow(db, conf, stmt, id) + return conf, err +} + +func (db *datastore) ConfigFind(repo *model.Repo, hash string) (*model.Config, error) { + stmt := sql.Lookup(db.driver, "config-find-repo-hash") + conf := new(model.Config) + err := meddler.QueryRow(db, conf, stmt, repo.ID, hash) + return conf, err +} + +func (db *datastore) ConfigUpdate(config *model.Config) error { + return meddler.Update(db, "config", config) +} + +func (db *datastore) ConfigInsert(config *model.Config) error { + return meddler.Insert(db, "config", config) +} diff --git a/store/datastore/config_test.go b/store/datastore/config_test.go new file mode 100644 index 00000000..bc1082c0 --- /dev/null +++ b/store/datastore/config_test.go @@ -0,0 +1,106 @@ +package datastore + +import ( + "testing" + + "github.com/drone/drone/model" +) + +func TestConfig(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.ConfigInsert( + &model.Config{ + RepoID: 2, + Data: data, + Hash: hash, + Approved: false, + }, + ); err != nil { + t.Errorf("Unexpected error: insert config: %s", err) + return + } + + config, err := s.ConfigFind(&model.Repo{ID: 2}, hash) + if err != nil { + t.Error(err) + return + } + if got, want := config.ID, int64(1); got != want { + t.Errorf("Want config id %d, got %d", want, got) + } + if got, want := config.RepoID, int64(2); got != want { + t.Errorf("Want config repo id %d, got %d", want, got) + } + if got, want := config.Data, data; got != want { + t.Errorf("Want config data %s, got %s", want, got) + } + 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) + if err != nil { + t.Errorf("Want config updated, 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) + } +} + +// +// 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") +// } +// } diff --git a/store/datastore/ddl/mysql/16.sql b/store/datastore/ddl/mysql/16.sql new file mode 100644 index 00000000..1636663e --- /dev/null +++ b/store/datastore/ddl/mysql/16.sql @@ -0,0 +1,21 @@ +-- +migrate Up + +CREATE TABLE config ( + config_id INTEGER PRIMARY KEY AUTO_INCREMENT +,config_repo_id INTEGER +,config_hash VARCHAR(250) +,config_data MEDIUMBLOB +,config_approved BOOLEAN + +,UNIQUE(config_hash, config_repo_id) +); + +ALTER TABLE builds ADD COLUMN build_config_id INTEGER; +UPDATE builds set build_config_id = 0; + +ALTER TABLE repos ADD COLUMN repo_gated_conf BOOLEAN; +UPDATE repos SET repo_gated_conf = 0; + +-- +migrate Down + +DROP TABLE config; diff --git a/store/datastore/ddl/postgres/16.sql b/store/datastore/ddl/postgres/16.sql new file mode 100644 index 00000000..e15397c8 --- /dev/null +++ b/store/datastore/ddl/postgres/16.sql @@ -0,0 +1,21 @@ +-- +migrate Up + +CREATE TABLE config ( + config_id SERIAL PRIMARY KEY +,config_repo_id INTEGER +,config_hash VARCHAR(250) +,config_data BYTEA +,config_approved BOOLEAN + +,UNIQUE(config_hash, config_repo_id) +); + +ALTER TABLE builds ADD COLUMN build_config_id INTEGER; +UPDATE builds set build_config_id = 0; + +ALTER TABLE repos ADD COLUMN repo_gated_conf BOOLEAN; +UPDATE repos SET repo_gated_conf = 0; + +-- +migrate Down + +DROP TABLE config; diff --git a/store/datastore/ddl/sqlite3/16.sql b/store/datastore/ddl/sqlite3/16.sql new file mode 100644 index 00000000..31622944 --- /dev/null +++ b/store/datastore/ddl/sqlite3/16.sql @@ -0,0 +1,21 @@ +-- +migrate Up + +CREATE TABLE config ( + config_id INTEGER PRIMARY KEY AUTOINCREMENT +,config_repo_id INTEGER +,config_hash TEXT +,config_data BLOB +,config_approved BOOLEAN + +,UNIQUE(config_hash, config_repo_id) +); + +ALTER TABLE builds ADD COLUMN build_config_id INTEGER; +UPDATE builds set build_config_id = 0; + +ALTER TABLE repos ADD COLUMN repo_gated_conf BOOLEAN; +UPDATE repos SET repo_gated_conf = 0; + +-- +migrate Down + +DROP TABLE config; diff --git a/store/datastore/sql/postgres/files/config.sql b/store/datastore/sql/postgres/files/config.sql new file mode 100644 index 00000000..0385c27c --- /dev/null +++ b/store/datastore/sql/postgres/files/config.sql @@ -0,0 +1,22 @@ +-- name: config-find-id + +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_id = $1 + +-- name: config-find-repo-hash + +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_repo_id = $1 + AND config_hash = $2 diff --git a/store/datastore/sql/postgres/sql_gen.go b/store/datastore/sql/postgres/sql_gen.go index 08be6c7d..c27fa554 100644 --- a/store/datastore/sql/postgres/sql_gen.go +++ b/store/datastore/sql/postgres/sql_gen.go @@ -6,6 +6,8 @@ func Lookup(name string) string { } var index = map[string]string{ + "config-find-id": configFindId, + "config-find-repo-hash": configFindRepoHash, "count-users": countUsers, "count-repos": countRepos, "count-builds": countBuilds, @@ -33,6 +35,29 @@ var index = map[string]string{ "task-delete": taskDelete, } +var configFindId = ` +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_id = $1 +` + +var configFindRepoHash = ` +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_repo_id = $1 + AND config_hash = $2 +` + var countUsers = ` SELECT reltuples FROM pg_class WHERE relname = 'users'; diff --git a/store/datastore/sql/sqlite/files/config.sql b/store/datastore/sql/sqlite/files/config.sql new file mode 100644 index 00000000..dd07a4f1 --- /dev/null +++ b/store/datastore/sql/sqlite/files/config.sql @@ -0,0 +1,22 @@ +-- name: config-find-id + +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_id = ? + +-- name: config-find-repo-hash + +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_repo_id = ? + AND config_hash = ? diff --git a/store/datastore/sql/sqlite/sql_gen.go b/store/datastore/sql/sqlite/sql_gen.go index c2e082dd..15cc0e7b 100644 --- a/store/datastore/sql/sqlite/sql_gen.go +++ b/store/datastore/sql/sqlite/sql_gen.go @@ -6,6 +6,8 @@ func Lookup(name string) string { } var index = map[string]string{ + "config-find-id": configFindId, + "config-find-repo-hash": configFindRepoHash, "count-users": countUsers, "count-repos": countRepos, "count-builds": countBuilds, @@ -33,6 +35,29 @@ var index = map[string]string{ "task-delete": taskDelete, } +var configFindId = ` +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_id = ? +` + +var configFindRepoHash = ` +SELECT + config_id +,config_repo_id +,config_hash +,config_data +,config_approved +FROM config +WHERE config_repo_id = ? + AND config_hash = ? +` + var countUsers = ` SELECT count(1) FROM users diff --git a/store/store.go b/store/store.go index 4ba35832..18a17782 100644 --- a/store/store.go +++ b/store/store.go @@ -92,6 +92,11 @@ type Store interface { // new functions // + ConfigLoad(int64) (*model.Config, error) + ConfigFind(*model.Repo, string) (*model.Config, error) + ConfigUpdate(*model.Config) error + ConfigInsert(*model.Config) error + SenderFind(*model.Repo, string) (*model.Sender, error) SenderList(*model.Repo) ([]*model.Sender, error) SenderCreate(*model.Sender) error