Browse Source

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

* 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.

* 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.

* 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:
1. 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)
2. The admin command was missing the filer_etc import, so the store
   was never registered

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:
- Added filer_etc import to weed/command/admin.go to register the store
- 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)
- Hoisted credentialManager assignment to reduce code duplication

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.
copilot/fix-s3-object-tagging-issue
Chris Lu 2 days ago
committed by GitHub
parent
commit
f6f3859826
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 27
      weed/admin/dash/admin_server.go
  2. 109
      weed/admin/dash/user_management_test.go
  3. 1
      weed/command/admin.go

27
weed/admin/dash/admin_server.go

@ -98,33 +98,30 @@ 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
server.credentialManager = credentialManager
// 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
// 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 configure 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)
// 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) // Retry every 5 seconds
time.Sleep(5 * time.Second)
}
}()
} else {
server.credentialManager = credentialManager
}
} else {
server.credentialManager = credentialManager
}
}

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

1
weed/command/admin.go

@ -22,6 +22,7 @@ import (
"github.com/seaweedfs/seaweedfs/weed/admin"
"github.com/seaweedfs/seaweedfs/weed/admin/dash"
"github.com/seaweedfs/seaweedfs/weed/admin/handlers"
_ "github.com/seaweedfs/seaweedfs/weed/credential/filer_etc" // Register filer_etc credential store
"github.com/seaweedfs/seaweedfs/weed/pb"
"github.com/seaweedfs/seaweedfs/weed/security"
"github.com/seaweedfs/seaweedfs/weed/util"

Loading…
Cancel
Save