-
Notifications
You must be signed in to change notification settings - Fork 644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Namespace Improvements #3862
base: main
Are you sure you want to change the base?
Namespace Improvements #3862
Conversation
Signed-off-by: Mike Interlandi <[email protected]>
Signed-off-by: Mike Interlandi <[email protected]>
Signed-off-by: Mike Interlandi <[email protected]>
Signed-off-by: Mike Interlandi <[email protected]>
Signed-off-by: Mike Interlandi <[email protected]>
Signed-off-by: Mike Interlandi <[email protected]>
Signed-off-by: Mike Interlandi <[email protected]>
Signed-off-by: Mike Interlandi <[email protected]>
63738c1
to
d3454d4
Compare
Thanks for this @Minterl There is a few issues with your PR - suggesting you run Also, for identifiers validation, Happy to review more in-depth once these two are out of the way. |
@@ -39,3 +43,19 @@ func labelArgs(labelStrings []string) map[string]string { | |||
|
|||
return labels | |||
} | |||
|
|||
// Returns an error if name is invalid. | |||
func validateNamespaceName(name string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Minterl Could we add test cases for this?
// Returns an error if name is invalid. | ||
func validateNamespaceName(name string) error { | ||
for _, c := range(name) { | ||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hyphen is allowed but not checked in the if statement.
Unexpected Behavior: "abc-123" (hyphen is neither allowed nor rejected explicitly, meaning it gets rejected)
Addresses come concerns in #3851.
The decision not to enforce name validation in namespace.Remove is intentional as it is likely namespaces do exists that violate the validation rules. Not being able to easily delete them would present an issue on its own.