From 96304b68706081f4e59bc406ecafa05f8ad0b441 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 21 Aug 2026 22:32:47 -0700 Subject: [PATCH] S3: source config credentials from the environment, and let the chart point at an existing secret (#10868) * s3: resolve ${VAR} in static config credentials from the environment A deployment that keeps its S3 keys in a secret store had no way to hand them to the gateway: -config takes a file, so the keys had to be written into that file. Let a key in the static config name an environment variable instead, and drop any credential whose reference stays unset so the placeholder never becomes a usable key. * helm: source the generated s3 identities from an existing secret The only way to reuse credentials that already live in a Secret was to hand-author the whole seaweedfs_s3_config JSON, since the literal keys in values.yaml end up in git and a lookup-based keyRef renders empty under helm template and Argo CD. Let s3.credentials.admin/read name a Secret and its keys instead: the generated config references them as ${VAR} and the gateway resolves them from the environment, so nothing is read from the cluster at render time. * s3: treat an empty environment value as an unresolved credential reference A secret store can hand over a key that exists but is blank. Resolving it would leave an access key whose signing secret is empty, so count it as unresolved and drop the credential. * helm: render the s3 secret when only the all-in-one auth flag is set The all-in-one deployment mounts the s3 secret whenever any of the three enableAuth flags is set, but the secret itself only rendered for the s3 and filer flags, so allInOne.s3.enableAuth on its own left the pod waiting on a secret nothing creates. * helm ci: check the credential wiring on every workload that mounts it The render check only looked at the standalone s3 deployment and only at one of the four variables, so a helper that bound a variable to the wrong secret key would still pass. * helm: create the all-in-one s3 secret for every flag that mounts it The all-in-one pod mounts the secret on any of the three enableAuth flags, so keying its creation off allInOne.s3.enableAuth alone still left filer.s3.enableAuth without filer.s3.enabled pointing at a secret nothing creates. Mirror the deployment's own condition instead, and check each flag renders both the mount and the secret. * s3: reject a malformed credential reference instead of keying on it A typo such as ${MY-VAR} matches no substitution, so it survived expansion and the placeholder itself became the access key the gateway accepted. Require every ${ in a static credential to open a well-formed reference. --- .github/workflows/helm_ci.yml | 42 ++++++ k8s/charts/seaweedfs/README.md | 26 ++++ .../all-in-one/all-in-one-deployment.yaml | 3 + .../templates/filer/filer-statefulset.yaml | 3 + .../seaweedfs/templates/s3/s3-deployment.yaml | 3 + .../seaweedfs/templates/s3/s3-secret.yaml | 21 ++- .../seaweedfs/templates/shared/_helpers.tpl | 33 ++++ k8s/charts/seaweedfs/values.yaml | 11 ++ weed/s3api/auth_credentials.go | 55 +++++++ .../auth_credentials_static_config_test.go | 142 ++++++++++++++++++ 10 files changed, 336 insertions(+), 3 deletions(-) diff --git a/.github/workflows/helm_ci.yml b/.github/workflows/helm_ci.yml index 554f85fbc..30d9ba069 100644 --- a/.github/workflows/helm_ci.yml +++ b/.github/workflows/helm_ci.yml @@ -58,10 +58,52 @@ jobs: grep -q "kind: Deployment" /tmp/s3.yaml && grep -q "seaweedfs-s3" /tmp/s3.yaml echo "S3 deployment renders correctly" + echo "=== Testing S3 credentials from an existing secret ===" + credential_args=( + --set s3.credentials.admin.existingSecret=minio-root + --set s3.credentials.admin.accessKeyKey=root-user + --set s3.credentials.admin.secretKeyKey=root-password + --set s3.credentials.read.existingSecret=minio-root + ) + for workload in \ + "s3.enabled=true,s3.enableAuth=true" \ + "filer.s3.enabled=true,filer.s3.enableAuth=true" \ + "allInOne.enabled=true,allInOne.s3.enabled=true,allInOne.s3.enableAuth=true" + do + helm template test $CHART_DIR --set "$workload" "${credential_args[@]}" > /tmp/s3-existing-credentials.yaml + # The identities file names the variables, and each is bound to the key it was pointed at. + grep -q 'accessKey":"${SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID}' /tmp/s3-existing-credentials.yaml + grep -q 'secretKey":"${SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY}' /tmp/s3-existing-credentials.yaml + grep -q 'accessKey":"${SEAWEEDFS_S3_READ_ACCESS_KEY_ID}' /tmp/s3-existing-credentials.yaml + for pair in \ + "SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID root-user" \ + "SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY root-password" \ + "SEAWEEDFS_S3_READ_ACCESS_KEY_ID read_access_key_id" \ + "SEAWEEDFS_S3_READ_SECRET_ACCESS_KEY read_secret_access_key" + do + set -- $pair + grep -A 4 -- "- name: $1\$" /tmp/s3-existing-credentials.yaml | grep -q "name: \"minio-root\"" + grep -A 4 -- "- name: $1\$" /tmp/s3-existing-credentials.yaml | grep -q "key: \"$2\"" + done + # The keys stay in the user's secret rather than being copied into the chart's. + ! grep -qE "^ (admin|read)_(access_key_id|secret_access_key):" /tmp/s3-existing-credentials.yaml + echo "S3 credentials reference the existing secret for $workload" + done + echo "=== Testing with all-in-one mode ===" helm template test $CHART_DIR --set allInOne.enabled=true > /tmp/allinone.yaml grep -q "seaweedfs-all-in-one" /tmp/allinone.yaml echo "All-in-one deployment renders correctly" + + echo "=== Testing the all-in-one s3 secret is created by every flag that mounts it ===" + for auth in allInOne.s3.enableAuth s3.enableAuth filer.s3.enableAuth; do + helm template test $CHART_DIR \ + --set allInOne.enabled=true --set allInOne.s3.enabled=true --set "$auth=true" \ + > /tmp/allinone-s3-auth.yaml + grep -q "secretName: test-seaweedfs-s3-secret" /tmp/allinone-s3-auth.yaml + grep -q "name: test-seaweedfs-s3-secret" /tmp/allinone-s3-auth.yaml + echo "All-in-one s3 secret renders for $auth" + done echo "=== Testing with security enabled ===" helm template test $CHART_DIR --set global.seaweedfs.enableSecurity=true > /tmp/security.yaml diff --git a/k8s/charts/seaweedfs/README.md b/k8s/charts/seaweedfs/README.md index 7edde6754..2db8cf31a 100644 --- a/k8s/charts/seaweedfs/README.md +++ b/k8s/charts/seaweedfs/README.md @@ -289,6 +289,32 @@ stringData: seaweedfs_s3_config: '{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":"snu8yoP6QAlY0ne4","secretKey":"PNzBcmeLNEdR0oviwm04NQAicOrDH1Km"}],"actions":["Admin","Read","Write"]},{"name":"anvReadOnly","credentials":[{"accessKey":"SCigFee6c5lbi04A","secretKey":"kgFhbT38R8WUYVtiFQ1OiSVOrYr3NKku"}],"actions":["Read"]}]}' ``` +#### Source S3 credentials from an existing Secret + +To keep the keys out of `values.yaml` while still letting the chart generate the +identities file, point an identity at an existing Secret: + +```yaml +s3: + enabled: true + enableAuth: true + credentials: + admin: + existingSecret: minio-root + accessKeyKey: root-user + secretKeyKey: root-password +``` + +`accessKeyKey` and `secretKeyKey` default to the chart's own key names +(`admin_access_key_id`, `admin_secret_access_key`, and the `read_` pair). The +generated `seaweedfs_s3_config` references the keys as `${SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID}` +and the gateway resolves them from the environment, which the chart wires up +from the Secret. Nothing is read from the cluster at render time, so +`helm template`, `--dry-run` and an Argo CD diff all render what an install +applies. Rotating a key in the Secret takes effect on the next pod restart, as +with any other environment variable. The COSI driver parses the config itself +and does not resolve these references. + ## Admin Component The admin component provides a modern web-based administration interface for managing SeaweedFS clusters. It includes: diff --git a/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml b/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml index 00074e7ef..9202ae8cd 100644 --- a/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml +++ b/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml @@ -84,6 +84,9 @@ spec: imagePullPolicy: {{ default "IfNotPresent" .Values.global.seaweedfs.imagePullPolicy }} env: {{- include "seaweedfs.licenseEnv" . | nindent 12 }} + {{- if and .Values.allInOne.s3.enabled (or .Values.allInOne.s3.enableAuth .Values.s3.enableAuth .Values.filer.s3.enableAuth) }} + {{- include "seaweedfs.s3.credentialEnv" . | nindent 12 }} + {{- end }} {{- /* Determine default cluster alias and the corresponding env var keys to avoid conflicts */}} {{- $mergedExtraEnvironmentVars := dict }} {{- include "seaweedfs.mergeExtraEnvironmentVars" (dict "global" .Values.global.seaweedfs "component" .Values.allInOne "target" $mergedExtraEnvironmentVars) }} diff --git a/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml b/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml index 447f8ffb7..b29772870 100644 --- a/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml +++ b/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml @@ -118,6 +118,9 @@ spec: optional: true - name: SEAWEEDFS_FULLNAME value: "{{ include "seaweedfs.fullname" . }}" + {{- if and .Values.filer.s3.enabled .Values.filer.s3.enableAuth }} + {{- include "seaweedfs.s3.credentialEnv" . | nindent 12 }} + {{- end }} {{- $mergedExtraEnvironmentVars := dict }} {{- include "seaweedfs.mergeExtraEnvironmentVars" (dict "global" .Values.global.seaweedfs "component" .Values.filer "target" $mergedExtraEnvironmentVars) }} {{- range $key := keys $mergedExtraEnvironmentVars | sortAlpha }} diff --git a/k8s/charts/seaweedfs/templates/s3/s3-deployment.yaml b/k8s/charts/seaweedfs/templates/s3/s3-deployment.yaml index 299234cf6..1fff9dbc4 100644 --- a/k8s/charts/seaweedfs/templates/s3/s3-deployment.yaml +++ b/k8s/charts/seaweedfs/templates/s3/s3-deployment.yaml @@ -94,6 +94,9 @@ spec: fieldPath: metadata.namespace - name: SEAWEEDFS_FULLNAME value: "{{ include "seaweedfs.fullname" . }}" + {{- if .Values.s3.enableAuth }} + {{- include "seaweedfs.s3.credentialEnv" . | nindent 12 }} + {{- end }} {{- $mergedExtraEnvironmentVars := dict }} {{- include "seaweedfs.mergeExtraEnvironmentVars" (dict "global" .Values.global.seaweedfs "component" .Values.s3 "target" $mergedExtraEnvironmentVars) }} {{- range $key := keys $mergedExtraEnvironmentVars | sortAlpha }} diff --git a/k8s/charts/seaweedfs/templates/s3/s3-secret.yaml b/k8s/charts/seaweedfs/templates/s3/s3-secret.yaml index 5a2ab6774..2c1d87da5 100644 --- a/k8s/charts/seaweedfs/templates/s3/s3-secret.yaml +++ b/k8s/charts/seaweedfs/templates/s3/s3-secret.yaml @@ -1,4 +1,7 @@ -{{- if or (and (or .Values.s3.enabled .Values.allInOne.enabled) .Values.s3.enableAuth (not .Values.s3.existingConfigSecret)) (and .Values.filer.s3.enabled .Values.filer.s3.enableAuth (not .Values.filer.s3.existingConfigSecret)) }} +{{- /* Mirrors the condition the all-in-one deployment mounts this secret under, + so the flags that make it mount are exactly the flags that create it. */}} +{{- $allInOneAuth := and .Values.allInOne.enabled .Values.allInOne.s3.enabled (or .Values.allInOne.s3.enableAuth .Values.s3.enableAuth .Values.filer.s3.enableAuth) (not (or .Values.allInOne.s3.existingConfigSecret .Values.s3.existingConfigSecret .Values.filer.s3.existingConfigSecret)) }} +{{- if or (and (or .Values.s3.enabled .Values.allInOne.enabled) .Values.s3.enableAuth (not .Values.s3.existingConfigSecret)) (and .Values.filer.s3.enabled .Values.filer.s3.enableAuth (not .Values.filer.s3.existingConfigSecret)) $allInOneAuth }} {{- $secretName := printf "%s-s3-secret" (include "seaweedfs.fullname" .) }} {{- $legacySecretName := "seaweedfs-s3-secret" }} {{- $lookupName := $secretName }} @@ -14,14 +17,20 @@ {{- $adminCreds := $creds.admin | default dict -}} {{- $access_key_admin := $adminCreds.accessKey -}} {{- $secret_key_admin := $adminCreds.secretKey -}} -{{- if not (and $access_key_admin $secret_key_admin) -}} +{{- if $adminCreds.existingSecret -}} + {{- $access_key_admin = printf "${%s}" (include "seaweedfs.s3.credentialEnvName" (list "admin" "accessKey")) -}} + {{- $secret_key_admin = printf "${%s}" (include "seaweedfs.s3.credentialEnvName" (list "admin" "secretKey")) -}} +{{- else if not (and $access_key_admin $secret_key_admin) -}} {{- $access_key_admin = include "seaweedfs.getOrGeneratePassword" (dict "namespace" .Release.Namespace "secretName" $secretName "key" "admin_access_key_id" "length" 20 "existingSecret" (ternary $existingSecret nil $reuse)) -}} {{- $secret_key_admin = include "seaweedfs.getOrGeneratePassword" (dict "namespace" .Release.Namespace "secretName" $secretName "key" "admin_secret_access_key" "length" 40 "existingSecret" (ternary $existingSecret nil $reuse)) -}} {{- end -}} {{- $readCreds := $creds.read | default dict -}} {{- $access_key_read := $readCreds.accessKey -}} {{- $secret_key_read := $readCreds.secretKey -}} -{{- if not (and $access_key_read $secret_key_read) -}} +{{- if $readCreds.existingSecret -}} + {{- $access_key_read = printf "${%s}" (include "seaweedfs.s3.credentialEnvName" (list "read" "accessKey")) -}} + {{- $secret_key_read = printf "${%s}" (include "seaweedfs.s3.credentialEnvName" (list "read" "secretKey")) -}} +{{- else if not (and $access_key_read $secret_key_read) -}} {{- $access_key_read = include "seaweedfs.getOrGeneratePassword" (dict "namespace" .Release.Namespace "secretName" $secretName "key" "read_access_key_id" "length" 20 "existingSecret" (ternary $existingSecret nil $reuse)) -}} {{- $secret_key_read = include "seaweedfs.getOrGeneratePassword" (dict "namespace" .Release.Namespace "secretName" $secretName "key" "read_secret_access_key" "length" 40 "existingSecret" (ternary $existingSecret nil $reuse)) -}} {{- end -}} @@ -41,10 +50,16 @@ metadata: app.kubernetes.io/instance: {{ .Release.Name }} app.kubernetes.io/component: s3 stringData: + {{- /* An identity read from an existing Secret keeps its keys there; the + config below names the environment variables carrying them. */}} + {{- if not $adminCreds.existingSecret }} admin_access_key_id: {{ $access_key_admin }} admin_secret_access_key: {{ $secret_key_admin }} + {{- end }} + {{- if not $readCreds.existingSecret }} read_access_key_id: {{ $access_key_read }} read_secret_access_key: {{ $secret_key_read }} + {{- end }} seaweedfs_s3_config: '{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":"{{ $access_key_admin }}","secretKey":"{{ $secret_key_admin }}"}],"actions":["Admin","Read","Write"]},{"name":"anvReadOnly","credentials":[{"accessKey":"{{ $access_key_read }}","secretKey":"{{ $secret_key_read }}"}],"actions":["Read"]}]}' {{- if .Values.filer.s3.auditLogConfig }} filer_s3_auditLogConfig.json: | diff --git a/k8s/charts/seaweedfs/templates/shared/_helpers.tpl b/k8s/charts/seaweedfs/templates/shared/_helpers.tpl index d09edc6f5..75e3cb721 100644 --- a/k8s/charts/seaweedfs/templates/shared/_helpers.tpl +++ b/k8s/charts/seaweedfs/templates/shared/_helpers.tpl @@ -496,6 +496,39 @@ true {{- end }} {{- end -}} +{{/* Name of the environment variable carrying one generated S3 credential + field, e.g. SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID. The generated identities file + names it in place of the key when the key lives in an existing Secret. + Usage: include "seaweedfs.s3.credentialEnvName" (list "admin" "accessKey") */}} +{{- define "seaweedfs.s3.credentialEnvName" -}} +{{- $identity := index . 0 -}} +{{- $field := index . 1 -}} +{{- printf "SEAWEEDFS_S3_%s_%s" (upper $identity) (ternary "ACCESS_KEY_ID" "SECRET_ACCESS_KEY" (eq $field "accessKey")) -}} +{{- end -}} + +{{/* Environment for the S3 identities the chart generates from an existing + Secret. The gateway resolves the ${VAR} references the identities file + carries, so the keys never enter the rendered manifests and a dry run + renders the same as an install. */}} +{{- define "seaweedfs.s3.credentialEnv" -}} +{{- $creds := $.Values.s3.credentials | default dict -}} +{{- range $identity := list "admin" "read" -}} +{{- $identityCreds := index $creds $identity | default dict -}} +{{- if $identityCreds.existingSecret }} +- name: {{ include "seaweedfs.s3.credentialEnvName" (list $identity "accessKey") }} + valueFrom: + secretKeyRef: + name: {{ $identityCreds.existingSecret | quote }} + key: {{ default (printf "%s_access_key_id" $identity) $identityCreds.accessKeyKey | quote }} +- name: {{ include "seaweedfs.s3.credentialEnvName" (list $identity "secretKey") }} + valueFrom: + secretKeyRef: + name: {{ $identityCreds.existingSecret | quote }} + key: {{ default (printf "%s_secret_access_key" $identity) $identityCreds.secretKeyKey | quote }} +{{- end -}} +{{- end -}} +{{- end -}} + {{/* Generate a compatible trafficDistribution value due to "PreferClose" fast deprecation in k8s v1.35. Accepts a dict with "value" (the trafficDistribution string) and "Capabilities". */}} {{- define "seaweedfs.trafficDistribution" -}} diff --git a/k8s/charts/seaweedfs/values.yaml b/k8s/charts/seaweedfs/values.yaml index 94891c17c..76bf492a6 100644 --- a/k8s/charts/seaweedfs/values.yaml +++ b/k8s/charts/seaweedfs/values.yaml @@ -1014,13 +1014,24 @@ s3: # Optionally provide explicit credentials for the S3 gateway. # When set, these are used in the generated s3 secret instead of # auto-generating random credentials. + # An identity may instead name an existing Secret to read its keys from. The + # generated config then references the keys through environment variables, so + # nothing is looked up at render time and a dry run renders what an install + # applies. Note the COSI driver parses the config itself and does not resolve + # those references. # credentials: # admin: # accessKey: "" # secretKey: "" + # existingSecret: "" + # accessKeyKey: admin_access_key_id + # secretKeyKey: admin_secret_access_key # read: # accessKey: "" # secretKey: "" + # existingSecret: "" + # accessKeyKey: read_access_key_id + # secretKeyKey: read_secret_access_key auditLogConfig: {} # You may specify buckets to be created during the install or upgrade process. # Buckets may be exposed publicly by setting `anonymousRead` to `true` diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 37fcc26fb..599fa1923 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -9,6 +9,7 @@ import ( "net/http" "net/url" "os" + "regexp" "slices" "strings" "sync" @@ -661,6 +662,10 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromBytes(content []b s3ApiConfiguration.Groups = nil } + if fromStaticFile { + expandCredentialEnvRefs(s3ApiConfiguration) + } + if err := filer.CheckDuplicateAccessKey(s3ApiConfiguration); err != nil { return nil, err } @@ -671,6 +676,56 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromBytes(content []b return s3ApiConfiguration, nil } +var credentialEnvRef = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)\}`) + +// expandCredentialEnvRefs resolves ${VAR} references in the keys of a static +// config file, so a deployment can keep the keys in its own secret store and +// hand them to the process as environment variables. A credential still holding +// an unresolved reference is dropped instead of becoming a literal key. +// +// A variable that is set but empty counts as unresolved: a secret store handing +// over a blank value must not leave an access key signed by an empty secret. +func expandCredentialEnvRefs(config *iam_pb.S3ApiConfiguration) { + for _, ident := range config.Identities { + kept := ident.Credentials[:0] + for _, cred := range ident.Credentials { + accessKey, accessResolved := expandEnvRefs(cred.AccessKey) + secretKey, secretResolved := expandEnvRefs(cred.SecretKey) + if !accessResolved || !secretResolved { + glog.Warningf("identity %s: dropping credential %s, it references an unset environment variable", ident.Name, cred.AccessKey) + continue + } + cred.AccessKey, cred.SecretKey = accessKey, secretKey + kept = append(kept, cred) + } + ident.Credentials = kept + } +} + +// expandEnvRefs reports false when any reference is malformed or names a +// variable that is unset or empty, leaving the reference in place for the +// caller to reject. +func expandEnvRefs(value string) (string, bool) { + if !strings.Contains(value, "${") { + return value, true + } + // Every ${ has to open a well-formed reference. A typo like ${MY-VAR} matches + // nothing, so it would otherwise survive substitution as a literal key. + if strings.Count(value, "${") != len(credentialEnvRef.FindAllString(value, -1)) { + return value, false + } + resolved := true + expanded := credentialEnvRef.ReplaceAllStringFunc(value, func(ref string) string { + env, found := os.LookupEnv(ref[2 : len(ref)-1]) + if !found || env == "" { + resolved = false + return ref + } + return env + }) + return expanded, resolved +} + func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3ApiConfiguration) error { return iam.loadS3ApiConfigurationWithSource(config, false) } diff --git a/weed/s3api/auth_credentials_static_config_test.go b/weed/s3api/auth_credentials_static_config_test.go index 70c791a80..fe66efc3c 100644 --- a/weed/s3api/auth_credentials_static_config_test.go +++ b/weed/s3api/auth_credentials_static_config_test.go @@ -343,3 +343,145 @@ func writeTempIamConfig(t *testing.T, content string) string { } return path } + +// A static config file may hold ${VAR} in place of a key, so a deployment can +// keep the keys in its own secret store and pass them in as environment +// variables rather than baking them into the file. +func TestStaticConfigExpandsEnvCredentialRefs(t *testing.T) { + t.Setenv("SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID", "AKIAFROMENV") + t.Setenv("SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY", "secretfromenv") + + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, `{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":"${SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID}","secretKey":"${SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY}"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load identity config: %v", err) + } + + _, cred, found := s3a.iam.lookupByAccessKey("AKIAFROMENV") + if !found { + t.Fatalf("expected the access key from the environment to be loaded") + } + if cred.SecretKey != "secretfromenv" { + t.Fatalf("expected the secret key from the environment, got %q", cred.SecretKey) + } + if _, _, found := s3a.iam.lookupByAccessKey("${SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID}"); found { + t.Fatalf("the unexpanded reference must not remain usable as an access key") + } +} + +// An unset variable must not leave the reference behind as a literal key. +func TestStaticConfigDropsUnresolvedEnvCredentialRefs(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, `{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":"${SEAWEEDFS_S3_MISSING_ACCESS_KEY_ID}","secretKey":"${SEAWEEDFS_S3_MISSING_SECRET_ACCESS_KEY}"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load identity config: %v", err) + } + + if _, _, found := s3a.iam.lookupByAccessKey("${SEAWEEDFS_S3_MISSING_ACCESS_KEY_ID}"); found { + t.Fatalf("a credential referencing an unset variable must be dropped") + } + if !hasIdentity(s3a.iam, "anvAdmin") { + t.Fatalf("expected the identity itself to still load") + } +} + +// Keys that merely contain a dollar sign are literal, and identities coming +// from the filer are never expanded. +func TestEnvCredentialRefsOnlyApplyToStaticConfig(t *testing.T) { + t.Setenv("SEAWEEDFS_S3_DYNAMIC_SECRET_ACCESS_KEY", "secretfromenv") + + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + if err := s3a.iam.LoadS3ApiConfigurationFromBytes([]byte(`{"identities":[{"name":"dynamic","credentials":[{"accessKey":"AKIALITERAL","secretKey":"${SEAWEEDFS_S3_DYNAMIC_SECRET_ACCESS_KEY}"}],"actions":["Admin"]}]}`)); err != nil { + t.Fatalf("failed to load dynamic config: %v", err) + } + + _, cred, found := s3a.iam.lookupByAccessKey("AKIALITERAL") + if !found { + t.Fatalf("expected the dynamic identity to load") + } + if cred.SecretKey != "${SEAWEEDFS_S3_DYNAMIC_SECRET_ACCESS_KEY}" { + t.Fatalf("a dynamic identity must keep its secret key verbatim, got %q", cred.SecretKey) + } +} + +// A key holding a dollar sign that is not a reference stays untouched. +func TestStaticConfigKeepsLiteralDollarSigns(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, `{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":"AKIALITERAL","secretKey":"pa$$word"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load identity config: %v", err) + } + + _, cred, found := s3a.iam.lookupByAccessKey("AKIALITERAL") + if !found { + t.Fatalf("expected the identity to load") + } + if cred.SecretKey != "pa$$word" { + t.Fatalf("expected the literal secret key, got %q", cred.SecretKey) + } +} + +// A secret store handing over a blank value must not leave an access key that +// any signature matches. +func TestStaticConfigDropsEmptyEnvCredentialRefs(t *testing.T) { + t.Setenv("SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID", "AKIAFROMENV") + t.Setenv("SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY", "") + + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, `{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":"${SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID}","secretKey":"${SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY}"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load identity config: %v", err) + } + + if _, _, found := s3a.iam.lookupByAccessKey("AKIAFROMENV"); found { + t.Fatalf("a credential whose secret key resolves to empty must be dropped") + } +} + +// A reference the substitution cannot match, such as a typo in the variable +// name, must not survive as a literal key. +func TestStaticConfigDropsMalformedEnvCredentialRefs(t *testing.T) { + for _, malformed := range []string{"${MY-VAR}", "${1VAR}", "${}", "${UNTERMINATED", "${A}${B"} { + t.Run(malformed, func(t *testing.T) { + t.Setenv("SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY", "secretfromenv") + t.Setenv("A", "a") + + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, fmt.Sprintf(`{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":%q,"secretKey":"${SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY}"}],"actions":["Admin"]}]}`, malformed)) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load identity config: %v", err) + } + + if _, _, found := s3a.iam.lookupByAccessKey(malformed); found { + t.Fatalf("%s must not become a usable access key", malformed) + } + }) + } +} + +// A resolved value that happens to contain ${ is still the key the operator set. +func TestStaticConfigKeepsBracesComingFromTheEnvironment(t *testing.T) { + t.Setenv("SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID", "AKIAFROMENV") + t.Setenv("SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY", "pa${ss}word") + + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{}) + + path := writeTempIamConfig(t, `{"identities":[{"name":"anvAdmin","credentials":[{"accessKey":"${SEAWEEDFS_S3_ADMIN_ACCESS_KEY_ID}","secretKey":"${SEAWEEDFS_S3_ADMIN_SECRET_ACCESS_KEY}"}],"actions":["Admin"]}]}`) + if err := s3a.iam.loadS3ApiConfigurationFromFile(path); err != nil { + t.Fatalf("failed to load identity config: %v", err) + } + + _, cred, found := s3a.iam.lookupByAccessKey("AKIAFROMENV") + if !found { + t.Fatalf("expected the identity to load") + } + if cred.SecretKey != "pa${ss}word" { + t.Fatalf("expected the secret key from the environment verbatim, got %q", cred.SecretKey) + } +}