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) + } +}