From 85bd593936126157fbb5f01a048d8258fab045c5 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 28 Oct 2025 19:05:29 -0700 Subject: [PATCH] S3: adjust for loading credentials (#7400) * adjust for loading credentials * Update weed/s3api/auth_credentials_test.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * simplify --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- weed/s3api/auth_credentials.go | 12 ++++---- weed/s3api/auth_credentials_test.go | 46 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index e3e7c0bbb..66b9c7296 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -153,10 +153,10 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, explicitSto if err := iam.loadS3ApiConfigurationFromFile(option.Config); err != nil { glog.Fatalf("fail to load config file %s: %v", option.Config, err) } - // Mark as loaded since an explicit config file was provided - // This prevents fallback to environment variables even if no identities were loaded - // (e.g., config file contains only KMS settings) - configLoaded = true + // Check if any identities were actually loaded from the config file + iam.m.RLock() + configLoaded = len(iam.identities) > 0 + iam.m.RUnlock() } else { glog.V(3).Infof("no static config file specified... loading config from credential manager") if err := iam.loadS3ApiConfigurationFromFiler(option); err != nil { @@ -164,9 +164,7 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, explicitSto } else { // Check if any identities were actually loaded from filer iam.m.RLock() - if len(iam.identities) > 0 { - configLoaded = true - } + configLoaded = len(iam.identities) > 0 iam.m.RUnlock() } } diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index c7521ad76..0753a833e 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -362,6 +362,52 @@ func TestNewIdentityAccessManagementWithStoreEnvVars(t *testing.T) { } } +// TestConfigFileWithNoIdentitiesAllowsEnvVars tests that when a config file exists +// but contains no identities (e.g., only KMS settings), environment variables should still work. +// This test validates the fix for issue #7311. +func TestConfigFileWithNoIdentitiesAllowsEnvVars(t *testing.T) { + // Set environment variables + testAccessKey := "AKIATEST1234567890AB" + testSecretKey := "testSecret1234567890123456789012345678901234" + t.Setenv("AWS_ACCESS_KEY_ID", testAccessKey) + t.Setenv("AWS_SECRET_ACCESS_KEY", testSecretKey) + + // Create a temporary config file with only KMS settings (no identities) + configContent := `{ + "kms": { + "default": { + "provider": "local", + "config": { + "keyPath": "/tmp/test-key" + } + } + } +}` + tmpFile, err := os.CreateTemp("", "s3-config-*.json") + assert.NoError(t, err, "Should create temp config file") + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.Write([]byte(configContent)) + assert.NoError(t, err, "Should write config content") + tmpFile.Close() + + // Create IAM instance with config file that has no identities + option := &S3ApiServerOption{ + Config: tmpFile.Name(), + } + iam := NewIdentityAccessManagementWithStore(option, string(credential.StoreTypeMemory)) + + // Should have exactly one identity from environment variables + assert.Len(t, iam.identities, 1, "Should have exactly one identity from environment variables even when config file exists with no identities") + + identity := iam.identities[0] + assert.Equal(t, "admin-AKIATEST", identity.Name, "Identity name should be based on access key") + assert.Len(t, identity.Credentials, 1, "Should have one credential") + assert.Equal(t, testAccessKey, identity.Credentials[0].AccessKey, "Access key should match environment variable") + assert.Equal(t, testSecretKey, identity.Credentials[0].SecretKey, "Secret key should match environment variable") + assert.Contains(t, identity.Actions, Action(ACTION_ADMIN), "Should have admin action") +} + // TestBucketLevelListPermissions tests that bucket-level List permissions work correctly // This test validates the fix for issue #7066 func TestBucketLevelListPermissions(t *testing.T) {