From 30ca88b9e66b6e12769d4cb65de37d9aef7ea3c6 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 3 Sep 2019 15:05:53 -0700 Subject: [PATCH] added validation plugins --- CHANGELOG.md | 4 +- cmd/drone-server/inject_plugin.go | 8 ++- cmd/drone-server/wire_gen.go | 3 +- mock/mock.go | 2 +- mock/mock_gen.go | 39 ++++++++++- plugin/validator/combine_test.go | 58 +++++++++++++++ plugin/validator/noop.go | 27 +++++++ plugin/validator/remote.go | 113 ++++++++++++++++++++++++++++++ plugin/validator/remote_oss.go | 27 +++++++ plugin/validator/remote_test.go | 7 ++ trigger/trigger.go | 52 +++++++++----- trigger/trigger_test.go | 24 +++++++ 12 files changed, 339 insertions(+), 25 deletions(-) create mode 100644 plugin/validator/combine_test.go create mode 100644 plugin/validator/noop.go create mode 100644 plugin/validator/remote.go create mode 100644 plugin/validator/remote_oss.go create mode 100644 plugin/validator/remote_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index eb04e4714..6f25efd2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added -- support for configuration conversion plugins (e.g. convert from starlark) -- moved native jsonnet to a conversion plugin +- support for validation plugins, by [@bradrydzewski](https://github.com/bradrydzewski). [#2266](https://github.com/drone/drone/issues/2266). +- support for conversion plugins, by [@bradrydzewski](https://github.com/bradrydzewski). ### Removed - Support for basic auth as an option for Gitea, by [@techknowlogick](https://giteahub.com/techknowlogick). [#2721](https://github.com/drone/drone/issues/2721) diff --git a/cmd/drone-server/inject_plugin.go b/cmd/drone-server/inject_plugin.go index b60ac4e59..2b5780730 100644 --- a/cmd/drone-server/inject_plugin.go +++ b/cmd/drone-server/inject_plugin.go @@ -120,7 +120,13 @@ func provideSecretPlugin(config spec.Config) core.SecretService { // returns a yaml validation plugin based on the environment // configuration. func provideValidatePlugin(conf spec.Config) core.ValidateService { - return validator.Combine() + return validator.Combine( + validator.Remote( + conf.Validate.Endpoint, + conf.Validate.Secret, + conf.Validate.SkipVerify, + ), + ) } // provideWebhookPlugin is a Wire provider function that returns diff --git a/cmd/drone-server/wire_gen.go b/cmd/drone-server/wire_gen.go index 85020f339..db5c12202 100644 --- a/cmd/drone-server/wire_gen.go +++ b/cmd/drone-server/wire_gen.go @@ -56,9 +56,10 @@ func InitializeApplication(config2 config.Config) (application, error) { buildStore := provideBuildStore(db) stageStore := provideStageStore(db) scheduler := provideScheduler(stageStore, config2) + validateService := provideValidatePlugin(config2) system := provideSystem(config2) webhookSender := provideWebhookPlugin(config2, system) - triggerer := trigger.New(configService, convertService, commitService, statusService, buildStore, scheduler, repositoryStore, userStore, webhookSender) + triggerer := trigger.New(configService, convertService, commitService, statusService, buildStore, scheduler, repositoryStore, userStore, validateService, webhookSender) cronScheduler := cron2.New(commitService, cronStore, repositoryStore, userStore, triggerer) coreLicense := provideLicense(client, config2) datadog := provideDatadog(userStore, repositoryStore, buildStore, system, coreLicense, config2) diff --git a/mock/mock.go b/mock/mock.go index fd5b8f427..ea58f9255 100644 --- a/mock/mock.go +++ b/mock/mock.go @@ -6,4 +6,4 @@ package mock -//go:generate mockgen -package=mock -destination=mock_gen.go github.com/drone/drone/core ConvertService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Triggerer,Syncer,LogStream,WebhookSender,LicenseService +//go:generate mockgen -package=mock -destination=mock_gen.go github.com/drone/drone/core ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Triggerer,Syncer,LogStream,WebhookSender,LicenseService diff --git a/mock/mock_gen.go b/mock/mock_gen.go index c3fc97461..9a896228a 100644 --- a/mock/mock_gen.go +++ b/mock/mock_gen.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/drone/drone/core (interfaces: ConvertService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Triggerer,Syncer,LogStream,WebhookSender,LicenseService) +// Source: github.com/drone/drone/core (interfaces: ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Triggerer,Syncer,LogStream,WebhookSender,LicenseService) // Package mock is a generated GoMock package. package mock @@ -51,6 +51,43 @@ func (mr *MockConvertServiceMockRecorder) Convert(arg0, arg1 interface{}) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Convert", reflect.TypeOf((*MockConvertService)(nil).Convert), arg0, arg1) } +// MockValidateService is a mock of ValidateService interface +type MockValidateService struct { + ctrl *gomock.Controller + recorder *MockValidateServiceMockRecorder +} + +// MockValidateServiceMockRecorder is the mock recorder for MockValidateService +type MockValidateServiceMockRecorder struct { + mock *MockValidateService +} + +// NewMockValidateService creates a new mock instance +func NewMockValidateService(ctrl *gomock.Controller) *MockValidateService { + mock := &MockValidateService{ctrl: ctrl} + mock.recorder = &MockValidateServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockValidateService) EXPECT() *MockValidateServiceMockRecorder { + return m.recorder +} + +// Validate mocks base method +func (m *MockValidateService) Validate(arg0 context.Context, arg1 *core.ValidateArgs) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Validate", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// Validate indicates an expected call of Validate +func (mr *MockValidateServiceMockRecorder) Validate(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Validate", reflect.TypeOf((*MockValidateService)(nil).Validate), arg0, arg1) +} + // MockNetrcService is a mock of NetrcService interface type MockNetrcService struct { ctrl *gomock.Controller diff --git a/plugin/validator/combine_test.go b/plugin/validator/combine_test.go new file mode 100644 index 000000000..22627c7a3 --- /dev/null +++ b/plugin/validator/combine_test.go @@ -0,0 +1,58 @@ +// Copyright 2019 Drone.IO Inc. All rights reserved. +// Use of this source code is governed by the Drone Non-Commercial License +// that can be found in the LICENSE file. + +package validator + +import ( + "context" + "errors" + "testing" + + "github.com/drone/drone/core" + "github.com/drone/drone/mock" + + "github.com/golang/mock/gomock" +) + +var noContext = context.Background() + +var mockFile = ` +kind: pipeline +type: docker +name: testing +` + +func TestCombine(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + args := &core.ValidateArgs{ + User: &core.User{Login: "octocat"}, + Repo: &core.Repository{Slug: "octocat/hello-world", Config: ".drone.yml"}, + Build: &core.Build{After: "6d144de7"}, + Config: &core.Config{}, + } + + service := mock.NewMockValidateService(controller) + service.EXPECT().Validate(noContext, args).Return(nil) + + err := Combine(service).Validate(noContext, args) + if err != nil { + t.Error(err) + } +} + +func TestCombineErr(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + resp := errors.New("") + service := mock.NewMockValidateService(controller) + service.EXPECT().Validate(noContext, nil).Return(resp) + + err := Combine(service).Validate(noContext, nil) + if err != resp { + t.Errorf("expected convert service error") + } +} diff --git a/plugin/validator/noop.go b/plugin/validator/noop.go new file mode 100644 index 000000000..3fca78329 --- /dev/null +++ b/plugin/validator/noop.go @@ -0,0 +1,27 @@ +// Copyright 2019 Drone IO, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build oss + +package converter + +import ( + "context" + + "github.com/drone/drone/core" +) + +type noop struct{} + +func (noop) Validate(context.Context, *core.ConvertArgs) error { return nil } diff --git a/plugin/validator/remote.go b/plugin/validator/remote.go new file mode 100644 index 000000000..5c1323280 --- /dev/null +++ b/plugin/validator/remote.go @@ -0,0 +1,113 @@ +// Copyright 2019 Drone.IO Inc. All rights reserved. +// Use of this source code is governed by the Drone Non-Commercial License +// that can be found in the LICENSE file. + +// +build !oss + +package validator + +import ( + "context" + "time" + + "github.com/drone/drone-go/drone" + "github.com/drone/drone-go/plugin/validator" + "github.com/drone/drone/core" +) + +// Remote returns a conversion service that converts the +// configuration file using a remote http service. +func Remote(endpoint, signer string, skipVerify bool) core.ValidateService { + return &remote{ + endpoint: endpoint, + secret: signer, + skipVerify: skipVerify, + } +} + +type remote struct { + endpoint string + secret string + skipVerify bool +} + +func (g *remote) Validate(ctx context.Context, in *core.ValidateArgs) error { + if g.endpoint == "" { + return nil + } + // include a timeout to prevent an API call from + // hanging the build process indefinitely. The + // external service must return a request within + // one minute. + ctx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + + req := &validator.Request{ + Repo: toRepo(in.Repo), + Build: toBuild(in.Build), + Config: drone.Config{ + Data: in.Config.Data, + }, + } + client := validator.Client(g.endpoint, g.secret, g.skipVerify) + return client.Validate(ctx, req) +} + +func toRepo(from *core.Repository) drone.Repo { + return drone.Repo{ + ID: from.ID, + UID: from.UID, + UserID: from.UserID, + Namespace: from.Namespace, + Name: from.Name, + Slug: from.Slug, + SCM: from.SCM, + HTTPURL: from.HTTPURL, + SSHURL: from.SSHURL, + Link: from.Link, + Branch: from.Branch, + Private: from.Private, + Visibility: from.Visibility, + Active: from.Active, + Config: from.Config, + Trusted: from.Trusted, + Protected: from.Protected, + Timeout: from.Timeout, + } +} + +func toBuild(from *core.Build) drone.Build { + return drone.Build{ + ID: from.ID, + RepoID: from.RepoID, + Trigger: from.Trigger, + Number: from.Number, + Parent: from.Parent, + Status: from.Status, + Error: from.Error, + Event: from.Event, + Action: from.Action, + Link: from.Link, + Timestamp: from.Timestamp, + Title: from.Title, + Message: from.Message, + Before: from.Before, + After: from.After, + Ref: from.Ref, + Fork: from.Fork, + Source: from.Source, + Target: from.Target, + Author: from.Author, + AuthorName: from.AuthorName, + AuthorEmail: from.AuthorEmail, + AuthorAvatar: from.AuthorAvatar, + Sender: from.Sender, + Params: from.Params, + Deploy: from.Deploy, + Started: from.Started, + Finished: from.Finished, + Created: from.Created, + Updated: from.Updated, + Version: from.Version, + } +} diff --git a/plugin/validator/remote_oss.go b/plugin/validator/remote_oss.go new file mode 100644 index 000000000..23165c410 --- /dev/null +++ b/plugin/validator/remote_oss.go @@ -0,0 +1,27 @@ +// Copyright 2019 Drone IO, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build oss + +package validator + +import ( + "github.com/drone/drone/core" +) + +// Remote returns a conversion service that converts the +// configuration file using a remote http service. +func Remote(endpoint, signer string, skipVerify bool) core.ValidateService { + return new(noop) +} diff --git a/plugin/validator/remote_test.go b/plugin/validator/remote_test.go new file mode 100644 index 000000000..518de4719 --- /dev/null +++ b/plugin/validator/remote_test.go @@ -0,0 +1,7 @@ +// Copyright 2019 Drone.IO Inc. All rights reserved. +// Use of this source code is governed by the Drone Non-Commercial License +// that can be found in the LICENSE file. + +// +build !oss + +package validator diff --git a/trigger/trigger.go b/trigger/trigger.go index c6f0b34b1..826b15106 100644 --- a/trigger/trigger.go +++ b/trigger/trigger.go @@ -32,15 +32,16 @@ import ( ) type triggerer struct { - config core.ConfigService - convert core.ConvertService - commits core.CommitService - status core.StatusService - builds core.BuildStore - sched core.Scheduler - repos core.RepositoryStore - users core.UserStore - hooks core.WebhookSender + config core.ConfigService + convert core.ConvertService + commits core.CommitService + status core.StatusService + builds core.BuildStore + sched core.Scheduler + repos core.RepositoryStore + users core.UserStore + validate core.ValidateService + hooks core.WebhookSender } // New returns a new build triggerer. @@ -53,18 +54,20 @@ func New( sched core.Scheduler, repos core.RepositoryStore, users core.UserStore, + validate core.ValidateService, hooks core.WebhookSender, ) core.Triggerer { return &triggerer{ - config: config, - convert: convert, - commits: commits, - status: status, - builds: builds, - sched: sched, - repos: repos, - users: users, - hooks: hooks, + config: config, + convert: convert, + commits: commits, + status: status, + builds: builds, + sched: sched, + repos: repos, + users: users, + validate: validate, + hooks: hooks, } } @@ -189,7 +192,6 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co Repo: repo, Build: tmpBuild, } - raw, err := t.config.Find(ctx, req) if err != nil { logger = logger.WithError(err) @@ -229,6 +231,18 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co return t.createBuildError(ctx, repo, base, err.Error()) } + err = t.validate.Validate(ctx, &core.ValidateArgs{ + User: user, + Repo: repo, + Build: tmpBuild, + Config: raw, + }) + if err != nil { + logger = logger.WithError(err) + logger.Warnln("trigger: yaml validation error") + return t.createBuildError(ctx, repo, base, err.Error()) + } + err = linter.Manifest(manifest, repo.Trusted) if err != nil { logger = logger.WithError(err) diff --git a/trigger/trigger_test.go b/trigger/trigger_test.go index 56977de1d..978d59226 100644 --- a/trigger/trigger_test.go +++ b/trigger/trigger_test.go @@ -63,6 +63,9 @@ func TestTrigger(t *testing.T) { mockConvertService := mock.NewMockConvertService(controller) mockConvertService.EXPECT().Convert(gomock.Any(), gomock.Any()).Return(dummyYaml, nil) + mockValidateService := mock.NewMockValidateService(controller) + mockValidateService.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + mockStatus := mock.NewMockStatusService(controller) mockStatus.EXPECT().Send(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Do(checkStatus) @@ -84,6 +87,7 @@ func TestTrigger(t *testing.T) { mockQueue, mockRepos, mockUsers, + mockValidateService, mockWebhooks, ) @@ -110,6 +114,7 @@ func TestTrigger_SkipCI(t *testing.T) { nil, nil, nil, + nil, ) dummyHookSkip := *dummyHook dummyHookSkip.Message = "foo [CI SKIP] bar" @@ -137,6 +142,7 @@ func TestTrigger_NoOwner(t *testing.T) { nil, mockUsers, nil, + nil, ) _, err := triggerer.Trigger(noContext, dummyRepo, dummyHook) @@ -167,6 +173,7 @@ func TestTrigger_MissingYaml(t *testing.T) { nil, mockUsers, nil, + nil, ) _, err := triggerer.Trigger(noContext, dummyRepo, dummyHook) @@ -206,6 +213,7 @@ func TestTrigger_ErrorYaml(t *testing.T) { mockRepos, mockUsers, nil, + nil, ) build, err := triggerer.Trigger(noContext, dummyRepo, dummyHook) @@ -239,6 +247,9 @@ func TestTrigger_SkipBranch(t *testing.T) { mockConvertService := mock.NewMockConvertService(controller) mockConvertService.EXPECT().Convert(gomock.Any(), gomock.Any()).Return(dummyYamlSkipBranch, nil) + mockValidateService := mock.NewMockValidateService(controller) + mockValidateService.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + triggerer := New( mockConfigService, mockConvertService, @@ -248,6 +259,7 @@ func TestTrigger_SkipBranch(t *testing.T) { nil, nil, mockUsers, + mockValidateService, nil, ) @@ -272,6 +284,9 @@ func TestTrigger_SkipEvent(t *testing.T) { mockConvertService := mock.NewMockConvertService(controller) mockConvertService.EXPECT().Convert(gomock.Any(), gomock.Any()).Return(dummyYamlSkipEvent, nil) + mockValidateService := mock.NewMockValidateService(controller) + mockValidateService.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + triggerer := New( mockConfigService, mockConvertService, @@ -281,6 +296,7 @@ func TestTrigger_SkipEvent(t *testing.T) { nil, nil, mockUsers, + mockValidateService, nil, ) @@ -305,6 +321,9 @@ func TestTrigger_SkipAction(t *testing.T) { mockConvertService := mock.NewMockConvertService(controller) mockConvertService.EXPECT().Convert(gomock.Any(), gomock.Any()).Return(dummyYamlSkipAction, nil) + mockValidateService := mock.NewMockValidateService(controller) + mockValidateService.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + triggerer := New( mockConfigService, mockConvertService, @@ -314,6 +333,7 @@ func TestTrigger_SkipAction(t *testing.T) { nil, nil, mockUsers, + mockValidateService, nil, ) @@ -342,6 +362,9 @@ func TestTrigger_ErrorIncrement(t *testing.T) { mockConvertService := mock.NewMockConvertService(controller) mockConvertService.EXPECT().Convert(gomock.Any(), gomock.Any()).Return(dummyYaml, nil) + mockValidateService := mock.NewMockValidateService(controller) + mockValidateService.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + triggerer := New( mockConfigService, mockConvertService, @@ -351,6 +374,7 @@ func TestTrigger_ErrorIncrement(t *testing.T) { nil, mockRepos, mockUsers, + mockValidateService, nil, )