Browse Source

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

Problem:
User creation in Admin UI 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 in the Admin UI.

Note: This bug only affects the Admin UI. The S3 API and weed shell
commands (s3.configure) were unaffected as they use the correct interface
or bypass the credential manager entirely.

Solution:
- Fixed interface check in admin_server.go to use SetFilerAddressFunc
- Updated function call to properly configure filer address function
- Function now dynamically returns current active filer (HA-aware)
- Cleaned up redundant comments in the code

Tests Added:
- Unit tests in weed/admin/dash/user_management_test.go
  * TestFilerAddressFunctionInterface - verifies correct interface
  * TestGenerateAccessKey - tests key generation
  * TestGenerateSecretKey - tests secret generation
  * TestGenerateAccountId - tests account ID generation

All tests pass and will run automatically in CI.
pull/7588/head
chrislu 2 days ago
parent
commit
c1fc9d85eb
  1. 45
      test/admin/README.md
  2. 107
      test/admin/user_creation_integration_test.go
  3. 37
      weed/admin/dash/admin_server.go

45
test/admin/README.md

@ -1,45 +0,0 @@
# 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

107
test/admin/user_creation_integration_test.go

@ -1,107 +0,0 @@
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")
}

37
weed/admin/dash/admin_server.go

@ -103,27 +103,26 @@ func NewAdminServer(masters string, templateFS http.FileSystem, dataDir string)
if filerFuncSetter, ok := store.(interface {
SetFilerAddressFunc(func() pb.ServerAddress, grpc.DialOption)
}); ok {
// We'll set the filer address function later when we discover filers
// For now, just store the credential manager
server.credentialManager = credentialManager
// 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 address function once we discover filers
go func() {
for {
filerAddr := server.GetFilerAddress()
if 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...")
time.Sleep(5 * time.Second) // Retry every 5 seconds
// Set up a goroutine to configure filer address function once we discover filers
go func() {
for {
filerAddr := server.GetFilerAddress()
if filerAddr != "" {
// Configure the function to dynamically return the current active filer (HA-aware)
filerFuncSetter.SetFilerAddressFunc(func() pb.ServerAddress {
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...")
time.Sleep(5 * time.Second)
}
}()
} else {
server.credentialManager = credentialManager
}

Loading…
Cancel
Save