Merge pull request #19 from OctopusDeploy/mattr/fix-config-parsing

Fix config parsing of bools

Author: octocolby

config.go

@@ -0,0 +1,85 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/spf13/pflag"
+	"github.com/spf13/viper"
+)
+
+type Config struct {
+	URL     string
+	APIKey  string
+	Name    string
+	Version string
+	Parent  string
+	Tags    string
+	SBOM    string
+	Poll    bool
+	Latest  bool
+}
+
+func (c *Config) validate() error {
+	if c.URL == "" {
+		return fmt.Errorf("missing required input: url (via --url or SBOM_UPLOADER_URL)")
+	}
+	if c.APIKey == "" {
+		return fmt.Errorf("missing required input: api-key (via --api-key or SBOM_UPLOADER_API_KEY)")
+	}
+	if c.Name == "" {
+		return fmt.Errorf("missing required input: name (via --name or SBOM_UPLOADER_NAME)")
+	}
+	if c.Parent == "" {
+		return fmt.Errorf("missing required input: parent (via --parent or SBOM_UPLOADER_PARENT)")
+	}
+	if c.Version == "" {
+		return fmt.Errorf("missing required input: version (via --version or SBOM_UPLOADER_VERSION)")
+	}
+	return nil
+}
+
+// loadConfig resolves configuration from flags and environment variables.
+// Flags take precedence over env vars; env vars take precedence over defaults.
+//
+// AutomaticEnv + BindPFlags has a known issue where bool flags with a false
+// default have their pflag default shadow the env var. We work around it with
+// explicit BindEnv calls for bool flags.
+// See https://github.com/spf13/viper/issues/671
+func loadConfig(flags *pflag.FlagSet) (*Config, error) {
+	v := viper.New()
+	v.SetEnvPrefix("SBOM_UPLOADER")
+	v.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
+	v.AutomaticEnv()
+	if err := errors.Join(
+		v.BindPFlags(flags),
+		v.BindEnv("poll", "SBOM_UPLOADER_POLL"),
+		v.BindEnv("latest", "SBOM_UPLOADER_LATEST"),
+	); err != nil {
+		return nil, err
+	}
+	return &Config{
+		URL:     v.GetString("url"),
+		APIKey:  v.GetString("api-key"),
+		Name:    v.GetString("name"),
+		Version: v.GetString("version"),
+		Parent:  v.GetString("parent"),
+		Tags:    v.GetString("tags"),
+		SBOM:    v.GetString("sbom"),
+		Poll:    v.GetBool("poll"),
+		Latest:  v.GetBool("latest"),
+	}, nil
+}
+
+func setFlags(s *pflag.FlagSet) {
+	s.String("url", "", "Dependency-Track API base URL or env SBOM_UPLOADER_URL")
+	s.String("api-key", "", "Dependency-Track API key or env SBOM_UPLOADER_API_KEY")
+	s.String("name", "", "Project name or env SBOM_UPLOADER_NAME")
+	s.String("version", "", "Project version or env SBOM_UPLOADER_VERSION")
+	s.String("parent", "", "Parent project name or env SBOM_UPLOADER_PARENT")
+	s.Bool("latest", true, "Mark as latest version (default true)")
+	s.Bool("poll", false, "Poll until import completes or env SBOM_UPLOADER_POLL")
+	s.String("tags", "", "Comma-separated project tags or env SBOM_UPLOADER_TAGS")
+	s.String("sbom", "", "Path to SBOM file (optional; otherwise read from stdin)")
+}

config_test.go

@@ -0,0 +1,203 @@
+package main
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/spf13/pflag"
+)
+
+// newFlagSet returns a FlagSet populated with the same flags as the real CLI.
+func newFlagSet() *pflag.FlagSet {
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	setFlags(flags)
+	return flags
+}
+
+func TestLoadConfig_StringsFromEnvVars(t *testing.T) {
+	t.Setenv("SBOM_UPLOADER_URL", "https://example.com")
+	t.Setenv("SBOM_UPLOADER_API_KEY", "my-api-key")
+	t.Setenv("SBOM_UPLOADER_NAME", "my-project")
+	t.Setenv("SBOM_UPLOADER_VERSION", "1.2.3")
+	t.Setenv("SBOM_UPLOADER_PARENT", "my-parent")
+	t.Setenv("SBOM_UPLOADER_TAGS", "tag1,tag2")
+	t.Setenv("SBOM_UPLOADER_SBOM", "/path/to/sbom.json")
+
+	cfg, err := loadConfig(newFlagSet())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if cfg.URL != "https://example.com" {
+		t.Errorf("URL: got %q, want %q", cfg.URL, "https://example.com")
+	}
+	if cfg.APIKey != "my-api-key" {
+		t.Errorf("APIKey: got %q, want %q", cfg.APIKey, "my-api-key")
+	}
+	if cfg.Name != "my-project" {
+		t.Errorf("Name: got %q, want %q", cfg.Name, "my-project")
+	}
+	if cfg.Version != "1.2.3" {
+		t.Errorf("Version: got %q, want %q", cfg.Version, "1.2.3")
+	}
+	if cfg.Parent != "my-parent" {
+		t.Errorf("Parent: got %q, want %q", cfg.Parent, "my-parent")
+	}
+	if cfg.Tags != "tag1,tag2" {
+		t.Errorf("Tags: got %q, want %q", cfg.Tags, "tag1,tag2")
+	}
+	if cfg.SBOM != "/path/to/sbom.json" {
+		t.Errorf("SBOM: got %q, want %q", cfg.SBOM, "/path/to/sbom.json")
+	}
+}
+
+func TestLoadConfig_PollFromEnvVar(t *testing.T) {
+	t.Setenv("SBOM_UPLOADER_POLL", "true")
+
+	cfg, err := loadConfig(newFlagSet())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !cfg.Poll {
+		t.Error("Poll: expected true from SBOM_UPLOADER_POLL env var, got false")
+	}
+}
+
+func TestLoadConfig_LatestFromEnvVar(t *testing.T) {
+	t.Setenv("SBOM_UPLOADER_LATEST", "false")
+
+	cfg, err := loadConfig(newFlagSet())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if cfg.Latest {
+		t.Error("Latest: expected false from SBOM_UPLOADER_LATEST env var, got true")
+	}
+}
+
+func TestLoadConfig_FromFlags(t *testing.T) {
+	flags := newFlagSet()
+	err := flags.Parse([]string{
+		"--url", "https://from-flag.com",
+		"--api-key", "flag-key",
+		"--name", "flag-project",
+		"--version", "2.0.0",
+		"--parent", "flag-parent",
+		"--tags", "a,b",
+		"--sbom", "/flag/sbom.json",
+		"--poll",
+		"--latest=false",
+	})
+	if err != nil {
+		t.Fatalf("failed to parse flags: %v", err)
+	}
+
+	cfg, err := loadConfig(flags)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if cfg.URL != "https://from-flag.com" {
+		t.Errorf("URL: got %q, want %q", cfg.URL, "https://from-flag.com")
+	}
+	if cfg.APIKey != "flag-key" {
+		t.Errorf("APIKey: got %q, want %q", cfg.APIKey, "flag-key")
+	}
+	if cfg.Name != "flag-project" {
+		t.Errorf("Name: got %q, want %q", cfg.Name, "flag-project")
+	}
+	if cfg.Version != "2.0.0" {
+		t.Errorf("Version: got %q, want %q", cfg.Version, "2.0.0")
+	}
+	if cfg.Parent != "flag-parent" {
+		t.Errorf("Parent: got %q, want %q", cfg.Parent, "flag-parent")
+	}
+	if cfg.Tags != "a,b" {
+		t.Errorf("Tags: got %q, want %q", cfg.Tags, "a,b")
+	}
+	if cfg.SBOM != "/flag/sbom.json" {
+		t.Errorf("SBOM: got %q, want %q", cfg.SBOM, "/flag/sbom.json")
+	}
+	if !cfg.Poll {
+		t.Error("Poll: expected true from --poll flag, got false")
+	}
+	if cfg.Latest {
+		t.Error("Latest: expected false from --latest=false flag, got true")
+	}
+}
+
+func TestLoadConfig_FlagOverridesEnvVar(t *testing.T) {
+	t.Setenv("SBOM_UPLOADER_URL", "https://from-env.com")
+
+	flags := newFlagSet()
+	if err := flags.Parse([]string{"--url", "https://from-flag.com"}); err != nil {
+		t.Fatalf("failed to parse flags: %v", err)
+	}
+
+	cfg, err := loadConfig(flags)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if cfg.URL != "https://from-flag.com" {
+		t.Errorf("URL: expected flag to override env var, got %q", cfg.URL)
+	}
+}
+
+func TestLoadConfig_Defaults(t *testing.T) {
+	cfg, err := loadConfig(newFlagSet())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if cfg.Poll {
+		t.Error("Poll: expected false by default, got true")
+	}
+	if !cfg.Latest {
+		t.Error("Latest: expected true by default, got false")
+	}
+}
+
+// --- Config.validate ---
+
+func validConfig() *Config {
+	return &Config{
+		URL:     "https://example.com",
+		APIKey:  "key",
+		Name:    "project",
+		Parent:  "parent",
+		Version: "1.0.0",
+	}
+}
+
+func TestValidate_PassesWithAllRequiredFields(t *testing.T) {
+	if err := validConfig().validate(); err != nil {
+		t.Errorf("expected no error, got: %v", err)
+	}
+}
+
+func TestValidate_RequiredFields(t *testing.T) {
+	tests := []struct {
+		field   string
+		mutate  func(*Config)
+		wantMsg string
+	}{
+		{"URL", func(c *Config) { c.URL = "" }, "url"},
+		{"APIKey", func(c *Config) { c.APIKey = "" }, "api-key"},
+		{"Name", func(c *Config) { c.Name = "" }, "name"},
+		{"Parent", func(c *Config) { c.Parent = "" }, "parent"},
+		{"Version", func(c *Config) { c.Version = "" }, "version"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.field, func(t *testing.T) {
+			cfg := validConfig()
+			tt.mutate(cfg)
+			err := cfg.validate()
+			if err == nil {
+				t.Fatalf("expected error for missing %s, got nil", tt.field)
+			}
+			if !strings.Contains(err.Error(), tt.wantMsg) {
+				t.Errorf("error %q does not mention %q", err.Error(), tt.wantMsg)
+			}
+		})
+	}
+}