label: Invalidate empty or invalid value labels #8556
Conversation
0xmichalis
changed the title
|
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain @ixdy. |
|
ok to test |
| @@ -103,7 +104,7 @@ func parseLabels(spec []string) (map[string]string, []string, error) { | |||
| for _, labelSpec := range spec { | |||
| if strings.Index(labelSpec, "=") != -1 { | |||
| parts := strings.Split(labelSpec, "=") | |||
| if len(parts) != 2 { | |||
| if len(parts) != 2 || len(parts[1]) == 0 || !util.IsValidLabelValue(parts[1]) { | |||
| return nil, nil, fmt.Errorf("invalid label spec: %v", labelSpec) | |||
There was a problem hiding this comment.
I know it's preexisting, but invalid user input should return UsageError. Do you mind fixing references in this file?
There was a problem hiding this comment.
Added this in the main RunLabel function.
|
Users shouldn't be expected to remember the rules for a valid label spec. Let's include the valid label regexp in the help string. |
I've updated the help string with a simple explanation of what's needed for a valid label value. |
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| const ( | ||
| label_long = `Update the labels on a resource. | ||
|
|
||
| A valid label value is consisted of letters and/or numbers with a max length of %[1]d characters. |
There was a problem hiding this comment.
Labels can also contain -, ., and_, which is why I was suggesting putting the regex into the help string. Rule is here.
There was a problem hiding this comment.
How about mentioning those characters as well?
A valid label value is consisted of letters, numbers, and some special characters (hyphen, dot, underscore) with a max length of 63 characters.
Imho it feels much more user-friendly than eg.
A valid label value has to comply with the ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] regural expression with a max length of 63 characters.
There was a problem hiding this comment.
I'm probably weird in that I prefer the regex. Fine with me to stick to English, so long as the rule is concisely and accurately stated. Label must begin with a letter or number, and may contain letters, numbers, hyphens, dots, and underscores, up to 63 characters.
|
LGTM, thanks! |
j3ffml
added
the
lgtm
|
needs rebase |
|
rebased |
label: Invalidate empty or invalid value labels
No description provided.