Merge pull request #2025 from bradrydzewski/master

persist and compare yaml for gating

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)
@@ -399,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)
 	}

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"`

model/config.go

@@ -1,24 +1,17 @@
 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)
+	ConfigFindApproved(*Config) (bool, error)
+	ConfigCreate(*Config) error
+}
+
+// Config represents a pipeline configuration.
 type Config struct {
-	Open   bool            // Enables open registration
+	ID     int64  `json:"-"    meddler:"config_id,pk"`
-	Secret string          // Secret token used to authenticate agents
+	RepoID int64  `json:"-"    meddler:"config_repo_id"`
-	Admins map[string]bool // Administrative users
+	Data   string `json:"data" meddler:"config_data"`
-	Orgs   map[string]bool // Organization whitelist
+	Hash   string `json:"hash" meddler:"config_hash"`
-}
-
-// 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
 }

model/sender.go

@@ -1,7 +1,7 @@
 package model
 
 type SenderService interface {
-	SenderAllowed(*User, *Repo, *Build) (bool, error)
+	SenderAllowed(*User, *Repo, *Build, *Config) (bool, error)
 	SenderCreate(*Repo, *Sender) error
 	SenderUpdate(*Repo, *Sender) error
 	SenderDelete(*Repo, string) error

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
+}

plugins/sender/builtin.go

@@ -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 {
+func New(store model.SenderStore, conf model.ConfigStore) model.SenderService {
-	return &builtin{store}
+	return &builtin{store, conf}
 }
 
-func (b *builtin) SenderAllowed(user *model.User, repo *model.Repo, build *model.Build) (bool, error) {
+func (b *builtin) SenderAllowed(user *model.User, repo *model.Repo, build *model.Build, conf *model.Config) (bool, error) {
-	if repo.IsPrivate == false && build.Event == model.EventPull && build.Sender != user.Login {
+	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

plugins/sender/plugin.go

@@ -16,7 +16,7 @@ func NewRemote(endpoint string) model.SenderService {
 	return &plugin{endpoint}
 }
 
-func (p *plugin) SenderAllowed(user *model.User, repo *model.Repo, build *model.Build) (bool, error) {
+func (p *plugin) SenderAllowed(user *model.User, repo *model.Repo, build *model.Build, conf *model.Config) (bool, error) {
 	path := fmt.Sprintf("%s/senders/%s/%s/%s/verify", p.endpoint, repo.Owner, repo.Name, build.Sender)
 	err := internal.Send("POST", path, build, nil)
 	if err != nil {

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 {
+func setupConfig(c *cli.Context) *model.Settings {
-	return &model.Config{
+	return &model.Settings{
 		Open:   c.Bool("open"),
 		Secret: c.String("agent-secret"),
 		Admins: sliceToMap2(c.StringSlice("admin")),

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)

server/build.go

@@ -174,7 +174,7 @@ 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)
@@ -222,7 +222,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,7 +394,7 @@ 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)
@@ -493,7 +493,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()

server/hook.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"context"
+	"crypto/sha256"
 	"encoding/json"
 	"fmt"
 	"regexp"
@@ -131,12 +132,28 @@ 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,
+		}
+		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
+		}
+	}
+	build.ConfigID = conf.ID
 
 	netrc, err := remote_.Netrc(user, repo)
 	if err != nil {
@@ -145,7 +162,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")
@@ -169,7 +186,7 @@ func PostHook(c *gin.Context) {
 	build.Status = model.StatusPending
 
 	if repo.IsGated {
-		allowed, _ := Config.Services.Senders.SenderAllowed(user, repo, build)
+		allowed, _ := Config.Services.Senders.SenderAllowed(user, repo, build, conf)
 		if !allowed {
 			build.Status = model.StatusBlocked
 		}
@@ -212,7 +229,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 +459,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 +528,8 @@ func (b *builder) Build() ([]*buildItem, error) {
 
 	return items, nil
 }
+
+func shasum(raw []byte) string {
+	sum := sha256.Sum256(raw)
+	return fmt.Sprintf("%x", sum)
+}

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)
 }

server/rpc.go

@@ -41,6 +41,7 @@ var Config = struct {
 		// Repos  model.RepoStore
 		// Builds model.BuildStore
 		// Logs   model.LogStore
+		Config model.ConfigStore
 		Files  model.FileStore
 		Procs  model.ProcStore
 		// Registries model.RegistryStore

store/datastore/config.go

@@ -0,0 +1,39 @@
+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-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) 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) ConfigCreate(config *model.Config) error {
+	return meddler.Insert(db, "config", config)
+}

store/datastore/config_test.go

@@ -0,0 +1,147 @@
+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.ConfigCreate(
+		&model.Config{
+			RepoID: 2,
+			Data:   data,
+			Hash:   hash,
+		},
+	); 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)
+	}
+
+	loaded, err := s.ConfigLoad(config.ID)
+	if err != nil {
+		t.Errorf("Want config by id, got error %q", err)
+		return
+	}
+	if got, want := loaded.ID, config.ID; got != want {
+		t.Errorf("Want config by id %d, got %d", want, got)
+	}
+}
+
+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")
+	}
+}

store/datastore/ddl/mysql/16.sql

@@ -0,0 +1,17 @@
+-- +migrate Up
+
+CREATE TABLE config (
+ config_id       INTEGER PRIMARY KEY AUTO_INCREMENT
+,config_repo_id  INTEGER
+,config_hash     VARCHAR(250)
+,config_data     MEDIUMBLOB
+
+,UNIQUE(config_hash, config_repo_id)
+);
+
+ALTER TABLE builds ADD COLUMN build_config_id INTEGER;
+UPDATE builds set build_config_id = 0;
+
+-- +migrate Down
+
+DROP TABLE config;

store/datastore/ddl/postgres/16.sql

@@ -0,0 +1,17 @@
+-- +migrate Up
+
+CREATE TABLE config (
+ config_id       SERIAL PRIMARY KEY
+,config_repo_id  INTEGER
+,config_hash     VARCHAR(250)
+,config_data     BYTEA
+
+,UNIQUE(config_hash, config_repo_id)
+);
+
+ALTER TABLE builds ADD COLUMN build_config_id INTEGER;
+UPDATE builds set build_config_id = 0;
+
+-- +migrate Down
+
+DROP TABLE config;

store/datastore/ddl/sqlite3/16.sql

@@ -0,0 +1,17 @@
+-- +migrate Up
+
+CREATE TABLE config (
+ config_id       INTEGER PRIMARY KEY AUTOINCREMENT
+,config_repo_id  INTEGER
+,config_hash     TEXT
+,config_data     BLOB
+
+,UNIQUE(config_hash, config_repo_id)
+);
+
+ALTER TABLE builds ADD COLUMN build_config_id INTEGER;
+UPDATE builds set build_config_id = 0;
+
+-- +migrate Down
+
+DROP TABLE config;

store/datastore/sql/postgres/files/config.sql

@@ -0,0 +1,28 @@
+-- name: config-find-id
+
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+FROM config
+WHERE config_id = $1
+
+-- name: config-find-repo-hash
+
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+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

store/datastore/sql/postgres/sql_gen.go

@@ -6,6 +6,9 @@ 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,
@@ -33,6 +36,35 @@ var index = map[string]string{
 	"task-delete":               taskDelete,
 }
 
+var configFindId = `
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+FROM config
+WHERE config_id = $1
+`
+
+var configFindRepoHash = `
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+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';

store/datastore/sql/sqlite/files/config.sql

@@ -0,0 +1,28 @@
+-- name: config-find-id
+
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+FROM config
+WHERE config_id = ?
+
+-- name: config-find-repo-hash
+
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+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

store/datastore/sql/sqlite/sql_gen.go

@@ -6,6 +6,9 @@ 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,
@@ -33,6 +36,35 @@ var index = map[string]string{
 	"task-delete":               taskDelete,
 }
 
+var configFindId = `
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+FROM config
+WHERE config_id = ?
+`
+
+var configFindRepoHash = `
+SELECT
+ config_id
+,config_repo_id
+,config_hash
+,config_data
+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

store/store.go

@@ -92,6 +92,11 @@ type Store interface {
 	// new functions
 	//
 
+	ConfigLoad(int64) (*model.Config, error)
+	ConfigFind(*model.Repo, string) (*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)
 	SenderCreate(*model.Sender) error