From 29b4d1bff42bd2e368c1e1fe8ad5dce789d5c11f Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 14 Oct 2025 12:09:06 -0400 Subject: [PATCH 01/13] feat(service/config): introduce typed Secret with inline env/file/literal, BindServiceConfig; integrate in KAS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add Secret type with redaction and safe JSON/log masking - Support inline secret directives: env:NAME, file:/path, literal:VALUE - Add BindServiceConfig helper with mapstructure decode hooks + validation - Implement file-based and env-based secret resolution (lazy/eager) - Integrate KAS: use Secret for root_key and BindServiceConfig - Improve env handling during reload to surface env-only keys - Add unit tests for Secret, BindServiceConfig, and inline decode forms - Update docs: “Secrets In Config” with examples and guidance - Remove temporary debug logs and experimental nested secret usage in KAS DX - Plain strings are literals by default; use literal: to disambiguate values starting with env:/file: - Example: - services.kas.root_key: "env:OPENTDF_SERVICES_KAS_ROOT_KEY" - services.kas.preview.key_management: true --- docs/Configuring.md | 37 ++++- service/kas/access/provider.go | 2 +- service/kas/kas.go | 107 ++++++++------ service/pkg/config/bind.go | 77 +++++++++++ service/pkg/config/bind_test.go | 42 ++++++ service/pkg/config/config.go | 34 +++++ service/pkg/config/decode.go | 53 +++++++ service/pkg/config/decode_test.go | 48 +++++++ .../pkg/config/environment_value_loader.go | 39 +++++- service/pkg/config/secret.go | 130 ++++++++++++++++++ service/pkg/config/secret_test.go | 53 +++++++ service/pkg/config/walk.go | 67 +++++++++ 12 files changed, 645 insertions(+), 44 deletions(-) create mode 100644 service/pkg/config/bind.go create mode 100644 service/pkg/config/bind_test.go create mode 100644 service/pkg/config/decode.go create mode 100644 service/pkg/config/decode_test.go create mode 100644 service/pkg/config/secret.go create mode 100644 service/pkg/config/secret_test.go create mode 100644 service/pkg/config/walk.go diff --git a/docs/Configuring.md b/docs/Configuring.md index 0671954c2e..7a60a56481 100644 --- a/docs/Configuring.md +++ b/docs/Configuring.md @@ -22,7 +22,8 @@ The platform leverages [viper](https://github.com/spf13/viper) to help load conf - [Configuration in opentdf-example.yaml](#configuration-in-opentdf-exampleyaml) - [Role Permissions](#role-permissions) - [Managing Authorization Policy](#managing-authorization-policy) - - [Cache Configuration](#cache-configuration) +- [Cache Configuration](#cache-configuration) + - [Secrets In Config](#secrets-in-config) ## Deployment Mode @@ -305,6 +306,40 @@ Root level key `policy` | `list_request_limit_default` | Policy List request limit default when not provided | 1000 | OPENTDF_SERVICES_POLICY_LIST_REQUEST_LIMIT_DEFAULT | | `list_request_limit_max` | Policy List request limit maximum enforced by services | 2500 | OPENTDF_SERVICES_POLICY_LIST_REQUEST_LIMIT_MAX | +## Secrets In Config + +Some service configuration fields are sensitive (secrets). The platform supports convenient, safe ways to provide these values in YAML without leaking them in logs: + +- Literal value: use `literal:` prefix +- Environment variable reference: use `env:` prefix +- File reference (e.g., mounted secret): use `file:` prefix + +Examples: + +```yaml +services: + kas: + # Provide a root key via environment variable + root_key: "env:OPENTDF_SERVICES_KAS_ROOT_KEY" + + # Or as a literal (avoid in production) + # root_key: "literal:493ff7acd07b..." + + # Or from a file path (e.g., k8s secret volume) + # root_key: "file:/var/run/secrets/opentdf/kas_root_key" + + preview: + key_management: true +``` + +Notes: +- Secrets are redacted in logs and JSON output; only a redacted summary is shown. +- For nested secret fields in service-specific configs, prefer the inline `env:` form in YAML to avoid underscore-to-dot mapping issues with environment variables alone. +- You can still use a structured form for references: + - `root_key: { fromEnv: "OPENTDF_SERVICES_KAS_ROOT_KEY" }` + - `root_key: { fromFile: "/path/to/secret" }` + + Example: ```yaml diff --git a/service/kas/access/provider.go b/service/kas/access/provider.go index fc6235abb1..8c06e59a88 100644 --- a/service/kas/access/provider.go +++ b/service/kas/access/provider.go @@ -40,7 +40,7 @@ type KASConfig struct { // Deprecated RSACertID string `mapstructure:"rsacertid" json:"rsacertid"` - RootKey string `mapstructure:"root_key" json:"root_key"` + RootKey config.Secret `mapstructure:"root_key" json:"root_key"` KeyCacheExpiration time.Duration `mapstructure:"key_cache_expiration" json:"key_cache_expiration"` diff --git a/service/kas/kas.go b/service/kas/kas.go index 95ffdc3876..c7ae5453d4 100644 --- a/service/kas/kas.go +++ b/service/kas/kas.go @@ -2,13 +2,13 @@ package kas import ( "context" + "errors" "fmt" "log/slog" "net" "net/url" "strings" - "github.com/go-viper/mapstructure/v2" kaspb "github.com/opentdf/platform/protocol/go/kas" "github.com/opentdf/platform/protocol/go/kas/kasconnect" "github.com/opentdf/platform/service/internal/security" @@ -21,9 +21,9 @@ import ( ) func OnConfigUpdate(p *access.Provider) serviceregistry.OnConfigUpdateHook { - return func(_ context.Context, cfg config.ServiceConfig) error { + return func(ctx context.Context, cfg config.ServiceConfig) error { var kasCfg access.KASConfig - if err := mapstructure.Decode(cfg, &kasCfg); err != nil { + if err := config.BindServiceConfig(ctx, cfg, &kasCfg); err != nil { return fmt.Errorf("invalid kas cfg [%v] %w", cfg, err) } @@ -46,10 +46,9 @@ func NewRegistration() *serviceregistry.Service[kasconnect.AccessServiceHandler] OnConfigUpdate: onConfigUpdate, RegisterFunc: func(srp serviceregistry.RegistrationParams) (kasconnect.AccessServiceHandler, serviceregistry.HandlerServer) { var kasCfg access.KASConfig - if err := mapstructure.Decode(srp.Config, &kasCfg); err != nil { + if err := config.BindServiceConfig(context.Background(), srp.Config, &kasCfg); err != nil { panic(fmt.Errorf("invalid kas cfg [%v] %w", srp.Config, err)) } - var cacheClient *cache.Cache if kasCfg.KeyCacheExpiration != 0 { var err error @@ -64,48 +63,20 @@ func NewRegistration() *serviceregistry.Service[kasconnect.AccessServiceHandler] var kmgrNames []string if kasCfg.Preview.KeyManagement { - srp.Logger.Info("preview feature: key management is enabled") - - kasURL, err := determineKASURL(srp, kasCfg) - if err != nil { - panic(fmt.Errorf("failed to determine KAS URL: %w", err)) + if err := handleKeyManagement(srp, kasCfg, p, cacheClient); err != nil { + panic(err) } - - srp.Logger.Debug("determined KAS URL", slog.String("kas_url", kasURL.String())) - - // Configure new delegation service - p.KeyDelegator = trust.NewDelegatingKeyService(NewPlatformKeyIndexer(srp.SDK, kasURL.String(), srp.Logger), srp.Logger, cacheClient) + // Track registered manager names for logging for _, manager := range srp.KeyManagerFactories { - p.KeyDelegator.RegisterKeyManager(manager.Name, manager.Factory) kmgrNames = append(kmgrNames, manager.Name) } - - // Register Basic Key Manager - p.KeyDelegator.RegisterKeyManager(security.BasicManagerName, func(opts *trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { - bm, err := security.NewBasicManager(opts.Logger, opts.Cache, kasCfg.RootKey) - if err != nil { - return nil, err - } - return bm, nil - }) kmgrNames = append(kmgrNames, security.BasicManagerName) - // Explicitly set the default manager for session key generation. - // This should be configurable, e.g., defaulting to BasicManager or an HSM if available. - p.KeyDelegator.SetDefaultMode(security.BasicManagerName) // Example: default to BasicManager } else { - // Set up both the legacy CryptoProvider and the new SecurityProvider - kasCfg.UpgradeMapToKeyring(srp.OTDF.CryptoProvider) - p.CryptoProvider = srp.OTDF.CryptoProvider - - inProcessService := initSecurityProviderAdapter(p.CryptoProvider, kasCfg, srp.Logger) - - p.KeyDelegator = trust.NewDelegatingKeyService(inProcessService, srp.Logger, nil) - p.KeyDelegator.RegisterKeyManager(inProcessService.Name(), func(*trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { - return inProcessService, nil - }) - // Set default for non-key-management mode - p.KeyDelegator.SetDefaultMode(inProcessService.Name()) - kmgrNames = append(kmgrNames, inProcessService.Name()) + name, err := handleLegacyMode(srp, kasCfg, p) + if err != nil { + panic(err) + } + kmgrNames = append(kmgrNames, name) } srp.Logger.Info("kas registered trust.KeyManagers", slog.Any("key_managers", kmgrNames)) @@ -181,6 +152,60 @@ func determineKASURL(srp serviceregistry.RegistrationParams, kasCfg access.KASCo return kasURL, nil } +func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider, cacheClient *cache.Cache) error { + srp.Logger.Info("preview feature: key management is enabled") + srp.Logger.Debug("kas root_key summary", slog.Any("root_key", kasCfg.RootKey)) + srp.Logger.Debug("kas preview settings", slog.Any("preview", kasCfg.Preview)) + + kasURL, err := determineKASURL(srp, kasCfg) + if err != nil { + return fmt.Errorf("failed to determine KAS URL: %w", err) + } + srp.Logger.Debug("determined KAS URL", slog.String("kas_url", kasURL.String())) + + // Configure new delegation service + p.KeyDelegator = trust.NewDelegatingKeyService(NewPlatformKeyIndexer(srp.SDK, kasURL.String(), srp.Logger), srp.Logger, cacheClient) + for _, manager := range srp.KeyManagerFactories { + p.KeyDelegator.RegisterKeyManager(manager.Name, manager.Factory) + } + + // Register Basic Key Manager + p.KeyDelegator.RegisterKeyManager(security.BasicManagerName, func(opts *trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { + // RootKey is required when key management is enabled. + if kasCfg.RootKey.IsZero() { + return nil, errors.New("root_key is required when preview.key_management is enabled; set OPENTDF_SERVICES_KAS_ROOT_KEY or services.kas.root_key") + } + rk, err := kasCfg.RootKey.Resolve(context.Background()) + if err != nil { + return nil, fmt.Errorf("failed to resolve root_key: %w", err) + } + bm, err := security.NewBasicManager(opts.Logger, opts.Cache, rk) + if err != nil { + return nil, err + } + return bm, nil + }) + // Explicitly set the default manager for session key generation. + p.KeyDelegator.SetDefaultMode(security.BasicManagerName) + return nil +} + +func handleLegacyMode(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider) (string, error) { //nolint:unparam // maintains a consistent signature with other handlers + // Set up both the legacy CryptoProvider and the new SecurityProvider + kasCfg.UpgradeMapToKeyring(srp.OTDF.CryptoProvider) + p.CryptoProvider = srp.OTDF.CryptoProvider + + inProcessService := initSecurityProviderAdapter(p.CryptoProvider, kasCfg, srp.Logger) + + p.KeyDelegator = trust.NewDelegatingKeyService(inProcessService, srp.Logger, nil) + p.KeyDelegator.RegisterKeyManager(inProcessService.Name(), func(*trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { + return inProcessService, nil + }) + // Set default for non-key-management mode + p.KeyDelegator.SetDefaultMode(inProcessService.Name()) + return inProcessService.Name(), nil +} + func initSecurityProviderAdapter(cryptoProvider *security.StandardCrypto, kasCfg access.KASConfig, l *logger.Logger) trust.KeyService { var defaults []string var legacies []string diff --git a/service/pkg/config/bind.go b/service/pkg/config/bind.go new file mode 100644 index 0000000000..baa54293d8 --- /dev/null +++ b/service/pkg/config/bind.go @@ -0,0 +1,77 @@ +package config + +import ( + "context" + "errors" + "fmt" + + "github.com/go-playground/validator/v10" + "github.com/go-viper/mapstructure/v2" +) + +// BindOptions controls how service config binding behaves. +type BindOptions struct { + // Eagerly resolve secrets during binding; otherwise they resolve lazily. + EagerResolve bool +} + +// BindOption functional option. +type BindOption func(*BindOptions) + +// WithEagerSecretResolution enables eager secret resolution during bind. +func WithEagerSecretResolution() BindOption { return func(o *BindOptions) { o.EagerResolve = true } } + +// BindServiceConfig decodes a ServiceConfig map into a typed struct target using +// mapstructure with custom decode hooks (e.g., Secret). It optionally validates +// the result and can eagerly resolve secret values. +func BindServiceConfig[T any](ctx context.Context, svcCfg ServiceConfig, out *T, opts ...BindOption) error { + var options BindOptions + for _, o := range opts { + o(&options) + } + + if out == nil { + return errors.New("nil output target") + } + + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.ComposeDecodeHookFunc(secretDecodeHook), + Result: out, + TagName: "mapstructure", + Squash: true, + ZeroFields: true, + }) + if err != nil { + return fmt.Errorf("bind decoder: %w", err) + } + if err := dec.Decode(svcCfg); err != nil { + return fmt.Errorf("bind decode: %w", err) + } + + // Eager secret resolution + if options.EagerResolve { + if err := resolveSecrets(ctx, out); err != nil { + return err + } + } + + // Validate struct using go-playground/validator tags, if present + if err := validator.New().Struct(out); err != nil { + return err + } + return nil +} + +// resolveSecrets walks the struct and resolves any Secret fields. +func resolveSecrets(ctx context.Context, v any) error { + // Reflectively find Secret fields; keep it minimal and safe. + // We only walk exported fields of structs and slices/maps. + return walk(v, func(s *Secret) error { + // Skip zero-value secrets (unset optional fields) + if s.IsZero() { + return nil + } + _, err := s.Resolve(ctx) + return err + }) +} diff --git a/service/pkg/config/bind_test.go b/service/pkg/config/bind_test.go new file mode 100644 index 0000000000..89f02a8a34 --- /dev/null +++ b/service/pkg/config/bind_test.go @@ -0,0 +1,42 @@ +package config + +import ( + "context" + "testing" +) + +type demoCfg struct { + User string `mapstructure:"user" validate:"required"` + Pass Secret `mapstructure:"pass"` + Nested struct { + Token Secret `mapstructure:"token"` + } `mapstructure:"nested"` +} + +func TestBindServiceConfig_LiteralAndEnv(t *testing.T) { + t.Setenv("OPENTDF_DEMO_TOKEN", "tok") + + in := ServiceConfig{ + "user": "alice", + "pass": "p@ss", + "nested": map[string]any{ + "token": map[string]any{"fromEnv": "OPENTDF_DEMO_TOKEN"}, + }, + } + + var out demoCfg + if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { + t.Fatalf("bind: %v", err) + } + if out.User != "alice" { + t.Fatalf("user mismatch: %q", out.User) + } + pass, err := out.Pass.Resolve(context.Background()) + if err != nil || pass != "p@ss" { + t.Fatalf("pass resolve: %v %q", err, pass) + } + tok, err := out.Nested.Token.Resolve(context.Background()) + if err != nil || tok != "tok" { + t.Fatalf("token resolve: %v %q", err, tok) + } +} diff --git a/service/pkg/config/config.go b/service/pkg/config/config.go index 26d1510b85..74991ab09e 100644 --- a/service/pkg/config/config.go +++ b/service/pkg/config/config.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log/slog" + "strings" "sync" "github.com/go-playground/validator/v10" @@ -190,6 +191,22 @@ func (c *Config) Reload(ctx context.Context) error { assigned = make(map[string]struct{}) orderedViper := viper.NewWithOptions(viper.WithLogger(slog.Default())) + // If an environment loader is present, configure orderedViper to read env directly. + for _, loader := range c.loaders { + if loader.Name() == LoaderNameEnvironmentValue { + // Match the environment loader's behavior + orderedViper.SetEnvPrefix("OPENTDF") // default; overridden below if loader exposes different + if evl, ok := loader.(*EnvironmentValueLoader); ok { + if evl.envPrefix != "" { + orderedViper.SetEnvPrefix(evl.envPrefix) + } + } + orderedViper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + orderedViper.AutomaticEnv() + break + } + } + // Loop through loaders in their order of priority. for _, loader := range c.loaders { // The Load call allows the loader to refresh its internal state (e.g., re-read file, re-query DB). @@ -228,6 +245,23 @@ func (c *Config) Reload(ctx context.Context) error { continue } if loaderValue != nil { + // Avoid logging sensitive values + if strings.Contains(strings.ToLower(key), "secret") { + slog.DebugContext( + ctx, + "config merge", + slog.String("loader", loader.Name()), + slog.String("key", key), + slog.String("value", "[REDACTED]"), + ) + } else { + slog.DebugContext( + ctx, + "config merge", + slog.String("loader", loader.Name()), + slog.String("key", key), + ) + } orderedViper.Set(key, loaderValue) assigned[key] = struct{}{} } diff --git a/service/pkg/config/decode.go b/service/pkg/config/decode.go new file mode 100644 index 0000000000..475ef546f4 --- /dev/null +++ b/service/pkg/config/decode.go @@ -0,0 +1,53 @@ +package config + +import ( + "errors" + "fmt" + "reflect" + "strings" +) + +// secretDecodeHook converts supported inputs into Secret. +// Supported inputs: +// - string -> literal secret +// - map[string]any -> reference, e.g., {"fromEnv":"OPENTDF_FOO"} +func secretDecodeHook(from, to reflect.Type, data any) (any, error) { + // Only target Secret type + if to != reflect.TypeOf(Secret{}) { + return data, nil + } + + //nolint:exhaustive // reflect.Kind has many variants; we only handle string and map inputs here + switch from.Kind() { + case reflect.String: + // Support friendly inline directives: "env:VAR", "file:/path", "literal:..." + s := reflect.ValueOf(data).String() + switch { + case strings.HasPrefix(s, "env:") && len(s) > len("env:"): + return NewEnvSecret(strings.TrimPrefix(s, "env:")), nil + case strings.HasPrefix(s, "file:") && len(s) > len("file:"): + return NewFileSecret(strings.TrimPrefix(s, "file:")), nil + case strings.HasPrefix(s, "literal:"): + return NewLiteralSecret(strings.TrimPrefix(s, "literal:")), nil + default: + // Default to literal + return NewLiteralSecret(s), nil + } + case reflect.Map: + // Must be map[string]any + m, okm := data.(map[string]any) + if !okm { + return nil, fmt.Errorf("invalid secret map type: %T", data) + } + if env, ok := m["fromEnv"].(string); ok && env != "" { + return NewEnvSecret(env), nil + } + if file, ok2 := m["fromFile"].(string); ok2 && file != "" { + return NewFileSecret(file), nil + } + // Future: support {"fromURI":"aws-secretsmanager://..."} + return nil, errors.New("unsupported secret map, expected {fromEnv:string}") + default: + return nil, fmt.Errorf("cannot decode %s into Secret", from.Kind()) + } +} diff --git a/service/pkg/config/decode_test.go b/service/pkg/config/decode_test.go new file mode 100644 index 0000000000..e0c0a8ec70 --- /dev/null +++ b/service/pkg/config/decode_test.go @@ -0,0 +1,48 @@ +package config + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +type inlineCfg struct { + A Secret `mapstructure:"a"` + B Secret `mapstructure:"b"` + C Secret `mapstructure:"c"` +} + +func TestDecodeHook_InlineForms(t *testing.T) { + // Prepare a temp file for file: form + dir := t.TempDir() + f := filepath.Join(dir, "secret.txt") + if err := os.WriteFile(f, []byte("from-file"), 0o600); err != nil { + t.Fatalf("write file: %v", err) + } + t.Setenv("OPENTDF_TEST_INLINE", "from-env") + + in := ServiceConfig{ + "a": "literal:abc", + "b": "env:OPENTDF_TEST_INLINE", + "c": "file:" + f, + } + + var out inlineCfg + if err := BindServiceConfig(context.Background(), in, &out); err != nil { + t.Fatalf("bind: %v", err) + } + + a, err := out.A.Resolve(context.Background()) + if err != nil || a != "abc" { + t.Fatalf("literal: %v %q", err, a) + } + b, err := out.B.Resolve(context.Background()) + if err != nil || b != "from-env" { + t.Fatalf("env: %v %q", err, b) + } + c, err := out.C.Resolve(context.Background()) + if err != nil || c != "from-file" { + t.Fatalf("file: %v %q", err, c) + } +} diff --git a/service/pkg/config/environment_value_loader.go b/service/pkg/config/environment_value_loader.go index ef57c54bf9..d262dab78b 100644 --- a/service/pkg/config/environment_value_loader.go +++ b/service/pkg/config/environment_value_loader.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log/slog" + "os" "strings" "github.com/spf13/viper" @@ -15,6 +16,7 @@ const LoaderNameEnvironmentValue = "environment-value" type EnvironmentValueLoader struct { allowListMap map[string]struct{} viper *viper.Viper + envPrefix string } // NewEnvironmentValueLoader creates a new Viper-based configuration loader @@ -40,6 +42,7 @@ func NewEnvironmentValueLoader(key string, allowList []string) (*EnvironmentValu result := &EnvironmentValueLoader{ allowListMap: allowListMap, viper: v, + envPrefix: strings.ToUpper(key), } return result, nil } @@ -56,7 +59,41 @@ func (l *EnvironmentValueLoader) Get(key string) (any, error) { // GetConfigKeys returns all the configuration keys found in the environment variables. func (l *EnvironmentValueLoader) GetConfigKeys() ([]string, error) { - return l.viper.AllKeys(), nil + // Start with keys Viper already knows about (from defaults/file loads) + keys := make([]string, 0, len(l.viper.AllKeys())) + keys = append(keys, l.viper.AllKeys()...) + + // Discover environment variables with the configured prefix and convert them to dotted keys + // Example: OPENTDF_SERVICES_KAS_ROOT_KEY -> services.kas.root_key + // Note: Viper treats keys case-insensitively; we normalize to lower-case dotted form here + prefix := l.envPrefix + "_" + for _, env := range os.Environ() { + // env is in the form KEY=VALUE + if !strings.HasPrefix(env, prefix) { + continue + } + // Split only on first '=' to get the key + eq := strings.IndexByte(env, '=') + if eq <= 0 { + continue + } + key := env[:eq] + // Trim prefix + raw := strings.TrimPrefix(key, prefix) + if raw == "" { + continue + } + // Convert to dotted lower-case key path + dotted := strings.ToLower(strings.ReplaceAll(raw, "_", ".")) + // If an allow-list is set, skip keys not present in it + if l.allowListMap != nil { + if _, ok := l.allowListMap[dotted]; !ok { + continue + } + } + keys = append(keys, dotted) + } + return keys, nil } // Load loads the configuration into the provided struct diff --git a/service/pkg/config/secret.go b/service/pkg/config/secret.go new file mode 100644 index 0000000000..eaf8aca69e --- /dev/null +++ b/service/pkg/config/secret.go @@ -0,0 +1,130 @@ +package config + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io/fs" + "log/slog" + "os" + "strings" +) + +// Secret represents a sensitive value that should not be logged or marshaled plainly. +// It can be provided literally or by reference (e.g., environment variable). +type Secret struct { + // value holds the resolved secret when available. + value string + + // source is a human-readable origin for the secret (e.g., "literal", "env:OPENTDF_FOO"). + source string + + // resolved indicates whether value has been materialized. + resolved bool +} + +var ( + // ErrSecretNotResolved is returned when attempting to access a secret that hasn't been resolved yet. + ErrSecretNotResolved = errors.New("secret not resolved") + // ErrSecretMissingEnv indicates the requested env var does not exist. + ErrSecretMissingEnv = errors.New("secret env var not set") +) + +// NewLiteralSecret creates a Secret from a literal value and marks it resolved. +func NewLiteralSecret(v string) Secret { + return Secret{value: v, source: "literal", resolved: true} +} + +// NewEnvSecret creates a Secret that will resolve from the given environment variable. +func NewEnvSecret(envName string) Secret { + return Secret{source: "env:" + envName} +} + +// NewFileSecret creates a Secret that will resolve from the contents of a file path. +// The value is read as-is; callers can trim/parse if needed. +func NewFileSecret(path string) Secret { + // Normalize to absolute-ish source marker + if !strings.HasPrefix(path, "file:") { + return Secret{source: "file:" + path} + } + return Secret{source: path} +} + +// Resolve ensures the secret is materialized and returns its value. +// If the secret references an environment variable, it reads it from the process environment. +func (s *Secret) Resolve(_ context.Context) (string, error) { + if s.resolved { + return s.value, nil + } + + // Resolve based on source scheme + // If no source is set, the secret is unset/optional and not resolved + if s.source == "" { + return "", ErrSecretNotResolved + } + + switch { + case len(s.source) > 4 && s.source[:4] == "env:": + envName := s.source[4:] + if v, ok := os.LookupEnv(envName); ok { + s.value = v + s.resolved = true + return s.value, nil + } + return "", fmt.Errorf("%w: %s", ErrSecretMissingEnv, envName) + case len(s.source) > 5 && s.source[:5] == "file:": + path := s.source[5:] + b, err := os.ReadFile(path) + if err != nil { + // Mask file not found vs other errors in public message + if errors.Is(err, fs.ErrNotExist) { + return "", fmt.Errorf("secret file not found: %s", path) + } + return "", fmt.Errorf("error reading secret file: %w", err) + } + s.value = string(b) + s.resolved = true + return s.value, nil + case s.source == "literal": + // Should have been resolved already; treat as not resolved if empty. + if s.resolved { + return s.value, nil + } + return "", ErrSecretNotResolved + default: + // Placeholder for future resolvers (e.g., fromURI) + return "", fmt.Errorf("unrecognized secret source: %s", s.source) + } +} + +// String implements fmt.Stringer and returns a redacted representation. +func (s Secret) String() string { return "[REDACTED]" } + +// LogValue implements slog.LogValuer to prevent accidental secret leakage in logs. +func (s Secret) LogValue() slog.Value { + if s.source != "" { + return slog.GroupValue( + slog.String("value", "[REDACTED]"), + slog.String("source", s.source), + ) + } + return slog.StringValue("[REDACTED]") +} + +// MarshalJSON redacts the value when serialized to JSON. +func (s Secret) MarshalJSON() ([]byte, error) { + return json.Marshal("[REDACTED]") +} + +// Export returns the raw secret value if resolved, otherwise returns an error. +// Intended for explicit, narrow use when the raw value is required. +func (s Secret) Export() (string, error) { + if !s.resolved { + return "", ErrSecretNotResolved + } + return s.value, nil +} + +// IsZero reports whether the secret has no value and no source. +func (s Secret) IsZero() bool { return !s.resolved && s.source == "" && s.value == "" } diff --git a/service/pkg/config/secret_test.go b/service/pkg/config/secret_test.go new file mode 100644 index 0000000000..b6d2f39ee2 --- /dev/null +++ b/service/pkg/config/secret_test.go @@ -0,0 +1,53 @@ +package config + +import ( + "context" + "encoding/json" + "testing" +) + +func TestSecret_Literal(t *testing.T) { + s := NewLiteralSecret("super-secret") + if s.String() != "[REDACTED]" { + t.Fatalf("expected redacted String, got %q", s.String()) + } + got, err := s.Resolve(context.Background()) + if err != nil { + t.Fatalf("resolve literal: %v", err) + } + if got != "super-secret" { + t.Fatalf("unexpected value: %q", got) + } + raw, err := s.Export() + if err != nil || raw != "super-secret" { + t.Fatalf("export literal: %v, %q", err, raw) + } +} + +func TestSecret_FromEnv(t *testing.T) { + const env = "OPENTDF_TEST_SECRET" + t.Setenv(env, "env-secret") + + s := NewEnvSecret(env) + if s.String() != "[REDACTED]" { + t.Fatalf("expected redacted String, got %q", s.String()) + } + got, err := s.Resolve(context.Background()) + if err != nil { + t.Fatalf("resolve env: %v", err) + } + if got != "env-secret" { + t.Fatalf("unexpected value: %q", got) + } +} + +func TestSecret_JSONRedacted(t *testing.T) { + s := NewLiteralSecret("dont-log-me") + b, err := json.Marshal(s) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if string(b) != "\"[REDACTED]\"" { + t.Fatalf("expected redacted json, got %s", b) + } +} diff --git a/service/pkg/config/walk.go b/service/pkg/config/walk.go new file mode 100644 index 0000000000..6cd9345c44 --- /dev/null +++ b/service/pkg/config/walk.go @@ -0,0 +1,67 @@ +package config + +import ( + "reflect" +) + +// walk traverses v and calls fn for each Secret encountered. +func walk(v any, fn func(*Secret) error) error { + if v == nil { + return nil + } + rv := reflect.ValueOf(v) + return walkValue(rv, fn) +} + +func walkValue(rv reflect.Value, fn func(*Secret) error) error { + if !rv.IsValid() { + return nil + } + // Follow pointers + for rv.Kind() == reflect.Pointer { + if rv.IsNil() { + return nil + } + rv = rv.Elem() + } + + //nolint:exhaustive // Only handling relevant kinds for Secret traversal + switch rv.Kind() { + case reflect.Struct: + rt := rv.Type() + // Handle Secret itself + if rt == reflect.TypeOf(Secret{}) { + if rv.CanAddr() { + s, ok := rv.Addr().Interface().(*Secret) + if ok { + return fn(s) + } + } + return nil + } + // Iterate exported fields + for i := 0; i < rv.NumField(); i++ { + // Only exported fields + if rt.Field(i).IsExported() { + if err := walkValue(rv.Field(i), fn); err != nil { + return err + } + } + } + case reflect.Map: + for _, k := range rv.MapKeys() { + if err := walkValue(rv.MapIndex(k), fn); err != nil { + return err + } + } + case reflect.Slice, reflect.Array: + for i := 0; i < rv.Len(); i++ { + if err := walkValue(rv.Index(i), fn); err != nil { + return err + } + } + default: + // Other kinds: nothing to do + } + return nil +} From 2248256e3d6cf7a5582c069b685aa3735f1e1359 Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 14 Oct 2025 14:36:27 -0400 Subject: [PATCH 02/13] fix: implement review feedback --- service/kas/kas.go | 5 +- service/pkg/config/bind.go | 6 +- service/pkg/config/decode.go | 16 ++- service/pkg/config/decode_test.go | 17 +++ service/pkg/config/secret.go | 111 ++++++++++++------ service/pkg/config/secret_concurrency_test.go | 43 +++++++ service/pkg/config/secret_test.go | 18 +++ 7 files changed, 169 insertions(+), 47 deletions(-) create mode 100644 service/pkg/config/secret_concurrency_test.go diff --git a/service/kas/kas.go b/service/kas/kas.go index c7ae5453d4..5b0359d5c7 100644 --- a/service/kas/kas.go +++ b/service/kas/kas.go @@ -152,12 +152,11 @@ func determineKASURL(srp serviceregistry.RegistrationParams, kasCfg access.KASCo return kasURL, nil } -func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider, cacheClient *cache.Cache) error { +func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg *access.KASConfig, p *access.Provider, cacheClient *cache.Cache) error { srp.Logger.Info("preview feature: key management is enabled") - srp.Logger.Debug("kas root_key summary", slog.Any("root_key", kasCfg.RootKey)) srp.Logger.Debug("kas preview settings", slog.Any("preview", kasCfg.Preview)) - kasURL, err := determineKASURL(srp, kasCfg) + kasURL, err := determineKASURL(srp, *kasCfg) if err != nil { return fmt.Errorf("failed to determine KAS URL: %w", err) } diff --git a/service/pkg/config/bind.go b/service/pkg/config/bind.go index baa54293d8..e6f907b68a 100644 --- a/service/pkg/config/bind.go +++ b/service/pkg/config/bind.go @@ -57,7 +57,11 @@ func BindServiceConfig[T any](ctx context.Context, svcCfg ServiceConfig, out *T, // Validate struct using go-playground/validator tags, if present if err := validator.New().Struct(out); err != nil { - return err + var verrs validator.ValidationErrors + if errors.As(err, &verrs) { + return fmt.Errorf("validation failed: %s", verrs.Error()) + } + return fmt.Errorf("validation failed: %w", err) } return nil } diff --git a/service/pkg/config/decode.go b/service/pkg/config/decode.go index 475ef546f4..b5719f062f 100644 --- a/service/pkg/config/decode.go +++ b/service/pkg/config/decode.go @@ -23,10 +23,18 @@ func secretDecodeHook(from, to reflect.Type, data any) (any, error) { // Support friendly inline directives: "env:VAR", "file:/path", "literal:..." s := reflect.ValueOf(data).String() switch { - case strings.HasPrefix(s, "env:") && len(s) > len("env:"): - return NewEnvSecret(strings.TrimPrefix(s, "env:")), nil - case strings.HasPrefix(s, "file:") && len(s) > len("file:"): - return NewFileSecret(strings.TrimPrefix(s, "file:")), nil + case strings.HasPrefix(s, "env:"): + v := strings.TrimPrefix(s, "env:") + if v == "" { + return nil, errors.New("empty env directive") + } + return NewEnvSecret(v), nil + case strings.HasPrefix(s, "file:"): + v := strings.TrimPrefix(s, "file:") + if v == "" { + return nil, errors.New("empty file directive") + } + return NewFileSecret(v), nil case strings.HasPrefix(s, "literal:"): return NewLiteralSecret(strings.TrimPrefix(s, "literal:")), nil default: diff --git a/service/pkg/config/decode_test.go b/service/pkg/config/decode_test.go index e0c0a8ec70..f8ff980b05 100644 --- a/service/pkg/config/decode_test.go +++ b/service/pkg/config/decode_test.go @@ -46,3 +46,20 @@ func TestDecodeHook_InlineForms(t *testing.T) { t.Fatalf("file: %v %q", err, c) } } + +func TestDecodeHook_MalformedDirectives(t *testing.T) { + cases := []ServiceConfig{ + {"x": "env:"}, + {"x": "file:"}, + {"x": map[string]any{"fromEnv": ""}}, + {"x": map[string]any{"fromFile": ""}}, + } + for _, in := range cases { + var out struct { + X Secret `mapstructure:"x"` + } + if err := BindServiceConfig(context.Background(), in, &out); err == nil { + t.Fatalf("expected error for malformed directive: %+v", in) + } + } +} diff --git a/service/pkg/config/secret.go b/service/pkg/config/secret.go index eaf8aca69e..b964ed5db5 100644 --- a/service/pkg/config/secret.go +++ b/service/pkg/config/secret.go @@ -9,18 +9,17 @@ import ( "log/slog" "os" "strings" + "sync" ) -// Secret represents a sensitive value that should not be logged or marshaled plainly. -// It can be provided literally or by reference (e.g., environment variable). -type Secret struct { - // value holds the resolved secret when available. - value string +// Secret represents a sensitive value. It holds an internal pointer to state +// so copying Secret values does not copy the underlying lock. +type Secret struct{ state *secretState } - // source is a human-readable origin for the secret (e.g., "literal", "env:OPENTDF_FOO"). - source string - - // resolved indicates whether value has been materialized. +type secretState struct { + mu sync.Mutex + value string + source string resolved bool } @@ -33,12 +32,12 @@ var ( // NewLiteralSecret creates a Secret from a literal value and marks it resolved. func NewLiteralSecret(v string) Secret { - return Secret{value: v, source: "literal", resolved: true} + return Secret{state: &secretState{value: v, source: "literal", resolved: true}} } // NewEnvSecret creates a Secret that will resolve from the given environment variable. func NewEnvSecret(envName string) Secret { - return Secret{source: "env:" + envName} + return Secret{state: &secretState{source: "env:" + envName}} } // NewFileSecret creates a Secret that will resolve from the contents of a file path. @@ -46,55 +45,77 @@ func NewEnvSecret(envName string) Secret { func NewFileSecret(path string) Secret { // Normalize to absolute-ish source marker if !strings.HasPrefix(path, "file:") { - return Secret{source: "file:" + path} + return Secret{state: &secretState{source: "file:" + path}} } - return Secret{source: path} + return Secret{state: &secretState{source: path}} } // Resolve ensures the secret is materialized and returns its value. // If the secret references an environment variable, it reads it from the process environment. -func (s *Secret) Resolve(_ context.Context) (string, error) { - if s.resolved { - return s.value, nil +func (s Secret) Resolve(ctx context.Context) (string, error) { + if s.state == nil { + return "", ErrSecretNotResolved + } + st := s.state + st.mu.Lock() + defer st.mu.Unlock() + + if st.resolved { + return st.value, nil } - // Resolve based on source scheme - // If no source is set, the secret is unset/optional and not resolved - if s.source == "" { + if st.source == "" { return "", ErrSecretNotResolved } + if ctx != nil { + if err := ctx.Err(); err != nil { + return "", err + } + } + switch { - case len(s.source) > 4 && s.source[:4] == "env:": - envName := s.source[4:] + case len(st.source) > 4 && st.source[:4] == "env:": + envName := st.source[4:] + if envName == "" { + return "", errors.New("empty env directive") + } if v, ok := os.LookupEnv(envName); ok { - s.value = v - s.resolved = true - return s.value, nil + st.value = v + st.resolved = true + return st.value, nil } return "", fmt.Errorf("%w: %s", ErrSecretMissingEnv, envName) - case len(s.source) > 5 && s.source[:5] == "file:": - path := s.source[5:] + case len(st.source) > 5 && st.source[:5] == "file:": + path := st.source[5:] + if path == "" { + return "", errors.New("empty file directive") + } + if ctx != nil { + if err := ctx.Err(); err != nil { + return "", err + } + } b, err := os.ReadFile(path) if err != nil { // Mask file not found vs other errors in public message if errors.Is(err, fs.ErrNotExist) { return "", fmt.Errorf("secret file not found: %s", path) } - return "", fmt.Errorf("error reading secret file: %w", err) + return "", fmt.Errorf("error reading secret file %s: %w", path, err) } - s.value = string(b) - s.resolved = true - return s.value, nil - case s.source == "literal": + st.value = strings.TrimSpace(string(b)) + st.resolved = true + return st.value, nil + case st.source == "literal": // Should have been resolved already; treat as not resolved if empty. - if s.resolved { - return s.value, nil + if st.resolved { + return st.value, nil } return "", ErrSecretNotResolved default: // Placeholder for future resolvers (e.g., fromURI) - return "", fmt.Errorf("unrecognized secret source: %s", s.source) + return "", fmt.Errorf("unrecognized secret source: %s", st.source) } } @@ -103,10 +124,10 @@ func (s Secret) String() string { return "[REDACTED]" } // LogValue implements slog.LogValuer to prevent accidental secret leakage in logs. func (s Secret) LogValue() slog.Value { - if s.source != "" { + if s.state != nil && s.state.source != "" { return slog.GroupValue( slog.String("value", "[REDACTED]"), - slog.String("source", s.source), + slog.String("source", s.state.source), ) } return slog.StringValue("[REDACTED]") @@ -120,11 +141,23 @@ func (s Secret) MarshalJSON() ([]byte, error) { // Export returns the raw secret value if resolved, otherwise returns an error. // Intended for explicit, narrow use when the raw value is required. func (s Secret) Export() (string, error) { - if !s.resolved { + if s.state == nil { return "", ErrSecretNotResolved } - return s.value, nil + s.state.mu.Lock() + defer s.state.mu.Unlock() + if !s.state.resolved { + return "", ErrSecretNotResolved + } + return s.state.value, nil } // IsZero reports whether the secret has no value and no source. -func (s Secret) IsZero() bool { return !s.resolved && s.source == "" && s.value == "" } +func (s Secret) IsZero() bool { + if s.state == nil { + return true + } + s.state.mu.Lock() + defer s.state.mu.Unlock() + return !s.state.resolved && s.state.source == "" && s.state.value == "" +} diff --git a/service/pkg/config/secret_concurrency_test.go b/service/pkg/config/secret_concurrency_test.go new file mode 100644 index 0000000000..240244ef1f --- /dev/null +++ b/service/pkg/config/secret_concurrency_test.go @@ -0,0 +1,43 @@ +package config + +import ( + "context" + "sync" + "testing" +) + +func TestSecret_Resolve_Concurrent(t *testing.T) { + t.Setenv("OPENTDF_CONCUR", "concur") + s := NewEnvSecret("OPENTDF_CONCUR") + + const n = 50 + var wg sync.WaitGroup + wg.Add(n) + errs := make(chan error, n) + vals := make(chan string, n) + for i := 0; i < n; i++ { + go func() { + defer wg.Done() + v, err := s.Resolve(context.Background()) + if err != nil { + errs <- err + return + } + vals <- v + }() + } + wg.Wait() + close(errs) + close(vals) + if len(errs) > 0 { + for e := range errs { + t.Errorf("resolve error: %v", e) + } + t.FailNow() + } + for v := range vals { + if v != "concur" { + t.Fatalf("unexpected value: %q", v) + } + } +} diff --git a/service/pkg/config/secret_test.go b/service/pkg/config/secret_test.go index b6d2f39ee2..5d67e76595 100644 --- a/service/pkg/config/secret_test.go +++ b/service/pkg/config/secret_test.go @@ -3,6 +3,7 @@ package config import ( "context" "encoding/json" + "os" "testing" ) @@ -41,6 +42,23 @@ func TestSecret_FromEnv(t *testing.T) { } } +func TestSecret_FromFile_Trim(t *testing.T) { + dir := t.TempDir() + p := dir + "/s.txt" + // Include trailing newline to simulate typical secret mounts + if err := os.WriteFile(p, []byte("abc\n"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + s := NewFileSecret(p) + got, err := s.Resolve(context.Background()) + if err != nil { + t.Fatalf("resolve file: %v", err) + } + if got != "abc" { + t.Fatalf("expected trimmed value 'abc', got %q", got) + } +} + func TestSecret_JSONRedacted(t *testing.T) { s := NewLiteralSecret("dont-log-me") b, err := json.Marshal(s) From 5861113ea024e178398167643bcd193f519a4b84 Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 14 Oct 2025 16:32:12 -0400 Subject: [PATCH 03/13] minor cleanups --- service/pkg/config/bind_test.go | 7 +++---- service/pkg/config/decode_test.go | 11 +++++------ service/pkg/config/secret_concurrency_test.go | 3 +-- service/pkg/server/start_test.go | 2 +- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/service/pkg/config/bind_test.go b/service/pkg/config/bind_test.go index 89f02a8a34..787773dfb2 100644 --- a/service/pkg/config/bind_test.go +++ b/service/pkg/config/bind_test.go @@ -1,7 +1,6 @@ package config import ( - "context" "testing" ) @@ -25,17 +24,17 @@ func TestBindServiceConfig_LiteralAndEnv(t *testing.T) { } var out demoCfg - if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { + if err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()); err != nil { t.Fatalf("bind: %v", err) } if out.User != "alice" { t.Fatalf("user mismatch: %q", out.User) } - pass, err := out.Pass.Resolve(context.Background()) + pass, err := out.Pass.Resolve(t.Context()) if err != nil || pass != "p@ss" { t.Fatalf("pass resolve: %v %q", err, pass) } - tok, err := out.Nested.Token.Resolve(context.Background()) + tok, err := out.Nested.Token.Resolve(t.Context()) if err != nil || tok != "tok" { t.Fatalf("token resolve: %v %q", err, tok) } diff --git a/service/pkg/config/decode_test.go b/service/pkg/config/decode_test.go index f8ff980b05..9399db8514 100644 --- a/service/pkg/config/decode_test.go +++ b/service/pkg/config/decode_test.go @@ -1,7 +1,6 @@ package config import ( - "context" "os" "path/filepath" "testing" @@ -29,19 +28,19 @@ func TestDecodeHook_InlineForms(t *testing.T) { } var out inlineCfg - if err := BindServiceConfig(context.Background(), in, &out); err != nil { + if err := BindServiceConfig(t.Context(), in, &out); err != nil { t.Fatalf("bind: %v", err) } - a, err := out.A.Resolve(context.Background()) + a, err := out.A.Resolve(t.Context()) if err != nil || a != "abc" { t.Fatalf("literal: %v %q", err, a) } - b, err := out.B.Resolve(context.Background()) + b, err := out.B.Resolve(t.Context()) if err != nil || b != "from-env" { t.Fatalf("env: %v %q", err, b) } - c, err := out.C.Resolve(context.Background()) + c, err := out.C.Resolve(t.Context()) if err != nil || c != "from-file" { t.Fatalf("file: %v %q", err, c) } @@ -58,7 +57,7 @@ func TestDecodeHook_MalformedDirectives(t *testing.T) { var out struct { X Secret `mapstructure:"x"` } - if err := BindServiceConfig(context.Background(), in, &out); err == nil { + if err := BindServiceConfig(t.Context(), in, &out); err == nil { t.Fatalf("expected error for malformed directive: %+v", in) } } diff --git a/service/pkg/config/secret_concurrency_test.go b/service/pkg/config/secret_concurrency_test.go index 240244ef1f..55c8276565 100644 --- a/service/pkg/config/secret_concurrency_test.go +++ b/service/pkg/config/secret_concurrency_test.go @@ -1,7 +1,6 @@ package config import ( - "context" "sync" "testing" ) @@ -18,7 +17,7 @@ func TestSecret_Resolve_Concurrent(t *testing.T) { for i := 0; i < n; i++ { go func() { defer wg.Done() - v, err := s.Resolve(context.Background()) + v, err := s.Resolve(t.Context()) if err != nil { errs <- err return diff --git a/service/pkg/server/start_test.go b/service/pkg/server/start_test.go index bff90e197a..6fabf8e639 100644 --- a/service/pkg/server/start_test.go +++ b/service/pkg/server/start_test.go @@ -353,7 +353,7 @@ func (s *StartTestSuite) Test_Start_When_Extra_Service_Registered() { require.NoError(t, err) // Start services with test service - cleanup, err := startServices(context.Background(), startServicesParams{ + cleanup, err := startServices(t.Context(), startServicesParams{ cfg: &config.Config{ Mode: tc.mode, Services: map[string]config.ServiceConfig{ From 79d43200f5168838bfb39e7d0dba5162b9cb3b3d Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 14 Oct 2025 18:16:53 -0400 Subject: [PATCH 04/13] minor fixes and doc updates --- docs/Configuring.md | 29 ++++++++++++++++++++++++++--- service/pkg/config/secret.go | 30 +++++++++++++++--------------- service/pkg/config/secret_test.go | 7 +++---- 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/docs/Configuring.md b/docs/Configuring.md index 7a60a56481..78c9c09c2e 100644 --- a/docs/Configuring.md +++ b/docs/Configuring.md @@ -335,9 +335,32 @@ services: Notes: - Secrets are redacted in logs and JSON output; only a redacted summary is shown. - For nested secret fields in service-specific configs, prefer the inline `env:` form in YAML to avoid underscore-to-dot mapping issues with environment variables alone. -- You can still use a structured form for references: - - `root_key: { fromEnv: "OPENTDF_SERVICES_KAS_ROOT_KEY" }` - - `root_key: { fromFile: "/path/to/secret" }` +- You can still use a structured map form for references (YAML): + + Block style (preferred for readability): + ```yaml + services: + kas: + root_key: + fromEnv: OPENTDF_SERVICES_KAS_ROOT_KEY + ``` + + Or referencing a file path: + ```yaml + services: + kas: + root_key: + fromFile: /var/run/secrets/opentdf/kas_root_key + ``` + + Flow style (compact): + ```yaml + services: + kas: + root_key: { fromEnv: OPENTDF_SERVICES_KAS_ROOT_KEY } + # or + root_key: { fromFile: /var/run/secrets/opentdf/kas_root_key } + ``` Example: diff --git a/service/pkg/config/secret.go b/service/pkg/config/secret.go index b964ed5db5..ab97bf0b5f 100644 --- a/service/pkg/config/secret.go +++ b/service/pkg/config/secret.go @@ -24,12 +24,14 @@ type secretState struct { } var ( - // ErrSecretNotResolved is returned when attempting to access a secret that hasn't been resolved yet. - ErrSecretNotResolved = errors.New("secret not resolved") - // ErrSecretMissingEnv indicates the requested env var does not exist. - ErrSecretMissingEnv = errors.New("secret env var not set") + // ErrSecretNotResolved is returned when attempting to access a secret that hasn't been resolved yet. + ErrSecretNotResolved = errors.New("secret not resolved") + // ErrSecretMissingEnv indicates the requested env var does not exist. + ErrSecretMissingEnv = errors.New("secret env var not set") ) +const redactedPlaceholder = "[REDACTED]" + // NewLiteralSecret creates a Secret from a literal value and marks it resolved. func NewLiteralSecret(v string) Secret { return Secret{state: &secretState{value: v, source: "literal", resolved: true}} @@ -120,23 +122,21 @@ func (s Secret) Resolve(ctx context.Context) (string, error) { } // String implements fmt.Stringer and returns a redacted representation. -func (s Secret) String() string { return "[REDACTED]" } +func (s Secret) String() string { return redactedPlaceholder } // LogValue implements slog.LogValuer to prevent accidental secret leakage in logs. func (s Secret) LogValue() slog.Value { - if s.state != nil && s.state.source != "" { - return slog.GroupValue( - slog.String("value", "[REDACTED]"), - slog.String("source", s.state.source), - ) - } - return slog.StringValue("[REDACTED]") + if s.state != nil && s.state.source != "" { + return slog.GroupValue( + slog.String("value", redactedPlaceholder), + slog.String("source", s.state.source), + ) + } + return slog.StringValue(redactedPlaceholder) } // MarshalJSON redacts the value when serialized to JSON. -func (s Secret) MarshalJSON() ([]byte, error) { - return json.Marshal("[REDACTED]") -} +func (s Secret) MarshalJSON() ([]byte, error) { return json.Marshal(redactedPlaceholder) } // Export returns the raw secret value if resolved, otherwise returns an error. // Intended for explicit, narrow use when the raw value is required. diff --git a/service/pkg/config/secret_test.go b/service/pkg/config/secret_test.go index 5d67e76595..3fb158bf07 100644 --- a/service/pkg/config/secret_test.go +++ b/service/pkg/config/secret_test.go @@ -1,7 +1,6 @@ package config import ( - "context" "encoding/json" "os" "testing" @@ -12,7 +11,7 @@ func TestSecret_Literal(t *testing.T) { if s.String() != "[REDACTED]" { t.Fatalf("expected redacted String, got %q", s.String()) } - got, err := s.Resolve(context.Background()) + got, err := s.Resolve(t.Context()) if err != nil { t.Fatalf("resolve literal: %v", err) } @@ -33,7 +32,7 @@ func TestSecret_FromEnv(t *testing.T) { if s.String() != "[REDACTED]" { t.Fatalf("expected redacted String, got %q", s.String()) } - got, err := s.Resolve(context.Background()) + got, err := s.Resolve(t.Context()) if err != nil { t.Fatalf("resolve env: %v", err) } @@ -50,7 +49,7 @@ func TestSecret_FromFile_Trim(t *testing.T) { t.Fatalf("write: %v", err) } s := NewFileSecret(p) - got, err := s.Resolve(context.Background()) + got, err := s.Resolve(t.Context()) if err != nil { t.Fatalf("resolve file: %v", err) } From 5a4f9025fc4c0b95e0e88105acb4c2f5cdec3937 Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 14 Oct 2025 18:22:06 -0400 Subject: [PATCH 05/13] format --- service/pkg/config/secret.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/service/pkg/config/secret.go b/service/pkg/config/secret.go index ab97bf0b5f..9789ddc676 100644 --- a/service/pkg/config/secret.go +++ b/service/pkg/config/secret.go @@ -24,10 +24,10 @@ type secretState struct { } var ( - // ErrSecretNotResolved is returned when attempting to access a secret that hasn't been resolved yet. - ErrSecretNotResolved = errors.New("secret not resolved") - // ErrSecretMissingEnv indicates the requested env var does not exist. - ErrSecretMissingEnv = errors.New("secret env var not set") + // ErrSecretNotResolved is returned when attempting to access a secret that hasn't been resolved yet. + ErrSecretNotResolved = errors.New("secret not resolved") + // ErrSecretMissingEnv indicates the requested env var does not exist. + ErrSecretMissingEnv = errors.New("secret env var not set") ) const redactedPlaceholder = "[REDACTED]" @@ -126,13 +126,13 @@ func (s Secret) String() string { return redactedPlaceholder } // LogValue implements slog.LogValuer to prevent accidental secret leakage in logs. func (s Secret) LogValue() slog.Value { - if s.state != nil && s.state.source != "" { - return slog.GroupValue( - slog.String("value", redactedPlaceholder), - slog.String("source", s.state.source), - ) - } - return slog.StringValue(redactedPlaceholder) + if s.state != nil && s.state.source != "" { + return slog.GroupValue( + slog.String("value", redactedPlaceholder), + slog.String("source", s.state.source), + ) + } + return slog.StringValue(redactedPlaceholder) } // MarshalJSON redacts the value when serialized to JSON. From a2473ba397e238f9f3b4b02fa68f6ea6129a126f Mon Sep 17 00:00:00 2001 From: strantalis Date: Wed, 15 Oct 2025 10:35:32 -0400 Subject: [PATCH 06/13] simplify resolve switch statement --- service/pkg/config/secret.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/service/pkg/config/secret.go b/service/pkg/config/secret.go index 9789ddc676..e2060da27a 100644 --- a/service/pkg/config/secret.go +++ b/service/pkg/config/secret.go @@ -77,8 +77,8 @@ func (s Secret) Resolve(ctx context.Context) (string, error) { } switch { - case len(st.source) > 4 && st.source[:4] == "env:": - envName := st.source[4:] + case strings.HasPrefix(st.source, "env:"): + envName := strings.TrimPrefix(st.source, "env:") if envName == "" { return "", errors.New("empty env directive") } @@ -88,8 +88,8 @@ func (s Secret) Resolve(ctx context.Context) (string, error) { return st.value, nil } return "", fmt.Errorf("%w: %s", ErrSecretMissingEnv, envName) - case len(st.source) > 5 && st.source[:5] == "file:": - path := st.source[5:] + case strings.HasPrefix(st.source, "file:"): + path := strings.TrimPrefix(st.source, "file:") if path == "" { return "", errors.New("empty file directive") } From 5d9b126082f6d5cda08c4019e0b5e68de434c461 Mon Sep 17 00:00:00 2001 From: strantalis Date: Wed, 15 Oct 2025 12:56:38 -0400 Subject: [PATCH 07/13] remove logging value --- service/pkg/config/config.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/service/pkg/config/config.go b/service/pkg/config/config.go index 74991ab09e..2795505ddb 100644 --- a/service/pkg/config/config.go +++ b/service/pkg/config/config.go @@ -245,23 +245,13 @@ func (c *Config) Reload(ctx context.Context) error { continue } if loaderValue != nil { - // Avoid logging sensitive values - if strings.Contains(strings.ToLower(key), "secret") { - slog.DebugContext( - ctx, - "config merge", - slog.String("loader", loader.Name()), - slog.String("key", key), - slog.String("value", "[REDACTED]"), - ) - } else { - slog.DebugContext( - ctx, - "config merge", - slog.String("loader", loader.Name()), - slog.String("key", key), - ) - } + // Log loader and key only; never log values in merge path + slog.DebugContext( + ctx, + "config merge", + slog.String("loader", loader.Name()), + slog.String("key", key), + ) orderedViper.Set(key, loaderValue) assigned[key] = struct{}{} } From 3fefb48c40405d50bc531303f62b85024de51f2f Mon Sep 17 00:00:00 2001 From: strantalis Date: Wed, 15 Oct 2025 12:58:37 -0400 Subject: [PATCH 08/13] fix error return message --- service/pkg/config/decode.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/pkg/config/decode.go b/service/pkg/config/decode.go index b5719f062f..856e41d6e5 100644 --- a/service/pkg/config/decode.go +++ b/service/pkg/config/decode.go @@ -54,7 +54,7 @@ func secretDecodeHook(from, to reflect.Type, data any) (any, error) { return NewFileSecret(file), nil } // Future: support {"fromURI":"aws-secretsmanager://..."} - return nil, errors.New("unsupported secret map, expected {fromEnv:string}") + return nil, errors.New("unsupported secret map, expected {fromEnv:string} or {fromFile:string}") default: return nil, fmt.Errorf("cannot decode %s into Secret", from.Kind()) } From 42759426ba75134ac79e5069ccd988c1de95fa0e Mon Sep 17 00:00:00 2001 From: strantalis Date: Wed, 15 Oct 2025 16:58:59 -0400 Subject: [PATCH 09/13] add more tests for nested arrays and maps --- service/pkg/config/complex_decode_test.go | 105 ++++++++++++++++++++++ service/pkg/config/decode.go | 11 ++- service/pkg/config/walk.go | 10 ++- service/pkg/config/yaml_array_test.go | 98 ++++++++++++++++++++ 4 files changed, 218 insertions(+), 6 deletions(-) create mode 100644 service/pkg/config/complex_decode_test.go create mode 100644 service/pkg/config/yaml_array_test.go diff --git a/service/pkg/config/complex_decode_test.go b/service/pkg/config/complex_decode_test.go new file mode 100644 index 0000000000..7f45f0e5bc --- /dev/null +++ b/service/pkg/config/complex_decode_test.go @@ -0,0 +1,105 @@ +package config + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +// A nested service configuration with multiple tenants and lists of credentials. +type tenantCfg struct { + Credential Secret `mapstructure:"credential"` + Passwords []Secret `mapstructure:"passwords"` +} + +type svcCfgComplex struct { + ClientSecret Secret `mapstructure:"client_secret"` + Tenants map[string]tenantCfg `mapstructure:"tenants"` +} + +func TestBindServiceConfig_NestedTenants_SecretsAndLists(t *testing.T) { + // Prepare env vars and a temp file for fromFile + t.Setenv("OPENTDF_TEST_CLIENT_SECRET", "client-secret") + t.Setenv("OPENTDF_TENANT_A_CRED", "tenant-a-cred") + t.Setenv("OPENTDF_PASS1", "p1") + + dir := t.TempDir() + filePath := filepath.Join(dir, "pass.txt") + if err := os.WriteFile(filePath, []byte("from-file\n"), 0o600); err != nil { + t.Fatalf("write temp file: %v", err) + } + + in := ServiceConfig{ + "client_secret": "env:OPENTDF_TEST_CLIENT_SECRET", + "tenants": map[string]any{ + "tenantA": map[string]any{ + "credential": "env:OPENTDF_TENANT_A_CRED", + "passwords": []any{ + "env:OPENTDF_PASS1", + "literal:abc", + map[string]any{"fromFile": filePath}, + }, + }, + "tenantB": map[string]any{ + "credential": "literal:credB", + }, + }, + } + + var out svcCfgComplex + // Eagerly resolve to validate that nested secrets are materialized + if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { + t.Fatalf("bind: %v", err) + } + + // Assert top-level secret + v, err := out.ClientSecret.Resolve(context.Background()) + if err != nil || v != "client-secret" { + t.Fatalf("client_secret resolve: %v %q", err, v) + } + + // Assert tenant map + tenantA, ok := out.Tenants["tenantA"] + if !ok { + t.Fatalf("expected tenant 'tenantA' present") + } + credA, err := tenantA.Credential.Resolve(context.Background()) + if err != nil || credA != "tenant-a-cred" { + t.Fatalf("credential resolve: %v %q", err, credA) + } + if len(tenantA.Passwords) != 3 { + t.Fatalf("expected 3 passwords, got %d", len(tenantA.Passwords)) + } + p0, _ := tenantA.Passwords[0].Resolve(context.Background()) + p1, _ := tenantA.Passwords[1].Resolve(context.Background()) + p2, _ := tenantA.Passwords[2].Resolve(context.Background()) + if p0 != "p1" || p1 != "abc" || p2 != "from-file" { + t.Fatalf("passwords mismatch: %q, %q, %q", p0, p1, p2) + } + + // Second tenant literal credential + tenantB, ok := out.Tenants["tenantB"] + if !ok { + t.Fatalf("expected tenant 'tenantB' present") + } + credB, err := tenantB.Credential.Resolve(context.Background()) + if err != nil || credB != "credB" { + t.Fatalf("credential resolve (tenantB): %v %q", err, credB) + } +} + +func TestBindServiceConfig_NestedTenants_EagerFailureOnMissingEnv(t *testing.T) { + in := ServiceConfig{ + "tenants": map[string]any{ + "tenantA": map[string]any{ + // Missing env value should cause eager resolution to fail + "credential": "env:OPENTDF_TEST_MISSING_ENV_ABC123", + }, + }, + } + var out svcCfgComplex + if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err == nil { + t.Fatalf("expected bind failure on missing env in eager resolution") + } +} diff --git a/service/pkg/config/decode.go b/service/pkg/config/decode.go index 856e41d6e5..0f006b87c3 100644 --- a/service/pkg/config/decode.go +++ b/service/pkg/config/decode.go @@ -42,15 +42,20 @@ func secretDecodeHook(from, to reflect.Type, data any) (any, error) { return NewLiteralSecret(s), nil } case reflect.Map: - // Must be map[string]any + // Must be map[string]any (case-insensitive key handling) m, okm := data.(map[string]any) if !okm { return nil, fmt.Errorf("invalid secret map type: %T", data) } - if env, ok := m["fromEnv"].(string); ok && env != "" { + // Normalize keys to lowercase for robust matching (Viper may lowercase keys) + lower := make(map[string]any, len(m)) + for k, v := range m { + lower[strings.ToLower(k)] = v + } + if env, ok := lower["fromenv"].(string); ok && env != "" { return NewEnvSecret(env), nil } - if file, ok2 := m["fromFile"].(string); ok2 && file != "" { + if file, ok2 := lower["fromfile"].(string); ok2 && file != "" { return NewFileSecret(file), nil } // Future: support {"fromURI":"aws-secretsmanager://..."} diff --git a/service/pkg/config/walk.go b/service/pkg/config/walk.go index 6cd9345c44..ced28e5d18 100644 --- a/service/pkg/config/walk.go +++ b/service/pkg/config/walk.go @@ -25,17 +25,21 @@ func walkValue(rv reflect.Value, fn func(*Secret) error) error { rv = rv.Elem() } - //nolint:exhaustive // Only handling relevant kinds for Secret traversal + //nolint:exhaustive // We only need to traverse struct, map, slice, and array kinds for secrets switch rv.Kind() { case reflect.Struct: rt := rv.Type() // Handle Secret itself if rt == reflect.TypeOf(Secret{}) { if rv.CanAddr() { - s, ok := rv.Addr().Interface().(*Secret) - if ok { + if s, ok := rv.Addr().Interface().(*Secret); ok { return fn(s) } + return nil + } + // For non-addressable Secret values (e.g., map index), operate on a copy. + if s, ok := rv.Interface().(Secret); ok { + return fn(&s) } return nil } diff --git a/service/pkg/config/yaml_array_test.go b/service/pkg/config/yaml_array_test.go new file mode 100644 index 0000000000..6810cc838c --- /dev/null +++ b/service/pkg/config/yaml_array_test.go @@ -0,0 +1,98 @@ +package config + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + + "github.com/spf13/viper" +) + +type arrayCfg struct { + Secrets []Secret `mapstructure:"secrets"` +} + +type provider struct { + Name string `mapstructure:"name"` + Password Secret `mapstructure:"password"` +} + +type providersCfg struct { + Providers []provider `mapstructure:"providers"` +} + +func TestBind_FromYAMLArray_SecretsSlice(t *testing.T) { + t.Setenv("OPENTDF_YAML_ARR", "arr-env") + dir := t.TempDir() + fp := filepath.Join(dir, "s.txt") + if err := os.WriteFile(fp, []byte("from-file\n"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + yaml := "" + + "secrets:\n" + + " - \"env:OPENTDF_YAML_ARR\"\n" + + " - { fromFile: \"" + fp + "\" }\n" + + " - \"literal:abc\"\n" + + v := viper.New() + v.SetConfigType("yaml") + if err := v.ReadConfig(bytes.NewBufferString(yaml)); err != nil { + t.Fatalf("read yaml: %v", err) + } + + in := ServiceConfig{ + "secrets": v.Get("secrets"), + } + var out arrayCfg + if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { + t.Fatalf("bind: %v", err) + } + if len(out.Secrets) != 3 { + t.Fatalf("expected 3 secrets, got %d", len(out.Secrets)) + } + s0, _ := out.Secrets[0].Resolve(context.Background()) + s1, _ := out.Secrets[1].Resolve(context.Background()) + s2, _ := out.Secrets[2].Resolve(context.Background()) + if s0 != "arr-env" || s1 != "from-file" || s2 != "abc" { + t.Fatalf("values mismatch: %q %q %q", s0, s1, s2) + } +} + +func TestBind_FromYAMLArray_StructsWithSecretFields(t *testing.T) { + t.Setenv("OPENTDF_PROVIDER_B_PASS", "b-pass") + + yaml := "" + + "providers:\n" + + " - name: a\n" + + " password: \"literal:alpha\"\n" + + " - name: b\n" + + " password: \"env:OPENTDF_PROVIDER_B_PASS\"\n" + + v := viper.New() + v.SetConfigType("yaml") + if err := v.ReadConfig(bytes.NewBufferString(yaml)); err != nil { + t.Fatalf("read yaml: %v", err) + } + + in := ServiceConfig{ + "providers": v.Get("providers"), + } + var out providersCfg + if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { + t.Fatalf("bind: %v", err) + } + if len(out.Providers) != 2 { + t.Fatalf("expected 2 providers, got %d", len(out.Providers)) + } + if out.Providers[0].Name != "a" || out.Providers[1].Name != "b" { + t.Fatalf("names mismatch: %q %q", out.Providers[0].Name, out.Providers[1].Name) + } + p0, _ := out.Providers[0].Password.Resolve(context.Background()) + p1, _ := out.Providers[1].Password.Resolve(context.Background()) + if p0 != "alpha" || p1 != "b-pass" { + t.Fatalf("passwords mismatch: %q %q", p0, p1) + } +} From 867b0d673fd01e469842758fe7c997174e27d339 Mon Sep 17 00:00:00 2001 From: strantalis Date: Fri, 17 Oct 2025 10:31:07 -0400 Subject: [PATCH 10/13] rebase on main --- service/kas/kas.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/kas/kas.go b/service/kas/kas.go index 5b0359d5c7..464788a397 100644 --- a/service/kas/kas.go +++ b/service/kas/kas.go @@ -152,11 +152,11 @@ func determineKASURL(srp serviceregistry.RegistrationParams, kasCfg access.KASCo return kasURL, nil } -func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg *access.KASConfig, p *access.Provider, cacheClient *cache.Cache) error { +func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider, cacheClient *cache.Cache) error { srp.Logger.Info("preview feature: key management is enabled") srp.Logger.Debug("kas preview settings", slog.Any("preview", kasCfg.Preview)) - kasURL, err := determineKASURL(srp, *kasCfg) + kasURL, err := determineKASURL(srp, kasCfg) if err != nil { return fmt.Errorf("failed to determine KAS URL: %w", err) } From a3df7345a608e7b9b8612e04aa7e98dd1b7e3182 Mon Sep 17 00:00:00 2001 From: strantalis Date: Fri, 17 Oct 2025 16:21:53 -0400 Subject: [PATCH 11/13] fix lint issues --- service/pkg/config/complex_decode_test.go | 105 ---------------------- service/pkg/config/decode_test.go | 97 ++++++++++++++++++++ 2 files changed, 97 insertions(+), 105 deletions(-) delete mode 100644 service/pkg/config/complex_decode_test.go diff --git a/service/pkg/config/complex_decode_test.go b/service/pkg/config/complex_decode_test.go deleted file mode 100644 index 7f45f0e5bc..0000000000 --- a/service/pkg/config/complex_decode_test.go +++ /dev/null @@ -1,105 +0,0 @@ -package config - -import ( - "context" - "os" - "path/filepath" - "testing" -) - -// A nested service configuration with multiple tenants and lists of credentials. -type tenantCfg struct { - Credential Secret `mapstructure:"credential"` - Passwords []Secret `mapstructure:"passwords"` -} - -type svcCfgComplex struct { - ClientSecret Secret `mapstructure:"client_secret"` - Tenants map[string]tenantCfg `mapstructure:"tenants"` -} - -func TestBindServiceConfig_NestedTenants_SecretsAndLists(t *testing.T) { - // Prepare env vars and a temp file for fromFile - t.Setenv("OPENTDF_TEST_CLIENT_SECRET", "client-secret") - t.Setenv("OPENTDF_TENANT_A_CRED", "tenant-a-cred") - t.Setenv("OPENTDF_PASS1", "p1") - - dir := t.TempDir() - filePath := filepath.Join(dir, "pass.txt") - if err := os.WriteFile(filePath, []byte("from-file\n"), 0o600); err != nil { - t.Fatalf("write temp file: %v", err) - } - - in := ServiceConfig{ - "client_secret": "env:OPENTDF_TEST_CLIENT_SECRET", - "tenants": map[string]any{ - "tenantA": map[string]any{ - "credential": "env:OPENTDF_TENANT_A_CRED", - "passwords": []any{ - "env:OPENTDF_PASS1", - "literal:abc", - map[string]any{"fromFile": filePath}, - }, - }, - "tenantB": map[string]any{ - "credential": "literal:credB", - }, - }, - } - - var out svcCfgComplex - // Eagerly resolve to validate that nested secrets are materialized - if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { - t.Fatalf("bind: %v", err) - } - - // Assert top-level secret - v, err := out.ClientSecret.Resolve(context.Background()) - if err != nil || v != "client-secret" { - t.Fatalf("client_secret resolve: %v %q", err, v) - } - - // Assert tenant map - tenantA, ok := out.Tenants["tenantA"] - if !ok { - t.Fatalf("expected tenant 'tenantA' present") - } - credA, err := tenantA.Credential.Resolve(context.Background()) - if err != nil || credA != "tenant-a-cred" { - t.Fatalf("credential resolve: %v %q", err, credA) - } - if len(tenantA.Passwords) != 3 { - t.Fatalf("expected 3 passwords, got %d", len(tenantA.Passwords)) - } - p0, _ := tenantA.Passwords[0].Resolve(context.Background()) - p1, _ := tenantA.Passwords[1].Resolve(context.Background()) - p2, _ := tenantA.Passwords[2].Resolve(context.Background()) - if p0 != "p1" || p1 != "abc" || p2 != "from-file" { - t.Fatalf("passwords mismatch: %q, %q, %q", p0, p1, p2) - } - - // Second tenant literal credential - tenantB, ok := out.Tenants["tenantB"] - if !ok { - t.Fatalf("expected tenant 'tenantB' present") - } - credB, err := tenantB.Credential.Resolve(context.Background()) - if err != nil || credB != "credB" { - t.Fatalf("credential resolve (tenantB): %v %q", err, credB) - } -} - -func TestBindServiceConfig_NestedTenants_EagerFailureOnMissingEnv(t *testing.T) { - in := ServiceConfig{ - "tenants": map[string]any{ - "tenantA": map[string]any{ - // Missing env value should cause eager resolution to fail - "credential": "env:OPENTDF_TEST_MISSING_ENV_ABC123", - }, - }, - } - var out svcCfgComplex - if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err == nil { - t.Fatalf("expected bind failure on missing env in eager resolution") - } -} diff --git a/service/pkg/config/decode_test.go b/service/pkg/config/decode_test.go index 9399db8514..ab277206cb 100644 --- a/service/pkg/config/decode_test.go +++ b/service/pkg/config/decode_test.go @@ -62,3 +62,100 @@ func TestDecodeHook_MalformedDirectives(t *testing.T) { } } } + +// A nested service configuration with multiple tenants and lists of credentials. +type tenantCfg struct { + Credential Secret `mapstructure:"credential"` + Passwords []Secret `mapstructure:"passwords"` +} + +type svcCfgComplex struct { + ClientSecret Secret `mapstructure:"client_secret"` + Tenants map[string]tenantCfg `mapstructure:"tenants"` +} + +func TestBindServiceConfig_NestedTenants_SecretsAndLists(t *testing.T) { + // Prepare env vars and a temp file for fromFile + t.Setenv("OPENTDF_TEST_CLIENT_SECRET", "client-secret") + t.Setenv("OPENTDF_TENANT_A_CRED", "tenant-a-cred") + t.Setenv("OPENTDF_PASS1", "p1") + + dir := t.TempDir() + filePath := filepath.Join(dir, "pass.txt") + if err := os.WriteFile(filePath, []byte("from-file\n"), 0o600); err != nil { + t.Fatalf("write temp file: %v", err) + } + + in := ServiceConfig{ + "client_secret": "env:OPENTDF_TEST_CLIENT_SECRET", + "tenants": map[string]any{ + "tenantA": map[string]any{ + "credential": "env:OPENTDF_TENANT_A_CRED", + "passwords": []any{ + "env:OPENTDF_PASS1", + "literal:abc", + map[string]any{"fromFile": filePath}, + }, + }, + "tenantB": map[string]any{ + "credential": "literal:credB", + }, + }, + } + + var out svcCfgComplex + // Eagerly resolve to validate that nested secrets are materialized + if err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()); err != nil { + t.Fatalf("bind: %v", err) + } + + // Assert top-level secret + v, err := out.ClientSecret.Resolve(t.Context()) + if err != nil || v != "client-secret" { + t.Fatalf("client_secret resolve: %v %q", err, v) + } + + // Assert tenant map + tenantA, ok := out.Tenants["tenantA"] + if !ok { + t.Fatalf("expected tenant 'tenantA' present") + } + credA, err := tenantA.Credential.Resolve(t.Context()) + if err != nil || credA != "tenant-a-cred" { + t.Fatalf("credential resolve: %v %q", err, credA) + } + if len(tenantA.Passwords) != 3 { + t.Fatalf("expected 3 passwords, got %d", len(tenantA.Passwords)) + } + p0, _ := tenantA.Passwords[0].Resolve(t.Context()) + p1, _ := tenantA.Passwords[1].Resolve(t.Context()) + p2, _ := tenantA.Passwords[2].Resolve(t.Context()) + if p0 != "p1" || p1 != "abc" || p2 != "from-file" { + t.Fatalf("passwords mismatch: %q, %q, %q", p0, p1, p2) + } + + // Second tenant literal credential + tenantB, ok := out.Tenants["tenantB"] + if !ok { + t.Fatalf("expected tenant 'tenantB' present") + } + credB, err := tenantB.Credential.Resolve(t.Context()) + if err != nil || credB != "credB" { + t.Fatalf("credential resolve (tenantB): %v %q", err, credB) + } +} + +func TestBindServiceConfig_NestedTenants_EagerFailureOnMissingEnv(t *testing.T) { + in := ServiceConfig{ + "tenants": map[string]any{ + "tenantA": map[string]any{ + // Missing env value should cause eager resolution to fail + "credential": "env:OPENTDF_TEST_MISSING_ENV_ABC123", + }, + }, + } + var out svcCfgComplex + if err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()); err == nil { + t.Fatalf("expected bind failure on missing env in eager resolution") + } +} From 126f006d113f7dbd83b4b8a8eee3a2aac3b9a538 Mon Sep 17 00:00:00 2001 From: strantalis Date: Mon, 20 Oct 2025 12:18:08 -0400 Subject: [PATCH 12/13] cleanup --- service/kas/kas.go | 27 ++++--- service/pkg/config/bind_test.go | 25 +++--- service/pkg/config/decode_test.go | 77 +++++++----------- service/pkg/config/secret_concurrency_test.go | 42 ---------- service/pkg/config/secret_test.go | 78 +++++++++++-------- service/pkg/config/yaml_array_test.go | 46 ++++------- 6 files changed, 118 insertions(+), 177 deletions(-) delete mode 100644 service/pkg/config/secret_concurrency_test.go diff --git a/service/kas/kas.go b/service/kas/kas.go index 464788a397..886a13c3a9 100644 --- a/service/kas/kas.go +++ b/service/kas/kas.go @@ -60,26 +60,20 @@ func NewRegistration() *serviceregistry.Service[kasconnect.AccessServiceHandler] } } - var kmgrNames []string - if kasCfg.Preview.KeyManagement { if err := handleKeyManagement(srp, kasCfg, p, cacheClient); err != nil { panic(err) } // Track registered manager names for logging for _, manager := range srp.KeyManagerFactories { - kmgrNames = append(kmgrNames, manager.Name) + p.KeyDelegator.RegisterKeyManager(manager.Name, manager.Factory) } - kmgrNames = append(kmgrNames, security.BasicManagerName) } else { - name, err := handleLegacyMode(srp, kasCfg, p) + err := handleLegacyMode(srp, kasCfg, p) if err != nil { panic(err) } - kmgrNames = append(kmgrNames, name) } - srp.Logger.Info("kas registered trust.KeyManagers", slog.Any("key_managers", kmgrNames)) - p.SDK = srp.SDK p.Logger = srp.Logger p.KASConfig = kasCfg @@ -162,11 +156,15 @@ func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg access.K } srp.Logger.Debug("determined KAS URL", slog.String("kas_url", kasURL.String())) + var kmgrNames []string + // Configure new delegation service p.KeyDelegator = trust.NewDelegatingKeyService(NewPlatformKeyIndexer(srp.SDK, kasURL.String(), srp.Logger), srp.Logger, cacheClient) for _, manager := range srp.KeyManagerFactories { p.KeyDelegator.RegisterKeyManager(manager.Name, manager.Factory) + kmgrNames = append(kmgrNames, manager.Name) } + kmgrNames = append(kmgrNames, security.BasicManagerName) // Register Basic Key Manager p.KeyDelegator.RegisterKeyManager(security.BasicManagerName, func(opts *trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { @@ -184,12 +182,16 @@ func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg access.K } return bm, nil }) + + srp.Logger.Info("kas registered trust.KeyManagers", slog.Any("key_managers", kmgrNames)) + // Explicitly set the default manager for session key generation. - p.KeyDelegator.SetDefaultMode(security.BasicManagerName) + // This should be configurable, e.g., defaulting to BasicManager or an HSM if available. + p.KeyDelegator.SetDefaultMode(security.BasicManagerName) // Example: default to BasicManager return nil } -func handleLegacyMode(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider) (string, error) { //nolint:unparam // maintains a consistent signature with other handlers +func handleLegacyMode(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider) error { //nolint:unparam // maintains a consistent signature with other handlers // Set up both the legacy CryptoProvider and the new SecurityProvider kasCfg.UpgradeMapToKeyring(srp.OTDF.CryptoProvider) p.CryptoProvider = srp.OTDF.CryptoProvider @@ -200,9 +202,12 @@ func handleLegacyMode(srp serviceregistry.RegistrationParams, kasCfg access.KASC p.KeyDelegator.RegisterKeyManager(inProcessService.Name(), func(*trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { return inProcessService, nil }) + + srp.Logger.Info("kas registered trust.KeyManagers", slog.Any("key_managers", []string{inProcessService.Name()})) + // Set default for non-key-management mode p.KeyDelegator.SetDefaultMode(inProcessService.Name()) - return inProcessService.Name(), nil + return nil } func initSecurityProviderAdapter(cryptoProvider *security.StandardCrypto, kasCfg access.KASConfig, l *logger.Logger) trust.KeyService { diff --git a/service/pkg/config/bind_test.go b/service/pkg/config/bind_test.go index 787773dfb2..fdf05506ad 100644 --- a/service/pkg/config/bind_test.go +++ b/service/pkg/config/bind_test.go @@ -2,6 +2,9 @@ package config import ( "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type demoCfg struct { @@ -24,18 +27,16 @@ func TestBindServiceConfig_LiteralAndEnv(t *testing.T) { } var out demoCfg - if err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()); err != nil { - t.Fatalf("bind: %v", err) - } - if out.User != "alice" { - t.Fatalf("user mismatch: %q", out.User) - } + err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()) + require.NoError(t, err) + + assert.Equal(t, "alice", out.User) + pass, err := out.Pass.Resolve(t.Context()) - if err != nil || pass != "p@ss" { - t.Fatalf("pass resolve: %v %q", err, pass) - } + require.NoError(t, err) + assert.Equal(t, "p@ss", pass) + tok, err := out.Nested.Token.Resolve(t.Context()) - if err != nil || tok != "tok" { - t.Fatalf("token resolve: %v %q", err, tok) - } + require.NoError(t, err) + assert.Equal(t, "tok", tok) } diff --git a/service/pkg/config/decode_test.go b/service/pkg/config/decode_test.go index ab277206cb..422b6d0cf2 100644 --- a/service/pkg/config/decode_test.go +++ b/service/pkg/config/decode_test.go @@ -4,6 +4,9 @@ import ( "os" "path/filepath" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type inlineCfg struct { @@ -16,9 +19,7 @@ func TestDecodeHook_InlineForms(t *testing.T) { // Prepare a temp file for file: form dir := t.TempDir() f := filepath.Join(dir, "secret.txt") - if err := os.WriteFile(f, []byte("from-file"), 0o600); err != nil { - t.Fatalf("write file: %v", err) - } + require.NoError(t, os.WriteFile(f, []byte("from-file"), 0o600)) t.Setenv("OPENTDF_TEST_INLINE", "from-env") in := ServiceConfig{ @@ -28,22 +29,17 @@ func TestDecodeHook_InlineForms(t *testing.T) { } var out inlineCfg - if err := BindServiceConfig(t.Context(), in, &out); err != nil { - t.Fatalf("bind: %v", err) - } + require.NoError(t, BindServiceConfig(t.Context(), in, &out)) a, err := out.A.Resolve(t.Context()) - if err != nil || a != "abc" { - t.Fatalf("literal: %v %q", err, a) - } + require.NoError(t, err) + assert.Equal(t, "abc", a) b, err := out.B.Resolve(t.Context()) - if err != nil || b != "from-env" { - t.Fatalf("env: %v %q", err, b) - } + require.NoError(t, err) + assert.Equal(t, "from-env", b) c, err := out.C.Resolve(t.Context()) - if err != nil || c != "from-file" { - t.Fatalf("file: %v %q", err, c) - } + require.NoError(t, err) + assert.Equal(t, "from-file", c) } func TestDecodeHook_MalformedDirectives(t *testing.T) { @@ -57,9 +53,8 @@ func TestDecodeHook_MalformedDirectives(t *testing.T) { var out struct { X Secret `mapstructure:"x"` } - if err := BindServiceConfig(t.Context(), in, &out); err == nil { - t.Fatalf("expected error for malformed directive: %+v", in) - } + err := BindServiceConfig(t.Context(), in, &out) + require.Error(t, err) } } @@ -82,9 +77,7 @@ func TestBindServiceConfig_NestedTenants_SecretsAndLists(t *testing.T) { dir := t.TempDir() filePath := filepath.Join(dir, "pass.txt") - if err := os.WriteFile(filePath, []byte("from-file\n"), 0o600); err != nil { - t.Fatalf("write temp file: %v", err) - } + require.NoError(t, os.WriteFile(filePath, []byte("from-file\n"), 0o600)) in := ServiceConfig{ "client_secret": "env:OPENTDF_TEST_CLIENT_SECRET", @@ -105,44 +98,33 @@ func TestBindServiceConfig_NestedTenants_SecretsAndLists(t *testing.T) { var out svcCfgComplex // Eagerly resolve to validate that nested secrets are materialized - if err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()); err != nil { - t.Fatalf("bind: %v", err) - } + require.NoError(t, BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution())) // Assert top-level secret v, err := out.ClientSecret.Resolve(t.Context()) - if err != nil || v != "client-secret" { - t.Fatalf("client_secret resolve: %v %q", err, v) - } + require.NoError(t, err) + assert.Equal(t, "client-secret", v) // Assert tenant map tenantA, ok := out.Tenants["tenantA"] - if !ok { - t.Fatalf("expected tenant 'tenantA' present") - } + require.True(t, ok, "expected tenant 'tenantA' present") credA, err := tenantA.Credential.Resolve(t.Context()) - if err != nil || credA != "tenant-a-cred" { - t.Fatalf("credential resolve: %v %q", err, credA) - } - if len(tenantA.Passwords) != 3 { - t.Fatalf("expected 3 passwords, got %d", len(tenantA.Passwords)) - } + require.NoError(t, err) + assert.Equal(t, "tenant-a-cred", credA) + require.Len(t, tenantA.Passwords, 3) p0, _ := tenantA.Passwords[0].Resolve(t.Context()) p1, _ := tenantA.Passwords[1].Resolve(t.Context()) p2, _ := tenantA.Passwords[2].Resolve(t.Context()) - if p0 != "p1" || p1 != "abc" || p2 != "from-file" { - t.Fatalf("passwords mismatch: %q, %q, %q", p0, p1, p2) - } + assert.Equal(t, "p1", p0) + assert.Equal(t, "abc", p1) + assert.Equal(t, "from-file", p2) // Second tenant literal credential tenantB, ok := out.Tenants["tenantB"] - if !ok { - t.Fatalf("expected tenant 'tenantB' present") - } + require.True(t, ok, "expected tenant 'tenantB' present") credB, err := tenantB.Credential.Resolve(t.Context()) - if err != nil || credB != "credB" { - t.Fatalf("credential resolve (tenantB): %v %q", err, credB) - } + require.NoError(t, err) + assert.Equal(t, "credB", credB) } func TestBindServiceConfig_NestedTenants_EagerFailureOnMissingEnv(t *testing.T) { @@ -155,7 +137,6 @@ func TestBindServiceConfig_NestedTenants_EagerFailureOnMissingEnv(t *testing.T) }, } var out svcCfgComplex - if err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()); err == nil { - t.Fatalf("expected bind failure on missing env in eager resolution") - } + err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()) + require.Error(t, err) } diff --git a/service/pkg/config/secret_concurrency_test.go b/service/pkg/config/secret_concurrency_test.go deleted file mode 100644 index 55c8276565..0000000000 --- a/service/pkg/config/secret_concurrency_test.go +++ /dev/null @@ -1,42 +0,0 @@ -package config - -import ( - "sync" - "testing" -) - -func TestSecret_Resolve_Concurrent(t *testing.T) { - t.Setenv("OPENTDF_CONCUR", "concur") - s := NewEnvSecret("OPENTDF_CONCUR") - - const n = 50 - var wg sync.WaitGroup - wg.Add(n) - errs := make(chan error, n) - vals := make(chan string, n) - for i := 0; i < n; i++ { - go func() { - defer wg.Done() - v, err := s.Resolve(t.Context()) - if err != nil { - errs <- err - return - } - vals <- v - }() - } - wg.Wait() - close(errs) - close(vals) - if len(errs) > 0 { - for e := range errs { - t.Errorf("resolve error: %v", e) - } - t.FailNow() - } - for v := range vals { - if v != "concur" { - t.Fatalf("unexpected value: %q", v) - } - } -} diff --git a/service/pkg/config/secret_test.go b/service/pkg/config/secret_test.go index 3fb158bf07..c941c5317b 100644 --- a/service/pkg/config/secret_test.go +++ b/service/pkg/config/secret_test.go @@ -3,25 +3,22 @@ package config import ( "encoding/json" "os" + "sync" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSecret_Literal(t *testing.T) { s := NewLiteralSecret("super-secret") - if s.String() != "[REDACTED]" { - t.Fatalf("expected redacted String, got %q", s.String()) - } + assert.Equal(t, "[REDACTED]", s.String()) got, err := s.Resolve(t.Context()) - if err != nil { - t.Fatalf("resolve literal: %v", err) - } - if got != "super-secret" { - t.Fatalf("unexpected value: %q", got) - } + require.NoError(t, err) + assert.Equal(t, "super-secret", got) raw, err := s.Export() - if err != nil || raw != "super-secret" { - t.Fatalf("export literal: %v, %q", err, raw) - } + require.NoError(t, err) + assert.Equal(t, "super-secret", raw) } func TestSecret_FromEnv(t *testing.T) { @@ -29,42 +26,55 @@ func TestSecret_FromEnv(t *testing.T) { t.Setenv(env, "env-secret") s := NewEnvSecret(env) - if s.String() != "[REDACTED]" { - t.Fatalf("expected redacted String, got %q", s.String()) - } + assert.Equal(t, "[REDACTED]", s.String()) got, err := s.Resolve(t.Context()) - if err != nil { - t.Fatalf("resolve env: %v", err) - } - if got != "env-secret" { - t.Fatalf("unexpected value: %q", got) - } + require.NoError(t, err) + assert.Equal(t, "env-secret", got) } func TestSecret_FromFile_Trim(t *testing.T) { dir := t.TempDir() p := dir + "/s.txt" // Include trailing newline to simulate typical secret mounts - if err := os.WriteFile(p, []byte("abc\n"), 0o600); err != nil { - t.Fatalf("write: %v", err) - } + require.NoError(t, os.WriteFile(p, []byte("abc\n"), 0o600)) s := NewFileSecret(p) got, err := s.Resolve(t.Context()) - if err != nil { - t.Fatalf("resolve file: %v", err) - } - if got != "abc" { - t.Fatalf("expected trimmed value 'abc', got %q", got) - } + require.NoError(t, err) + assert.Equal(t, "abc", got, "expected trimmed value") } func TestSecret_JSONRedacted(t *testing.T) { s := NewLiteralSecret("dont-log-me") b, err := json.Marshal(s) - if err != nil { - t.Fatalf("marshal: %v", err) + require.NoError(t, err) + assert.Equal(t, "\"[REDACTED]\"", string(b)) +} + +func TestSecret_Resolve_Concurrent(t *testing.T) { + t.Setenv("OPENTDF_CONCUR", "concur") + s := NewEnvSecret("OPENTDF_CONCUR") + + const n = 50 + var wg sync.WaitGroup + wg.Add(n) + errs := make(chan error, n) + vals := make(chan string, n) + for i := 0; i < n; i++ { + go func() { + defer wg.Done() + v, err := s.Resolve(t.Context()) + if err != nil { + errs <- err + return + } + vals <- v + }() } - if string(b) != "\"[REDACTED]\"" { - t.Fatalf("expected redacted json, got %s", b) + wg.Wait() + close(errs) + close(vals) + require.Empty(t, errs, "expected no resolve errors") + for v := range vals { + assert.Equal(t, "concur", v) } } diff --git a/service/pkg/config/yaml_array_test.go b/service/pkg/config/yaml_array_test.go index 6810cc838c..be4583e379 100644 --- a/service/pkg/config/yaml_array_test.go +++ b/service/pkg/config/yaml_array_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type arrayCfg struct { @@ -27,9 +29,7 @@ func TestBind_FromYAMLArray_SecretsSlice(t *testing.T) { t.Setenv("OPENTDF_YAML_ARR", "arr-env") dir := t.TempDir() fp := filepath.Join(dir, "s.txt") - if err := os.WriteFile(fp, []byte("from-file\n"), 0o600); err != nil { - t.Fatalf("write: %v", err) - } + require.NoError(t, os.WriteFile(fp, []byte("from-file\n"), 0o600)) yaml := "" + "secrets:\n" + @@ -39,26 +39,20 @@ func TestBind_FromYAMLArray_SecretsSlice(t *testing.T) { v := viper.New() v.SetConfigType("yaml") - if err := v.ReadConfig(bytes.NewBufferString(yaml)); err != nil { - t.Fatalf("read yaml: %v", err) - } + require.NoError(t, v.ReadConfig(bytes.NewBufferString(yaml))) in := ServiceConfig{ "secrets": v.Get("secrets"), } var out arrayCfg - if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { - t.Fatalf("bind: %v", err) - } - if len(out.Secrets) != 3 { - t.Fatalf("expected 3 secrets, got %d", len(out.Secrets)) - } + require.NoError(t, BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution())) + require.Len(t, out.Secrets, 3) s0, _ := out.Secrets[0].Resolve(context.Background()) s1, _ := out.Secrets[1].Resolve(context.Background()) s2, _ := out.Secrets[2].Resolve(context.Background()) - if s0 != "arr-env" || s1 != "from-file" || s2 != "abc" { - t.Fatalf("values mismatch: %q %q %q", s0, s1, s2) - } + assert.Equal(t, "arr-env", s0) + assert.Equal(t, "from-file", s1) + assert.Equal(t, "abc", s2) } func TestBind_FromYAMLArray_StructsWithSecretFields(t *testing.T) { @@ -73,26 +67,18 @@ func TestBind_FromYAMLArray_StructsWithSecretFields(t *testing.T) { v := viper.New() v.SetConfigType("yaml") - if err := v.ReadConfig(bytes.NewBufferString(yaml)); err != nil { - t.Fatalf("read yaml: %v", err) - } + require.NoError(t, v.ReadConfig(bytes.NewBufferString(yaml))) in := ServiceConfig{ "providers": v.Get("providers"), } var out providersCfg - if err := BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution()); err != nil { - t.Fatalf("bind: %v", err) - } - if len(out.Providers) != 2 { - t.Fatalf("expected 2 providers, got %d", len(out.Providers)) - } - if out.Providers[0].Name != "a" || out.Providers[1].Name != "b" { - t.Fatalf("names mismatch: %q %q", out.Providers[0].Name, out.Providers[1].Name) - } + require.NoError(t, BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution())) + require.Len(t, out.Providers, 2) + assert.Equal(t, "a", out.Providers[0].Name) + assert.Equal(t, "b", out.Providers[1].Name) p0, _ := out.Providers[0].Password.Resolve(context.Background()) p1, _ := out.Providers[1].Password.Resolve(context.Background()) - if p0 != "alpha" || p1 != "b-pass" { - t.Fatalf("passwords mismatch: %q %q", p0, p1) - } + assert.Equal(t, "alpha", p0) + assert.Equal(t, "b-pass", p1) } From d445f8ef73676b716656611038689680588273a4 Mon Sep 17 00:00:00 2001 From: strantalis Date: Mon, 20 Oct 2025 13:15:21 -0400 Subject: [PATCH 13/13] fix lint issues --- service/pkg/config/yaml_array_test.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/service/pkg/config/yaml_array_test.go b/service/pkg/config/yaml_array_test.go index be4583e379..0a4fb9abe5 100644 --- a/service/pkg/config/yaml_array_test.go +++ b/service/pkg/config/yaml_array_test.go @@ -2,7 +2,6 @@ package config import ( "bytes" - "context" "os" "path/filepath" "testing" @@ -45,11 +44,11 @@ func TestBind_FromYAMLArray_SecretsSlice(t *testing.T) { "secrets": v.Get("secrets"), } var out arrayCfg - require.NoError(t, BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution())) + require.NoError(t, BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution())) require.Len(t, out.Secrets, 3) - s0, _ := out.Secrets[0].Resolve(context.Background()) - s1, _ := out.Secrets[1].Resolve(context.Background()) - s2, _ := out.Secrets[2].Resolve(context.Background()) + s0, _ := out.Secrets[0].Resolve(t.Context()) + s1, _ := out.Secrets[1].Resolve(t.Context()) + s2, _ := out.Secrets[2].Resolve(t.Context()) assert.Equal(t, "arr-env", s0) assert.Equal(t, "from-file", s1) assert.Equal(t, "abc", s2) @@ -73,12 +72,12 @@ func TestBind_FromYAMLArray_StructsWithSecretFields(t *testing.T) { "providers": v.Get("providers"), } var out providersCfg - require.NoError(t, BindServiceConfig(context.Background(), in, &out, WithEagerSecretResolution())) + require.NoError(t, BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution())) require.Len(t, out.Providers, 2) assert.Equal(t, "a", out.Providers[0].Name) assert.Equal(t, "b", out.Providers[1].Name) - p0, _ := out.Providers[0].Password.Resolve(context.Background()) - p1, _ := out.Providers[1].Password.Resolve(context.Background()) + p0, _ := out.Providers[0].Password.Resolve(t.Context()) + p1, _ := out.Providers[1].Password.Resolve(t.Context()) assert.Equal(t, "alpha", p0) assert.Equal(t, "b-pass", p1) }