Ensure all configs are used or checked in the constructor

config/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@rules_go//go:def.bzl", "go_library")
+load("@rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "config",
@@ -12,3 +12,10 @@ go_library(
     visibility = ["//visibility:public"],
     deps = ["@com_github_goccy_go_yaml//:go-yaml"],
 )
+
+go_test(
+    name = "config_test",
+    srcs = ["config_test.go"],
+    embed = [":config"],
+    deps = ["@com_github_stretchr_testify//require"],
+)

config/config.go

@@ -70,12 +70,24 @@ func Parse(configFilePath string) (*Config, error) {
 	if config.Service.WorkerPoolSize <= 0 {
 		return nil, fmt.Errorf("service.worker_pool_size must be > 0, got %d", config.Service.WorkerPoolSize)
 	}
+	if config.Service.Chunking.TargetChunkSize <= 0 {
+		return nil, fmt.Errorf("service.chunking.target_chunk_size must be > 0, got %d", config.Service.Chunking.TargetChunkSize)
+	}
+	if config.Service.Chunking.ChangedTargetChunkSize <= 0 {
+		return nil, fmt.Errorf("service.chunking.changed_target_chunk_size must be > 0, got %d", config.Service.Chunking.ChangedTargetChunkSize)
+	}
+	if config.Service.Chunking.MetadataMapChunkSize <= 0 {
+		return nil, fmt.Errorf("service.chunking.metadata_map_chunk_size must be > 0, got %d", config.Service.Chunking.MetadataMapChunkSize)
+	}
 	config.repositoryByRemote = make(map[string]*RepositoryConfig, len(config.Repository))
 	for i := range config.Repository {
 		remote := config.Repository[i].Remote
 		if remote == "" {
 			return nil, fmt.Errorf("repository[%d].remote must not be empty", i)
 		}
+		if config.Repository[i].QueryTimeoutSeconds <= 0 {
+			return nil, fmt.Errorf("repository[%d].query_timeout_seconds must be > 0", i)
+		}
 		if _, exists := config.repositoryByRemote[remote]; exists {
 			return nil, fmt.Errorf("duplicate repository remote %q", remote)
 		}

config/config_test.go

@@ -0,0 +1,186 @@
+// 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 config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestParse_Validation(t *testing.T) {
+	t.Parallel()
+
+	validConfig := `
+repository:
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+storage:
+  type: memory
+service:
+  worker_pool_size: 5
+  repo_manager_clone_path: /tmp/tango
+  chunking:
+    target_chunk_size: 250
+    changed_target_chunk_size: 125
+    metadata_map_chunk_size: 50000
+`
+
+	tests := []struct {
+		name    string
+		yaml    string
+		wantErr bool
+	}{
+		{
+			name: "valid config",
+			yaml: validConfig,
+		},
+		{
+			name: "worker_pool_size not set",
+			yaml: `
+repository:
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+service:
+  chunking:
+    target_chunk_size: 250
+    changed_target_chunk_size: 125
+    metadata_map_chunk_size: 50000
+`,
+			wantErr: true,
+		},
+		{
+			name: "target_chunk_size not set",
+			yaml: `
+repository:
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+service:
+  worker_pool_size: 5
+  chunking:
+    changed_target_chunk_size: 125
+    metadata_map_chunk_size: 50000
+`,
+			wantErr: true,
+		},
+		{
+			name: "changed_target_chunk_size not set",
+			yaml: `
+repository:
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+service:
+  worker_pool_size: 5
+  chunking:
+    target_chunk_size: 250
+    metadata_map_chunk_size: 50000
+`,
+			wantErr: true,
+		},
+		{
+			name: "metadata_map_chunk_size not set",
+			yaml: `
+repository:
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+service:
+  worker_pool_size: 5
+  chunking:
+    target_chunk_size: 250
+    changed_target_chunk_size: 125
+`,
+			wantErr: true,
+		},
+		{
+			name: "repository remote not set",
+			yaml: `
+repository:
+  - query_timeout_seconds: 300
+service:
+  worker_pool_size: 5
+  chunking:
+    target_chunk_size: 250
+    changed_target_chunk_size: 125
+    metadata_map_chunk_size: 50000
+`,
+			wantErr: true,
+		},
+		{
+			name: "query_timeout_seconds not set",
+			yaml: `
+repository:
+  - remote: "git@github.com:org/repo"
+service:
+  worker_pool_size: 5
+  chunking:
+    target_chunk_size: 250
+    changed_target_chunk_size: 125
+    metadata_map_chunk_size: 50000
+`,
+			wantErr: true,
+		},
+		{
+			name: "duplicate repository remote",
+			yaml: `
+repository:
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+service:
+  worker_pool_size: 5
+  chunking:
+    target_chunk_size: 250
+    changed_target_chunk_size: 125
+    metadata_map_chunk_size: 50000
+`,
+			wantErr: true,
+		},
+		{
+			name: "worker_root_path without repo_manager_clone_path",
+			yaml: `
+repository:
+  - remote: "git@github.com:org/repo"
+    query_timeout_seconds: 300
+service:
+  worker_pool_size: 5
+  worker_root_path: /tmp/workers
+  chunking:
+    target_chunk_size: 250
+    changed_target_chunk_size: 125
+    metadata_map_chunk_size: 50000
+`,
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			path := filepath.Join(t.TempDir(), "config.yaml")
+			require.NoError(t, os.WriteFile(path, []byte(tt.yaml), 0o644))
+
+			cfg, err := Parse(path)
+			if tt.wantErr {
+				require.Error(t, err, "expected error when %s", tt.name)
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, cfg)
+			}
+		})
+	}
+}

config/repository_config.go

@@ -22,7 +22,7 @@ type RepositoryConfig struct {
 	ExcludeExternalTargets bool     `yaml:"exclude_external_targets"`
 	BzlmodEnabled          bool     `yaml:"bzlmod_enabled"`
 	BazelCommand           string   `yaml:"bazel_command"`
-	QueryTimeout           int64    `yaml:"query_timeout"` // in seconds
+	QueryTimeoutSeconds    int64    `yaml:"query_timeout_seconds"`
 	BazelExtraArgs         []string `yaml:"bazel_extra_args"`
 	StreamBazelLogs        bool     `yaml:"stream_bazel_logs"`
 }

controller/BUILD.bazel

@@ -33,10 +33,10 @@ go_test(
         "getchangedtargets_test.go",
         "gettargetgraph_test.go",
         "output_filter_test.go",
-        "testhelper_test.go",
     ],
     embed = [":controller"],
     deps = [
+        "//config",
         "//core/common",
         "//core/storage",
         "//core/storage/storagemock",
@@ -46,7 +46,6 @@ go_test(
         "@com_github_gogo_protobuf//io",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
-        "@com_github_uber_go_tally//:tally",
         "@org_uber_go_mock//gomock",
         "@org_uber_go_zap//:zap",
         "@org_uber_go_zap//zaptest",

controller/controller.go

@@ -16,11 +16,11 @@ package controller
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/uber-go/tally"
 	"github.com/uber/tango/config"
-	"github.com/uber/tango/core/common"
 	"github.com/uber/tango/core/storage"
 	"github.com/uber/tango/orchestrator"
 	pb "github.com/uber/tango/tangopb"
@@ -59,34 +59,31 @@ type controller struct {
 
 // NewController creates a new controller. appCtx is cancelled on process
 // shutdown to abort background work.
-func NewController(appCtx context.Context, p Params) pb.TangoYARPCServer {
+func NewController(appCtx context.Context, p Params) (pb.TangoYARPCServer, error) {
 	scope := p.Scope
 	if scope == nil {
 		scope = tally.NoopScope
 	}
-	targetChunkSize := p.ChunkConfig.TargetChunkSize
-	if targetChunkSize <= 0 {
-		targetChunkSize = common.DefaultTargetChunkSize
+	if p.ChunkConfig.TargetChunkSize <= 0 {
+		return nil, fmt.Errorf("target_chunk_size must be > 0, got %d", p.ChunkConfig.TargetChunkSize)
 	}
-	changedTargetChunkSize := p.ChunkConfig.ChangedTargetChunkSize
-	if changedTargetChunkSize <= 0 {
-		changedTargetChunkSize = common.DefaultChangedTargetChunkSize
+	if p.ChunkConfig.ChangedTargetChunkSize <= 0 {
+		return nil, fmt.Errorf("changed_target_chunk_size must be > 0, got %d", p.ChunkConfig.ChangedTargetChunkSize)
 	}
-	metadataMapChunkSize := p.ChunkConfig.MetadataMapChunkSize
-	if metadataMapChunkSize <= 0 {
-		metadataMapChunkSize = common.DefaultMetadataMapChunkSize
+	if p.ChunkConfig.MetadataMapChunkSize <= 0 {
+		return nil, fmt.Errorf("metadata_map_chunk_size must be > 0, got %d", p.ChunkConfig.MetadataMapChunkSize)
 	}
 	return &controller{
 		logger:                 p.Logger,
 		storage:                p.Storage,
 		orchestrator:           p.Orchestrator,
 		scope:                  scope.SubScope("controller"),
-		targetChunkSize:        targetChunkSize,
-		changedTargetChunkSize: changedTargetChunkSize,
-		metadataMapChunkSize:   metadataMapChunkSize,
+		targetChunkSize:        p.ChunkConfig.TargetChunkSize,
+		changedTargetChunkSize: p.ChunkConfig.ChangedTargetChunkSize,
+		metadataMapChunkSize:   p.ChunkConfig.MetadataMapChunkSize,
 		totalDurationBuckets:   _totalDurationBuckets,
 		appCtx:                 appCtx,
-	}
+	}, nil
 }
 
 // linkRequestCtx returns a context derived from reqCtx that is also cancelled