diff options
| author | Chris Lu <chrislusf@users.noreply.github.com> | 2025-10-28 19:05:29 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-10-28 19:05:29 -0700 |
| commit | 85bd593936126157fbb5f01a048d8258fab045c5 (patch) | |
| tree | 2a0a3c3587649622f58fc168c4a2bac8a9cb6a9a | |
| parent | d19eca71eb1da170b4398d5a207f6951c8fa20d5 (diff) | |
| download | seaweedfs-85bd593936126157fbb5f01a048d8258fab045c5.tar.xz seaweedfs-85bd593936126157fbb5f01a048d8258fab045c5.zip | |
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>
| -rw-r--r-- | weed/s3api/auth_credentials.go | 12 | ||||
| -rw-r--r-- | 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) { |
