diff --git a/weed/command/s3.go b/weed/command/s3.go index 8dcb681b9..027bb9cd0 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -160,13 +160,13 @@ var cmdS3 = &Command{ ] } - Alternatively, you can use environment variables to supplement admin credentials: + Alternatively, you can use environment variables as fallback admin credentials: AWS_ACCESS_KEY_ID=your_access_key AWS_SECRET_ACCESS_KEY=your_secret_key weed s3 - This will add admin credentials from environment variables to any existing - configuration. Environment variables are added after loading file/filer - configurations and are skipped if the same access key already exists. + Environment variables are only used when no S3 configuration file is provided + and no configuration is available from the filer. This provides a simple way + to get started without requiring configuration files. `, } diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 4f00639ff..e2e8c1752 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -131,33 +131,38 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, explicitSto iam.credentialManager = credentialManager - // First, load configurations from file or filer + // Track whether any configuration was successfully loaded + configLoaded := false + + // First, try to load configurations from file or filer if option.Config != "" { glog.V(3).Infof("loading static config file %s", option.Config) if err := iam.loadS3ApiConfigurationFromFile(option.Config); err != nil { glog.Fatalf("fail to load config file %s: %v", option.Config, err) } + configLoaded = true } else { glog.V(3).Infof("no static config file specified... loading config from credential manager") if err := iam.loadS3ApiConfigurationFromFiler(option); err != nil { glog.Warningf("fail to load config: %v", err) + } else { + // Check if any identities were actually loaded from filer + iam.m.RLock() + if len(iam.identities) > 0 { + configLoaded = true + } + iam.m.RUnlock() } } - // Then, add admin credentials from environment variables if available -// This supplements the configuration by adding admin credentials from environment variables if they don't already exist. - accessKeyId := os.Getenv("AWS_ACCESS_KEY_ID") - secretAccessKey := os.Getenv("AWS_SECRET_ACCESS_KEY") + // Only use environment variables as fallback if no configuration was loaded + if !configLoaded { + accessKeyId := os.Getenv("AWS_ACCESS_KEY_ID") + secretAccessKey := os.Getenv("AWS_SECRET_ACCESS_KEY") - if accessKeyId != "" && secretAccessKey != "" { - glog.V(0).Infof("Adding S3 admin credentials from AWS environment variables") + if accessKeyId != "" && secretAccessKey != "" { + glog.V(0).Infof("No S3 configuration found, using AWS environment variables as fallback") - // Check if an identity with this access key already exists - iam.m.RLock() - _, accessKeyExists := iam.accessKeyIdent[accessKeyId] - iam.m.RUnlock() - - if !accessKeyExists { // Create environment variable identity name identityNameSuffix := accessKeyId if len(accessKeyId) > 8 { @@ -179,18 +184,16 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, explicitSto }, } - // Add to existing configuration + // Set as the only configuration iam.m.Lock() - iam.identities = append(iam.identities, envIdentity) - iam.accessKeyIdent[accessKeyId] = envIdentity - if !iam.isAuthEnabled { + if len(iam.identities) == 0 { + iam.identities = []*Identity{envIdentity} + iam.accessKeyIdent = map[string]*Identity{accessKeyId: envIdentity} iam.isAuthEnabled = true } iam.m.Unlock() glog.V(0).Infof("Added admin identity from AWS environment variables: %s", envIdentity.Name) - } else { - glog.V(0).Infof("Access key %s already exists, skipping environment variable credentials", accessKeyId) } } diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index 0ed6e65db..b751eb8bc 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -292,27 +292,31 @@ func TestNewIdentityAccessManagementWithStoreEnvVars(t *testing.T) { secretAccessKey string expectEnvIdentity bool expectedName string + description string }{ { - name: "Both env vars set", + name: "Environment variables used as fallback", accessKeyId: "AKIA1234567890ABCDEF", secretAccessKey: "secret123456789012345678901234567890abcdef12", expectEnvIdentity: true, expectedName: "admin-AKIA1234", + description: "When no config file and no filer config, environment variables should be used", }, { - name: "Short access key", + name: "Short access key fallback", accessKeyId: "SHORT", secretAccessKey: "secret123456789012345678901234567890abcdef12", expectEnvIdentity: true, expectedName: "admin-SHORT", + description: "Short access keys should work correctly as fallback", }, { - name: "No env vars set", + name: "No env vars means no identities", accessKeyId: "", secretAccessKey: "", expectEnvIdentity: false, expectedName: "", + description: "When no env vars and no config, should have no identities", }, } @@ -330,26 +334,22 @@ func TestNewIdentityAccessManagementWithStoreEnvVars(t *testing.T) { os.Unsetenv("AWS_SECRET_ACCESS_KEY") } - // Create IAM instance with memory store for testing + // Create IAM instance with memory store for testing (no config file) option := &S3ApiServerOption{ - Config: "", // No config file, should use environment variables + Config: "", // No config file - this should trigger environment variable fallback } iam := NewIdentityAccessManagementWithStore(option, string(credential.StoreTypeMemory)) if tt.expectEnvIdentity { - // Check that environment variable identity was created - found := false - for _, identity := range iam.identities { - if identity.Name == tt.expectedName { - found = true - assert.Len(t, identity.Credentials, 1, "Should have one credential") - assert.Equal(t, tt.accessKeyId, identity.Credentials[0].AccessKey, "Access key should match environment variable") - assert.Equal(t, tt.secretAccessKey, identity.Credentials[0].SecretKey, "Secret key should match environment variable") - assert.Contains(t, identity.Actions, Action(ACTION_ADMIN), "Should have admin action") - break - } - } - assert.True(t, found, "Should find identity created from environment variables") + // Should have exactly one identity from environment variables + assert.Len(t, iam.identities, 1, "Should have exactly one identity from environment variables") + + identity := iam.identities[0] + assert.Equal(t, tt.expectedName, identity.Name, "Identity name should match expected") + assert.Len(t, identity.Credentials, 1, "Should have one credential") + assert.Equal(t, tt.accessKeyId, identity.Credentials[0].AccessKey, "Access key should match environment variable") + assert.Equal(t, tt.secretAccessKey, identity.Credentials[0].SecretKey, "Secret key should match environment variable") + assert.Contains(t, identity.Actions, Action(ACTION_ADMIN), "Should have admin action") } else { // When no env vars, should have no identities (since no config file) assert.Len(t, iam.identities, 0, "Should have no identities when no env vars and no config file")