change priority of admin credentials from env varaibles (#7032)

* change priority of admin credentials from env varaibles

* address comment

3 files changed, 44 insertions, 41 deletions

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")