From 29e9f0c5121e18c481527ab1ebdb54bda83c3dab Mon Sep 17 00:00:00 2001 From: David Yu Date: Tue, 28 Apr 2026 13:03:25 -0700 Subject: [PATCH 1/2] rpk: warn when role name is not RFC 1123-compliant Adds a stderr warning to `rpk security role create` when the role name isn't a valid DNS-1123 subdomain. The warning is informational only -- the role is still created and the command exits 0. The Redpanda Kubernetes operator binds a RedpandaRole CR's role name to its metadata.name, which the Kubernetes API server validates against RFC 1123. Roles like "MyRole" or "App_Service" cannot be adopted by an operator-managed CR and require a manual rename (create new role, copy members, recreate ACLs, delete old) before migration. Surfacing this constraint at create time gives operators a chance to choose operator- friendly names up front. A warning rather than a hard rejection avoids breaking existing users with non-compliant names. Stdout is unchanged so JSON/YAML formatters and scripted callers parsing stdout aren't affected. --- src/go/rpk/pkg/cli/security/role/BUILD | 7 ++- src/go/rpk/pkg/cli/security/role/create.go | 10 ++++ .../rpk/pkg/cli/security/role/validation.go | 27 +++++++++ .../pkg/cli/security/role/validation_test.go | 59 +++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 src/go/rpk/pkg/cli/security/role/validation.go create mode 100644 src/go/rpk/pkg/cli/security/role/validation_test.go diff --git a/src/go/rpk/pkg/cli/security/role/BUILD b/src/go/rpk/pkg/cli/security/role/BUILD index be45f36374477..e276d6f127546 100644 --- a/src/go/rpk/pkg/cli/security/role/BUILD +++ b/src/go/rpk/pkg/cli/security/role/BUILD @@ -10,6 +10,7 @@ go_library( "list.go", "role.go", "unassign.go", + "validation.go", ], importpath = "github.com/redpanda-data/redpanda/src/go/rpk/pkg/cli/security/role", visibility = ["//visibility:public"], @@ -26,12 +27,16 @@ go_library( "@com_github_spf13_cobra//:cobra", "@com_github_twmb_franz_go_pkg_kadm//:kadm", "@com_github_twmb_types//:types", + "@io_k8s_apimachinery//pkg/util/validation", ], ) go_test( name = "role_test", - srcs = ["role_test.go"], + srcs = [ + "role_test.go", + "validation_test.go", + ], embed = [":role"], deps = ["@com_github_stretchr_testify//require"], ) diff --git a/src/go/rpk/pkg/cli/security/role/create.go b/src/go/rpk/pkg/cli/security/role/create.go index a9499bc7a4a08..419e8a5f1911d 100644 --- a/src/go/rpk/pkg/cli/security/role/create.go +++ b/src/go/rpk/pkg/cli/security/role/create.go @@ -11,6 +11,7 @@ package role import ( "fmt" + "os" dataplanev1 "buf.build/gen/go/redpandadata/dataplane/protocolbuffers/go/redpanda/api/dataplane/v1" "connectrpc.com/connect" @@ -45,6 +46,15 @@ flag in the 'rpk security acl create' command.`, config.CheckExitServerlessAdmin(prof) roleName := args[0] + if msgs := validateRoleNameForK8s(roleName); len(msgs) > 0 { + fmt.Fprintf(os.Stderr, "Warning: role name %q is not a valid DNS-1123 subdomain (RFC 1123):\n", roleName) + for _, m := range msgs { + fmt.Fprintf(os.Stderr, " - %s\n", m) + } + fmt.Fprintln(os.Stderr, " This role cannot be adopted by a RedpandaRole CR in the Redpanda Kubernetes operator,") + fmt.Fprintln(os.Stderr, " since the operator binds the role name to the CR's metadata.name. Consider using a") + fmt.Fprintln(os.Stderr, " lowercase name (letters, digits, '-', '.') if you may migrate to operator-managed roles.") + } if prof.CheckFromCloud() { cl, err := publicapi.DataplaneClientFromRpkProfile(prof) out.MaybeDie(err, "unable to initialize cloud API client: %v", err) diff --git a/src/go/rpk/pkg/cli/security/role/validation.go b/src/go/rpk/pkg/cli/security/role/validation.go new file mode 100644 index 0000000000000..83859545c8b78 --- /dev/null +++ b/src/go/rpk/pkg/cli/security/role/validation.go @@ -0,0 +1,27 @@ +// Copyright 2026 Redpanda Data, Inc. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.md +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0 + +package role + +import ( + "k8s.io/apimachinery/pkg/util/validation" +) + +// validateRoleNameForK8s reports whether the role name is a valid DNS-1123 +// subdomain. Returns nil if the name is compliant; otherwise returns the +// validation messages from k8s.io/apimachinery so the caller can surface them +// to the user. +// +// The Redpanda Kubernetes operator binds a RedpandaRole CR's role name to its +// metadata.name, which the Kubernetes API server constrains to DNS-1123. Names +// that don't satisfy this constraint cannot be adopted by a RedpandaRole CR +// and will block migration to operator-managed roles. +func validateRoleNameForK8s(name string) []string { + return validation.IsDNS1123Subdomain(name) +} diff --git a/src/go/rpk/pkg/cli/security/role/validation_test.go b/src/go/rpk/pkg/cli/security/role/validation_test.go new file mode 100644 index 0000000000000..aa0d4fc3ab4c5 --- /dev/null +++ b/src/go/rpk/pkg/cli/security/role/validation_test.go @@ -0,0 +1,59 @@ +// Copyright 2026 Redpanda Data, Inc. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.md +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0 + +package role + +import ( + "strings" + "testing" +) + +func TestValidateRoleNameForK8s(t *testing.T) { + for _, tc := range []struct { + name string + input string + wantOK bool + mustHave string + }{ + {name: "lowercase simple", input: "my-role", wantOK: true}, + {name: "lowercase with dots", input: "team.read-only", wantOK: true}, + {name: "alphanumeric", input: "role01", wantOK: true}, + {name: "single char", input: "a", wantOK: true}, + + {name: "uppercase rejected", input: "MyRole", wantOK: false, mustHave: "lower case"}, + {name: "leading hyphen rejected", input: "-foo", wantOK: false}, + {name: "trailing hyphen rejected", input: "foo-", wantOK: false}, + {name: "underscore rejected", input: "App_Role", wantOK: false}, + {name: "space rejected", input: "my role", wantOK: false}, + {name: "empty rejected", input: "", wantOK: false}, + {name: "too long rejected", input: strings.Repeat("a", 254), wantOK: false}, + } { + t.Run(tc.name, func(t *testing.T) { + msgs := validateRoleNameForK8s(tc.input) + gotOK := len(msgs) == 0 + if gotOK != tc.wantOK { + t.Fatalf("validateRoleNameForK8s(%q) compliant=%v msgs=%v; want compliant=%v", + tc.input, gotOK, msgs, tc.wantOK) + } + if tc.mustHave != "" { + found := false + for _, m := range msgs { + if strings.Contains(m, tc.mustHave) { + found = true + break + } + } + if !found { + t.Fatalf("validateRoleNameForK8s(%q) msgs=%v; expected one to contain %q", + tc.input, msgs, tc.mustHave) + } + } + }) + } +} From d3a8897ad223a208826b596512e02e670a4f2ebc Mon Sep 17 00:00:00 2001 From: David Yu Date: Tue, 28 Apr 2026 13:36:49 -0700 Subject: [PATCH 2/2] rpk: address review - inline IsDNS1123Subdomain, collapse warning Per @r-vasquez review on PR #30325: - Drop the validateRoleNameForK8s wrapper and its tests; the wrapper added no logic over validation.IsDNS1123Subdomain, so testing it just exercised the upstream function. Call IsDNS1123Subdomain directly at the use site. - Collapse the multi-line warning into a single Fprintf with a raw string template, joining messages with strings.Join. --- src/go/rpk/pkg/cli/security/role/BUILD | 6 +- src/go/rpk/pkg/cli/security/role/create.go | 21 ++++--- .../rpk/pkg/cli/security/role/validation.go | 27 --------- .../pkg/cli/security/role/validation_test.go | 59 ------------------- 4 files changed, 14 insertions(+), 99 deletions(-) delete mode 100644 src/go/rpk/pkg/cli/security/role/validation.go delete mode 100644 src/go/rpk/pkg/cli/security/role/validation_test.go diff --git a/src/go/rpk/pkg/cli/security/role/BUILD b/src/go/rpk/pkg/cli/security/role/BUILD index e276d6f127546..75e422a2fccd9 100644 --- a/src/go/rpk/pkg/cli/security/role/BUILD +++ b/src/go/rpk/pkg/cli/security/role/BUILD @@ -10,7 +10,6 @@ go_library( "list.go", "role.go", "unassign.go", - "validation.go", ], importpath = "github.com/redpanda-data/redpanda/src/go/rpk/pkg/cli/security/role", visibility = ["//visibility:public"], @@ -33,10 +32,7 @@ go_library( go_test( name = "role_test", - srcs = [ - "role_test.go", - "validation_test.go", - ], + srcs = ["role_test.go"], embed = [":role"], deps = ["@com_github_stretchr_testify//require"], ) diff --git a/src/go/rpk/pkg/cli/security/role/create.go b/src/go/rpk/pkg/cli/security/role/create.go index 419e8a5f1911d..3b711f8a23492 100644 --- a/src/go/rpk/pkg/cli/security/role/create.go +++ b/src/go/rpk/pkg/cli/security/role/create.go @@ -12,6 +12,7 @@ package role import ( "fmt" "os" + "strings" dataplanev1 "buf.build/gen/go/redpandadata/dataplane/protocolbuffers/go/redpanda/api/dataplane/v1" "connectrpc.com/connect" @@ -21,6 +22,7 @@ import ( "github.com/redpanda-data/redpanda/src/go/rpk/pkg/publicapi" "github.com/spf13/afero" "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/util/validation" ) type createResponse struct { @@ -46,14 +48,17 @@ flag in the 'rpk security acl create' command.`, config.CheckExitServerlessAdmin(prof) roleName := args[0] - if msgs := validateRoleNameForK8s(roleName); len(msgs) > 0 { - fmt.Fprintf(os.Stderr, "Warning: role name %q is not a valid DNS-1123 subdomain (RFC 1123):\n", roleName) - for _, m := range msgs { - fmt.Fprintf(os.Stderr, " - %s\n", m) - } - fmt.Fprintln(os.Stderr, " This role cannot be adopted by a RedpandaRole CR in the Redpanda Kubernetes operator,") - fmt.Fprintln(os.Stderr, " since the operator binds the role name to the CR's metadata.name. Consider using a") - fmt.Fprintln(os.Stderr, " lowercase name (letters, digits, '-', '.') if you may migrate to operator-managed roles.") + if msgs := validation.IsDNS1123Subdomain(roleName); len(msgs) > 0 { + fmt.Fprintf(os.Stderr, + `Warning: role name %q is not a valid DNS-1123 subdomain (RFC 1123): + - %s + This role cannot be adopted by a RedpandaRole CR in the Redpanda + Kubernetes operator, since the operator binds the role name to the + CR's metadata.name. Consider using a lowercase name (letters, digits, + '-', '.') if you may migrate to operator-managed roles. +`, + roleName, strings.Join(msgs, "\n - "), + ) } if prof.CheckFromCloud() { cl, err := publicapi.DataplaneClientFromRpkProfile(prof) diff --git a/src/go/rpk/pkg/cli/security/role/validation.go b/src/go/rpk/pkg/cli/security/role/validation.go deleted file mode 100644 index 83859545c8b78..0000000000000 --- a/src/go/rpk/pkg/cli/security/role/validation.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2026 Redpanda Data, Inc. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.md -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0 - -package role - -import ( - "k8s.io/apimachinery/pkg/util/validation" -) - -// validateRoleNameForK8s reports whether the role name is a valid DNS-1123 -// subdomain. Returns nil if the name is compliant; otherwise returns the -// validation messages from k8s.io/apimachinery so the caller can surface them -// to the user. -// -// The Redpanda Kubernetes operator binds a RedpandaRole CR's role name to its -// metadata.name, which the Kubernetes API server constrains to DNS-1123. Names -// that don't satisfy this constraint cannot be adopted by a RedpandaRole CR -// and will block migration to operator-managed roles. -func validateRoleNameForK8s(name string) []string { - return validation.IsDNS1123Subdomain(name) -} diff --git a/src/go/rpk/pkg/cli/security/role/validation_test.go b/src/go/rpk/pkg/cli/security/role/validation_test.go deleted file mode 100644 index aa0d4fc3ab4c5..0000000000000 --- a/src/go/rpk/pkg/cli/security/role/validation_test.go +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2026 Redpanda Data, Inc. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.md -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0 - -package role - -import ( - "strings" - "testing" -) - -func TestValidateRoleNameForK8s(t *testing.T) { - for _, tc := range []struct { - name string - input string - wantOK bool - mustHave string - }{ - {name: "lowercase simple", input: "my-role", wantOK: true}, - {name: "lowercase with dots", input: "team.read-only", wantOK: true}, - {name: "alphanumeric", input: "role01", wantOK: true}, - {name: "single char", input: "a", wantOK: true}, - - {name: "uppercase rejected", input: "MyRole", wantOK: false, mustHave: "lower case"}, - {name: "leading hyphen rejected", input: "-foo", wantOK: false}, - {name: "trailing hyphen rejected", input: "foo-", wantOK: false}, - {name: "underscore rejected", input: "App_Role", wantOK: false}, - {name: "space rejected", input: "my role", wantOK: false}, - {name: "empty rejected", input: "", wantOK: false}, - {name: "too long rejected", input: strings.Repeat("a", 254), wantOK: false}, - } { - t.Run(tc.name, func(t *testing.T) { - msgs := validateRoleNameForK8s(tc.input) - gotOK := len(msgs) == 0 - if gotOK != tc.wantOK { - t.Fatalf("validateRoleNameForK8s(%q) compliant=%v msgs=%v; want compliant=%v", - tc.input, gotOK, msgs, tc.wantOK) - } - if tc.mustHave != "" { - found := false - for _, m := range msgs { - if strings.Contains(m, tc.mustHave) { - found = true - break - } - } - if !found { - t.Fatalf("validateRoleNameForK8s(%q) msgs=%v; expected one to contain %q", - tc.input, msgs, tc.mustHave) - } - } - }) - } -}