Fix #7575: Correct interface check for filer address function in admin server

Problem: User creation in object store was failing with error: 'filer_etc: filer address function not configured' Root Cause: In admin_server.go, the code checked for incorrect interface method SetFilerClient(string, grpc.DialOption) instead of the actual SetFilerAddressFunc(func() pb.ServerAddress, grpc.DialOption) This interface mismatch prevented the filer address function from being configured, causing user creation operations to fail. Solution: - Fixed interface check to use SetFilerAddressFunc - Updated function call to properly configure filer address function - Function now dynamically returns current active filer address Tests Added: - Unit tests in weed/admin/dash/user_management_test.go - Integration tests in test/admin/user_creation_integration_test.go - Documentation in test/admin/README.md All tests pass successfully.
2 days ago · 15620dda68
4 changed files with 272 additions and 7 deletions
--- a/test/admin/README.md
+++ b/test/admin/README.md
@ -0,0 +1,45 @@
+# Admin Integration Tests
+
+This directory contains integration tests for the SeaweedFS admin server functionality.
+
+## Tests
+
+### user_creation_integration_test.go
+
+Integration tests for user creation and credential management, specifically testing:
+
+- **Issue #7575**: Fixed bug where user creation failed with "filer address function not configured" error
+- Verification that the `FilerEtcStore` correctly implements the `SetFilerAddressFunc` interface
+- Admin server setup process with filer_etc credential store
+
+## Running Tests
+
+Run all integration tests:
+```bash
+go test -v ./test/admin
+```
+
+Run specific tests:
+```bash
+go test -v ./test/admin -run TestUserCreationWithFilerEtcStore
+```
+
+Skip integration tests (short mode):
+```bash
+go test -short ./test/admin
+```
+
+## Test Requirements
+
+These tests do not require a running filer instance. They verify that:
+1. The credential store interface is correctly implemented
+2. The filer address function can be properly configured
+3. Errors are appropriate (connection errors, not configuration errors)
+
+## Related Files
+
+- `weed/admin/dash/admin_server.go` - Admin server initialization with credential manager setup
+- `weed/credential/filer_etc/filer_etc_store.go` - FilerEtcStore implementation
+- `weed/admin/dash/user_management.go` - User management operations
+- `weed/admin/dash/user_management_test.go` - Unit tests for user management
+
--- a/test/admin/user_creation_integration_test.go
+++ b/test/admin/user_creation_integration_test.go
@ -0,0 +1,107 @@
+package admin
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/seaweedfs/seaweedfs/weed/credential"
+	_ "github.com/seaweedfs/seaweedfs/weed/credential/filer_etc" // Import to register filer_etc store
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/seaweedfs/seaweedfs/weed/security"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+	"google.golang.org/grpc"
+)
+
+// TestUserCreationWithFilerEtcStore is an integration test for issue #7575
+// It tests that user creation properly works with the filer_etc credential store
+// after the filer address function is correctly set up
+func TestUserCreationWithFilerEtcStore(t *testing.T) {
+	// This is an integration test - skip in short mode
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	// Initialize credential manager with filer_etc store (default)
+	credentialManager, err := credential.NewCredentialManagerWithDefaults("")
+	if err != nil {
+		t.Fatalf("Failed to initialize credential manager: %v", err)
+	}
+
+	// Set up filer address function for the credential store
+	// This simulates what the fixed AdminServer constructor should do
+	grpcDialOption := security.LoadClientTLS(util.GetViper(), "grpc.admin")
+	
+	if store := credentialManager.GetStore(); store != nil {
+		// Check if store supports filer address function setup
+		if filerFuncSetter, ok := store.(interface {
+			SetFilerAddressFunc(func() pb.ServerAddress, grpc.DialOption)
+		}); ok {
+			// Mock filer address
+			mockFilerAddress := "localhost:8888"
+			
+			// Set up the function to return the mock filer address
+			filerFuncSetter.SetFilerAddressFunc(func() pb.ServerAddress {
+				return pb.ServerAddress(mockFilerAddress)
+			}, grpcDialOption)
+			t.Log("Successfully set filer address function for credential store")
+		} else {
+			t.Fatal("Credential store does not implement SetFilerAddressFunc interface - bug #7575 not fixed")
+		}
+	}
+
+	// Try to create a user through the credential store directly
+	// This should not fail with "filer address function not configured"
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	// Note: This will fail trying to connect to the filer, but it should NOT
+	// fail with "filer address function not configured"
+	err = credentialManager.GetStore().CreateUser(ctx, nil)
+	
+	// Check that the error is NOT about missing filer address function
+	if err != nil {
+		errMsg := err.Error()
+		if errMsg == "filer_etc: filer address function not configured" {
+			t.Fatalf("User creation failed with 'filer address function not configured' error - bug #7575 still present")
+		}
+		// Other errors are acceptable for this test since we don't have a real filer running
+		t.Logf("User creation failed with expected error (no real filer): %v", err)
+	}
+}
+
+// TestAdminServerSetupWithFilerEtcStore tests the admin server setup process
+// to ensure the filer address function is properly configured
+func TestAdminServerSetupWithFilerEtcStore(t *testing.T) {
+	// This is an integration test - skip in short mode
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	// Test that the credential manager can be initialized
+	credentialManager, err := credential.NewCredentialManagerWithDefaults("")
+	if err != nil {
+		t.Fatalf("Failed to initialize credential manager: %v", err)
+	}
+
+	store := credentialManager.GetStore()
+	if store == nil {
+		t.Fatal("Credential store is nil")
+	}
+
+	// Check if store is filer_etc type
+	if store.GetName() != credential.StoreTypeFilerEtc {
+		t.Skipf("Skipping test - store is not filer_etc (got: %s)", store.GetName())
+	}
+
+	// Verify the store implements the correct interface
+	_, ok := store.(interface {
+		SetFilerAddressFunc(func() pb.ServerAddress, grpc.DialOption)
+	})
+	if !ok {
+		t.Fatal("FilerEtcStore does not implement SetFilerAddressFunc interface - this indicates the interface definition is wrong")
+	}
+
+	t.Log("FilerEtcStore correctly implements SetFilerAddressFunc interface")
+}
+
--- a/weed/admin/dash/admin_server.go
+++ b/weed/admin/dash/admin_server.go
@ -98,22 +98,26 @@ func NewAdminServer(masters string, templateFS http.FileSystem, dataDir string)
 		glog.Warningf("Failed to initialize credential manager: %v", err)
 		// Continue without credential manager - will fall back to legacy approach
 	} else {
-		// For stores that need filer client details, set them
+		// For stores that need filer address function, set them
 		if store := credentialManager.GetStore(); store != nil {
-			if filerClientSetter, ok := store.(interface {
-				SetFilerClient(string, grpc.DialOption)
+			if filerFuncSetter, ok := store.(interface {
+				SetFilerAddressFunc(func() pb.ServerAddress, grpc.DialOption)
 			}); ok {
-				// We'll set the filer client later when we discover filers
+				// We'll set the filer address function later when we discover filers
 				// For now, just store the credential manager
 				server.credentialManager = credentialManager

-				// Set up a goroutine to set filer client once we discover filers
+				// Set up a goroutine to set filer address function once we discover filers
 				go func() {
 					for {
 						filerAddr := server.GetFilerAddress()
 						if filerAddr != "" {
-							filerClientSetter.SetFilerClient(filerAddr, server.grpcDialOption)
-							glog.V(1).Infof("Set filer client for credential manager: %s", filerAddr)
+							// Set up the filer address function to always return the discovered filer
+							filerFuncSetter.SetFilerAddressFunc(func() pb.ServerAddress {
+								// Use GetFilerAddress() to get the current active filer
+								return pb.ServerAddress(server.GetFilerAddress())
+							}, server.grpcDialOption)
+							glog.V(1).Infof("Set filer address function for credential manager: %s", filerAddr)
 							break
 						}
 						glog.V(1).Infof("Waiting for filer discovery for credential manager...")
--- a/weed/admin/dash/user_management_test.go
+++ b/weed/admin/dash/user_management_test.go
@ -0,0 +1,109 @@
+package dash
+
+import (
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/credential"
+	_ "github.com/seaweedfs/seaweedfs/weed/credential/filer_etc" // Import to register filer_etc store
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"google.golang.org/grpc"
+)
+
+// TestFilerAddressFunctionInterface tests that the filer_etc store
+// implements the correct SetFilerAddressFunc interface (issue #7575)
+func TestFilerAddressFunctionInterface(t *testing.T) {
+	// Create credential manager with filer_etc store
+	credentialManager, err := credential.NewCredentialManagerWithDefaults("")
+	if err != nil {
+		t.Fatalf("Failed to initialize credential manager: %v", err)
+	}
+
+	store := credentialManager.GetStore()
+	if store == nil {
+		t.Fatal("Credential store is nil")
+	}
+
+	// Check if store is filer_etc type
+	if store.GetName() != credential.StoreTypeFilerEtc {
+		t.Skipf("Skipping test - store is not filer_etc (got: %s)", store.GetName())
+	}
+
+	// Check if store implements SetFilerAddressFunc interface
+	// This is the critical check for bug #7575
+	filerFuncSetter, ok := store.(interface {
+		SetFilerAddressFunc(func() pb.ServerAddress, grpc.DialOption)
+	})
+	if !ok {
+		t.Fatal("FilerEtcStore does not implement SetFilerAddressFunc interface - bug #7575")
+	}
+
+	// Verify we can call the method without panic
+	mockFilerAddress := pb.ServerAddress("localhost:8888")
+	filerFuncSetter.SetFilerAddressFunc(func() pb.ServerAddress {
+		return mockFilerAddress
+	}, grpc.WithInsecure())
+
+	t.Log("FilerEtcStore correctly implements SetFilerAddressFunc interface")
+}
+
+// TestGenerateAccessKey tests the access key generation function
+func TestGenerateAccessKey(t *testing.T) {
+	key1 := generateAccessKey()
+	key2 := generateAccessKey()
+
+	// Check length
+	if len(key1) != 20 {
+		t.Errorf("Expected access key length 20, got %d", len(key1))
+	}
+
+	// Check uniqueness
+	if key1 == key2 {
+		t.Error("Generated access keys should be unique")
+	}
+
+	// Check character set
+	for _, c := range key1 {
+		if !((c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9')) {
+			t.Errorf("Access key contains invalid character: %c", c)
+		}
+	}
+}
+
+// TestGenerateSecretKey tests the secret key generation function
+func TestGenerateSecretKey(t *testing.T) {
+	key1 := generateSecretKey()
+	key2 := generateSecretKey()
+
+	// Check length (base64 encoding of 30 bytes = 40 characters)
+	if len(key1) != 40 {
+		t.Errorf("Expected secret key length 40, got %d", len(key1))
+	}
+
+	// Check uniqueness
+	if key1 == key2 {
+		t.Error("Generated secret keys should be unique")
+	}
+}
+
+// TestGenerateAccountId tests the account ID generation function
+func TestGenerateAccountId(t *testing.T) {
+	id1 := generateAccountId()
+	id2 := generateAccountId()
+
+	// Check length
+	if len(id1) != 12 {
+		t.Errorf("Expected account ID length 12, got %d", len(id1))
+	}
+
+	// Check that it's a number
+	for _, c := range id1 {
+		if c < '0' || c > '9' {
+			t.Errorf("Account ID contains non-digit character: %c", c)
+		}
+	}
+
+	// Check uniqueness (they should usually be different)
+	if id1 == id2 {
+		t.Log("Warning: Generated account IDs are the same (rare but possible)")
+	}
+}