diff --git a/MODULE.bazel b/MODULE.bazel index eabace86..d8e387f5 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -36,6 +36,7 @@ use_repo( "com_github_spf13_cobra", "com_github_stretchr_testify", "com_github_uber_go_tally_v4", + "in_gopkg_yaml_v3", "org_golang_google_grpc", "org_golang_google_protobuf", "org_golang_x_oauth2", diff --git a/Makefile b/Makefile index 241ea9d4..4fb97bb0 100644 --- a/Makefile +++ b/Makefile @@ -274,7 +274,7 @@ local-stovepipe-stop: ## Stop Stovepipe service mocks: ## Generate mock files using mockgen @echo "Generating mocks..." - @$(BAZEL) run @rules_go//go -- generate ./extension/storage/... ./extension/changestore/... ./extension/counter/... ./extension/queue/... ./extension/mergechecker/... ./extension/pusher/... ./extension/scorer/... ./extension/conflict/... ./core/consumer/... + @$(BAZEL) run @rules_go//go -- generate ./extension/storage/... ./extension/changestore/... ./extension/counter/... ./extension/queue/... ./extension/queueconfig/... ./extension/mergechecker/... ./extension/pusher/... ./extension/scorer/... ./extension/conflict/... ./core/consumer/... @echo "Mocks generated successfully!" proto: ## Generate protobuf files from .proto definitions diff --git a/entity/queue_config.go b/entity/queue_config.go index 57986aca..940d5319 100644 --- a/entity/queue_config.go +++ b/entity/queue_config.go @@ -42,7 +42,7 @@ type QueueConfig struct { Target string `json:"target" yaml:"target"` // BuildRunner identifies the CI pipeline or job that runs builds for this queue. - // Opaque to the system; meaningful only to the build extension implementation. + // Opaque to the system; meaningful only to the build runner extension implementation. // Examples: // - Buildkite: "buildkite.com/uber/submitqueue-ci" // - Jenkins: "jenkins.example.com/job/submitqueue-verify" diff --git a/example/server/docker-compose.yml b/example/server/docker-compose.yml index 16877d95..f93bd6a6 100644 --- a/example/server/docker-compose.yml +++ b/example/server/docker-compose.yml @@ -53,6 +53,8 @@ services: - MYSQL_DSN=root:root@tcp(mysql-app:3306)/submitqueue?parseTime=true # Queue infrastructure connection (separate database) - QUEUE_MYSQL_DSN=root:root@tcp(mysql-queue:3306)/submitqueue?parseTime=true + # Path to YAML queue configuration baked into the image + - QUEUE_CONFIG_PATH=/root/queues.yaml depends_on: mysql-app: condition: service_healthy diff --git a/example/server/gateway/BUILD.bazel b/example/server/gateway/BUILD.bazel index 801eae7f..9367297a 100644 --- a/example/server/gateway/BUILD.bazel +++ b/example/server/gateway/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//core/consumer", "//extension/counter/mysql", "//extension/queue/mysql", + "//extension/queueconfig/yaml", "//extension/storage/mysql", "//gateway/controller", "//gateway/protopb", diff --git a/example/server/gateway/Dockerfile b/example/server/gateway/Dockerfile index 4a581628..eab468fd 100644 --- a/example/server/gateway/Dockerfile +++ b/example/server/gateway/Dockerfile @@ -7,6 +7,10 @@ WORKDIR /root/ # Built via: make build-gateway-linux COPY .docker-bin/gateway ./gateway +# Sample queue configuration; the gateway reads it on startup via +# QUEUE_CONFIG_PATH (set in docker-compose.yml). +COPY example/server/gateway/queues.yaml ./queues.yaml + EXPOSE 8080 CMD ["./gateway"] diff --git a/example/server/gateway/docker-compose.yml b/example/server/gateway/docker-compose.yml index b5fc783d..3d55cdbb 100644 --- a/example/server/gateway/docker-compose.yml +++ b/example/server/gateway/docker-compose.yml @@ -53,6 +53,8 @@ services: - MYSQL_DSN=root:root@tcp(mysql-app:3306)/submitqueue?parseTime=true # Queue infrastructure connection (separate database) - QUEUE_MYSQL_DSN=root:root@tcp(mysql-queue:3306)/submitqueue?parseTime=true + # Path to YAML queue configuration baked into the image + - QUEUE_CONFIG_PATH=/root/queues.yaml depends_on: mysql-app: condition: service_healthy diff --git a/example/server/gateway/main.go b/example/server/gateway/main.go index c449743b..5088bd19 100644 --- a/example/server/gateway/main.go +++ b/example/server/gateway/main.go @@ -31,6 +31,7 @@ import ( "github.com/uber/submitqueue/core/consumer" mysqlcounter "github.com/uber/submitqueue/extension/counter/mysql" queueMySQL "github.com/uber/submitqueue/extension/queue/mysql" + yamlqueueconfig "github.com/uber/submitqueue/extension/queueconfig/yaml" mysqlstorage "github.com/uber/submitqueue/extension/storage/mysql" "github.com/uber/submitqueue/gateway/controller" pb "github.com/uber/submitqueue/gateway/protopb" @@ -175,9 +176,20 @@ func run() error { // Initialize request log store from shared app database connection requestLogStore := mysqlstorage.NewRequestLogStore(appDB, scope.SubScope("request_log_store")) + // Load queue configurations from YAML. Path is required so the gateway + // can reject requests for unknown queues at the edge. + queueConfigPath := os.Getenv("QUEUE_CONFIG_PATH") + if queueConfigPath == "" { + return fmt.Errorf("QUEUE_CONFIG_PATH environment variable is required") + } + queueConfigs, err := yamlqueueconfig.NewStore(queueConfigPath) + if err != nil { + return fmt.Errorf("failed to load queue configs: %w", err) + } + // Create controllers and wrap them for gRPC pingController := controller.NewPingController(logger, scope) - landController := controller.NewLandController(logger.Sugar(), scope, cnt, requestLogStore, registry) + landController := controller.NewLandController(logger.Sugar(), scope, cnt, requestLogStore, queueConfigs, registry) gatewayServer := &GatewayServer{ pingController: pingController, landController: landController, diff --git a/example/server/gateway/queues.yaml b/example/server/gateway/queues.yaml new file mode 100644 index 00000000..84d9aa49 --- /dev/null +++ b/example/server/gateway/queues.yaml @@ -0,0 +1,22 @@ +# Example queue configurations consumed by the gateway YAML store. +# The gateway loads this file at startup; QUEUE_CONFIG_PATH must point +# at it. Each entry maps a queue name to the VCS repository + target +# branch and selects the extension implementations used downstream. +queues: + - name: test-queue + vcs_type: git + vcs_address: git@github.com:uber/submitqueue.git + target: main + build_runner: buildkite.com/uber/submitqueue-ci + change_provider: github + merge_checker: github + land_provider: github + + - name: e2e-test-queue + vcs_type: git + vcs_address: git@github.com:uber/submitqueue.git + target: main + build_runner: buildkite.com/uber/submitqueue-ci + change_provider: github + merge_checker: github + land_provider: github diff --git a/extension/queueconfig/mock/BUILD.bazel b/extension/queueconfig/mock/BUILD.bazel new file mode 100644 index 00000000..a220d409 --- /dev/null +++ b/extension/queueconfig/mock/BUILD.bazel @@ -0,0 +1,12 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "mock", + srcs = ["queueconfig_mock.go"], + importpath = "github.com/uber/submitqueue/extension/queueconfig/mock", + visibility = ["//visibility:public"], + deps = [ + "//entity", + "@org_uber_go_mock//gomock", + ], +) diff --git a/extension/queueconfig/mock/queueconfig_mock.go b/extension/queueconfig/mock/queueconfig_mock.go new file mode 100644 index 00000000..e3717164 --- /dev/null +++ b/extension/queueconfig/mock/queueconfig_mock.go @@ -0,0 +1,72 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: queueconfig.go +// +// Generated by this command: +// +// mockgen -source=queueconfig.go -destination=mock/queueconfig_mock.go -package=mock +// + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + entity "github.com/uber/submitqueue/entity" + gomock "go.uber.org/mock/gomock" +) + +// MockStore is a mock of Store interface. +type MockStore struct { + ctrl *gomock.Controller + recorder *MockStoreMockRecorder + isgomock struct{} +} + +// MockStoreMockRecorder is the mock recorder for MockStore. +type MockStoreMockRecorder struct { + mock *MockStore +} + +// NewMockStore creates a new mock instance. +func NewMockStore(ctrl *gomock.Controller) *MockStore { + mock := &MockStore{ctrl: ctrl} + mock.recorder = &MockStoreMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockStore) EXPECT() *MockStoreMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockStore) Get(ctx context.Context, name string) (entity.QueueConfig, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", ctx, name) + ret0, _ := ret[0].(entity.QueueConfig) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockStoreMockRecorder) Get(ctx, name any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockStore)(nil).Get), ctx, name) +} + +// List mocks base method. +func (m *MockStore) List(ctx context.Context) ([]entity.QueueConfig, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "List", ctx) + ret0, _ := ret[0].([]entity.QueueConfig) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List. +func (mr *MockStoreMockRecorder) List(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockStore)(nil).List), ctx) +} diff --git a/extension/queueconfig/queueconfig.go b/extension/queueconfig/queueconfig.go index ba87a203..d056d46d 100644 --- a/extension/queueconfig/queueconfig.go +++ b/extension/queueconfig/queueconfig.go @@ -14,6 +14,8 @@ package queueconfig +//go:generate mockgen -source=queueconfig.go -destination=mock/queueconfig_mock.go -package=mock + import ( "context" "errors" diff --git a/extension/queueconfig/yaml/BUILD.bazel b/extension/queueconfig/yaml/BUILD.bazel new file mode 100644 index 00000000..21c06957 --- /dev/null +++ b/extension/queueconfig/yaml/BUILD.bazel @@ -0,0 +1,24 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "yaml", + srcs = ["yaml.go"], + importpath = "github.com/uber/submitqueue/extension/queueconfig/yaml", + visibility = ["//visibility:public"], + deps = [ + "//entity", + "//extension/queueconfig", + "@in_gopkg_yaml_v3//:yaml_v3", + ], +) + +go_test( + name = "yaml_test", + srcs = ["yaml_test.go"], + embed = [":yaml"], + deps = [ + "//extension/queueconfig", + "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", + ], +) diff --git a/extension/queueconfig/yaml/yaml.go b/extension/queueconfig/yaml/yaml.go new file mode 100644 index 00000000..ab339eec --- /dev/null +++ b/extension/queueconfig/yaml/yaml.go @@ -0,0 +1,90 @@ +// 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 yaml provides a YAML-file-backed implementation of +// queueconfig.Store. The file is read once at construction time and held +// in memory; the file is not watched for changes. +package yaml + +import ( + "context" + "fmt" + "os" + + "github.com/uber/submitqueue/entity" + "github.com/uber/submitqueue/extension/queueconfig" + yamlv3 "gopkg.in/yaml.v3" +) + +// fileContents is the top-level YAML schema. A configuration file is a +// single document with a "queues" key holding a list of QueueConfig. +type fileContents struct { + Queues []entity.QueueConfig `yaml:"queues"` +} + +// Store is a queueconfig.Store backed by an in-memory snapshot of a YAML +// file. Construct via NewStore. Safe for concurrent reads. +type Store struct { + byName map[string]entity.QueueConfig + all []entity.QueueConfig +} + +// NewStore reads queue configurations from the YAML file at path and +// returns a Store. If the file omits the top-level "queues" key, it is +// treated as an empty queue list. +// Returns an error if the file is unreadable, malformed, contains a queue +// with an empty name, or contains duplicate queue names. +func NewStore(path string) (Store, error) { + data, err := os.ReadFile(path) + if err != nil { + return Store{}, fmt.Errorf("failed to read queue config file %q: %w", path, err) + } + + var contents fileContents + if err := yamlv3.Unmarshal(data, &contents); err != nil { + return Store{}, fmt.Errorf("failed to parse queue config file %q: %w", path, err) + } + + byName := make(map[string]entity.QueueConfig, len(contents.Queues)) + for _, q := range contents.Queues { + if q.Name == "" { + return Store{}, fmt.Errorf("queue config in %q has empty name", path) + } + if _, dup := byName[q.Name]; dup { + return Store{}, fmt.Errorf("queue config in %q has duplicate name %q", path, q.Name) + } + byName[q.Name] = q + } + + all := make([]entity.QueueConfig, len(contents.Queues)) + copy(all, contents.Queues) + + return Store{byName: byName, all: all}, nil +} + +// Get returns the configuration for the named queue, or queueconfig.ErrNotFound. +func (s Store) Get(_ context.Context, name string) (entity.QueueConfig, error) { + cfg, ok := s.byName[name] + if !ok { + return entity.QueueConfig{}, queueconfig.ErrNotFound + } + return cfg, nil +} + +// List returns a copy of all configured queues in file order. +func (s Store) List(_ context.Context) ([]entity.QueueConfig, error) { + out := make([]entity.QueueConfig, len(s.all)) + copy(out, s.all) + return out, nil +} diff --git a/extension/queueconfig/yaml/yaml_test.go b/extension/queueconfig/yaml/yaml_test.go new file mode 100644 index 00000000..a8bb21ec --- /dev/null +++ b/extension/queueconfig/yaml/yaml_test.go @@ -0,0 +1,165 @@ +// 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 yaml + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber/submitqueue/extension/queueconfig" +) + +func writeTempYAML(t *testing.T, contents string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "queues.yaml") + require.NoError(t, os.WriteFile(path, []byte(contents), 0o600)) + return path +} + +func TestNewStore(t *testing.T) { + cases := []struct { + name string + yaml string + wantErr bool + wantSize int + }{ + { + name: "single queue", + yaml: `queues: + - name: main + vcs_type: git + vcs_address: git@github.com:uber/submitqueue.git + target: main +`, + wantSize: 1, + }, + { + name: "multiple queues", + yaml: `queues: + - name: main + vcs_type: git + vcs_address: git@github.com:uber/submitqueue.git + target: main + - name: release + vcs_type: git + vcs_address: git@github.com:uber/submitqueue.git + target: release/v2 +`, + wantSize: 2, + }, + { + name: "empty queues list", + yaml: `queues: []`, + wantSize: 0, + }, + { + name: "missing queues key", + yaml: `other: value`, + wantSize: 0, + }, + { + name: "duplicate names rejected", + yaml: `queues: + - name: main + target: main + - name: main + target: release +`, + wantErr: true, + }, + { + name: "empty name rejected", + yaml: `queues: + - target: main +`, + wantErr: true, + }, + { + name: "malformed yaml rejected", + yaml: `queues: [`, + wantErr: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + path := writeTempYAML(t, tc.yaml) + store, err := NewStore(path) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + got, err := store.List(context.Background()) + require.NoError(t, err) + assert.Len(t, got, tc.wantSize) + }) + } +} + +func TestNewStore_MissingFile(t *testing.T) { + _, err := NewStore(filepath.Join(t.TempDir(), "does-not-exist.yaml")) + require.Error(t, err) +} + +func TestStore_Get(t *testing.T) { + path := writeTempYAML(t, `queues: + - name: main + vcs_type: git + vcs_address: git@github.com:uber/submitqueue.git + target: main + change_provider: github +`) + store, err := NewStore(path) + require.NoError(t, err) + + t.Run("known queue returns config", func(t *testing.T) { + cfg, err := store.Get(context.Background(), "main") + require.NoError(t, err) + assert.Equal(t, "main", cfg.Name) + assert.Equal(t, "git", cfg.VCSType) + assert.Equal(t, "github", cfg.ChangeProvider) + }) + + t.Run("unknown queue returns ErrNotFound", func(t *testing.T) { + _, err := store.Get(context.Background(), "nope") + require.Error(t, err) + assert.True(t, errors.Is(err, queueconfig.ErrNotFound)) + }) +} + +func TestStore_List_ReturnsCopy(t *testing.T) { + path := writeTempYAML(t, `queues: + - name: main + target: main +`) + store, err := NewStore(path) + require.NoError(t, err) + + first, err := store.List(context.Background()) + require.NoError(t, err) + require.Len(t, first, 1) + first[0].Name = "mutated" + + second, err := store.List(context.Background()) + require.NoError(t, err) + assert.Equal(t, "main", second[0].Name, "mutating returned slice must not affect store") +} diff --git a/gateway/controller/BUILD.bazel b/gateway/controller/BUILD.bazel index 627babbd..85190ecd 100644 --- a/gateway/controller/BUILD.bazel +++ b/gateway/controller/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//entity", "//entity/queue", "//extension/counter", + "//extension/queueconfig", "//extension/storage", "//gateway/protopb", "@com_github_uber_go_tally_v4//:tally", @@ -30,10 +31,13 @@ go_test( embed = [":controller"], deps = [ "//core/consumer", + "//core/errs", "//entity", "//entity/queue", "//extension/counter/mock", "//extension/queue/mock", + "//extension/queueconfig", + "//extension/queueconfig/mock", "//extension/storage/mock", "//gateway/protopb", "@com_github_stretchr_testify//assert", diff --git a/gateway/controller/land.go b/gateway/controller/land.go index a43bfc93..16936dd9 100644 --- a/gateway/controller/land.go +++ b/gateway/controller/land.go @@ -26,6 +26,7 @@ import ( "github.com/uber/submitqueue/entity" "github.com/uber/submitqueue/entity/queue" "github.com/uber/submitqueue/extension/counter" + "github.com/uber/submitqueue/extension/queueconfig" "github.com/uber/submitqueue/extension/storage" pb "github.com/uber/submitqueue/gateway/protopb" "go.uber.org/zap" @@ -40,24 +41,44 @@ func IsInvalidRequest(err error) bool { return errors.Is(err, ErrInvalidRequest) } +// UnrecognizedQueueError indicates the request named a queue that is not +// present in the queue configuration store. +type UnrecognizedQueueError struct { + Queue string +} + +// Error implements the error interface. +func (e *UnrecognizedQueueError) Error() string { + return fmt.Sprintf("unrecognized queue %q", e.Queue) +} + +// IsUnrecognizedQueue returns true if any error in the chain is an +// *UnrecognizedQueueError. +func IsUnrecognizedQueue(err error) bool { + var target *UnrecognizedQueueError + return errors.As(err, &target) +} + // LandController handles land business logic for the gateway type LandController struct { logger *zap.SugaredLogger metricsScope tally.Scope counter counter.Counter requestLogStore storage.RequestLogStore + queueConfigs queueconfig.Store registry consumer.TopicRegistry } // NewLandController creates a new instance of the gateway land controller. // The controller publishes land requests to the topic registered under // consumer.TopicKeyStart in the registry. -func NewLandController(logger *zap.SugaredLogger, scope tally.Scope, counter counter.Counter, requestLogStore storage.RequestLogStore, registry consumer.TopicRegistry) *LandController { +func NewLandController(logger *zap.SugaredLogger, scope tally.Scope, counter counter.Counter, requestLogStore storage.RequestLogStore, queueConfigs queueconfig.Store, registry consumer.TopicRegistry) *LandController { return &LandController{ logger: logger, metricsScope: scope, counter: counter, requestLogStore: requestLogStore, + queueConfigs: queueConfigs, registry: registry, } } @@ -83,8 +104,13 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan URIs: req.Change.GetUris(), } - // TODO: validate that queue is configured. Return error if not. queue := req.Queue + if _, err := c.queueConfigs.Get(ctx, queue); err != nil { + if errors.Is(err, queueconfig.ErrNotFound) { + return nil, errs.NewUserError(&UnrecognizedQueueError{Queue: queue}) + } + return nil, fmt.Errorf("LandController failed to look up queue %q: %w", queue, err) + } // TODO: pass default queue land strategy to resolver function to process a default. strategy, err := resolveRequestLandStrategy(req.Strategy) diff --git a/gateway/controller/land_test.go b/gateway/controller/land_test.go index 1d629f5e..9f02276b 100644 --- a/gateway/controller/land_test.go +++ b/gateway/controller/land_test.go @@ -23,10 +23,13 @@ import ( "github.com/stretchr/testify/require" "github.com/uber-go/tally/v4" "github.com/uber/submitqueue/core/consumer" + "github.com/uber/submitqueue/core/errs" "github.com/uber/submitqueue/entity" "github.com/uber/submitqueue/entity/queue" countermock "github.com/uber/submitqueue/extension/counter/mock" queuemock "github.com/uber/submitqueue/extension/queue/mock" + "github.com/uber/submitqueue/extension/queueconfig" + qcmock "github.com/uber/submitqueue/extension/queueconfig/mock" storagemock "github.com/uber/submitqueue/extension/storage/mock" pb "github.com/uber/submitqueue/gateway/protopb" "go.uber.org/mock/gomock" @@ -65,11 +68,19 @@ func noopRequestLogStore(ctrl *gomock.Controller) *storagemock.MockRequestLogSto return s } +// noopQueueConfigStore returns a mock queueconfig.Store that always reports +// the queue as configured. +func noopQueueConfigStore(ctrl *gomock.Controller) *qcmock.MockStore { + s := qcmock.NewMockStore(ctrl) + s.EXPECT().Get(gomock.Any(), gomock.Any()).Return(entity.QueueConfig{}, nil).AnyTimes() + return s +} + func TestNewLandController(t *testing.T) { ctrl := gomock.NewController(t) cnt := countermock.NewMockCounter(ctrl) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) require.NotNil(t, controller) } @@ -78,7 +89,7 @@ func TestLand_ReturnsSqid(t *testing.T) { cnt := countermock.NewMockCounter(ctrl) cnt.EXPECT().Next(gomock.Any(), gomock.Any()).Return(int64(1), nil) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) ctx := context.Background() req := &pb.LandRequest{ @@ -96,7 +107,7 @@ func TestLand_ReturnsErrorOnCounterFailure(t *testing.T) { cnt := countermock.NewMockCounter(ctrl) cnt.EXPECT().Next(gomock.Any(), gomock.Any()).Return(int64(0), fmt.Errorf("counter unavailable")) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) ctx := context.Background() req := &pb.LandRequest{ @@ -120,7 +131,7 @@ func TestLand_CounterDomainIncludesQueue(t *testing.T) { return 1, nil }, ) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) ctx := context.Background() req := &pb.LandRequest{ @@ -137,7 +148,7 @@ func TestLand_ReturnsErrorOnEmptyQueue(t *testing.T) { ctrl := gomock.NewController(t) cnt := countermock.NewMockCounter(ctrl) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) ctx := context.Background() req := &pb.LandRequest{ @@ -154,7 +165,7 @@ func TestLand_ReturnsErrorOnEmptyChangeUri(t *testing.T) { ctrl := gomock.NewController(t) cnt := countermock.NewMockCounter(ctrl) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) ctx := context.Background() req := &pb.LandRequest{ @@ -171,7 +182,7 @@ func TestLand_ReturnsErrorOnNilChange(t *testing.T) { ctrl := gomock.NewController(t) cnt := countermock.NewMockCounter(ctrl) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), newTestRegistryWithNoopPublisher(t, ctrl)) ctx := context.Background() req := &pb.LandRequest{ @@ -184,6 +195,53 @@ func TestLand_ReturnsErrorOnNilChange(t *testing.T) { assert.True(t, IsInvalidRequest(err)) } +func TestLand_ReturnsUnrecognizedQueueWhenStoreReportsNotFound(t *testing.T) { + ctrl := gomock.NewController(t) + + cnt := countermock.NewMockCounter(ctrl) + qcs := qcmock.NewMockStore(ctrl) + qcs.EXPECT().Get(gomock.Any(), "missing-queue").Return(entity.QueueConfig{}, queueconfig.ErrNotFound) + + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), qcs, newTestRegistryWithNoopPublisher(t, ctrl)) + ctx := context.Background() + + req := &pb.LandRequest{ + Queue: "missing-queue", + Change: &pb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + } + _, err := controller.Land(ctx, req) + + require.Error(t, err) + assert.True(t, IsUnrecognizedQueue(err)) + assert.True(t, errs.IsUserError(err)) + assert.False(t, errs.IsRetryable(err)) + + var typed *UnrecognizedQueueError + require.ErrorAs(t, err, &typed) + assert.Equal(t, "missing-queue", typed.Queue) +} + +func TestLand_PropagatesQueueConfigStoreError(t *testing.T) { + ctrl := gomock.NewController(t) + + cnt := countermock.NewMockCounter(ctrl) + qcs := qcmock.NewMockStore(ctrl) + qcs.EXPECT().Get(gomock.Any(), "test-queue").Return(entity.QueueConfig{}, fmt.Errorf("config backend down")) + + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), qcs, newTestRegistryWithNoopPublisher(t, ctrl)) + ctx := context.Background() + + req := &pb.LandRequest{ + Queue: "test-queue", + Change: &pb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + } + _, err := controller.Land(ctx, req) + + require.Error(t, err) + assert.False(t, IsUnrecognizedQueue(err)) + assert.False(t, IsInvalidRequest(err)) +} + func TestLand_PublishesToQueue(t *testing.T) { var publishedTopic string var publishedMessage queue.Message @@ -202,7 +260,7 @@ func TestLand_PublishesToQueue(t *testing.T) { }, ) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), registry) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), registry) ctx := context.Background() req := &pb.LandRequest{ @@ -238,7 +296,7 @@ func TestLand_ContinuesWhenPublishFails(t *testing.T) { registry, publisher := newTestRegistry(t, ctrl) publisher.EXPECT().Publish(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("queue unavailable")) - controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), registry) + controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, cnt, noopRequestLogStore(ctrl), noopQueueConfigStore(ctrl), registry) ctx := context.Background() req := &pb.LandRequest{ diff --git a/go.mod b/go.mod index 0f6b9c46..46eadf56 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( golang.org/x/oauth2 v0.34.0 google.golang.org/grpc v1.79.3 google.golang.org/protobuf v1.36.10 + gopkg.in/yaml.v3 v3.0.1 ) require ( @@ -56,6 +57,5 @@ require ( golang.org/x/tools/go/expect v0.1.1-deprecated // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect honnef.co/go/tools v0.4.3 // indirect )