Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion pkg/controller/perconaservermongodb/custom_users.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import (
s "github.com/percona/percona-server-mongodb-operator/pkg/psmdb/secret"
)

// maxAnnotationNameLength is the maximum length for Kubernetes annotation key names (63 characters).
const maxAnnotationNameLength = 63

func (r *ReconcilePerconaServerMongoDB) reconcileCustomUsers(ctx context.Context, cr *api.PerconaServerMongoDB) error {
if cr.Spec.Users == nil && len(cr.Spec.Users) == 0 && cr.Spec.Roles == nil && len(cr.Spec.Roles) == 0 {
return nil
Expand Down Expand Up @@ -105,7 +108,7 @@ func handleUsers(ctx context.Context, cr *api.PerconaServerMongoDB, mongoCli mon
continue
}

annotationKey := fmt.Sprintf("percona.com/%s-%s-hash", cr.Name, user.Name)
annotationKey := buildAnnotationKey(cr.Name, user.Name)

if userInfo == nil && !user.IsExternalDB() {
err = createUser(ctx, client, mongoCli, &user, sec, annotationKey, userSecretPassKey)
Expand Down Expand Up @@ -422,6 +425,21 @@ func createUser(
return nil
}

// buildAnnotationKey creates a Kubernetes annotation key for tracking user password hashes.
// Kubernetes annotation keys have a limit of 63 characters for the name part.
// Format: "percona.com/<name>" where it must be less than or equal to 63 characters.
// We need to keep the "-hash" suffix (5 chars), so we have 58 chars for the prefix.
func buildAnnotationKey(crName, userName string) string {
annotationKeyBase := fmt.Sprintf("percona.com/%s-%s", crName, userName)
const hashSuffix = "-hash"
const maxPrefixLength = maxAnnotationNameLength - len(hashSuffix)

if len(annotationKeyBase) > maxPrefixLength {
annotationKeyBase = annotationKeyBase[:maxPrefixLength]
}
return fmt.Sprintf("%s%s", annotationKeyBase, hashSuffix)
}

// getCustomUserSecret gets secret by name defined by `user.PasswordSecretRef.Name` or returns a secret
// with newly generated password if name matches defaultName
func getCustomUserSecret(ctx context.Context, cl client.Client, cr *api.PerconaServerMongoDB, user *api.User, passKey string) (*corev1.Secret, error) {
Expand Down
96 changes: 96 additions & 0 deletions pkg/controller/perconaservermongodb/custom_users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package perconaservermongodb

import (
"context"
"strings"
"testing"

"github.com/pkg/errors"
Expand Down Expand Up @@ -411,3 +412,98 @@ func TestGetCustomUserSecret(t *testing.T) {
})
}
}

func TestBuildAnnotationKey(t *testing.T) {
tests := []struct {
name string
crName string
userName string
want string
wantLen int
maxLength int
}{
{
name: "short names within limit",
crName: "my-cluster",
userName: "user1",
want: "percona.com/my-cluster-user1-hash",
wantLen: 33,
maxLength: maxAnnotationNameLength,
},
{
name: "exactly at limit",
crName: "a",
userName: strings.Repeat("x", 44), // 1 + 5 + 44 + 13 (percona.com/-) = 63, will not be truncated
want: "percona.com/a-" + strings.Repeat("x", 44) + "-hash",
wantLen: maxAnnotationNameLength,
maxLength: maxAnnotationNameLength,
},
{
name: "exceeds limit - truncates but keeps hash suffix",
crName: "very-long-cluster-name-that-exceeds",
userName: "very-long-user-name-that-also-exceeds",
want: "percona.com/very-long-cluster-name-that-exceeds-very-long--hash",
wantLen: maxAnnotationNameLength,
maxLength: maxAnnotationNameLength,
},
{
name: "very long cluster name",
crName: strings.Repeat("a", 100),
userName: "user",
want: "percona.com/" + strings.Repeat("a", 46) + "-hash",
wantLen: maxAnnotationNameLength,
maxLength: maxAnnotationNameLength,
},
{
name: "very long user name",
crName: "cluster",
userName: strings.Repeat("b", 100),
want: "percona.com/cluster-" + strings.Repeat("b", 38) + "-hash",
wantLen: maxAnnotationNameLength,
maxLength: maxAnnotationNameLength,
},
{
name: "both names very long",
crName: strings.Repeat("c", 50),
userName: strings.Repeat("d", 50),
want: "percona.com/" + strings.Repeat("c", 46) + "-hash",
wantLen: maxAnnotationNameLength,
maxLength: maxAnnotationNameLength,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := buildAnnotationKey(tt.crName, tt.userName)

// Extract the name part (after "percona.com/")
prefix := "percona.com/"
if !strings.HasPrefix(got, prefix) {
t.Errorf("buildAnnotationKey() = %v, should start with %v", got, prefix)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using these conditions, we can use assert and require from testify as we do in other tests in this repo.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same of some other cases in the same test

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravisingal could you please check these comments

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated tests.


namePart := got[len(prefix):]
gotLen := len(got)

// Verify the annotation key name part is within Kubernetes limit
if len(namePart) > tt.maxLength {
t.Errorf("buildAnnotationKey() name part length = %v, should be <= %v. Got: %v", len(namePart), tt.maxLength, got)
}

// Verify it ends with "-hash"
if !strings.HasSuffix(got, "-hash") {
t.Errorf("buildAnnotationKey() = %v, should end with '-hash'", got)
}

// Verify exact match for non-truncated cases
if gotLen <= tt.maxLength && tt.want != "" {
assert.Equal(t, tt.want, got, "buildAnnotationKey() = %v, want %v", got, tt.want)
}

// Verify length matches expected for all cases
if tt.wantLen > 0 {
assert.Equal(t, tt.wantLen, gotLen, "buildAnnotationKey() length = %v, want %v", gotLen, tt.wantLen)
}
})
}
}
Loading