diff --git a/test/admin/README.md b/test/admin/README.md new file mode 100644 index 000000000..d820e3239 --- /dev/null +++ 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 + diff --git a/test/admin/user_creation_integration_test.go b/test/admin/user_creation_integration_test.go new file mode 100644 index 000000000..c50c9c5b1 --- /dev/null +++ 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") +} + diff --git a/weed/admin/dash/admin_server.go b/weed/admin/dash/admin_server.go index 4a1dd592f..223a2f687 100644 --- 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...") diff --git a/weed/admin/dash/user_management_test.go b/weed/admin/dash/user_management_test.go new file mode 100644 index 000000000..6e63bfa31 --- /dev/null +++ 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)") + } +}