Skip to content

Commit 5b61c9c

Browse files
author
Jonathan S. Katz
committed
Refactor of label validation function
This moves the function to a common area in the API code and adds some much needed testing to it.
1 parent d8a482d commit 5b61c9c

File tree

3 files changed

+122
-63
lines changed

3 files changed

+122
-63
lines changed

‎internal/apiserver/common.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
kerrors "k8s.io/apimachinery/pkg/api/errors"
2828
"k8s.io/apimachinery/pkg/api/resource"
2929
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/apimachinery/pkg/util/validation"
3031
)
3132

3233
const (
@@ -45,6 +46,8 @@ var (
4546
// ErrDBContainerNotFound is an error that indicates that a "database" container
4647
// could not be found in a specific pod
4748
ErrDBContainerNotFound = errors.New("\"database\" container not found in pod")
49+
// ErrLabelInvalid indicates that a label is invalid
50+
ErrLabelInvalid = errors.New("invalid label")
4851
// ErrStandbyNotAllowed contains the error message returned when an API call is not
4952
// permitted because it involves a cluster that is in standby mode
5053
ErrStandbyNotAllowed = errors.New("Action not permitted because standby mode is enabled")
@@ -110,6 +113,60 @@ func IsValidPVC(pvcName, ns string) bool {
110113
return pvc != nil
111114
}
112115

116+
// ValidateLabel is derived from a legacy method and validates if the input is a
117+
// valid Kubernetes label.
118+
//
119+
// A label is composed of a key and value.
120+
//
121+
// The key can either be a name or have an optional prefix that i
122+
// terminated by a "/", e.g. "prefix/name"
123+
//
124+
// The name must be a valid DNS 1123 value
125+
// THe prefix must be a valid DNS 1123 subdomain
126+
//
127+
// The value can be validated by machinery provided by Kubenretes
128+
//
129+
// Ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
130+
func ValidateLabel(labelStr string) (map[string]string, error) {
131+
labelMap := map[string]string{}
132+
133+
for _, v := range strings.Split(labelStr, ",") {
134+
pair := strings.Split(v, "=")
135+
if len(pair) != 2 {
136+
return labelMap, fmt.Errorf("%w: label format incorrect, requires key=value", ErrLabelInvalid)
137+
}
138+
139+
// first handle the key
140+
keyParts := strings.Split(pair[0], "/")
141+
142+
switch len(keyParts) {
143+
default:
144+
return labelMap, fmt.Errorf("%w: invalid key for "+v, ErrLabelInvalid)
145+
case 2:
146+
if errs := validation.IsDNS1123Subdomain(keyParts[0]); len(errs) > 0 {
147+
return labelMap, fmt.Errorf("%w: invalid key for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
148+
}
149+
150+
if errs := validation.IsDNS1123Label(keyParts[1]); len(errs) > 0 {
151+
return labelMap, fmt.Errorf("%w: invalid key for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
152+
}
153+
case 1:
154+
if errs := validation.IsDNS1123Label(keyParts[0]); len(errs) > 0 {
155+
return labelMap, fmt.Errorf("%w: invalid key for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
156+
}
157+
}
158+
159+
// handle the value
160+
if errs := validation.IsValidLabelValue(pair[1]); len(errs) > 0 {
161+
return labelMap, fmt.Errorf("%w: invalid value for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
162+
}
163+
164+
labelMap[pair[0]] = pair[1]
165+
}
166+
167+
return labelMap, nil
168+
}
169+
113170
// ValidateResourceRequestLimit validates that a Kubernetes Requests/Limit pair
114171
// is valid, both by validating the values are valid quantity values, and then
115172
// by checking that the limit >= request. This also needs to check against the

‎internal/apiserver/common_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,74 @@ limitations under the License.
1616
*/
1717

1818
import (
19+
"errors"
20+
"fmt"
21+
"strings"
1922
"testing"
2023

2124
"k8s.io/apimachinery/pkg/api/resource"
2225
)
2326

27+
func TestValidateLabel(t *testing.T) {
28+
t.Run("valid", func(t *testing.T) {
29+
inputs := []map[string]string{
30+
map[string]string{"key": "value"},
31+
map[string]string{"example.com/key": "value"},
32+
map[string]string{"key1": "value1", "key2": "value2"},
33+
}
34+
35+
for _, input := range inputs {
36+
labelStr := ""
37+
38+
for k, v := range input {
39+
labelStr += fmt.Sprintf("%s=%s,", k, v)
40+
}
41+
42+
labelStr = strings.Trim(labelStr, ",")
43+
44+
t.Run(labelStr, func(*testing.T) {
45+
labels, err := ValidateLabel(labelStr)
46+
47+
if err != nil {
48+
t.Fatalf("expected no error, got: %s", err.Error())
49+
}
50+
51+
for k := range labels {
52+
if v, ok := input[k]; !(ok || v == labels[k]) {
53+
t.Fatalf("label values do not matched (%s vs. %s)", input[k], labels[k])
54+
}
55+
}
56+
})
57+
}
58+
})
59+
60+
t.Run("invalid", func(t *testing.T) {
61+
inputs := []string{
62+
"key",
63+
"key=value=value",
64+
"key=value,",
65+
"b@d=value",
66+
"b@d-prefix/key=value",
67+
"really/bad/prefix/key=value",
68+
"key=v\\alue",
69+
}
70+
71+
for _, input := range inputs {
72+
t.Run(input, func(t *testing.T) {
73+
_, err := ValidateLabel(input)
74+
75+
if err == nil {
76+
t.Fatalf("expected an invalid input error.")
77+
}
78+
79+
if !errors.Is(err, ErrLabelInvalid) {
80+
t.Fatalf("expected an ErrLabelInvalid error.")
81+
}
82+
})
83+
}
84+
})
85+
}
86+
2487
func TestValidateResourceRequestLimit(t *testing.T) {
2588
t.Run("valid", func(t *testing.T) {
2689
resources := []struct{ request, limit, defaultRequest string }{

‎internal/apiserver/labelservice/labelimpl.go‎

Lines changed: 2 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,7 @@ limitations under the License.
1616
*/
1717

1818
import (
19-
<<<<<<< HEAD
2019
"encoding/json"
21-
"errors"
22-
=======
23-
"context"
24-
"fmt"
25-
>>>>>>> 5c56a341d... Update label validation to match Kubernetes rules
26-
"strings"
2720

2821
"github.com/crunchydata/postgres-operator/internal/apiserver"
2922
"github.com/crunchydata/postgres-operator/internal/config"
@@ -37,7 +30,6 @@ import (
3730
"k8s.io/apimachinery/pkg/api/meta"
3831
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3932
"k8s.io/apimachinery/pkg/types"
40-
"k8s.io/apimachinery/pkg/util/validation"
4133
)
4234

4335
// Label ... 2 forms ...
@@ -57,7 +49,7 @@ func Label(request *msgs.LabelRequest, ns, pgouser string) msgs.LabelResponse {
5749
return resp
5850
}
5951

60-
labelsMap, err = validateLabel(request.LabelCmdLabel, ns)
52+
labelsMap, err = apiserver.ValidateLabel(request.LabelCmdLabel)
6153
if err != nil {
6254
resp.Status.Code = msgs.Error
6355
resp.Status.Msg = err.Error()
@@ -264,59 +256,6 @@ func PatchPgcluster(newLabels map[string]string, oldCRD crv1.Pgcluster, ns strin
264256

265257
}
266258

267-
// validateLabel validates if the input is a valid Kubernetes label
268-
//
269-
// A label is composed of a key and value.
270-
//
271-
// The key can either be a name or have an optional prefix that i
272-
// terminated by a "/", e.g. "prefix/name"
273-
//
274-
// The name must be a valid DNS 1123 value
275-
// THe prefix must be a valid DNS 1123 subdomain
276-
//
277-
// The value can be validated by machinery provided by Kubenretes
278-
//
279-
// Ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
280-
func validateLabel(LabelCmdLabel string) (map[string]string, error) {
281-
labelMap := map[string]string{}
282-
283-
for _, v := range strings.Split(LabelCmdLabel, ",") {
284-
pair := strings.Split(v, "=")
285-
if len(pair) != 2 {
286-
return labelMap, fmt.Errorf("label format incorrect, requires key=value")
287-
}
288-
289-
// first handle the key
290-
keyParts := strings.Split(pair[0], "/")
291-
292-
switch len(keyParts) {
293-
default:
294-
return labelMap, fmt.Errorf("invalid key for " + v)
295-
case 2:
296-
if errs := validation.IsDNS1123Subdomain(keyParts[0]); len(errs) > 0 {
297-
return labelMap, fmt.Errorf("invalid key for %s: %s", v, strings.Join(errs, ","))
298-
}
299-
300-
if errs := validation.IsDNS1123Label(keyParts[1]); len(errs) > 0 {
301-
return labelMap, fmt.Errorf("invalid key for %s: %s", v, strings.Join(errs, ","))
302-
}
303-
case 1:
304-
if errs := validation.IsDNS1123Label(keyParts[0]); len(errs) > 0 {
305-
return labelMap, fmt.Errorf("invalid key for %s: %s", v, strings.Join(errs, ","))
306-
}
307-
}
308-
309-
// handle the value
310-
if errs := validation.IsValidLabelValue(pair[1]); len(errs) > 0 {
311-
return labelMap, fmt.Errorf("invalid value for %s: %s", v, strings.Join(errs, ","))
312-
}
313-
314-
labelMap[pair[0]] = pair[1]
315-
}
316-
317-
return labelMap, nil
318-
}
319-
320259
// DeleteLabel ...
321260
// pgo delete label mycluster yourcluster --label=env=prod
322261
// pgo delete label --label=env=prod --selector=group=somegroup
@@ -334,7 +273,7 @@ func DeleteLabel(request *msgs.DeleteLabelRequest, ns string) msgs.LabelResponse
334273
return resp
335274
}
336275

337-
labelsMap, err = validateLabel(request.LabelCmdLabel, ns)
276+
labelsMap, err = apiserver.ValidateLabel(request.LabelCmdLabel)
338277
if err != nil {
339278
resp.Status.Code = msgs.Error
340279
resp.Status.Msg = "labels not formatted correctly"

0 commit comments

Comments
 (0)