diff --git a/cmd/controller/controller.go b/cmd/controller/controller.go index 5b6472ce..290e1402 100644 --- a/cmd/controller/controller.go +++ b/cmd/controller/controller.go @@ -6,350 +6,164 @@ import ( "log" "log/slog" "os" - "reflect" - "regexp" - "strconv" - "strings" + "github.com/alecthomas/kong" "github.com/buildkite/agent-stack-k8s/v2/cmd/linter" "github.com/buildkite/agent-stack-k8s/v2/cmd/version" "github.com/buildkite/agent-stack-k8s/v2/internal/controller" "github.com/buildkite/agent-stack-k8s/v2/internal/controller/config" - "github.com/buildkite/agent-stack-k8s/v2/internal/controller/scheduler" - "github.com/lmittmann/tint" - "github.com/mattn/go-isatty" - "github.com/go-playground/locales/en" ut "github.com/go-playground/universal-translator" "github.com/go-playground/validator/v10" en_translations "github.com/go-playground/validator/v10/translations/en" - mapstructure "github.com/go-viper/mapstructure/v2" - "github.com/spf13/cobra" - "github.com/spf13/pflag" - "github.com/spf13/viper" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - "k8s.io/apimachinery/pkg/util/intstr" + "github.com/lmittmann/tint" + "github.com/mattn/go-isatty" "k8s.io/client-go/kubernetes" restconfig "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/manager/signals" ) -var configFile string - -// ReadConfigFromFileArgsAndEnv reads the config from the file, env and args in that order. -// an excaption is the path to the config file which is read from the args and env only. -func ReadConfigFromFileArgsAndEnv(cmd *cobra.Command, args []string) (*viper.Viper, error) { - // First parse the flags so we can settle on the config file - if err := cmd.Flags().Parse(args); err != nil { - return nil, fmt.Errorf("failed to parse flags: %w", err) - } +var ( + english = en.New() + uni = ut.New(english, english) + validate = validator.New() + trans, _ = uni.GetTranslator("en") +) - // Settle on the config file - if configFile == "" { - configFile = os.Getenv("CONFIG") +func init() { + if err := en_translations.RegisterDefaultTranslations(validate, trans); err != nil { + log.Fatalf("failed to register translations: %v", err) } +} - // By default Viper unmarshals a key like "a.b.c" as nested maps: - // map[string]any{"a": map[string]any{"b": map[string]any{"c": ... }}} - // which is frustrating, because `.` is commonly used in Kubernetes labels, - // annotations, and node selector keys (they tend to use domain names to - // "namespace" keys). So change Viper's delimiter to`::`. - v := viper.NewWithOptions( - viper.KeyDelimiter("::"), - viper.EnvKeyReplacer(strings.NewReplacer("-", "_")), - ) - v.SetConfigFile(configFile) - - // Bind the flags to the viper instance, but only those that can appear in the config file. - errs := []error{} - cmd.Flags().VisitAll(func(f *pflag.Flag) { - switch f.Name { - case "config", "help": - // skip - default: - if err := v.BindPFlag(f.Name, f); err != nil { - errs = append(errs, fmt.Errorf("failed to bind flag %s: %w", f.Name, err)) - } +// Run is the main entry point for the controller. +func Run() error { + // Check if first arg is a subcommand + if len(os.Args) > 1 { + switch os.Args[1] { + case "lint": + return linter.Run() + case "version": + return version.Run() } - }) - if len(errs) > 0 { - return nil, errors.Join(errs...) } - v.AutomaticEnv() - - if err := v.ReadInConfig(); err != nil { - if !errors.As(err, &viper.ConfigFileNotFoundError{}) { - return nil, fmt.Errorf("failed to read config: %w", err) + cfg, err := BuildConfigFromArgs(os.Args[1:]) + if err != nil { + var errs validator.ValidationErrors + if errors.As(err, &errs) { + for _, e := range errs { + log.Println(e.Translate(trans)) + } } + return err } - return v, nil + return runController(cfg) } -var resourceQuantityType = reflect.TypeOf(resource.Quantity{}) -var intOrStringType = reflect.TypeOf(intstr.IntOrString{}) - -// This mapstructure.DecodeHookFunc is needed to decode kubernetes objects (as -// used in podSpecs) properly. Without this, viper (which uses mapstructure) doesn't -// e.g. know how to put a string (e.g. "100m") into a "map" (resource.Quantity) and -// will error out. -func decodeKubeSpecials(f, t reflect.Type, data any) (any, error) { - switch t { - case resourceQuantityType: - switch f.Kind() { - case reflect.String: - return resource.ParseQuantity(data.(string)) - case reflect.Float64: - return resource.ParseQuantity(strconv.FormatFloat(data.(float64), 'f', -1, 64)) - case reflect.Float32: - return resource.ParseQuantity(strconv.FormatFloat(float64(data.(float32)), 'f', -1, 32)) - case reflect.Int: - return resource.ParseQuantity(strconv.Itoa(data.(int))) - default: - return nil, fmt.Errorf("invalid resource quantity: %v", data) - } - case intOrStringType: - switch f.Kind() { - case reflect.String: - return intstr.FromString(data.(string)), nil - case reflect.Int: - return intstr.FromInt(data.(int)), nil - default: - return nil, fmt.Errorf("invalid int/string: %v", data) - } - default: - return data, nil +// BuildConfigFromArgs parses CLI args and env vars, then builds config. +// Config file can be specified via --config flag or CONFIG env var. +// Priority: defaults < config file < environment variables < CLI args +func BuildConfigFromArgs(args []string) (*config.Config, error) { + cli := &CLI{} + parser, err := kong.New( + cli, + kong.Name("agent-stack-k8s"), + kong.Description("Buildkite Agent Stack for Kubernetes"), + kong.DefaultEnvars(""), + kong.ConfigureHelp(kong.HelpOptions{ + Compact: true, + }), + kong.UsageOnError(), + ) + if err != nil { + return nil, fmt.Errorf("failed to create parser: %w", err) } -} - -// This viper.DecoderConfigOption is needed to make mapstructure (used by viper) -// use the same struct tags that the k8s libraries provide. -func useJSONTagForDecoder(c *mapstructure.DecoderConfig) { - c.TagName = "json" - c.SquashTagOption = "inline" -} - -// ParseAndValidateConfig parses the config into a struct and validates the values. -func ParseAndValidateConfig(v *viper.Viper) (*config.Config, error) { - // We want to let the user know if they have any extra fields, so use UnmarshalExact. - // The user likely expects every part of their config to be meaningful, so if some of it is - // ignored in parsing, they almost certainly want to know about it. - cfg := &config.Config{} - // This decode hook = the default Viper decode hooks + decodeKubeSpecials - // (Setting this option overrides the default.) - decodeHook := viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( - decodeKubeSpecials, - config.StringToInterposer, - mapstructure.StringToTimeDurationHookFunc(), - mapstructure.StringToSliceHookFunc(","), - )) - if err := v.UnmarshalExact(cfg, useJSONTagForDecoder, decodeHook); err != nil { - return nil, fmt.Errorf("failed to parse config: %w", err) + _, err = parser.Parse(args) + if err != nil { + parser.FatalIfErrorf(err) } - if err := validate.Struct(cfg); err != nil { - return nil, fmt.Errorf("failed to validate config: %w", err) - } + return buildConfig(cli) +} - if cfg.PodSpecPatch != nil { - if err := processPodSpecPatch(cfg.PodSpecPatch); err != nil { - return nil, fmt.Errorf("invalid pod spec patch: %w", err) - } - } +// buildConfig builds the config from CLI values with correct precedence: +// defaults < config file < env vars/CLI flags. +// Kong has already applied env vars and CLI flags to the cli struct. +func buildConfig(cli *CLI) (*config.Config, error) { + // Step 1: Start with defaults + cfg := newConfigWithDefaults() - for name, rc := range cfg.ResourceClasses { - err := recapitalizeHugePagesResourceClasses(rc) + // Step 2: Apply config file values (overrides defaults) + if cli.ConfigFile != "" { + fileCfg, keys, err := loadConfigFile(cli.ConfigFile) if err != nil { - return nil, fmt.Errorf("invalid resource class %q: %w", name, err) - } - } - - if cfg.DefaultResourceClassName != "" { - if cfg.ResourceClasses == nil { - return nil, fmt.Errorf("default-resource-class-name %q specified but no resource-classes defined", cfg.DefaultResourceClassName) - } - if _, exists := cfg.ResourceClasses[cfg.DefaultResourceClassName]; !exists { - return nil, fmt.Errorf("default-resource-class-name %q not found in resource-classes", cfg.DefaultResourceClassName) + return nil, fmt.Errorf("failed to load config: %w", err) } + mergeConfigFromFile(cfg, fileCfg, keys) } - if _, err := resource.ParseQuantity(cfg.ImageCheckContainerCPULimit); err != nil { - return nil, fmt.Errorf("invalid CPU resource limit defined: %s", cfg.ImageCheckContainerCPULimit) - } + // Step 3: Apply CLI/env values (overrides config file) + applyCLIOverrides(cfg, cli) - if _, err := resource.ParseQuantity(cfg.ImageCheckContainerMemoryLimit); err != nil { - return nil, fmt.Errorf("invalid memory resource limit defined: %s", cfg.ImageCheckContainerMemoryLimit) + // Step 4: Validate + if err := validateConfig(cfg); err != nil { + return nil, fmt.Errorf("failed to validate config: %w", err) } return cfg, nil } -func processPodSpecPatch(podSpec *corev1.PodSpec) error { - for idx, c := range podSpec.Containers { - if c.Image != strings.ToLower(c.Image) { - return fmt.Errorf("container image for container at index %d contains uppercase letters: %s", idx, c.Image) - } - - if len(c.Command) != 0 || len(c.Args) != 0 { - return fmt.Errorf("container at index %d (image: %s): %w", idx, c.Image, scheduler.ErrNoCommandModification) - } - - var err error - if podSpec.Containers[idx].Resources.Requests, err = recapitalizeHugePagesResourceMap(c.Resources.Requests); err != nil { - return fmt.Errorf("processing resource requests for container at index %d (image: %s): %w", idx, c.Image, err) - } +func runController(cfg *config.Config) error { + ctx := signals.SetupSignalHandler() - if podSpec.Containers[idx].Resources.Limits, err = recapitalizeHugePagesResourceMap(c.Resources.Limits); err != nil { - return fmt.Errorf("processing resource limits for container at index %d (image: %s): %w", idx, c.Image, err) + level := slog.LevelInfo + if cfg.LogLevel != "" { + if err := level.UnmarshalText([]byte(cfg.LogLevel)); err != nil { + return fmt.Errorf("invalid log level %q: %w", cfg.LogLevel, err) } } - return nil -} - -// Viper downcases keys in maps used in inputs, but in the case of resource classes where hugepages are used, it -// gets downcased to "hugepages-2mi". This function fixes that by recapitlizing the "Mi" part for any hugepages resource -// claims. This is important because the hugepages resource name must be capitalized as "hugepages-2Mi" (or similar), -// otherwise k8s won't recognize the resource claim. -func recapitalizeHugePagesResourceClasses(rc *config.ResourceClass) error { - var err error - if rc.Resource.Limits, err = recapitalizeHugePagesResourceMap(rc.Resource.Limits); err != nil { - return fmt.Errorf("invalid resource class limits: %w", err) - } - - rc.Resource.Requests, err = recapitalizeHugePagesResourceMap(rc.Resource.Requests) - if err != nil { - return fmt.Errorf("invalid resource class requests: %w", err) - } - - return nil -} - -var hugepagesSizeRE = regexp.MustCompile(`^(\d+)([KMGPEkmgpe])i$`) - -func recapitalizeHugePagesResourceMap(m corev1.ResourceList) (corev1.ResourceList, error) { - if m == nil { - return nil, nil + if cfg.Debug { + level = slog.LevelDebug } - newResourceList := corev1.ResourceList{} - for res, qty := range m { - stringRes := string(res) - if !strings.HasPrefix(stringRes, corev1.ResourceHugePagesPrefix) { - newResourceList[res] = qty - continue - } - - hugepagesSize := strings.TrimPrefix(stringRes, corev1.ResourceHugePagesPrefix) - - matched := hugepagesSizeRE.FindStringSubmatch(hugepagesSize) - if len(matched) != 3 { - return nil, fmt.Errorf("couldn't match hugepages size %q to regex %s. This is a bug, please contact support@buildkite.com", hugepagesSize, hugepagesSizeRE) - } - - magnitude := matched[1] - unitFirstLetter := matched[2] + var handler slog.Handler + switch cfg.LogFormat { + case "json": + handler = slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: level}) - correctedSize := magnitude + strings.ToUpper(unitFirstLetter) + "i" - correctedResourceName := corev1.ResourceName(corev1.ResourceHugePagesPrefix + correctedSize) + case "logfmt", "": + handler = tint.NewHandler(os.Stdout, &tint.Options{ + AddSource: true, + Level: level, + NoColor: cfg.NoColor || !isatty.IsTerminal(os.Stdout.Fd()), + }) - newResourceList[correctedResourceName] = qty + default: + return fmt.Errorf("unknown log format: %s", cfg.LogFormat) } - return newResourceList, nil -} - -var ( - english = en.New() - uni = ut.New(english, english) - validate = validator.New() - trans, _ = uni.GetTranslator("en") -) - -func New() *cobra.Command { - cmd := &cobra.Command{ - Use: "agent-stack-k8s", - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - ctx := signals.SetupSignalHandler() - - v, err := ReadConfigFromFileArgsAndEnv(cmd, args) - if err != nil { - return err - } - - cfg, err := ParseAndValidateConfig(v) - if err != nil { - var errs validator.ValidationErrors - if errors.As(err, &errs) { - for _, e := range errs { - log.Println(e.Translate(trans)) - } - } - return fmt.Errorf("failed to parse config: %w", err) - } - - level := slog.LevelInfo - if cfg.LogLevel != "" { - err := level.UnmarshalText([]byte(cfg.LogLevel)) - if err != nil { - return fmt.Errorf("invalid log level %q: %w", cfg.LogLevel, err) - } - } - - if cfg.Debug { - // The debug flag trumps the log level flag - level = slog.LevelDebug - } - - var handler slog.Handler - switch cfg.LogFormat { - case "json": - handler = slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: level}) + logger := slog.New(handler) + logger.Debug("configuration loaded", "config", cfg) - case "logfmt": - handler = tint.NewHandler(os.Stdout, &tint.Options{ - AddSource: true, - Level: level, - NoColor: cfg.NoColor || !isatty.IsTerminal(os.Stdout.Fd()), - }) + clientConfig := restconfig.GetConfigOrDie() + clientConfig.QPS = float32(cfg.K8sClientRateLimiterQPS) + clientConfig.Burst = cfg.K8sClientRateLimiterBurst - default: - // Handled by the config validator above, but nice to have just in case. - return fmt.Errorf("unknown log format: %s", cfg.LogFormat) - } - - logger := slog.New(handler) - logger.Debug("configuration loaded", "config", cfg) - - clientConfig := restconfig.GetConfigOrDie() - clientConfig.QPS = float32(cfg.K8sClientRateLimiterQPS) - clientConfig.Burst = cfg.K8sClientRateLimiterBurst - - // Default to Protobuf encoding for API responses, support fallback to JSON - clientConfig.AcceptContentTypes = "application/vnd.kubernetes.protobuf,application/json" - clientConfig.ContentType = "application/vnd.kubernetes.protobuf" - - k8sClient, err := kubernetes.NewForConfig(clientConfig) - if err != nil { - logger.Error("failed to create clientset", "error", err) - } + // Default to Protobuf encoding for API responses, support fallback to JSON + clientConfig.AcceptContentTypes = "application/vnd.kubernetes.protobuf,application/json" + clientConfig.ContentType = "application/vnd.kubernetes.protobuf" - controller.Run(ctx, logger, k8sClient, cfg) - - return nil - }, + k8sClient, err := kubernetes.NewForConfig(clientConfig) + if err != nil { + logger.Error("failed to create clientset", "error", err) + return err } - AddConfigFlags(cmd) - cmd.AddCommand(linter.New()) - cmd.AddCommand(version.New()) - if err := en_translations.RegisterDefaultTranslations(validate, trans); err != nil { - log.Fatalf("failed to register translations: %v", err) - } + controller.Run(ctx, logger, k8sClient, cfg) - return cmd + return nil } diff --git a/cmd/controller/controller_test.go b/cmd/controller/controller_test.go index 34e0fba6..7bc44a16 100644 --- a/cmd/controller/controller_test.go +++ b/cmd/controller/controller_test.go @@ -1,13 +1,14 @@ package controller_test import ( + "os" "testing" "time" "github.com/buildkite/agent-stack-k8s/v2/cmd/controller" "github.com/buildkite/agent-stack-k8s/v2/internal/controller/config" "github.com/google/go-cmp/cmp" - "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -164,22 +165,9 @@ func TestReadAndParseConfig(t *testing.T) { }, } - // These need to be unset to as it is set in CI which pollutes the test environment - t.Setenv("INTEGRATION_TEST_BUILDKITE_TOKEN", "") - t.Setenv("IMAGE", "") - t.Setenv("NAMESPACE", "") - t.Setenv("AGENT_TOKEN_SECRET", "") + cleanTestEnv(t) - cmd := &cobra.Command{} - controller.AddConfigFlags(cmd) - v, err := controller.ReadConfigFromFileArgsAndEnv(cmd, []string{}) - require.NoError(t, err) - - // We need to read the config file from the test - v.SetConfigFile("../../examples/config.yaml") - require.NoError(t, v.ReadInConfig()) - - actual, err := controller.ParseAndValidateConfig(v) + actual, err := controller.BuildConfigFromArgs([]string{"--config=../../examples/config.yaml"}) require.NoError(t, err) if diff := cmp.Diff(*actual, expected); diff != "" { @@ -189,55 +177,272 @@ func TestReadAndParseConfig(t *testing.T) { func TestParseAndValidateConfig_DefaultResourceClassValidation(t *testing.T) { tests := []struct { - name string - config map[string]any - wantErr string + name string + configYAML string + wantErr string }{ { name: "default references non-existent resource class", - config: map[string]any{ - "agent-token-secret": "test", - "image": "test:latest", - "job-active-deadline-seconds": 3600, - "namespace": "default", - "default-resource-class-name": "nonexistent", - "resource-classes": map[string]any{ - "small": map[string]any{ - "resource": map[string]any{ - "requests": map[string]any{"cpu": "100m"}, - }, - }, - }, - }, + configYAML: ` +default-resource-class-name: nonexistent +resource-classes: + small: + resource: + requests: + cpu: "100m" +`, wantErr: `default-resource-class-name "nonexistent" not found in resource-classes`, }, { name: "default specified but no resource classes defined", - config: map[string]any{ - "agent-token-secret": "test", - "image": "test:latest", - "job-active-deadline-seconds": 3600, - "namespace": "default", - "default-resource-class-name": "small", - }, + configYAML: ` +default-resource-class-name: small +`, wantErr: `default-resource-class-name "small" specified but no resource-classes defined`, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cmd := &cobra.Command{} - controller.AddConfigFlags(cmd) - v, err := controller.ReadConfigFromFileArgsAndEnv(cmd, []string{}) - require.NoError(t, err) + cleanTestEnv(t) + configFile := createTempConfigFile(t, tt.configYAML) - for k, val := range tt.config { - v.Set(k, val) - } - - _, err = controller.ParseAndValidateConfig(v) + _, err := controller.BuildConfigFromArgs([]string{"--config=" + configFile}) require.Error(t, err) require.Contains(t, err.Error(), tt.wantErr) }) } } + +func TestConfigPrecedence(t *testing.T) { + // Helper to build config with optional config file + buildConfig := func(t *testing.T, args []string, configFile string) (*config.Config, error) { + t.Helper() + if configFile != "" { + args = append([]string{"--config=" + configFile}, args...) + } + return controller.BuildConfigFromArgs(args) + } + + t.Run("defaults applied when nothing set", func(t *testing.T) { + cleanTestEnv(t) + + cfg, err := buildConfig(t, []string{}, "") + require.NoError(t, err) + + assert.Equal(t, 25, cfg.MaxInFlight) + assert.Equal(t, "default", cfg.Namespace) + assert.Equal(t, 10*time.Minute, cfg.JobTTL) + assert.False(t, cfg.Debug) + }) + + t.Run("config file overrides defaults", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, ` +max-in-flight: 100 +namespace: from-file +debug: true +`) + + cfg, err := buildConfig(t, []string{}, configFile) + require.NoError(t, err) + + assert.Equal(t, 100, cfg.MaxInFlight) + assert.Equal(t, "from-file", cfg.Namespace) + assert.True(t, cfg.Debug) + }) + + t.Run("config file can set zero value", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, `max-in-flight: 0`) + + cfg, err := buildConfig(t, []string{}, configFile) + require.NoError(t, err) + + assert.Equal(t, 0, cfg.MaxInFlight) + }) + + t.Run("config file can set false", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, `debug: false`) + + cfg, err := buildConfig(t, []string{}, configFile) + require.NoError(t, err) + + assert.False(t, cfg.Debug) + }) + + t.Run("env var overrides config file", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, ` +max-in-flight: 100 +namespace: from-file +`) + t.Setenv("MAX_IN_FLIGHT", "50") + t.Setenv("NAMESPACE", "from-env") + + cfg, err := buildConfig(t, []string{}, configFile) + require.NoError(t, err) + + assert.Equal(t, 50, cfg.MaxInFlight) + assert.Equal(t, "from-env", cfg.Namespace) + }) + + t.Run("env var can override config file with zero", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, `max-in-flight: 100`) + t.Setenv("MAX_IN_FLIGHT", "0") + + cfg, err := buildConfig(t, []string{}, configFile) + require.NoError(t, err) + + assert.Equal(t, 0, cfg.MaxInFlight) + }) + + t.Run("env var can override config file with false", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, `debug: true`) + t.Setenv("DEBUG", "false") + + cfg, err := buildConfig(t, []string{}, configFile) + require.NoError(t, err) + + assert.False(t, cfg.Debug) + }) + + t.Run("CLI overrides env var", func(t *testing.T) { + cleanTestEnv(t) + t.Setenv("MAX_IN_FLIGHT", "50") + t.Setenv("NAMESPACE", "from-env") + + cfg, err := buildConfig(t, []string{ + "--max-in-flight=200", + "--namespace=from-cli", + }, "") + require.NoError(t, err) + + assert.Equal(t, 200, cfg.MaxInFlight) + assert.Equal(t, "from-cli", cfg.Namespace) + }) + + t.Run("CLI can override env var with zero", func(t *testing.T) { + cleanTestEnv(t) + t.Setenv("MAX_IN_FLIGHT", "50") + + cfg, err := buildConfig(t, []string{"--max-in-flight=0"}, "") + require.NoError(t, err) + + assert.Equal(t, 0, cfg.MaxInFlight) + }) + + t.Run("CLI can override env var with false", func(t *testing.T) { + cleanTestEnv(t) + t.Setenv("DEBUG", "true") + + cfg, err := buildConfig(t, []string{"--debug=false"}, "") + require.NoError(t, err) + + assert.False(t, cfg.Debug) + }) + + t.Run("full precedence chain", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, ` +max-in-flight: 100 +namespace: from-file +job-ttl: 5m +`) + t.Setenv("NAMESPACE", "from-env") + t.Setenv("JOB_TTL", "15m") + + cfg, err := buildConfig(t, []string{"--namespace=from-cli"}, configFile) + require.NoError(t, err) + + // max-in-flight: only in file + assert.Equal(t, 100, cfg.MaxInFlight) + // namespace: file -> env -> cli (cli wins) + assert.Equal(t, "from-cli", cfg.Namespace) + // job-ttl: file -> env (env wins) + assert.Equal(t, 15*time.Minute, cfg.JobTTL) + }) + + t.Run("duration fields work correctly", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, ` +job-ttl: 30m +poll-interval: 5s +`) + + cfg, err := buildConfig(t, []string{}, configFile) + require.NoError(t, err) + + assert.Equal(t, 30*time.Minute, cfg.JobTTL) + assert.Equal(t, 5*time.Second, cfg.PollInterval) + }) + + t.Run("config file with unknown field is rejected", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, ` +namespace: my-namespace +unknown-field: some-value +`) + + _, err := buildConfig(t, []string{}, configFile) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown-field") + }) + + t.Run("CLI can override config file tags with empty", func(t *testing.T) { + cleanTestEnv(t) + configFile := createTempConfigFile(t, ` +tags: + - queue=production + - env=prod +`) + + cfg, err := buildConfig(t, []string{"--tags="}, configFile) + require.NoError(t, err) + + // CLI explicitly set tags to empty, should override config file + assert.Empty(t, cfg.Tags) + }) +} + +// cleanTestEnv unsets environment variables that might be set in CI or .envrc +// which could pollute the test environment. +func cleanTestEnv(t *testing.T) { + t.Helper() + for _, env := range []string{ + "BUILDKITE_TOKEN", + "INTEGRATION_TEST_BUILDKITE_TOKEN", + "IMAGE", + "NAMESPACE", + "AGENT_TOKEN_SECRET", + "IMAGE_PULL_BACKOFF_GRACE_PERIOD", + "CONFIG", + "MAX_IN_FLIGHT", + "DEBUG", + "JOB_TTL", + } { + t.Setenv(env, "") + os.Unsetenv(env) + } +} + +// createTempConfigFile creates a temporary config file with the given content +// and returns its path. The file is automatically cleaned up after the test. +func createTempConfigFile(t *testing.T, content string) string { + t.Helper() + f, err := os.CreateTemp("", "config-*.yaml") + if err != nil { + t.Fatalf("failed to create temp config file: %v", err) + } + if _, err := f.WriteString(content); err != nil { + t.Fatalf("failed to write temp config file: %v", err) + } + if err := f.Close(); err != nil { + t.Fatalf("failed to close temp config file: %v", err) + } + t.Cleanup(func() { os.Remove(f.Name()) }) + return f.Name() +} diff --git a/cmd/controller/flags.go b/cmd/controller/flags.go index 5570299e..44062543 100644 --- a/cmd/controller/flags.go +++ b/cmd/controller/flags.go @@ -1,187 +1,308 @@ package controller import ( + "bytes" + "encoding/json" + "fmt" + "os" + "reflect" + "strings" "time" "github.com/buildkite/agent-stack-k8s/v2/internal/controller/config" - "github.com/spf13/cobra" + "github.com/buildkite/agent-stack-k8s/v2/internal/controller/scheduler" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "sigs.k8s.io/yaml" ) -func AddConfigFlags(cmd *cobra.Command) { - // the config file flag - cmd.Flags().StringVarP(&configFile, "config", "f", "", "config file path") - - // not in the config file - cmd.Flags().String( - "agent-token-secret", - "buildkite-agent-token", - "name of the Buildkite agent token secret", - ) - cmd.Flags().String("integration-test-buildkite-token", "", "Deprecated - Buildkite API token with GraphQL scopes") - - // in the config file - cmd.Flags().String( - "image", - config.DefaultAgentImage, - "The image to use for the Buildkite agent", - ) - cmd.Flags().String( - "job-prefix", - "buildkite-", - "The prefix to use when creating Kubernetes job names", - ) - cmd.Flags().StringSlice( - "tags", - []string{}, - `A comma-separated list of agent tags. The "queue" tag must be unique (e.g. "queue=kubernetes,os=linux")`, - ) - cmd.Flags().String( - "queue", - "", - "The Buildkite queue to poll for jobs (overrides the 'queue' tag if set)", - ) - cmd.Flags().String( - "namespace", - config.DefaultNamespace, - "kubernetes namespace to create resources in", - ) - cmd.Flags().Bool("debug", false, "sets log level to debug. Overrides --log-level if set") - cmd.Flags().Int("max-in-flight", 25, "max jobs in flight, 0 means no max") - cmd.Flags().Duration( - "job-ttl", - 10*time.Minute, - "time to retain kubernetes jobs after completion", - ) - cmd.Flags().Int( - "job-active-deadline-seconds", - 21600, - "maximum number of seconds a kubernetes job is allowed to run before terminating all pods and failing job", - ) - cmd.Flags().Int( - "default-termination-grace-period-seconds", - config.DefaultTerminationGracePeriodSeconds, - "maximum number of seconds a pod will run after being told to terminate, if not otherwise set by podSpec", - ) - cmd.Flags().Duration( - "poll-interval", - time.Second, - "time to wait between polling for new jobs (minimum 1s); note that increasing this causes jobs to be slower to start", - ) - cmd.Flags().String( - "profiler-address", - "", - "Bind address to expose the pprof profiler (e.g. localhost:6060)", - ) - cmd.Flags().Uint16( - "prometheus-port", - 0, - "Bind port to expose Prometheus /metrics; 0 disables it", - ) - cmd.Flags().String("graphql-endpoint", "", "Deprecated - Buildkite GraphQL endpoint URL") - - cmd.Flags().Int( - "job-creation-concurrency", - config.DefaultJobCreationConcurrency, - "Number of concurrent goroutines to run for converting Buildkite jobs into Kubernetes jobs", - ) - cmd.Flags().Int( - "k8s-client-rate-limiter-qps", - config.DefaultK8sClientRateLimiterQPS, - "The QPS value of the K8s client rate limiter.", - ) - cmd.Flags().Int( - "k8s-client-rate-limiter-burst", - config.DefaultK8sClientRateLimiterBurst, - "The burst value of the K8s client rate limiter.", - ) - cmd.Flags().Duration( - "image-pull-backoff-grace-period", - config.DefaultImagePullBackOffGracePeriod, - "Duration after starting a pod that the controller will wait before considering cancelling a job due to ImagePullBackOff (e.g. when the podSpec specifies container images that cannot be pulled)", - ) - cmd.Flags().Duration( - "job-cancel-checker-poll-interval", - config.DefaultJobCancelCheckerPollInterval, - "Controls the interval between job state queries while a pod is still Pending", - ) - cmd.Flags().Duration( - "empty-job-grace-period", - config.DefaultEmptyJobGracePeriod, - "Duration after starting a Kubernetes job that the controller will wait before considering failing the job due to a missing pod (e.g. when the podSpec specifies a missing service account)", - ) - cmd.Flags().String( - "default-image-pull-policy", - "", - "Configures a default image pull policy for containers that do not specify a pull policy and non-init containers created by the stack itself", - ) - cmd.Flags().String( - "default-image-check-pull-policy", - "", - "Sets a default PullPolicy for image-check init containers, used if an image pull policy is not set for the corresponding container in a podSpec or podSpecPatch", - ) - cmd.Flags().Bool( - "prohibit-kubernetes-plugin", - false, - "Causes the controller to prohibit the kubernetes plugin specified within jobs (pipeline YAML) - enabling this causes jobs with a kubernetes plugin to fail, preventing the pipeline YAML from having any influence over the podSpec", - ) - cmd.Flags().Bool( - "enable-queue-pause", - false, - "Allow controller to pause processing the jobs when queue is paused on Buildkite", - ) - cmd.Flags().Bool( - "allow-pod-spec-patch-unsafe-command-modification", - false, - "Permits PodSpecPatch to modify the command or args fields of stack-provided containers. See the warning in the README before enabling this option", - ) - cmd.Flags().Int( - "pagination-page-size", - config.DefaultPaginationPageSize, - "Sets the maximum number of Jobs per page when retrieving Buildkite Jobs to be Scheduled.", - ) - cmd.Flags().Int( - "pagination-depth-limit", - config.DefaultPaginationDepthLimit, - "Sets the maximum number of pages when retrieving Buildkite Jobs to be Scheduled. Increasing this value will increase the number of requests made to the Buildkite API and number of Jobs to be scheduled on the Kubernetes Cluster.", - ) - cmd.Flags().Duration( - "query-reset-interval", - config.DefaultQueryResetInterval, - "Controls the interval between pagination cursor resets. Increasing this value will increase the number of jobs to be scheduled but also delay picking up any jobs that were missed from the start of the query.", - ) - cmd.Flags().Int( - "work-queue-limit", - config.DefaultWorkQueueLimit, - "Sets the maximum number of Jobs the controller will hold in the work queue.", - ) - cmd.Flags().Bool( - "skip-image-check-containers", - false, - "Disable and skip all imagecheck-* init containers", - ) - cmd.Flags().String( - "image-check-container-cpu-limit", - config.DefaultImageCheckContainerCPULimit, - "Configures the CPU resource limits for all imagecheck-* containers", - ) - cmd.Flags().String( - "image-check-container-memory-limit", - config.DefaultImageCheckContainerMemoryLimit, - "Configures the memory resource limits for all imagecheck-* containers", - ) - cmd.Flags().String( - "log-format", - "logfmt", - `Sets the log format. One of "logfmt" (for plain/colored text output) or "json". Defaults to "logfmt"`, - ) - cmd.Flags().Bool( - "no-color", - false, - "Disable colored log output (ANSI escape codes). If the output is not a terminal, colors are disabled automatically", - ) - cmd.Flags().String( - "log-level", - "info", - `Sets the log level. One of "debug", "info", "warn", "error". Overridden by the --debug flag if set`, - ) +// CLI represents the command-line interface structure for Kong. +// Pointer types are used for bool, int, and duration fields so we can distinguish +// "not set" (nil) from "explicitly set to zero/false". This allows CLI/env vars +// to override config file values with zero/false values. +// Defaults are applied via newConfigWithDefaults(). +// +// Environment variables are auto-generated by Kong from flag names (kebab-case → SCREAMING_SNAKE_CASE). +// For example: --max-in-flight → MAX_IN_FLIGHT, --job-ttl → JOB_TTL +// Only flags with non-standard names or env vars need explicit name= or env= tags. +type CLI struct { + // name= needed: Go field "ConfigFile" → Kong default "--config-file", but we want "--config" + ConfigFile string `kong:"name='config',short='f',help='Path to config file'"` + + Debug *bool `kong:"help='Sets log level to debug. Overrides --log-level if set'"` + + // Job / Pod settings + JobTTL *time.Duration `kong:"help='Time to retain kubernetes jobs after completion (default: 10m)'"` + JobActiveDeadlineSeconds *int `kong:"help='Maximum number of seconds a kubernetes job is allowed to run before terminating all pods and failing job (default: 21600)'"` + JobPrefix string `kong:"help='The prefix to use when creating Kubernetes job names (default: buildkite-)'"` + DefaultTerminationGracePeriodSeconds *int `kong:"help='Maximum number of seconds a pod will run after being told to terminate, if not otherwise set by podSpec (default: 60)'"` + Namespace string `kong:"help='Kubernetes namespace to create resources in (default: default)'"` + + // Controller settings + JobCreationConcurrency *int `kong:"help='Number of concurrent goroutines to run for converting Buildkite jobs into Kubernetes jobs (default: 25)'"` + AgentTokenSecret string `kong:"help='Name of the Buildkite agent token secret (default: buildkite-agent-token)'"` + Image string `kong:"help='The image to use for the Buildkite agent'"` + MaxInFlight *int `kong:"help='Max jobs in flight, 0 means no max (default: 25)'"` + Tags []string `kong:"help='A comma-separated list of agent tags. The \"queue\" tag must be unique (e.g. \"queue=kubernetes,os=linux\")'"` + Queue string `kong:"help='The Buildkite queue to poll for jobs (overrides the queue tag if set)'"` + PrometheusPort *uint16 `kong:"help='Bind port to expose Prometheus /metrics; 0 disables it (default: 0)'"` + ProfilerAddress string `kong:"help='Bind address to expose the pprof profiler (e.g. localhost:6060)'"` + PollInterval *time.Duration `kong:"help='Time to wait between polling for new jobs (minimum 1s); note that increasing this causes jobs to be slower to start (default: 1s)'"` + PaginationPageSize *int `kong:"help='Sets the maximum number of Jobs per page when retrieving Buildkite Jobs to be Scheduled (default: 1000)'"` + PaginationDepthLimit *int `kong:"help='Sets the maximum number of pages when retrieving Buildkite Jobs to be Scheduled. Increasing this value will increase the number of requests made to the Buildkite API and number of Jobs to be scheduled on the Kubernetes Cluster (default: 2)'"` + QueryResetInterval *time.Duration `kong:"help='Controls the interval between pagination cursor resets. Increasing this value will increase the number of jobs to be scheduled but also delay picking up any jobs that were missed from the start of the query (default: 10s)'"` + EnableQueuePause *bool `kong:"help='Allow controller to pause processing the jobs when queue is paused on Buildkite'"` + WorkQueueLimit *int `kong:"help='Sets the maximum number of Jobs the controller will hold in the work queue (default: 1000000)'"` + + // name= and env= needed: Go field "K8s..." → Kong default "--k-8-s-..." and "$K_8_S_...", but we want "--k8s-..." and "$K8S_..." + K8sClientRateLimiterQPS *int `kong:"name='k8s-client-rate-limiter-qps',env='K8S_CLIENT_RATE_LIMITER_QPS',help='The QPS value of the K8s client rate limiter (default: 10)'"` + K8sClientRateLimiterBurst *int `kong:"name='k8s-client-rate-limiter-burst',env='K8S_CLIENT_RATE_LIMITER_BURST',help='The burst value of the K8s client rate limiter (default: 20)'"` + + // name= needed: Go field "ImagePullBackOff..." → Kong default "--image-pull-back-off-...", but we want "--image-pull-backoff-..." + ImagePullBackOffGracePeriod *time.Duration `kong:"name='image-pull-backoff-grace-period',help='Duration after starting a pod that the controller will wait before considering cancelling a job due to ImagePullBackOff (e.g. when the podSpec specifies container images that cannot be pulled) (default: 30s)'"` + JobCancelCheckerPollInterval *time.Duration `kong:"help='Controls the interval between job state queries while a pod is still Pending (default: 5s)'"` + EmptyJobGracePeriod *time.Duration `kong:"help='Duration after starting a Kubernetes job that the controller will wait before considering failing the job due to a missing pod (e.g. when the podSpec specifies a missing service account) (default: 30s)'"` + + // Image settings + DefaultImagePullPolicy corev1.PullPolicy `kong:"help='Configures a default image pull policy for containers that do not specify a pull policy and non-init containers created by the stack itself'"` + DefaultImageCheckPullPolicy corev1.PullPolicy `kong:"help='Sets a default PullPolicy for image-check init containers, used if an image pull policy is not set for the corresponding container in a podSpec or podSpecPatch'"` + + SkipImageCheckContainers *bool `kong:"help='Disable and skip all imagecheck-* init containers'"` + ImageCheckContainerCPULimit string `kong:"help='Configures the CPU resource limits for all imagecheck-* containers (default: 200m)'"` + ImageCheckContainerMemoryLimit string `kong:"help='Configures the memory resource limits for all imagecheck-* containers (default: 128Mi)'"` + + // Security settings + ProhibitKubernetesPlugin *bool `kong:"help='Causes the controller to prohibit the kubernetes plugin specified within jobs (pipeline YAML) - enabling this causes jobs with a kubernetes plugin to fail, preventing the pipeline YAML from having any influence over the podSpec'"` + AllowPodSpecPatchUnsafeCmdMod *bool `kong:"name='allow-pod-spec-patch-unsafe-command-modification',help='Permits PodSpecPatch to modify the command or args fields of stack-provided containers. See the warning in the README before enabling this option'"` + + // Integration test settings (hidden) + // env= needed: INTEGRATION_TEST_BUILDKITE_TOKEN doesn't match the flag name "buildkite-token" + BuildkiteToken string `kong:"name='buildkite-token',hidden,env='INTEGRATION_TEST_BUILDKITE_TOKEN',help='Buildkite API token with GraphQL scopes'"` + GraphQLEndpoint string `kong:"hidden,help='Buildkite GraphQL endpoint URL'"` + + // Logging settings + LogFormat string `kong:"help='Sets the log format. One of \"logfmt\" (for plain/colored text output) or \"json\" (default: logfmt)'"` + NoColor *bool `kong:"help='Disable colored log output (ANSI escape codes). If the output is not a terminal, colors are disabled automatically'"` + LogLevel string `kong:"help='Sets the log level. One of \"debug\", \"info\", \"warn\", \"error\". Overridden by the --debug flag if set (default: info)'"` +} + +// applyCLIOverrides applies CLI values to the config when explicitly set. +// Pointer fields allow distinguishing "not set" (nil) from "set to zero/false". +// Uses reflection to iterate over CLI fields and copy to matching Config fields. +// This ensures precedence: defaults < config file < env vars/CLI flags +func applyCLIOverrides(cfg *config.Config, cli *CLI) { + cfgVal := reflect.ValueOf(cfg).Elem() + cliVal := reflect.ValueOf(cli).Elem() + cliType := cliVal.Type() + + for i := range cliType.NumField() { + cliField := cliVal.Field(i) + fieldName := cliType.Field(i).Name + + // Skip ConfigFile - it's not a config field + if fieldName == "ConfigFile" { + continue + } + + cfgField := cfgVal.FieldByName(fieldName) + if !cfgField.IsValid() { + continue // CLI field doesn't exist in Config + } + + switch cliField.Kind() { + case reflect.Ptr: + // Pointer types (nil = not set) + if !cliField.IsNil() { + cfgField.Set(cliField.Elem()) + } + case reflect.String: + // String types (empty = not set) + if cliField.String() != "" { + cfgField.Set(cliField) + } + case reflect.Slice: + // Slice types (nil = not set, empty = explicitly cleared) + if !cliField.IsNil() { + cfgField.Set(cliField) + } + } + } +} + +// newConfigWithDefaults returns a config with default values. +func newConfigWithDefaults() *config.Config { + return &config.Config{ + AgentTokenSecret: "buildkite-agent-token", + Image: config.DefaultAgentImage, + JobPrefix: "buildkite-", + Namespace: "default", + MaxInFlight: 25, + JobTTL: 10 * time.Minute, + JobActiveDeadlineSeconds: 21600, + DefaultTerminationGracePeriodSeconds: 60, + PollInterval: time.Second, + JobCreationConcurrency: 25, + K8sClientRateLimiterQPS: 10, + K8sClientRateLimiterBurst: 20, + ImagePullBackOffGracePeriod: 30 * time.Second, + JobCancelCheckerPollInterval: 5 * time.Second, + EmptyJobGracePeriod: 30 * time.Second, + PaginationPageSize: 1000, + PaginationDepthLimit: 2, + QueryResetInterval: 10 * time.Second, + WorkQueueLimit: 1000000, + ImageCheckContainerCPULimit: "200m", + ImageCheckContainerMemoryLimit: "128Mi", + LogFormat: "logfmt", + LogLevel: "info", + } +} + +// loadConfigFile reads and parses a config file. +// Returns the parsed config and a set of keys that were present in the file. +// The keys set is used to determine which fields to merge (even if zero-valued). +func loadConfigFile(configFile string) (*config.Config, map[string]struct{}, error) { + data, err := os.ReadFile(configFile) + if err != nil { + return nil, nil, fmt.Errorf("failed to read config file: %w", err) + } + + // First pass: get the set of keys present in the file + var rawMap map[string]any + if err := yaml.Unmarshal(data, &rawMap); err != nil { + return nil, nil, fmt.Errorf("failed to parse config file: %w", err) + } + + keys := make(map[string]struct{}) + for k := range rawMap { + keys[k] = struct{}{} + } + + // Second pass: unmarshal into the config struct + cfg := &config.Config{} + if err := unmarshalConfigStrict(data, cfg); err != nil { + return nil, nil, fmt.Errorf("failed to parse config file: %w", err) + } + + return cfg, keys, nil +} + +// unmarshalConfigStrict unmarshals YAML config data with support for time.Duration fields. +// It returns an error if the config contains unknown fields, ensuring the user is aware +// if any part of their config is being ignored. +// +// We use sigs.k8s.io/yaml because the config contains Kubernetes types (corev1.PodSpec, +// corev1.Volume, etc.) that have custom JSON unmarshalers. The sigs.k8s.io/yaml library +// converts YAML to JSON and then uses encoding/json for unmarshaling, which correctly +// invokes these custom unmarshalers. Standard YAML libraries (like gopkg.in/yaml.v3) +// would not handle these types correctly without custom decode hooks (as was previously +// required with Viper/mapstructure). +func unmarshalConfigStrict(data []byte, cfg *config.Config) error { + var rawMap map[string]any + if err := yaml.Unmarshal(data, &rawMap); err != nil { + return err + } + + convertDurations(rawMap) + + jsonData, err := json.Marshal(rawMap) + if err != nil { + return err + } + + decoder := json.NewDecoder(bytes.NewReader(jsonData)) + decoder.DisallowUnknownFields() + return decoder.Decode(cfg) +} + +// convertDurations recursively converts duration strings (e.g., "5m", "30s") to nanoseconds. +func convertDurations(m map[string]any) { + durationFields := map[string]struct{}{ + "job-ttl": {}, + "poll-interval": {}, + "query-reset-interval": {}, + "image-pull-backoff-grace-period": {}, + "job-cancel-checker-poll-interval": {}, + "empty-job-grace-period": {}, + } + + for key, value := range m { + switch v := value.(type) { + case string: + if _, ok := durationFields[key]; ok { + if d, err := time.ParseDuration(v); err == nil { + m[key] = int64(d) + } + } + case map[string]any: + convertDurations(v) + } + } +} + +// mergeConfigFromFile merges file config values into the base config. +// The keys map indicates which fields were present in the file, allowing +// zero values (like max-in-flight: 0) to override defaults. +// Uses reflection to iterate over fields based on their JSON tags. +func mergeConfigFromFile(base, file *config.Config, keys map[string]struct{}) { + baseVal := reflect.ValueOf(base).Elem() + fileVal := reflect.ValueOf(file).Elem() + baseType := baseVal.Type() + + for i := range baseType.NumField() { + field := baseType.Field(i) + jsonTag := strings.Split(field.Tag.Get("json"), ",")[0] + if jsonTag == "" || jsonTag == "-" { + continue + } + if _, ok := keys[jsonTag]; ok { + baseVal.Field(i).Set(fileVal.Field(i)) + } + } +} + +// validateConfig validates the config after all sources have been merged. +func validateConfig(cfg *config.Config) error { + if err := validate.Struct(cfg); err != nil { + return err + } + + if cfg.PodSpecPatch != nil { + if err := validatePodSpecPatch(cfg.PodSpecPatch, cfg.AllowPodSpecPatchUnsafeCmdMod); err != nil { + return fmt.Errorf("invalid pod spec patch: %w", err) + } + } + + if _, err := resource.ParseQuantity(cfg.ImageCheckContainerCPULimit); err != nil { + return fmt.Errorf("invalid CPU resource limit defined: %s", cfg.ImageCheckContainerCPULimit) + } + + if _, err := resource.ParseQuantity(cfg.ImageCheckContainerMemoryLimit); err != nil { + return fmt.Errorf("invalid memory resource limit defined: %s", cfg.ImageCheckContainerMemoryLimit) + } + + if cfg.DefaultResourceClassName != "" { + if cfg.ResourceClasses == nil { + return fmt.Errorf("default-resource-class-name %q specified but no resource-classes defined", cfg.DefaultResourceClassName) + } + if _, exists := cfg.ResourceClasses[cfg.DefaultResourceClassName]; !exists { + return fmt.Errorf("default-resource-class-name %q not found in resource-classes", cfg.DefaultResourceClassName) + } + } + + return nil +} + +func validatePodSpecPatch(podSpec *corev1.PodSpec, allowCmdMod bool) error { + for idx, c := range podSpec.Containers { + if c.Image != strings.ToLower(c.Image) { + return fmt.Errorf("container image for container at index %d contains uppercase letters: %s", idx, c.Image) + } + + if !allowCmdMod && (len(c.Command) != 0 || len(c.Args) != 0) { + return fmt.Errorf("container at index %d (image: %s): %w", idx, c.Image, scheduler.ErrNoCommandModification) + } + } + + return nil } diff --git a/cmd/linter/linter.go b/cmd/linter/linter.go index d96cef91..b12a21f3 100644 --- a/cmd/linter/linter.go +++ b/cmd/linter/linter.go @@ -9,10 +9,9 @@ import ( "log" "os" + "github.com/alecthomas/kong" "github.com/buildkite/agent-stack-k8s/v2/internal/controller/scheduler" "github.com/buildkite/go-buildkite/v3/buildkite" - "github.com/go-playground/validator/v10" - "github.com/spf13/cobra" "github.com/xeipuuv/gojsonschema" "sigs.k8s.io/yaml" ) @@ -25,39 +24,32 @@ const ( //go:embed schema.json var schema string -type Options struct { - File string `validate:"required,file"` +// CLI represents the lint subcommand for Kong. +type CLI struct { + File string `kong:"arg,required,help='Path to the pipeline file'"` } -func (o *Options) AddFlags(cmd *cobra.Command) { - cmd.Flags().StringVarP(&o.File, "file", "f", "", "path to the pipeline file, or {-} for stdin") -} - -func (o *Options) Validate() error { - return validator.New().Struct(o) -} - -func New() *cobra.Command { - o := &Options{} - - cmd := &cobra.Command{ - Use: "lint", - Short: "A tool for linting Buildkite pipelines", - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - if err := o.Validate(); err != nil { - return fmt.Errorf("failed to validate config: %w", err) - } - return Lint(cmd.Context(), o) - }, +// Run parses CLI arguments and executes the lint command. +func Run() error { + cli := &CLI{} + parser, err := kong.New( + cli, + kong.Name("agent-stack-k8s lint"), + kong.Description("A tool for linting Buildkite pipelines"), + kong.UsageOnError(), + ) + if err != nil { + return err } - o.AddFlags(cmd) + _, err = parser.Parse(os.Args[2:]) + parser.FatalIfErrorf(err) - return cmd + return Lint(context.Background(), cli.File) } -func Lint(ctx context.Context, options *Options) error { - contents, err := os.ReadFile(options.File) +// Lint validates a Buildkite pipeline file against the schema. +func Lint(ctx context.Context, file string) error { + contents, err := os.ReadFile(file) if err != nil { return fmt.Errorf("failed to open file: %w", err) } @@ -105,7 +97,7 @@ func Lint(ctx context.Context, options *Options) error { if err := schemaLoader.AddSchema(k8sSchema, gojsonschema.NewReferenceLoader(k8sSchema)); err != nil { return fmt.Errorf("failed to add kubernetes schema: %w", err) } - schema, err := schemaLoader.Compile(gojsonschema.NewStringLoader(schema)) + compiledSchema, err := schemaLoader.Compile(gojsonschema.NewStringLoader(schema)) if err != nil { return fmt.Errorf("failed to compile schemas: %w", err) } @@ -115,7 +107,7 @@ func Lint(ctx context.Context, options *Options) error { } documentLoader := gojsonschema.NewBytesLoader(bs) - result, err := schema.Validate(documentLoader) + result, err := compiledSchema.Validate(documentLoader) if err != nil { return fmt.Errorf("failed to validate: %w", err) } diff --git a/cmd/version/version.go b/cmd/version/version.go index a7f9acc7..ca7f9b8c 100644 --- a/cmd/version/version.go +++ b/cmd/version/version.go @@ -1,27 +1,14 @@ package version import ( - "context" "fmt" - "io" "os" "github.com/buildkite/agent-stack-k8s/v2/internal/version" - "github.com/spf13/cobra" ) -func New() *cobra.Command { - return &cobra.Command{ - Use: "version", - Short: "Prints the version", - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - return Version(cmd.Context(), os.Stdout) - }, - } -} - -func Version(ctx context.Context, out io.WriteCloser) error { - fmt.Fprintf(out, "%s\n", version.Version()) +// Run prints the version to stdout. +func Run() error { + fmt.Fprintf(os.Stdout, "%s\n", version.Version()) return nil } diff --git a/go.mod b/go.mod index 9c15fd46..b729c660 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.25.4 require ( github.com/Khan/genqlient v0.8.1 + github.com/alecthomas/kong v1.13.0 github.com/buildkite/agent/v3 v3.115.2 github.com/buildkite/go-buildkite/v3 v3.13.0 github.com/buildkite/roko v1.4.0 @@ -12,13 +13,10 @@ require ( github.com/go-playground/locales v0.14.1 github.com/go-playground/universal-translator v0.18.1 github.com/go-playground/validator/v10 v10.30.1 - github.com/go-viper/mapstructure/v2 v2.4.0 github.com/google/uuid v1.6.0 github.com/jedib0t/go-pretty/v6 v6.7.8 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/lmittmann/tint v1.1.2 - github.com/spf13/cobra v1.10.2 - github.com/spf13/viper v1.21.0 github.com/xeipuuv/gojsonschema v1.2.0 gotest.tools/gotestsum v1.13.0 k8s.io/api v0.35.0 @@ -38,6 +36,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/s3 v1.93.2 // indirect github.com/aws/aws-sdk-go-v2/service/signin v1.0.4 // indirect github.com/buildkite/zstash v0.7.0 // indirect + github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/klauspost/compress v1.18.2 // indirect github.com/saracen/zipextra v0.0.0-20250129175152-f1aa42d25216 // indirect github.com/wolfeidau/quickzip v1.0.2 // indirect @@ -139,7 +138,6 @@ require ( github.com/gowebpki/jcs v1.0.1 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 // indirect github.com/hashicorp/go-version v1.7.0 // indirect - github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/kylelemons/godebug v1.1.0 // indirect @@ -163,7 +161,6 @@ require ( github.com/opentracing/opentracing-go v1.2.0 // indirect github.com/outcaste-io/ristretto v0.2.3 // indirect github.com/pborman/uuid v1.2.1 // indirect - github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/philhofer/fwd v1.2.0 // indirect github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pkg/errors v0.9.1 // indirect @@ -180,16 +177,11 @@ require ( github.com/qri-io/jsonschema v0.2.1 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect - github.com/sagikazarmark/locafero v0.11.0 // indirect github.com/secure-systems-lab/go-securesystemslib v0.9.1 // indirect github.com/segmentio/asm v1.2.1 // indirect github.com/shirou/gopsutil/v4 v4.25.9 // indirect - github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect - github.com/spf13/afero v1.15.0 // indirect - github.com/spf13/cast v1.10.0 // indirect - github.com/spf13/pflag v1.0.10 + github.com/spf13/pflag v1.0.10 // indirect github.com/stretchr/testify v1.11.1 - github.com/subosito/gotenv v1.6.0 // indirect github.com/theckman/httpforwarded v0.4.0 // indirect github.com/tinylib/msgp v1.4.0 // indirect github.com/tklauser/go-sysconf v0.3.15 // indirect diff --git a/go.sum b/go.sum index d45851b3..41c8c1d6 100644 --- a/go.sum +++ b/go.sum @@ -66,6 +66,12 @@ github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERo github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/agnivade/levenshtein v1.2.1 h1:EHBY3UOn1gwdy/VbFwgo4cxecRznFk7fKWN1KOX7eoM= github.com/agnivade/levenshtein v1.2.1/go.mod h1:QVVI16kDrtSuwcpd0p1+xMC6Z/VfhtCyDIjcwga4/DU= +github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0= +github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= +github.com/alecthomas/kong v1.13.0 h1:5e/7XC3ugvhP1DQBmTS+WuHtCbcv44hsohMgcvVxSrA= +github.com/alecthomas/kong v1.13.0/go.mod h1:wrlbXem1CWqUV5Vbmss5ISYhsVPkBb1Yo7YKJghju2I= +github.com/alecthomas/repr v0.5.2 h1:SU73FTI9D1P5UNtvseffFSGmdNci/O6RsqzeXJtP0Qs= +github.com/alecthomas/repr v0.5.2/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4= github.com/alexflint/go-arg v1.5.1 h1:nBuWUCpuRy0snAG+uIJ6N0UvYxpxA0/ghA/AaHxlT8Y= github.com/alexflint/go-arg v1.5.1/go.mod h1:A7vTJzvjoaSTypg4biM5uYNTkJ27SkNTArtYXnlqVO8= github.com/alexflint/go-scalar v1.2.0 h1:WR7JPKkeNpnYIOfHRa7ivM21aWAdHD0gEWHCx+WQBRw= @@ -157,7 +163,6 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UF github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 h1:kHaBemcxl8o/pQ5VM1c8PVE1PubbNx3mjUr09OqWGCs= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575/go.mod h1:9d6lWj8KzO/fd/NrVaLscBKmPigpZpn5YawRPw+e3Yo= -github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo= github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= @@ -193,8 +198,6 @@ github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= -github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/fxamacker/cbor/v2 v2.9.0 h1:NpKPmjDBgUfBms6tr6JZkTHtfFGcMKsw3eGcmD/sapM= @@ -279,8 +282,8 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 h1:NmZ1PKzSTQbuGHw9DGPFomqkkLW github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3/go.mod h1:zQrxl1YP88HQlA6i9c63DSVPFklWpGX4OWAc9bFuaH4= github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY= github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= -github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= -github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= +github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= github.com/jedib0t/go-pretty/v6 v6.7.8 h1:BVYrDy5DPBA3Qn9ICT+PokP9cvCv1KaHv2i+Hc8sr5o= github.com/jedib0t/go-pretty/v6 v6.7.8/go.mod h1:YwC5CE4fJ1HFUDeivSV1r//AmANFHyqczZk+U6BDALU= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= @@ -356,8 +359,6 @@ github.com/outcaste-io/ristretto v0.2.3 h1:AK4zt/fJ76kjlYObOeNwh4T3asEuaCmp26pOv github.com/outcaste-io/ristretto v0.2.3/go.mod h1:W8HywhmtlopSB1jeMg3JtdIhf+DYkLAr0VN/s4+MHac= github.com/pborman/uuid v1.2.1 h1:+ZZIw58t/ozdjRaXh/3awHfmWRbzYxJoAdNJxe/3pvw= github.com/pborman/uuid v1.2.1/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= -github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= -github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/philhofer/fwd v1.2.0 h1:e6DnBTl7vGY+Gz322/ASL4Gyp1FspeMvx1RNDoToZuM= github.com/philhofer/fwd v1.2.0/go.mod h1:RqIHx9QI14HlwKwm98g9Re5prTQ6LdeRQn+gXJFxsJM= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= @@ -396,8 +397,6 @@ github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0t github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/sagikazarmark/locafero v0.11.0 h1:1iurJgmM9G3PA/I+wWYIOw/5SyBtxapeHDcg+AAIFXc= -github.com/sagikazarmark/locafero v0.11.0/go.mod h1:nVIGvgyzw595SUSUE6tvCp3YYTeHs15MvlmU87WwIik= github.com/saracen/zipextra v0.0.0-20250129175152-f1aa42d25216 h1:8zyjtFyKi5NJySVOJRiHmSN1vl6qugQ5n9C4X7WyY3U= github.com/saracen/zipextra v0.0.0-20250129175152-f1aa42d25216/go.mod h1:hnzuad9d2wdd3z8fC6UouHQK5qZxqv3F/E6MMzXc7q0= github.com/secure-systems-lab/go-securesystemslib v0.9.1 h1:nZZaNz4DiERIQguNy0cL5qTdn9lR8XKHf4RUyG1Sx3g= @@ -410,21 +409,10 @@ github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NF github.com/shirou/gopsutil/v4 v4.25.9 h1:JImNpf6gCVhKgZhtaAHJ0serfFGtlfIlSC08eaKdTrU= github.com/shirou/gopsutil/v4 v4.25.9/go.mod h1:gxIxoC+7nQRwUl/xNhutXlD8lq+jxTgpIkEf3rADHL8= github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= -github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 h1:+jumHNA0Wrelhe64i8F6HNlS8pkoyMv5sreGx2Ry5Rw= -github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8/go.mod h1:3n1Cwaq1E1/1lhQhtRK2ts/ZwZEhjcQeJQ1RuC6Q/8U= github.com/spaolacci/murmur3 v1.1.0 h1:7c1g84S4BPRrfL5Xrdp6fOJ206sU9y293DDHaoy0bLI= github.com/spaolacci/murmur3 v1.1.0/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= -github.com/spf13/afero v1.15.0 h1:b/YBCLWAJdFWJTN9cLhiXXcD7mzKn9Dm86dNnfyQw1I= -github.com/spf13/afero v1.15.0/go.mod h1:NC2ByUVxtQs4b3sIUphxK0NioZnmxgyCrfzeuq8lxMg= -github.com/spf13/cast v1.10.0 h1:h2x0u2shc1QuLHfxi+cTJvs30+ZAHOGRic8uyGTDWxY= -github.com/spf13/cast v1.10.0/go.mod h1:jNfB8QC9IA6ZuY2ZjDp0KtFO2LZZlg4S/7bzP6qqeHo= -github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= -github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= -github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= -github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= @@ -441,8 +429,6 @@ github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXl github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= -github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/theckman/httpforwarded v0.4.0 h1:N55vGJT+6ojTnLY3LQCNliJC4TW0P0Pkeys1G1WpX2w= github.com/theckman/httpforwarded v0.4.0/go.mod h1:GVkFynv6FJreNbgH/bpOU9ITDZ7a5WuzdNCtIMI1pVI= github.com/tinylib/msgp v1.4.0 h1:SYOeDRiydzOw9kSiwdYp9UcBgPFtLU2WDHaJXyHruf8= diff --git a/internal/controller/config/command_params.go b/internal/controller/config/command_params.go index bd47d93c..40c12376 100644 --- a/internal/controller/config/command_params.go +++ b/internal/controller/config/command_params.go @@ -1,8 +1,8 @@ package config import ( + "encoding/json" "fmt" - "reflect" "slices" "strings" @@ -129,23 +129,16 @@ var AllowedInterposers = []Interposer{ // BUILDKITE_COMMAND. type Interposer string -var interposerType = reflect.TypeOf(InterposerBuildkite) - -// StringToInterposer implements a [mapstructure.DecodeHookFunc] for decoding -// a string into CmdInterposer. -func StringToInterposer(from, to reflect.Type, data any) (any, error) { - if from.Kind() != reflect.String { - return data, nil - } - if to != interposerType { - return data, nil - } - interp := Interposer(data.(string)) - if interp == "" { - return "", nil +// UnmarshalJSON validates that the interposer value is one of the allowed values. +func (i *Interposer) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return err } + interp := Interposer(s) if !slices.Contains(AllowedInterposers, interp) { - return data, fmt.Errorf("invalid command interposer %q", interp) + return fmt.Errorf("invalid interposer %q, must be one of: %v", s, AllowedInterposers) } - return interp, nil + *i = interp + return nil } diff --git a/internal/controller/config/config.go b/internal/controller/config/config.go index 6f1be7d1..38376be2 100644 --- a/internal/controller/config/config.go +++ b/internal/controller/config/config.go @@ -37,9 +37,7 @@ const ( var DefaultAgentImage = "ghcr.io/buildkite/agent:" + version.Version() -// viper requires mapstructure struct tags, but the k8s types only have json struct tags. -// mapstructure (the module) supports switching the struct tag to "json", viper does not. So we have -// to have the `mapstructure` tag for viper and the `json` tag is used by the mapstructure! +// Config holds the controller configuration. type Config struct { Debug bool `json:"debug"` diff --git a/internal/integration/main_test.go b/internal/integration/main_test.go index e83d70c9..a5671195 100644 --- a/internal/integration/main_test.go +++ b/internal/integration/main_test.go @@ -11,7 +11,6 @@ import ( "github.com/buildkite/agent-stack-k8s/v2/cmd/controller" "github.com/buildkite/agent-stack-k8s/v2/internal/controller/config" - "github.com/spf13/cobra" ) const ( @@ -32,14 +31,24 @@ var ( ) func TestMain(m *testing.M) { - cmd := &cobra.Command{} - controller.AddConfigFlags(cmd) - v, err := controller.ReadConfigFromFileArgsAndEnv(cmd, os.Args[1:]) - if err != nil { - log.Fatalf("Error reading config: %s", err) + // Build args for config parsing from os.Args + // Accepts -f, --config, -f=, --config= flags + var configArgs []string + for i, arg := range os.Args { + if arg == "-f" || arg == "--config" { + if i+1 < len(os.Args) { + configArgs = append(configArgs, "--config="+os.Args[i+1]) + } + } + if strings.HasPrefix(arg, "-f=") { + configArgs = append(configArgs, "--config="+strings.TrimPrefix(arg, "-f=")) + } + if strings.HasPrefix(arg, "--config=") { + configArgs = append(configArgs, arg) + } } - testCfg, err := controller.ParseAndValidateConfig(v) + testCfg, err := controller.BuildConfigFromArgs(configArgs) if err != nil { log.Fatalf("Error parsing config: %s", err) } diff --git a/main.go b/main.go index 51fa82dd..e5639d00 100644 --- a/main.go +++ b/main.go @@ -7,7 +7,7 @@ import ( ) func main() { - if err := controller.New().Execute(); err != nil { + if err := controller.Run(); err != nil { log.Fatal(err) } }