diff --git a/Makefile b/Makefile index 5aeff768..a0178520 100644 --- a/Makefile +++ b/Makefile @@ -364,7 +364,7 @@ local-stovepipe-stop: ## Stop the Stovepipe service mocks: ## Generate mock files using mockgen @echo "Generating mocks..." - @$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./submitqueue/extension/mergechecker/... ./submitqueue/extension/pusher/... ./submitqueue/extension/scorer/... ./submitqueue/extension/conflict/... ./platform/consumer/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/... + @$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./submitqueue/extension/mergechecker/... ./submitqueue/extension/pusher/... ./submitqueue/extension/scorer/... ./submitqueue/extension/conflict/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/... @echo "Mocks generated successfully!" proto: ## Generate protobuf files from .proto definitions diff --git a/service/submitqueue/orchestrator/server/BUILD.bazel b/service/submitqueue/orchestrator/server/BUILD.bazel index 8cf638a2..3bd852c8 100644 --- a/service/submitqueue/orchestrator/server/BUILD.bazel +++ b/service/submitqueue/orchestrator/server/BUILD.bazel @@ -43,6 +43,7 @@ go_library( "//submitqueue/extension/scorer/heuristic:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mysql:go_default_library", + "//submitqueue/extension/validator/fake:go_default_library", "//submitqueue/orchestrator/controller:go_default_library", "//submitqueue/orchestrator/controller/batch:go_default_library", "//submitqueue/orchestrator/controller/build:go_default_library", diff --git a/service/submitqueue/orchestrator/server/main.go b/service/submitqueue/orchestrator/server/main.go index 2ed845c3..2038b2e7 100644 --- a/service/submitqueue/orchestrator/server/main.go +++ b/service/submitqueue/orchestrator/server/main.go @@ -63,6 +63,7 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/scorer/heuristic" "github.com/uber/submitqueue/submitqueue/extension/storage" mysqlstorage "github.com/uber/submitqueue/submitqueue/extension/storage/mysql" + validatorfake "github.com/uber/submitqueue/submitqueue/extension/validator/fake" "github.com/uber/submitqueue/submitqueue/orchestrator/controller" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/batch" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/build" @@ -571,6 +572,7 @@ func registerPrimaryControllers(c consumer.Consumer, logger *zap.SugaredLogger, store, registry, cpf, + validatorfake.NewFactory(), runwaymq.TopicKeyMergeConflictCheck, topickey.TopicKeyValidate, "orchestrator-validate", diff --git a/submitqueue/extension/validator/BUILD.bazel b/submitqueue/extension/validator/BUILD.bazel new file mode 100644 index 00000000..39c569c5 --- /dev/null +++ b/submitqueue/extension/validator/BUILD.bazel @@ -0,0 +1,9 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["validator.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/validator", + visibility = ["//visibility:public"], + deps = ["//submitqueue/entity:go_default_library"], +) diff --git a/submitqueue/extension/validator/composite/BUILD.bazel b/submitqueue/extension/validator/composite/BUILD.bazel new file mode 100644 index 00000000..434491e6 --- /dev/null +++ b/submitqueue/extension/validator/composite/BUILD.bazel @@ -0,0 +1,26 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["validator.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/validator/composite", + visibility = ["//visibility:public"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/validator:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["validator_test.go"], + deps = [ + ":go_default_library", + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/validator:go_default_library", + "//submitqueue/extension/validator/mock:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + "@org_uber_go_mock//gomock:go_default_library", + ], +) diff --git a/submitqueue/extension/validator/composite/validator.go b/submitqueue/extension/validator/composite/validator.go new file mode 100644 index 00000000..5bdca52f --- /dev/null +++ b/submitqueue/extension/validator/composite/validator.go @@ -0,0 +1,44 @@ +// Copyright (c) 2025 Uber Technologies, 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. + +package composite + +import ( + "context" + "errors" + + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/validator" +) + +// compositeValidator runs all validators and joins their errors. +type compositeValidator struct { + // validators is the set of validators to run. + validators []validator.Validator +} + +// New creates a Validator that evaluates all child validators and joins their errors. +func New(validators []validator.Validator) validator.Validator { + return &compositeValidator{validators: validators} +} + +func (c *compositeValidator) Validate(ctx context.Context, request entity.Request) error { + var errs []error + for _, v := range c.validators { + if err := v.Validate(ctx, request); err != nil { + errs = append(errs, err) + } + } + return errors.Join(errs...) +} diff --git a/submitqueue/extension/validator/composite/validator_test.go b/submitqueue/extension/validator/composite/validator_test.go new file mode 100644 index 00000000..9785e31a --- /dev/null +++ b/submitqueue/extension/validator/composite/validator_test.go @@ -0,0 +1,107 @@ +// Copyright (c) 2025 Uber Technologies, 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. + +package composite_test + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/validator" + "github.com/uber/submitqueue/submitqueue/extension/validator/composite" + validatormock "github.com/uber/submitqueue/submitqueue/extension/validator/mock" + "go.uber.org/mock/gomock" +) + +func TestNew(t *testing.T) { + errA := errors.New("validation A failed") + errB := errors.New("validation B failed") + + tests := []struct { + name string + setup func(v1, v2 *validatormock.MockValidator) []validator.Validator + wantErrs []error + }{ + { + name: "no validators returns no error", + setup: func(v1, v2 *validatormock.MockValidator) []validator.Validator { return nil }, + }, + { + name: "single passing validator", + setup: func(v1, v2 *validatormock.MockValidator) []validator.Validator { + v1.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + return []validator.Validator{v1} + }, + }, + { + name: "single failing validator", + setup: func(v1, v2 *validatormock.MockValidator) []validator.Validator { + v1.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(errA) + return []validator.Validator{v1} + }, + wantErrs: []error{errA}, + }, + { + name: "all pass", + setup: func(v1, v2 *validatormock.MockValidator) []validator.Validator { + v1.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + v2.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + return []validator.Validator{v1, v2} + }, + }, + { + name: "some fail", + setup: func(v1, v2 *validatormock.MockValidator) []validator.Validator { + v1.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + v2.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(errA) + return []validator.Validator{v1, v2} + }, + wantErrs: []error{errA}, + }, + { + name: "all fail", + setup: func(v1, v2 *validatormock.MockValidator) []validator.Validator { + v1.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(errA) + v2.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(errB) + return []validator.Validator{v1, v2} + }, + wantErrs: []error{errA, errB}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + v1 := validatormock.NewMockValidator(ctrl) + v2 := validatormock.NewMockValidator(ctrl) + validators := tt.setup(v1, v2) + + v := composite.New(validators) + + err := v.Validate(context.Background(), entity.Request{}) + if len(tt.wantErrs) == 0 { + assert.NoError(t, err) + return + } + require.Error(t, err) + for _, want := range tt.wantErrs { + assert.ErrorIs(t, err, want) + } + }) + } +} diff --git a/submitqueue/extension/validator/fake/BUILD.bazel b/submitqueue/extension/validator/fake/BUILD.bazel new file mode 100644 index 00000000..1c863b4a --- /dev/null +++ b/submitqueue/extension/validator/fake/BUILD.bazel @@ -0,0 +1,23 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["fake.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/validator/fake", + visibility = ["//visibility:public"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/validator:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["fake_test.go"], + embed = [":go_default_library"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/validator:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + ], +) diff --git a/submitqueue/extension/validator/fake/fake.go b/submitqueue/extension/validator/fake/fake.go new file mode 100644 index 00000000..342bda6e --- /dev/null +++ b/submitqueue/extension/validator/fake/fake.go @@ -0,0 +1,47 @@ +// Copyright (c) 2025 Uber Technologies, 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. + +// Package fake provides a validator.Validator that always passes and a +// validator.Factory that returns it. Intended for examples, tests, and default +// wiring when no custom validation is needed. +package fake + +import ( + "context" + + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/validator" +) + +type fakeValidator struct{} + +// New returns a validator.Validator that always passes (returns nil). +func New() validator.Validator { + return fakeValidator{} +} + +func (fakeValidator) Validate(context.Context, entity.Request) error { + return nil +} + +type fakeFactory struct{} + +// NewFactory returns a validator.Factory that always returns a passing validator. +func NewFactory() validator.Factory { + return fakeFactory{} +} + +func (fakeFactory) For(validator.Config) (validator.Validator, error) { + return fakeValidator{}, nil +} diff --git a/submitqueue/extension/validator/fake/fake_test.go b/submitqueue/extension/validator/fake/fake_test.go new file mode 100644 index 00000000..5f914250 --- /dev/null +++ b/submitqueue/extension/validator/fake/fake_test.go @@ -0,0 +1,44 @@ +// Copyright (c) 2025 Uber Technologies, 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. + +package fake + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/validator" +) + +func TestNew_ImplementsInterface(t *testing.T) { + var _ validator.Validator = New() +} + +func TestNew_AlwaysPasses(t *testing.T) { + v := New() + assert.NoError(t, v.Validate(context.Background(), entity.Request{})) +} + +func TestNewFactory_ImplementsInterface(t *testing.T) { + var _ validator.Factory = NewFactory() +} + +func TestNewFactory_ReturnsPassingValidator(t *testing.T) { + f := NewFactory() + v, err := f.For(validator.Config{QueueName: "test-queue"}) + assert.NoError(t, err) + assert.NoError(t, v.Validate(context.Background(), entity.Request{})) +} diff --git a/submitqueue/extension/validator/mock/BUILD.bazel b/submitqueue/extension/validator/mock/BUILD.bazel new file mode 100644 index 00000000..e07ef68e --- /dev/null +++ b/submitqueue/extension/validator/mock/BUILD.bazel @@ -0,0 +1,13 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["validator_mock.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/validator/mock", + visibility = ["//visibility:public"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/validator:go_default_library", + "@org_uber_go_mock//gomock:go_default_library", + ], +) diff --git a/submitqueue/extension/validator/mock/validator_mock.go b/submitqueue/extension/validator/mock/validator_mock.go new file mode 100644 index 00000000..89b047ad --- /dev/null +++ b/submitqueue/extension/validator/mock/validator_mock.go @@ -0,0 +1,96 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: validator.go +// +// Generated by this command: +// +// mockgen -source=validator.go -destination=mock/validator_mock.go -package=mock +// + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + entity "github.com/uber/submitqueue/submitqueue/entity" + validator "github.com/uber/submitqueue/submitqueue/extension/validator" + gomock "go.uber.org/mock/gomock" +) + +// MockValidator is a mock of Validator interface. +type MockValidator struct { + ctrl *gomock.Controller + recorder *MockValidatorMockRecorder + isgomock struct{} +} + +// MockValidatorMockRecorder is the mock recorder for MockValidator. +type MockValidatorMockRecorder struct { + mock *MockValidator +} + +// NewMockValidator creates a new mock instance. +func NewMockValidator(ctrl *gomock.Controller) *MockValidator { + mock := &MockValidator{ctrl: ctrl} + mock.recorder = &MockValidatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockValidator) EXPECT() *MockValidatorMockRecorder { + return m.recorder +} + +// Validate mocks base method. +func (m *MockValidator) Validate(ctx context.Context, request entity.Request) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Validate", ctx, request) + ret0, _ := ret[0].(error) + return ret0 +} + +// Validate indicates an expected call of Validate. +func (mr *MockValidatorMockRecorder) Validate(ctx, request any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Validate", reflect.TypeOf((*MockValidator)(nil).Validate), ctx, request) +} + +// MockFactory is a mock of Factory interface. +type MockFactory struct { + ctrl *gomock.Controller + recorder *MockFactoryMockRecorder + isgomock struct{} +} + +// MockFactoryMockRecorder is the mock recorder for MockFactory. +type MockFactoryMockRecorder struct { + mock *MockFactory +} + +// NewMockFactory creates a new mock instance. +func NewMockFactory(ctrl *gomock.Controller) *MockFactory { + mock := &MockFactory{ctrl: ctrl} + mock.recorder = &MockFactoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFactory) EXPECT() *MockFactoryMockRecorder { + return m.recorder +} + +// For mocks base method. +func (m *MockFactory) For(cfg validator.Config) (validator.Validator, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "For", cfg) + ret0, _ := ret[0].(validator.Validator) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// For indicates an expected call of For. +func (mr *MockFactoryMockRecorder) For(cfg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "For", reflect.TypeOf((*MockFactory)(nil).For), cfg) +} diff --git a/submitqueue/extension/validator/validator.go b/submitqueue/extension/validator/validator.go new file mode 100644 index 00000000..c3929c31 --- /dev/null +++ b/submitqueue/extension/validator/validator.go @@ -0,0 +1,48 @@ +// Copyright (c) 2025 Uber Technologies, 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. + +package validator + +//go:generate mockgen -source=validator.go -destination=mock/validator_mock.go -package=mock + +import ( + "context" + + "github.com/uber/submitqueue/submitqueue/entity" +) + +// Validator runs custom validation checks against a request. +// Implementations are provided by integrators for company-specific checks +// (e.g. org-policy enforcement, repo-specific rules). The built-in validate +// controller invokes this after fetching change metadata and before claiming +// URIs in the change store. +type Validator interface { + // Validate runs custom validation on the request. + // Returns nil if validation passes, error if the request should be rejected. + Validate(ctx context.Context, request entity.Request) error +} + +// Config carries the routing identity handed to a Factory. +type Config struct { + // QueueName identifies the queue this request belongs to. + QueueName string +} + +// Factory builds the Validator for a given config. Implementations are provided +// by integrators and inject whatever they need at construction. +type Factory interface { + // For returns the Validator for the given config. Returning nil indicates + // no custom validation is needed for this config. + For(cfg Config) (Validator, error) +} diff --git a/submitqueue/orchestrator/controller/validate/BUILD.bazel b/submitqueue/orchestrator/controller/validate/BUILD.bazel index 7c012139..0b0beff7 100644 --- a/submitqueue/orchestrator/controller/validate/BUILD.bazel +++ b/submitqueue/orchestrator/controller/validate/BUILD.bazel @@ -17,6 +17,7 @@ go_library( "//submitqueue/entity:go_default_library", "//submitqueue/extension/changeprovider:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/extension/validator:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -40,6 +41,8 @@ go_test( "//submitqueue/extension/changeprovider/mock:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/extension/validator:go_default_library", + "//submitqueue/extension/validator/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/validate/validate.go b/submitqueue/orchestrator/controller/validate/validate.go index 222cdac3..390969d2 100644 --- a/submitqueue/orchestrator/controller/validate/validate.go +++ b/submitqueue/orchestrator/controller/validate/validate.go @@ -32,6 +32,7 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/changeprovider" "github.com/uber/submitqueue/submitqueue/extension/storage" + "github.com/uber/submitqueue/submitqueue/extension/validator" "go.uber.org/zap" ) @@ -46,6 +47,7 @@ type Controller struct { store storage.Storage registry consumer.TopicRegistry changeProviders changeprovider.Factory + validators validator.Factory runwayTopicKey consumer.TopicKey topicKey consumer.TopicKey consumerGroup string @@ -57,12 +59,14 @@ var _ consumer.Controller = (*Controller)(nil) // NewController creates a new validate controller for the orchestrator. // runwayTopicKey is the runway-owned topic the merge-conflict check request is // published to (TopicKeyMergeConflictCheck). +// validators is an optional factory for custom validation checks; pass nil to skip. func NewController( logger *zap.SugaredLogger, scope tally.Scope, store storage.Storage, registry consumer.TopicRegistry, changeProviders changeprovider.Factory, + validators validator.Factory, runwayTopicKey consumer.TopicKey, topicKey consumer.TopicKey, consumerGroup string, @@ -73,6 +77,7 @@ func NewController( store: store, registry: registry, changeProviders: changeProviders, + validators: validators, runwayTopicKey: runwayTopicKey, topicKey: topicKey, consumerGroup: consumerGroup, @@ -159,6 +164,24 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r "total_files", totalFiles(changeInfos), ) + // Run custom validation checks if a validator factory was provided. + if c.validators != nil { + cfg := validator.Config{ + QueueName: request.Queue, + } + v, err := c.validators.For(cfg) + if err != nil { + coremetrics.NamedCounter(c.metricsScope, "process", "validator_errors", 1) + return fmt.Errorf("failed to build validator for request %s: %w", request.ID, err) + } + if v != nil { + if err := v.Validate(ctx, request); err != nil { + coremetrics.NamedCounter(c.metricsScope, "process", "custom_validation_failures", 1) + return fmt.Errorf("custom validation failed for request %s: %w", request.ID, err) + } + } + } + // Claim each URI in the change store with its provider details. The claim is // created here — after duplicate detection and the merge/provider checks — so a // rejected request never leaves a claim, and the record is written once with its diff --git a/submitqueue/orchestrator/controller/validate/validate_test.go b/submitqueue/orchestrator/controller/validate/validate_test.go index 8f0c6fbc..d2c57efa 100644 --- a/submitqueue/orchestrator/controller/validate/validate_test.go +++ b/submitqueue/orchestrator/controller/validate/validate_test.go @@ -35,6 +35,8 @@ import ( changeprovidermock "github.com/uber/submitqueue/submitqueue/extension/changeprovider/mock" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + "github.com/uber/submitqueue/submitqueue/extension/validator" + validatormock "github.com/uber/submitqueue/submitqueue/extension/validator/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -119,7 +121,7 @@ func newTestController( cpFactory := changeprovidermock.NewMockFactory(ctrl) cpFactory.EXPECT().For(gomock.Any()).Return(cp, nil).AnyTimes() - return NewController(logger, scope, store, registry, cpFactory, runwaymq.TopicKeyMergeConflictCheck, topickey.TopicKeyValidate, "orchestrator-validate") + return NewController(logger, scope, store, registry, cpFactory, nil, runwaymq.TopicKeyMergeConflictCheck, topickey.TopicKeyValidate, "orchestrator-validate") } func TestNewController(t *testing.T) { @@ -201,7 +203,7 @@ func TestController_Process_PublishesCheckToRunway(t *testing.T) { cpFactory := changeprovidermock.NewMockFactory(ctrl) cpFactory.EXPECT().For(gomock.Any()).Return(&mockChangeProvider{}, nil).AnyTimes() - controller := NewController(logger, tally.NoopScope, store, registry, cpFactory, runwaymq.TopicKeyMergeConflictCheck, topickey.TopicKeyValidate, "orchestrator-validate") + controller := NewController(logger, tally.NoopScope, store, registry, cpFactory, nil, runwaymq.TopicKeyMergeConflictCheck, topickey.TopicKeyValidate, "orchestrator-validate") msg := entityqueue.NewMessage(request.ID, requestIDPayload(t, request.ID), request.Queue, nil) delivery := queuemock.NewMockDelivery(ctrl) @@ -563,3 +565,95 @@ func TestController_Process_TerminalShortCircuit(t *testing.T) { }) } } + +func TestController_Process_CustomValidatorPasses(t *testing.T) { + ctrl := gomock.NewController(t) + + request := entity.Request{ + ID: "test-queue/123", + Queue: "test-queue", + Change: change.Change{URIs: []string{"github://uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, + LandStrategy: mergestrategy.MergeStrategyRebase, + State: entity.RequestStateStarted, + Version: 1, + } + store, _ := newMockStorage(ctrl, request) + store.EXPECT().GetChangeStore().Return(newMockChangeStore(ctrl)).AnyTimes() + + logger := zaptest.NewLogger(t).Sugar() + + mockPub := queuemock.NewMockPublisher(ctrl) + mockPub.EXPECT().Publish(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockQ := queuemock.NewMockQueue(ctrl) + mockQ.EXPECT().Publisher().Return(mockPub).AnyTimes() + registry, err := consumer.NewTopicRegistry( + []consumer.TopicConfig{{Key: runwaymq.TopicKeyMergeConflictCheck, Name: "merge-conflict-check", Queue: mockQ}}, + ) + require.NoError(t, err) + + cpFactory := changeprovidermock.NewMockFactory(ctrl) + cpFactory.EXPECT().For(gomock.Any()).Return(&mockChangeProvider{}, nil).AnyTimes() + + mockValidator := validatormock.NewMockValidator(ctrl) + // mockValidator returns nil - validation succeeded + mockValidator.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(nil) + mockValidatorFactory := validatormock.NewMockFactory(ctrl) + mockValidatorFactory.EXPECT().For(validator.Config{ + QueueName: request.Queue, + }).Return(mockValidator, nil) + + controller := NewController(logger, tally.NoopScope, store, registry, cpFactory, mockValidatorFactory, runwaymq.TopicKeyMergeConflictCheck, topickey.TopicKeyValidate, "orchestrator-validate") + + msg := entityqueue.NewMessage(request.ID, requestIDPayload(t, request.ID), request.Queue, nil) + delivery := queuemock.NewMockDelivery(ctrl) + delivery.EXPECT().Message().Return(msg).AnyTimes() + delivery.EXPECT().Attempt().Return(1).AnyTimes() + + require.NoError(t, controller.Process(context.Background(), delivery)) +} + +func TestController_Process_CustomValidatorFails(t *testing.T) { + ctrl := gomock.NewController(t) + + request := entity.Request{ + ID: "test-queue/123", + Queue: "test-queue", + Change: change.Change{URIs: []string{"github://uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, + LandStrategy: mergestrategy.MergeStrategyRebase, + State: entity.RequestStateStarted, + Version: 1, + } + store, _ := newMockStorage(ctrl, request) + store.EXPECT().GetChangeStore().Return(newMockChangeStore(ctrl)).AnyTimes() + + logger := zaptest.NewLogger(t).Sugar() + + mockPub := queuemock.NewMockPublisher(ctrl) + mockQ := queuemock.NewMockQueue(ctrl) + mockQ.EXPECT().Publisher().Return(mockPub).AnyTimes() + registry, err := consumer.NewTopicRegistry( + []consumer.TopicConfig{{Key: runwaymq.TopicKeyMergeConflictCheck, Name: "merge-conflict-check", Queue: mockQ}}, + ) + require.NoError(t, err) + + cpFactory := changeprovidermock.NewMockFactory(ctrl) + cpFactory.EXPECT().For(gomock.Any()).Return(&mockChangeProvider{}, nil).AnyTimes() + + mockValidator := validatormock.NewMockValidator(ctrl) + // mockValidator returns an error - validation failed + mockValidator.EXPECT().Validate(gomock.Any(), gomock.Any()).Return(fmt.Errorf("some validation error")) + mockValidatorFactory := validatormock.NewMockFactory(ctrl) + mockValidatorFactory.EXPECT().For(validator.Config{ + QueueName: request.Queue, + }).Return(mockValidator, nil) + + controller := NewController(logger, tally.NoopScope, store, registry, cpFactory, mockValidatorFactory, runwaymq.TopicKeyMergeConflictCheck, topickey.TopicKeyValidate, "orchestrator-validate") + + msg := entityqueue.NewMessage(request.ID, requestIDPayload(t, request.ID), request.Queue, nil) + delivery := queuemock.NewMockDelivery(ctrl) + delivery.EXPECT().Message().Return(msg).AnyTimes() + delivery.EXPECT().Attempt().Return(1).AnyTimes() + + err = controller.Process(context.Background(), delivery) + require.ErrorContains(t, err, "some validation error") +}