diff --git a/.github/workflows/nats.yml b/.github/workflows/nats.yml new file mode 100644 index 0000000..9cdbe2f --- /dev/null +++ b/.github/workflows/nats.yml @@ -0,0 +1,36 @@ +name: nats + + +on: + push: + paths: + - 'nats/**' + - 'util/**' + - 'go.sum' + - 'go.mod' + - '.github/workflows/nats.yml' + +jobs: + golang_unit_tests: + name: Golang unit tests for nats + runs-on: ubuntu-latest + steps: + # See: https://github.com/marketplace/actions/checkout + # Checkout git repository and submodules + - name: Checkout + uses: actions/checkout@v4 + with: + submodules: recursive + + # See: https://github.com/marketplace/actions/setup-go-environment + - name: Setup Golang + uses: actions/setup-go@v5 + with: + go-version: '1.25' + check-latest: true + + # Run 'go test' in specified test target + - name: Run Golang tests + run: | + cd nats + go test diff --git a/ccStartup/README.md b/ccStartup/README.md index bfb8cf5..45d8ea0 100644 --- a/ccStartup/README.md +++ b/ccStartup/README.md @@ -36,3 +36,10 @@ The configuration file for the startup contains the targets where the topology s - `nats.url`: NATS server URL - `nats.subject`: NATS subject where to publish the topology as JSON - `nats.nkey_file`: Path to NKey file for authentification + +The auth token need not be stored in the configuration file. It is also read +from `$CC_STARTUP_AUTH_TOKEN`, or from the file named by +`$CC_STARTUP_AUTH_TOKEN_FILE`, either of which takes precedence over +`http.auth_token`. A named file that cannot be read is an error, so a request +is never sent unauthenticated in its place. See the +[`util`](../util/README.md) package for the full precedence rules. diff --git a/ccStartup/ccStartup.go b/ccStartup/ccStartup.go index 0d5643b..6b5e9ff 100644 --- a/ccStartup/ccStartup.go +++ b/ccStartup/ccStartup.go @@ -8,9 +8,19 @@ import ( cclog "github.com/ClusterCockpit/cc-lib/v2/ccLogger" "github.com/ClusterCockpit/cc-lib/v2/ccTopology" + "github.com/ClusterCockpit/cc-lib/v2/util" "github.com/nats-io/nats.go" ) +// EnvAuthToken overrides the configured HTTP endpoint auth token. It also +// accepts a "_FILE" variant naming a file that holds the token, so the token +// can come from a Docker or Kubernetes secret mount or from systemd +// LoadCredential; see util.SecretFromEnv for the precedence rules. +// +// The name is prefixed CC_ because cc-lib is linked into several applications, +// whose environments it must not silently claim names in. +const EnvAuthToken = "CC_STARTUP_AUTH_TOKEN" + // func StartupTopology(out chan lp.CCMessage) error { // topo, err := ccTopology.LocalTopology() // if err != nil { @@ -76,14 +86,25 @@ func CCStartup(config json.RawMessage) error { if len(out) > 0 { if len(conf.HttpEndpoint.URL) > 0 { + // The token may come from the environment instead of the config + // file. An unreadable secret file is fatal rather than a silent + // fallback, so an unauthenticated request is never sent in its + // place. + authToken, err := util.SecretFromEnv(EnvAuthToken, conf.HttpEndpoint.AuthToken) + if err != nil { + err = fmt.Errorf("resolving %s: %w", EnvAuthToken, err) + cclog.ComponentError("CCStartup", err.Error()) + return err + } + bodyReader := bytes.NewReader(out) req, err := http.NewRequest(http.MethodPost, conf.HttpEndpoint.URL, bodyReader) if err != nil { err = fmt.Errorf("failed to create HTTP request: %w", err) cclog.ComponentError("CCStartup", err.Error()) } else { - if len(conf.HttpEndpoint.AuthToken) > 0 { - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", conf.HttpEndpoint.AuthToken)) + if len(authToken) > 0 { + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", authToken)) } resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/nats/client.go b/nats/client.go index 2231d03..1eb25d1 100644 --- a/nats/client.go +++ b/nats/client.go @@ -21,6 +21,10 @@ // } // } // +// The username and password may instead come from the environment, via +// $CC_NATS_USERNAME and $CC_NATS_PASSWORD or the files named by their _FILE +// variants, which take precedence over the configuration file. +// // Or using a credentials file: // // { @@ -55,6 +59,7 @@ import ( "sync" cclog "github.com/ClusterCockpit/cc-lib/v2/ccLogger" + "github.com/ClusterCockpit/cc-lib/v2/util" "github.com/nats-io/nats.go" ) @@ -99,6 +104,27 @@ func GetClient() *Client { return clientInstance } +// resolveCredentials returns the username and password to authenticate with. +// They may come from the environment ($CC_NATS_USERNAME, $CC_NATS_PASSWORD, or +// the files named by their _FILE variants) so that they need not be stored in +// the configuration file. +// +// Resolution happens here rather than in Init for three reasons: an explicitly +// passed config then behaves the same as the global one, Init is optional, and +// the plaintext never lands in the exported Keys, which an application may +// re-marshal or dump. +func resolveCredentials(cfg *NatsConfig) (username, password string, err error) { + if username, err = util.SecretFromEnv(EnvUsername, cfg.Username); err != nil { + return "", "", fmt.Errorf("resolving %s: %w", EnvUsername, err) + } + + if password, err = util.SecretFromEnv(EnvPassword, cfg.Password); err != nil { + return "", "", fmt.Errorf("resolving %s: %w", EnvPassword, err) + } + + return username, password, nil +} + // NewClient creates a new NATS client. If cfg is nil, uses the global Keys config. func NewClient(cfg *NatsConfig) (*Client, error) { if cfg == nil { @@ -111,8 +137,13 @@ func NewClient(cfg *NatsConfig) (*Client, error) { var opts []nats.Option - if cfg.Username != "" && cfg.Password != "" { - opts = append(opts, nats.UserInfo(cfg.Username, cfg.Password)) + username, password, err := resolveCredentials(cfg) + if err != nil { + return nil, err + } + + if username != "" && password != "" { + opts = append(opts, nats.UserInfo(username, password)) } if cfg.CredsFilePath != "" { diff --git a/nats/client_test.go b/nats/client_test.go new file mode 100644 index 0000000..5a7434a --- /dev/null +++ b/nats/client_test.go @@ -0,0 +1,119 @@ +// Copyright (C) NHR@FAU, University Erlangen-Nuremberg. +// All rights reserved. This file is part of cc-lib. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package nats + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/ClusterCockpit/cc-lib/v2/util" +) + +func TestResolveCredentials_Precedence(t *testing.T) { + tests := []struct { + name string + cfg NatsConfig + envUser string + envPassFile string + wantUser string + wantPassword string + }{ + { + name: "config only", + cfg: NatsConfig{Username: "cfg-user", Password: "cfg-pass"}, + wantUser: "cfg-user", + wantPassword: "cfg-pass", + }, + { + name: "environment overrides config", + cfg: NatsConfig{Username: "cfg-user", Password: "cfg-pass"}, + envUser: "env-user", + wantUser: "env-user", + wantPassword: "cfg-pass", + }, + { + name: "secret file overrides config", + cfg: NatsConfig{Username: "cfg-user", Password: "cfg-pass"}, + envPassFile: "file-pass\n", + wantUser: "cfg-user", + wantPassword: "file-pass", + }, + { + name: "nothing configured", + wantUser: "", + wantPassword: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envUser != "" { + t.Setenv(EnvUsername, tt.envUser) + } + if tt.envPassFile != "" { + path := filepath.Join(t.TempDir(), "password") + if err := os.WriteFile(path, []byte(tt.envPassFile), 0o600); err != nil { + t.Fatalf("writing secret file: %v", err) + } + t.Setenv(EnvPassword+util.EnvFileSuffix, path) + } + + cfg := tt.cfg + user, password, err := resolveCredentials(&cfg) + if err != nil { + t.Fatalf("resolveCredentials failed: %v", err) + } + if user != tt.wantUser { + t.Errorf("expected username %q, got %q", tt.wantUser, user) + } + if password != tt.wantPassword { + t.Errorf("expected password %q, got %q", tt.wantPassword, password) + } + + // The resolved secret must never be written back into the config. + if cfg.Username != tt.cfg.Username || cfg.Password != tt.cfg.Password { + t.Error("expected the config to be left unmodified") + } + }) + } +} + +func TestResolveCredentials_UnreadableSecretFile(t *testing.T) { + t.Setenv(EnvPassword+util.EnvFileSuffix, filepath.Join(t.TempDir(), "absent")) + + cfg := NatsConfig{Username: "cfg-user", Password: "cfg-pass"} + _, _, err := resolveCredentials(&cfg) + if err == nil { + t.Fatal("expected an error for an unreadable secret file, got nil") + } + // The variable must be named so an operator can find the misconfiguration, + // and the config value must not be used as a silent fallback. + if !strings.Contains(err.Error(), EnvPassword) { + t.Errorf("expected the error to name %s, got %q", EnvPassword, err.Error()) + } +} + +func TestNewClient_RejectsUnreadableSecretFileBeforeConnecting(t *testing.T) { + t.Setenv(EnvPassword+util.EnvFileSuffix, filepath.Join(t.TempDir(), "absent")) + + // An unroutable address: if credential resolution did not fail first, this + // would block on a connection attempt instead of returning promptly. + _, err := NewClient(&NatsConfig{Address: "nats://127.0.0.1:1", Password: "cfg-pass"}) + if err == nil { + t.Fatal("expected an error, got nil") + } + if !strings.Contains(err.Error(), EnvPassword) { + t.Errorf("expected the error to name %s, got %q", EnvPassword, err.Error()) + } +} + +func TestNewClient_RequiresAddress(t *testing.T) { + if _, err := NewClient(&NatsConfig{}); err == nil { + t.Error("expected an error for an empty address, got nil") + } +} diff --git a/nats/config.go b/nats/config.go index c9ab48a..18f7a6c 100644 --- a/nats/config.go +++ b/nats/config.go @@ -23,6 +23,18 @@ type NatsConfig struct { // Keys holds the global NATS configuration loaded via Init. var Keys NatsConfig +// Environment variables that override the corresponding configuration values. +// Each also accepts a "_FILE" variant naming a file that holds the value, so a +// credential can be supplied from a Docker or Kubernetes secret mount or from +// systemd LoadCredential; see util.SecretFromEnv for the precedence rules. +// +// The names are prefixed CC_ because cc-lib is linked into several +// applications, whose environments it must not silently claim names in. +const ( + EnvUsername = "CC_NATS_USERNAME" + EnvPassword = "CC_NATS_PASSWORD" +) + const ConfigSchema = `{ "type": "object", "description": "Configuration for NATS messaging client.", @@ -32,11 +44,11 @@ const ConfigSchema = `{ "type": "string" }, "username": { - "description": "Username for NATS authentication (optional).", + "description": "Username for NATS authentication (optional). Overridden by the CC_NATS_USERNAME environment variable when set, or by the contents of the file named by CC_NATS_USERNAME_FILE.", "type": "string" }, "password": { - "description": "Password for NATS authentication (optional).", + "description": "Password for NATS authentication (optional). Overridden by the CC_NATS_PASSWORD environment variable when set, or by the contents of the file named by CC_NATS_PASSWORD_FILE.", "type": "string" }, "creds-file-path": { diff --git a/receivers/eecptReceiver.go b/receivers/eecptReceiver.go index 78b6902..6cc15cb 100644 --- a/receivers/eecptReceiver.go +++ b/receivers/eecptReceiver.go @@ -50,6 +50,13 @@ type EECPTReceiverConfig struct { Password string `json:"password"` useBasicAuth bool + // Alternative sources for the credentials above, so that they need not be + // stored in the configuration file. See resolveSecrets. + UsernameEnv string `json:"username_env,omitempty"` + UsernameFile string `json:"username_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` + AnalysisBufferLength int `json:"analysis_buffer_size"` AnalysisInterval string `json:"analysis_interval"` AnalysisMetric string `json:"analysis_metric"` @@ -420,6 +427,17 @@ func NewEECPTReceiver(name string, config json.RawMessage) (Receiver, error) { } } + // Resolve credentials before the basic authentication check below, so that + // values supplied from the environment or a secret file count as + // configured. + if err := resolveSecrets( + secretRef{"username", &r.config.Username, r.config.UsernameEnv, r.config.UsernameFile}, + secretRef{"password", &r.config.Password, r.config.PasswordEnv, r.config.PasswordFile}, + ); err != nil { + cclog.ComponentError(r.name, err.Error()) + return nil, err + } + if len(r.config.Username) > 0 || len(r.config.Password) > 0 { r.config.useBasicAuth = true } diff --git a/receivers/eecptReceiver.md b/receivers/eecptReceiver.md index a2aaccc..c64be25 100644 --- a/receivers/eecptReceiver.md +++ b/receivers/eecptReceiver.md @@ -69,3 +69,18 @@ When a phase transition is detected, the receiver generates an event: - **Tags**: `type=node`, `stype=application`, `stype-id=` - **Fields**: `value="region changed"` - **Timestamp**: Current time + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`username`, `password` has two sibling keys that select an alternative source: + +- `username_env`: name of an environment variable holding the value +- `username_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/receivers/httpReceiver.go b/receivers/httpReceiver.go index 97a3c49..b8913ba 100644 --- a/receivers/httpReceiver.go +++ b/receivers/httpReceiver.go @@ -39,6 +39,13 @@ type HttpReceiverConfig struct { Username string `json:"username"` // Basic auth username (optional) Password string `json:"password"` // Basic auth password (optional) useBasicAuth bool + + // Alternative sources for the credentials above, so that they need not be + // stored in the configuration file. See resolveSecrets. + UsernameEnv string `json:"username_env,omitempty"` + UsernameFile string `json:"username_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` } type HttpReceiver struct { @@ -139,6 +146,17 @@ func NewHttpReceiver(name string, config json.RawMessage) (Receiver, error) { } } + // Resolve credentials before the basic authentication check below, so that + // values supplied from the environment or a secret file count as + // configured. + if err := resolveSecrets( + secretRef{"username", &r.config.Username, r.config.UsernameEnv, r.config.UsernameFile}, + secretRef{"password", &r.config.Password, r.config.PasswordEnv, r.config.PasswordFile}, + ); err != nil { + cclog.ComponentError(r.name, err.Error()) + return nil, err + } + if len(r.config.Username) > 0 || len(r.config.Password) > 0 { r.config.useBasicAuth = true } diff --git a/receivers/httpReceiver.md b/receivers/httpReceiver.md index 13f812f..feacc27 100644 --- a/receivers/httpReceiver.md +++ b/receivers/httpReceiver.md @@ -60,3 +60,18 @@ curl http://localhost:8080/write \ "myMetric,hostname=myHost,type=hwthread,type-id=0,unit=Hz value=400000i 1694777161164284635 myMetric,hostname=myHost,type=hwthread,type-id=1,unit=Hz value=400001i 1694777161164284635" ``` + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`username`, `password` has two sibling keys that select an alternative source: + +- `username_env`: name of an environment variable holding the value +- `username_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/receivers/ipmiReceiver.go b/receivers/ipmiReceiver.go index abf16a3..6ce3d9e 100644 --- a/receivers/ipmiReceiver.go +++ b/receivers/ipmiReceiver.go @@ -373,6 +373,18 @@ func NewIPMIReceiver(name string, config json.RawMessage) (Receiver, error) { } } + // BMC credentials are the most sensitive part of this configuration, so + // allow the global defaults to come from the environment instead of + // plaintext JSON. Per-host client_config entries still take precedence. + if err := secretDefaultFromEnv(&configJSON.Username, EnvIPMIUsername); err != nil { + cclog.ComponentError(r.name, err.Error()) + return nil, err + } + if err := secretDefaultFromEnv(&configJSON.Password, EnvIPMIPassword); err != nil { + cclog.ComponentError(r.name, err.Error()) + return nil, err + } + // Convert interval string representation to duration var err error r.config.Interval, err = time.ParseDuration(configJSON.IntervalString) diff --git a/receivers/ipmiReceiver.md b/receivers/ipmiReceiver.md index 06e49d6..efcd104 100644 --- a/receivers/ipmiReceiver.md +++ b/receivers/ipmiReceiver.md @@ -68,3 +68,16 @@ These settings can be defined globally and overridden in `client_config`: - **Platform**: Linux only. - **Tools**: `ipmi-sensors` must be installed and available in the PATH. - **Permissions**: The user running the collector must have permission to execute `ipmi-sensors`. + +### Credentials from the environment + +The global default `username` and `password` need not be stored in the +configuration file. When they are absent, they are read from +`$CC_IPMI_USERNAME` and `$CC_IPMI_PASSWORD`, or from the files named by +`$CC_IPMI_USERNAME_FILE` and `$CC_IPMI_PASSWORD_FILE`. + +A `client_config` entry that sets its own `username` or `password` always takes +precedence, so per-host credentials keep working unchanged. Because the +variables are fixed names, several `ipmi` receiver instances in one process +share them. A named file that cannot be read is an error rather than a silent +fallback. See the [`util`](../util/README.md) package for the full rules. diff --git a/receivers/natsReceiver.go b/receivers/natsReceiver.go index e1430f1..4b65d47 100644 --- a/receivers/natsReceiver.go +++ b/receivers/natsReceiver.go @@ -28,6 +28,13 @@ type NatsReceiverConfig struct { User string `json:"user,omitempty"` // Username for authentication Password string `json:"password,omitempty"` // Password for authentication NkeyFile string `json:"nkey_file,omitempty"` // Path to NKey credentials file + + // Alternative sources for the credentials above, so that they need not be + // stored in the configuration file. See resolveSecrets. + UserEnv string `json:"user_env,omitempty"` + UserFile string `json:"user_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` } type NatsReceiver struct { @@ -109,6 +116,13 @@ func NewNatsReceiver(name string, config json.RawMessage) (Receiver, error) { len(r.config.Subject) == 0 { return nil, errors.New("not all configuration variables set required by NatsReceiver") } + if err := resolveSecrets( + secretRef{"user", &r.config.User, r.config.UserEnv, r.config.UserFile}, + secretRef{"password", &r.config.Password, r.config.PasswordEnv, r.config.PasswordFile}, + ); err != nil { + cclog.ComponentError(r.name, err.Error()) + return nil, err + } p, err := mp.NewMessageProcessor() if err != nil { return nil, fmt.Errorf("initialization of message processor failed: %w", err) diff --git a/receivers/natsReceiver.md b/receivers/natsReceiver.md index 6625e9a..0aae601 100644 --- a/receivers/natsReceiver.md +++ b/receivers/natsReceiver.md @@ -62,3 +62,18 @@ You can use the NATS command line client to interact with the server and verify "myMetric,hostname=myHost,type=hwthread,type-id=0,unit=Hz value=400000i 1694777161164284635 myMetric,hostname=myHost,type=hwthread,type-id=1,unit=Hz value=400001i 1694777161164284635" ``` + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`user`, `password` has two sibling keys that select an alternative source: + +- `user_env`: name of an environment variable holding the value +- `user_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/receivers/redfishReceiver.go b/receivers/redfishReceiver.go index 5671574..2e5d22b 100644 --- a/receivers/redfishReceiver.go +++ b/receivers/redfishReceiver.go @@ -871,6 +871,19 @@ func NewRedfishReceiver(name string, config json.RawMessage) (Receiver, error) { return nil, err } } + + // BMC credentials are the most sensitive part of this configuration, so + // allow the global defaults to come from the environment instead of + // plaintext JSON. Per-host client_config entries still take precedence. + if err := secretDefaultFromEnv(&configJSON.Username, EnvRedfishUsername); err != nil { + cclog.ComponentError(r.name, err.Error()) + return nil, err + } + if err := secretDefaultFromEnv(&configJSON.Password, EnvRedfishPassword); err != nil { + cclog.ComponentError(r.name, err.Error()) + return nil, err + } + p, err := mp.NewMessageProcessor() if err != nil { return nil, fmt.Errorf("initialization of message processor failed: %w", err) diff --git a/receivers/redfishReceiver.md b/receivers/redfishReceiver.md index ff35d9c..4923de8 100644 --- a/receivers/redfishReceiver.md +++ b/receivers/redfishReceiver.md @@ -76,3 +76,16 @@ These settings can be defined globally and overridden in `client_config`: - **Platform**: Linux only. - **Hardware**: Management controllers must support the Redfish API. + +### Credentials from the environment + +The global default `username` and `password` need not be stored in the +configuration file. When they are absent, they are read from +`$CC_REDFISH_USERNAME` and `$CC_REDFISH_PASSWORD`, or from the files named by +`$CC_REDFISH_USERNAME_FILE` and `$CC_REDFISH_PASSWORD_FILE`. + +A `client_config` entry that sets its own `username` or `password` always takes +precedence, so per-host credentials keep working unchanged. Because the +variables are fixed names, several `redfish` receiver instances in one process +share them. A named file that cannot be read is an error rather than a silent +fallback. See the [`util`](../util/README.md) package for the full rules. diff --git a/receivers/secret.go b/receivers/secret.go new file mode 100644 index 0000000..87fc732 --- /dev/null +++ b/receivers/secret.go @@ -0,0 +1,80 @@ +// Copyright (C) NHR@FAU, University Erlangen-Nuremberg. +// All rights reserved. This file is part of cc-lib. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package receivers + +import ( + "fmt" + + "github.com/ClusterCockpit/cc-lib/v2/util" +) + +// secretRef names the three places one credential of a receiver instance may +// come from. Value points at the inline configuration value and receives the +// resolved secret. +// +// Receivers are configured as a map of named instances, so no fixed +// environment variable name can address the credential of one particular +// receiver. Each instance therefore names its own sources, through the sibling +// configuration keys "_env" and "_file". +type secretRef struct { + Key string // configuration key, used in error messages only + Value *string + EnvName string + FilePath string +} + +// resolveSecrets resolves each secret in place, in the order given. A secret +// whose named file cannot be read is an error rather than a silent fallback to +// the inline value, so a receiver never connects with a stale credential. The +// resolved values are never logged. +func resolveSecrets(secrets ...secretRef) error { + for _, s := range secrets { + v, err := util.SecretFromConfig(*s.Value, s.EnvName, s.FilePath) + if err != nil { + return fmt.Errorf("resolving %q: %w", s.Key, err) + } + *s.Value = v + } + + return nil +} + +// Fixed environment variable names for the BMC credentials of the redfish and +// ipmi receivers. Unlike the credentials above these are one per receiver +// type, not one per instance, so several instances of the same receiver type +// share them; a per-host client_config entry always takes precedence. +// +// Each also accepts a "_FILE" variant naming a file that holds the value. The +// names are prefixed CC_ because cc-lib is linked into several applications, +// whose environments it must not silently claim names in. +const ( + EnvRedfishUsername = "CC_REDFISH_USERNAME" + EnvRedfishPassword = "CC_REDFISH_PASSWORD" + EnvIPMIUsername = "CC_IPMI_USERNAME" + EnvIPMIPassword = "CC_IPMI_PASSWORD" +) + +// secretDefaultFromEnv supplies a global default credential from the +// environment when the configuration file does not set one. It leaves a +// configured value untouched, so it needs no new configuration key, and an +// absent variable leaves the default unset for the caller to reject. +func secretDefaultFromEnv(target **string, envVar string) error { + if *target != nil { + return nil + } + + v, err := util.SecretFromEnv(envVar, "") + if err != nil { + return fmt.Errorf("resolving %s: %w", envVar, err) + } + if v == "" { + return nil + } + + *target = &v + + return nil +} diff --git a/sinks/httpSink.go b/sinks/httpSink.go index e4b21f3..0cdfcec 100644 --- a/sinks/httpSink.go +++ b/sinks/httpSink.go @@ -36,6 +36,16 @@ type HttpSinkConfig struct { Password string `json:"password"` useBasicAuth bool + // Alternative sources for the JWT and the basic authentication credentials + // above, so that they need not be stored in the configuration file. See + // resolveSecrets. + JWTEnv string `json:"jwt_env,omitempty"` + JWTFile string `json:"jwt_file,omitempty"` + UsernameEnv string `json:"username_env,omitempty"` + UsernameFile string `json:"username_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` + // time limit for requests made by the http client Timeout string `json:"timeout,omitempty"` timeout time.Duration @@ -232,6 +242,18 @@ func NewHttpSink(name string, config json.RawMessage) (Sink, error) { return nil, errors.New("`url` config option is required for HTTP sink") } + // Resolve credentials before the basic authentication check below, so that + // a username and password supplied from the environment or a secret file + // count as configured. + if err := resolveSecrets( + secretRef{"jwt", &s.config.JWT, s.config.JWTEnv, s.config.JWTFile}, + secretRef{"username", &s.config.Username, s.config.UsernameEnv, s.config.UsernameFile}, + secretRef{"password", &s.config.Password, s.config.PasswordEnv, s.config.PasswordFile}, + ); err != nil { + cclog.ComponentError(s.name, err.Error()) + return nil, err + } + // Check basic authentication config if len(s.config.Username) > 0 || len(s.config.Password) > 0 { s.config.useBasicAuth = true diff --git a/sinks/httpSink.md b/sinks/httpSink.md index 0287531..299db97 100644 --- a/sinks/httpSink.md +++ b/sinks/httpSink.md @@ -53,3 +53,20 @@ The `http` sink uses POST requests to a HTTP server to submit the metrics in the ### Using `http` sink for communication with cc-metric-store The cc-metric-store only accepts metrics with a timestamp precision in seconds, so it is required to use `"precision": "s"`. + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`jwt`, `username`, `password` has two sibling keys that select an alternative source: + +- `jwt_env`: name of an environment variable holding the value +- `jwt_file`: path to a file holding the value +- `username_env`: name of an environment variable holding the value +- `username_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/sinks/influxAsyncSink.go b/sinks/influxAsyncSink.go index 322cc24..6595420 100644 --- a/sinks/influxAsyncSink.go +++ b/sinks/influxAsyncSink.go @@ -33,6 +33,12 @@ type InfluxAsyncSinkConfig struct { Password string `json:"password,omitempty"` Organization string `json:"organization,omitempty"` SSL bool `json:"ssl,omitempty"` + // Alternative sources for the credentials above, so that they need not be + // stored in the configuration file. See resolveSecrets. + UserEnv string `json:"user_env,omitempty"` + UserFile string `json:"user_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` // Maximum number of points sent to server in single request. Default 5000 BatchSize uint `json:"batch_size,omitempty"` // Interval, in ms, in which is buffer flushed if it has not been already written (by reaching batch size) . Default 1000ms @@ -214,6 +220,16 @@ func NewInfluxAsyncSink(name string, config json.RawMessage) (Sink, error) { return nil, err } } + // Resolve credentials before the required-field checks below, since the + // password may be supplied from the environment or a secret file rather + // than from the configuration file. + if err := resolveSecrets( + secretRef{"user", &s.config.User, s.config.UserEnv, s.config.UserFile}, + secretRef{"password", &s.config.Password, s.config.PasswordEnv, s.config.PasswordFile}, + ); err != nil { + cclog.ComponentError(s.name, err.Error()) + return nil, err + } if len(s.config.Port) == 0 { return nil, errors.New("missing port configuration required by InfluxSink") } diff --git a/sinks/influxAsyncSink.md b/sinks/influxAsyncSink.md index df184d8..0149239 100644 --- a/sinks/influxAsyncSink.md +++ b/sinks/influxAsyncSink.md @@ -64,3 +64,18 @@ For information about the calculation of the retry interval settings, see [offic ### Using `influxasync` sink for communication with cc-metric-store The cc-metric-store only accepts metrics with a timestamp precision in seconds, so it is required to use `"precision": "s"`. + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`user`, `password` has two sibling keys that select an alternative source: + +- `user_env`: name of an environment variable holding the value +- `user_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/sinks/influxSink.go b/sinks/influxSink.go index 04d9c8f..e48d41e 100644 --- a/sinks/influxSink.go +++ b/sinks/influxSink.go @@ -38,6 +38,12 @@ type InfluxSink struct { Password string `json:"password,omitempty"` Organization string `json:"organization,omitempty"` SSL bool `json:"ssl,omitempty"` + // Alternative sources for the credentials above, so that they need not + // be stored in the configuration file. See resolveSecrets. + UserEnv string `json:"user_env,omitempty"` + UserFile string `json:"user_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` // Maximum number of points sent to server in single request. // Default: 1000 BatchSize int `json:"batch_size,omitempty"` @@ -455,6 +461,17 @@ func NewInfluxSink(name string, config json.RawMessage) (Sink, error) { } } + // Resolve credentials before the required-field checks below, since the + // password may be supplied from the environment or a secret file rather + // than from the configuration file. + if err := resolveSecrets( + secretRef{"user", &s.config.User, s.config.UserEnv, s.config.UserFile}, + secretRef{"password", &s.config.Password, s.config.PasswordEnv, s.config.PasswordFile}, + ); err != nil { + cclog.ComponentError(s.name, err.Error()) + return nil, err + } + if len(s.config.Host) == 0 { return s, errors.New("missing host configuration required by InfluxSink") } diff --git a/sinks/influxSink.md b/sinks/influxSink.md index 251fd10..715c94c 100644 --- a/sinks/influxSink.md +++ b/sinks/influxSink.md @@ -67,3 +67,18 @@ Influx client options: ### Using `influxdb` sink for communication with cc-metric-store The cc-metric-store only accepts metrics with a timestamp precision in seconds, so it is required to use `"precision": "s"`. + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`user`, `password` has two sibling keys that select an alternative source: + +- `user_env`: name of an environment variable holding the value +- `user_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/sinks/natsSink.go b/sinks/natsSink.go index b1dfdc4..ce76b41 100644 --- a/sinks/natsSink.go +++ b/sinks/natsSink.go @@ -25,14 +25,20 @@ import ( type NatsSinkConfig struct { defaultSinkConfig - Host string `json:"host,omitempty"` - Port string `json:"port,omitempty"` - Subject string `json:"subject,omitempty"` - User string `json:"user,omitempty"` - Password string `json:"password,omitempty"` - FlushDelay string `json:"flush_delay,omitempty"` - flushDelay time.Duration - NkeyFile string `json:"nkey_file,omitempty"` + Host string `json:"host,omitempty"` + Port string `json:"port,omitempty"` + Subject string `json:"subject,omitempty"` + User string `json:"user,omitempty"` + Password string `json:"password,omitempty"` + // Alternative sources for the credentials above, so that they need not be + // stored in the configuration file. See resolveSecrets. + UserEnv string `json:"user_env,omitempty"` + UserFile string `json:"user_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` + FlushDelay string `json:"flush_delay,omitempty"` + flushDelay time.Duration + NkeyFile string `json:"nkey_file,omitempty"` // Timestamp precision Precision string `json:"precision,omitempty"` } @@ -178,6 +184,13 @@ func NewNatsSink(name string, config json.RawMessage) (Sink, error) { len(s.config.Subject) == 0 { return nil, errors.New("not all configuration variables set required by NatsSink") } + if err := resolveSecrets( + secretRef{"user", &s.config.User, s.config.UserEnv, s.config.UserFile}, + secretRef{"password", &s.config.Password, s.config.PasswordEnv, s.config.PasswordFile}, + ); err != nil { + cclog.ComponentError(s.name, err.Error()) + return nil, err + } // Create a new message processor p, err := mp.NewMessageProcessor() if err != nil { diff --git a/sinks/natsSink.md b/sinks/natsSink.md index a2dde62..4f85e66 100644 --- a/sinks/natsSink.md +++ b/sinks/natsSink.md @@ -51,3 +51,18 @@ The `nats` sink publishes all metrics into a NATS network. The publishing key is ### Using `nats` sink for communication with cc-metric-store The cc-metric-store only accepts metrics with a timestamp precision in seconds, so it is required to use `"precision": "s"`. + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`user`, `password` has two sibling keys that select an alternative source: + +- `user_env`: name of an environment variable holding the value +- `user_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/sinks/questDbSink.go b/sinks/questDbSink.go index 3a9157d..1c7dae3 100644 --- a/sinks/questDbSink.go +++ b/sinks/questDbSink.go @@ -37,6 +37,14 @@ type QuestDBSinkConfig struct { Password string `json:"password,omitempty"` // Authentication with bearer token in HTTP header BearerToken string `json:"bearer_token,omitempty"` + // Alternative sources for the credentials above, so that they need not be + // stored in the configuration file. See resolveSecrets. + UsernameEnv string `json:"username_env,omitempty"` + UsernameFile string `json:"username_file,omitempty"` + PasswordEnv string `json:"password_env,omitempty"` + PasswordFile string `json:"password_file,omitempty"` + BearerTokenEnv string `json:"bearer_token_env,omitempty"` + BearerTokenFile string `json:"bearer_token_file,omitempty"` // Auto flush configuration // Interval at which the sender automatically flushes its buffer AutoFlushInterval string `json:"auto_flush_interval,omitempty"` @@ -138,6 +146,18 @@ func NewQuestDBSink(name string, config json.RawMessage) (Sink, error) { } } + // Resolve credentials before the authentication checks below, so that + // values supplied from the environment or a secret file count as + // configured. + if err := resolveSecrets( + secretRef{"username", &s.config.Username, s.config.UsernameEnv, s.config.UsernameFile}, + secretRef{"password", &s.config.Password, s.config.PasswordEnv, s.config.PasswordFile}, + secretRef{"bearer_token", &s.config.BearerToken, s.config.BearerTokenEnv, s.config.BearerTokenFile}, + ); err != nil { + cclog.ComponentError(s.name, err.Error()) + return nil, err + } + // Initialize and configure the message processor p, err := mp.NewMessageProcessor() if err != nil { diff --git a/sinks/questDbSink.md b/sinks/questDbSink.md index 4e4d25a..ce1ff04 100644 --- a/sinks/questDbSink.md +++ b/sinks/questDbSink.md @@ -41,3 +41,20 @@ The `questdb` sink sends metrics to the timeseries database QuestDB - `auto_flush_interval`: interval at which the sender automatically flushes its buffer (default `5s`) - `auto_flush_rows`: number of rows after which the sender automatically flushes its buffer - `use_tls`: Use https instead of http transport protocol + +### Credentials from the environment + +The credentials above need not be stored in the configuration file. Each of +`username`, `password`, `bearer_token` has two sibling keys that select an alternative source: + +- `username_env`: name of an environment variable holding the value +- `username_file`: path to a file holding the value +- `password_env`: name of an environment variable holding the value +- `password_file`: path to a file holding the value +- `bearer_token_env`: name of an environment variable holding the value +- `bearer_token_file`: path to a file holding the value + +The environment variable takes precedence over the file, and the file over the +inline value. A named file that cannot be read is an error rather than a silent +fallback, so a stale credential is never used in its place. See the +[`util`](../util/README.md) package for the full rules. diff --git a/sinks/secret.go b/sinks/secret.go new file mode 100644 index 0000000..a56751f --- /dev/null +++ b/sinks/secret.go @@ -0,0 +1,43 @@ +// Copyright (C) NHR@FAU, University Erlangen-Nuremberg. +// All rights reserved. This file is part of cc-lib. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package sinks + +import ( + "fmt" + + "github.com/ClusterCockpit/cc-lib/v2/util" +) + +// secretRef names the three places one credential of a sink instance may come +// from. Value points at the inline configuration value and receives the +// resolved secret. +// +// Sinks are configured as a map of named instances, so no fixed environment +// variable name can address the credential of one particular sink. Each +// instance therefore names its own sources, through the sibling configuration +// keys "_env" and "_file". +type secretRef struct { + Key string // configuration key, used in error messages only + Value *string + EnvName string + FilePath string +} + +// resolveSecrets resolves each secret in place, in the order given. A secret +// whose named file cannot be read is an error rather than a silent fallback to +// the inline value, so a sink never connects with a stale credential. The +// resolved values are never logged. +func resolveSecrets(secrets ...secretRef) error { + for _, s := range secrets { + v, err := util.SecretFromConfig(*s.Value, s.EnvName, s.FilePath) + if err != nil { + return fmt.Errorf("resolving %q: %w", s.Key, err) + } + *s.Value = v + } + + return nil +} diff --git a/util/README.md b/util/README.md index 186697d..dff5442 100644 --- a/util/README.md +++ b/util/README.md @@ -9,29 +9,77 @@ This package contains utilities for: - **File compression** - Gzip compression and decompression - **File/directory operations** - Copying files and directories - **Disk usage** - Calculating directory size -- **Custom types** - Float type with JSON NaN support, Selector types +- **Custom types** - Selector types - **File system watcher** - Event-based file system monitoring - **Statistics** - Basic statistical functions (mean, median, min, max) +- **Secrets** - Resolving secrets from environment variables or secret files + +The `Float` type with JSON NaN support is in the [`schema`](../schema) package, +not here. ## Key Features -### Float Type with NaN Support +### Secrets from the Environment + +A secret should not have to live in a configuration file. `SecretFromEnv` +resolves one from three sources, in this order of precedence: -Go's standard JSON encoder doesn't support NaN values (see [golang/go#3480](https://github.com/golang/go/issues/3480)). This package provides a `Float` type that properly handles NaN values in JSON by converting them to/from `null`. +1. the environment variable `$VAR`, if set and non-empty +2. the contents of the file named by `$VAR_FILE` +3. the value read from the configuration file ```go -import "github.com/ClusterCockpit/cc-lib/v2/util" +// Resolves $NATS_PASSWORD, else the file named by $NATS_PASSWORD_FILE, +// else the value from the config file. +password, err := util.SecretFromEnv("CC_NATS_PASSWORD", cfg.Password) +if err != nil { + return err +} +``` -// Create a Float value -f := util.Float(3.14) +The second step is what makes the standard secret-mounting mechanisms usable: -// Use NaN to represent missing data -missing := util.NaN +```bash +# Docker / docker compose +docker run -e CC_NATS_PASSWORD_FILE=/run/secrets/nats-pw ... + +# Kubernetes: mount the secret, then name the path +# env: +# - name: CC_NATS_PASSWORD_FILE +# value: /etc/secrets/nats-pw + +# systemd +[Service] +LoadCredential=nats-pw:/etc/cc/nats-pw +Environment=CC_NATS_PASSWORD_FILE=%d/nats-pw +``` + +For a repeated configuration section — one sink, one receiver — no fixed +variable name can address a single instance, so the instance names its own +sources through sibling configuration keys and resolves them with +`SecretFromConfig`: -// JSON marshaling - NaN becomes null -data, _ := json.Marshal(missing) // Returns: null +```go +// {"password": "...", "password_env": "...", "password_file": "..."} +password, err := util.SecretFromConfig(cfg.Password, cfg.PasswordEnv, cfg.PasswordFile) ``` +Rules that apply to both functions: + +- An empty environment variable counts as unset. +- File contents are trimmed of surrounding whitespace, so a secret file may end + in a newline. A secret with significant leading or trailing whitespace cannot + be supplied through a file. +- A secret file that cannot be read, or that holds no non-whitespace + characters, is an **error** rather than a silent fallback to the configured + value. An operator who names a file intends it to win, so falling back would + quietly start the process with a stale credential. +- Neither function ever logs a resolved secret. Errors carry only the path. + +Environment variables that cc-lib itself reads are prefixed `CC_`, so they +cannot collide with a variable already present in the environment of the +application linking cc-lib. + ### File Operations ```go diff --git a/util/secret.go b/util/secret.go new file mode 100644 index 0000000..1673b98 --- /dev/null +++ b/util/secret.go @@ -0,0 +1,115 @@ +// Copyright (C) NHR@FAU, University Erlangen-Nuremberg. +// All rights reserved. This file is part of cc-lib. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package util + +import ( + "fmt" + "os" + "strings" + + cclog "github.com/ClusterCockpit/cc-lib/v2/ccLogger" +) + +// EnvFileSuffix is appended to an environment variable name to obtain the name +// of the variable holding the path of a file that contains the secret. +const EnvFileSuffix = "_FILE" + +// SecretFromEnv resolves a secret from the environment or the configuration, +// in this order of precedence: +// +// 1. the environment variable envVar, if set and non-empty +// 2. the contents of the file named by the environment variable +// envVar+EnvFileSuffix, if that variable is set and non-empty +// 3. configValue, the value read from the configuration file +// +// Step 2 lets a deployment inject a secret from a Docker or Kubernetes secret +// mount, or from systemd LoadCredential, without ever placing it in the process +// environment or in a configuration file. +// +// An empty environment variable counts as unset. File contents are trimmed of +// leading and trailing whitespace so that secret files may end in a newline; a +// secret with significant leading or trailing whitespace can therefore not be +// supplied through a file. +// +// An error is returned only when envVar+EnvFileSuffix is set but the file +// cannot be read or holds no non-whitespace characters. An operator who sets +// that variable intends the file to win, so falling back to configValue there +// would silently start the process with a stale credential. +// +// envVar must not itself end in EnvFileSuffix, as it would then shadow the file +// variable of another secret. An empty envVar disables both env sources and +// returns configValue unchanged. +func SecretFromEnv(envVar, configValue string) (string, error) { + if envVar == "" { + return configValue, nil + } + + if v := os.Getenv(envVar); v != "" { + cclog.Debugf("using secret from environment variable %s", envVar) + return v, nil + } + + fileVar := envVar + EnvFileSuffix + if path := os.Getenv(fileVar); path != "" { + secret, err := readSecretFile(path) + if err != nil { + return "", fmt.Errorf("%s: %w", fileVar, err) + } + cclog.Debugf("using secret from the file named by %s", fileVar) + return secret, nil + } + + return configValue, nil +} + +// SecretFromConfig resolves a secret for one instance of a repeated +// configuration section (one sink, one receiver), where no fixed environment +// variable name exists. The instance names the sources itself, via sibling +// configuration keys. Precedence: +// +// 1. the environment variable envName, if envName is non-empty and that +// variable is set and non-empty +// 2. the contents of filePath, if filePath is non-empty +// 3. value, the secret configured inline +// +// An empty envName or filePath means that source is not configured. As in +// SecretFromEnv, file contents are whitespace-trimmed, and an unreadable or +// effectively empty file is an error rather than a silent fallback. +func SecretFromConfig(value, envName, filePath string) (string, error) { + if envName != "" { + if v := os.Getenv(envName); v != "" { + cclog.Debugf("using secret from environment variable %s", envName) + return v, nil + } + } + + if filePath != "" { + secret, err := readSecretFile(filePath) + if err != nil { + return "", err + } + cclog.Debugf("using secret from the file %s", filePath) + return secret, nil + } + + return value, nil +} + +// readSecretFile reads and trims a secret file. It never includes the file's +// contents in an error, only its path. +func readSecretFile(path string) (string, error) { + buf, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("read secret file: %w", err) + } + + secret := strings.TrimSpace(string(buf)) + if secret == "" { + return "", fmt.Errorf("secret file %s contains no non-whitespace characters", path) + } + + return secret, nil +} diff --git a/util/util_test.go b/util/util_test.go index d69809c..e5159c7 100644 --- a/util/util_test.go +++ b/util/util_test.go @@ -401,3 +401,194 @@ func TestMedian(t *testing.T) { t.Error("expected NaN for empty slice") } } + +func TestSecretFromEnv_Precedence(t *testing.T) { + tests := []struct { + name string + envSet bool + envValue string + fileSet bool + fileContent string + configValue string + want string + }{ + { + name: "env wins over file and config", envSet: true, envValue: "from-env", + fileSet: true, fileContent: "from-file", configValue: "from-config", want: "from-env", + }, + { + name: "empty env falls through to file", envSet: true, envValue: "", + fileSet: true, fileContent: "from-file", configValue: "from-config", want: "from-file", + }, + { + name: "empty env without file falls through to config", envSet: true, envValue: "", + configValue: "from-config", want: "from-config", + }, + { + name: "file wins over config", fileSet: true, fileContent: "from-file", + configValue: "from-config", want: "from-file", + }, + { + name: "file contents are trimmed", fileSet: true, fileContent: " s3cret\n", + configValue: "from-config", want: "s3cret", + }, + { + name: "nothing set returns config", configValue: "from-config", want: "from-config", + }, + { + name: "nothing set with empty config returns empty", want: "", + }, + } + + for i, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // A per-subtest name keeps an unset case genuinely unset, whatever + // the surrounding environment holds. + envVar := fmt.Sprintf("CC_LIB_TEST_SECRET_%d", i) + + if tt.envSet { + t.Setenv(envVar, tt.envValue) + } + if tt.fileSet { + path := filepath.Join(t.TempDir(), "secret") + if err := os.WriteFile(path, []byte(tt.fileContent), 0o600); err != nil { + t.Fatalf("writing secret file: %v", err) + } + t.Setenv(envVar+util.EnvFileSuffix, path) + } + + got, err := util.SecretFromEnv(envVar, tt.configValue) + if err != nil { + t.Fatalf("SecretFromEnv failed: %v", err) + } + if got != tt.want { + t.Errorf("expected %q, got %q", tt.want, got) + } + }) + } +} + +func TestSecretFromEnv_UnreadableFile(t *testing.T) { + dir := t.TempDir() + + // A path that does not exist. + t.Setenv("CC_LIB_TEST_MISSING"+util.EnvFileSuffix, filepath.Join(dir, "absent")) + if _, err := util.SecretFromEnv("CC_LIB_TEST_MISSING", "from-config"); err == nil { + t.Error("expected an error for a nonexistent secret file, got nil") + } + + // A directory rather than a file. + t.Setenv("CC_LIB_TEST_ISDIR"+util.EnvFileSuffix, dir) + if _, err := util.SecretFromEnv("CC_LIB_TEST_ISDIR", "from-config"); err == nil { + t.Error("expected an error for a directory secret file, got nil") + } +} + +func TestSecretFromEnv_EmptyFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "secret") + if err := os.WriteFile(path, []byte("\n \t\n"), 0o600); err != nil { + t.Fatalf("writing secret file: %v", err) + } + + t.Setenv("CC_LIB_TEST_EMPTYFILE"+util.EnvFileSuffix, path) + if _, err := util.SecretFromEnv("CC_LIB_TEST_EMPTYFILE", "from-config"); err == nil { + t.Error("expected an error for a whitespace-only secret file, got nil") + } +} + +func TestSecretFromEnv_NoVarName(t *testing.T) { + got, err := util.SecretFromEnv("", "from-config") + if err != nil { + t.Fatalf("SecretFromEnv failed: %v", err) + } + if got != "from-config" { + t.Errorf("expected \"from-config\", got %q", got) + } +} + +func TestSecretFromConfig_Precedence(t *testing.T) { + tests := []struct { + name string + useEnvName bool + envSet bool + envValue string + fileSet bool + fileContent string + value string + want string + }{ + { + name: "env wins over file and value", useEnvName: true, envSet: true, envValue: "from-env", + fileSet: true, fileContent: "from-file", value: "inline", want: "from-env", + }, + { + name: "empty env falls through to file", useEnvName: true, envSet: true, envValue: "", + fileSet: true, fileContent: "from-file", value: "inline", want: "from-file", + }, + { + name: "unset env falls through to value", useEnvName: true, value: "inline", want: "inline", + }, + { + name: "file wins over value", fileSet: true, fileContent: "from-file", + value: "inline", want: "from-file", + }, + { + name: "file contents are trimmed", fileSet: true, fileContent: "s3cret\n", + value: "inline", want: "s3cret", + }, + { + name: "no env name and no file returns value", value: "inline", want: "inline", + }, + } + + for i, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + envName := "" + if tt.useEnvName { + envName = fmt.Sprintf("CC_LIB_TEST_INSTANCE_SECRET_%d", i) + if tt.envSet { + t.Setenv(envName, tt.envValue) + } + } + + filePath := "" + if tt.fileSet { + filePath = filepath.Join(t.TempDir(), "secret") + if err := os.WriteFile(filePath, []byte(tt.fileContent), 0o600); err != nil { + t.Fatalf("writing secret file: %v", err) + } + } + + got, err := util.SecretFromConfig(tt.value, envName, filePath) + if err != nil { + t.Fatalf("SecretFromConfig failed: %v", err) + } + if got != tt.want { + t.Errorf("expected %q, got %q", tt.want, got) + } + }) + } +} + +func TestSecretFromConfig_UnreadableFile(t *testing.T) { + dir := t.TempDir() + + if _, err := util.SecretFromConfig("inline", "", filepath.Join(dir, "absent")); err == nil { + t.Error("expected an error for a nonexistent secret file, got nil") + } + + if _, err := util.SecretFromConfig("inline", "", dir); err == nil { + t.Error("expected an error for a directory secret file, got nil") + } +} + +func TestSecretFromConfig_EmptyFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "secret") + if err := os.WriteFile(path, []byte(" "), 0o600); err != nil { + t.Fatalf("writing secret file: %v", err) + } + + if _, err := util.SecretFromConfig("inline", "", path); err == nil { + t.Error("expected an error for a whitespace-only secret file, got nil") + } +}