Skip to content

Commit f87c6b1

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 7afc54c commit f87c6b1

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
@@ -26,6 +26,7 @@ import (
2626
kerrors "k8s.io/apimachinery/pkg/api/errors"
2727
"k8s.io/apimachinery/pkg/api/resource"
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
29+
"k8s.io/apimachinery/pkg/util/validation"
2930
)
3031

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

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

4234
// Label ... 2 forms ...
@@ -56,7 +48,7 @@ func Label(request *msgs.LabelRequest, ns, pgouser string) msgs.LabelResponse {
5648
return resp
5749
}
5850

59-
labelsMap, err = validateLabel(request.LabelCmdLabel, ns)
51+
labelsMap, err = apiserver.ValidateLabel(request.LabelCmdLabel)
6052
if err != nil {
6153
resp.Status.Code = msgs.Error
6254
resp.Status.Msg = err.Error()
@@ -255,59 +247,6 @@ func PatchPgcluster(newLabels map[string]string, oldCRD crv1.Pgcluster, ns strin
255247

256248
}
257249

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

328-
labelsMap, err = validateLabel(request.LabelCmdLabel, ns)
267+
labelsMap, err = apiserver.ValidateLabel(request.LabelCmdLabel)
329268
if err != nil {
330269
resp.Status.Code = msgs.Error
331270
resp.Status.Msg = "labels not formatted correctly"

0 commit comments

Comments
 (0)