Validate NNCP name like a label value (#757)

The NNCP name is used to label the NNCE so the NNCP name has the same restrictions as any other label. This change add a webhook validation for that so kubernetes-nmstate do a fail fast at this issue. Signed-off-by: Quique Llorente <[email protected]>
nmstate · May 26, 2021 · 3801c53 · 3801c53
1 parent b2bccf9
commit 3801c53
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 6 deletions.
diff --git a/pkg/webhook/nodenetworkconfigurationpolicy/handler.go b/pkg/webhook/nodenetworkconfigurationpolicy/handler.go
@@ -7,6 +7,7 @@ import (
 	"net/http"
 
 	"github.com/pkg/errors"
+	admissionv1 "k8s.io/api/admission/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -47,7 +48,7 @@ func mutatePolicyHandler(neededMutationFor func(nmstatev1beta1.NodeNetworkConfig
 	}
 }
 
-func validatePolicyHandler(cli client.Client, neededValidationFor func(nmstatev1beta1.NodeNetworkConfigurationPolicy, nmstatev1beta1.NodeNetworkConfigurationPolicy) bool, validators ...validator) admission.HandlerFunc {
+func validatePolicyHandler(cli client.Client, neededValidationFor func(admissionv1.Operation, nmstatev1beta1.NodeNetworkConfigurationPolicy, nmstatev1beta1.NodeNetworkConfigurationPolicy) bool, validators ...validator) admission.HandlerFunc {
 	log := logf.Log.WithName("webhook/nodenetworkconfigurationpolicy/validator")
 	return func(ctx context.Context, req webhook.AdmissionRequest) webhook.AdmissionResponse {
 		original := req.Object.Raw
@@ -64,7 +65,7 @@ func validatePolicyHandler(cli client.Client, neededValidationFor func(nmstatev1
 			return admission.Errored(http.StatusInternalServerError, errors.Wrapf(err, errMsg))
 		}
 
-		if !neededValidationFor(policy, currentPolicy) {
+		if !neededValidationFor(req.Operation, policy, currentPolicy) {
 			return admission.Allowed("validation not needed")
 		}
 

diff --git a/pkg/webhook/nodenetworkconfigurationpolicy/validation.go b/pkg/webhook/nodenetworkconfigurationpolicy/validation.go
@@ -5,6 +5,7 @@ import (
 	"reflect"
 	"strings"
 
+	admissionv1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -15,10 +16,14 @@ import (
 	nmstatev1beta1 "github.com/nmstate/kubernetes-nmstate/api/v1beta1"
 )
 
-func onPolicySpecChange(policy nmstatev1beta1.NodeNetworkConfigurationPolicy, currentPolicy nmstatev1beta1.NodeNetworkConfigurationPolicy) bool {
+func onPolicySpecChange(operation admissionv1.Operation, policy nmstatev1beta1.NodeNetworkConfigurationPolicy, currentPolicy nmstatev1beta1.NodeNetworkConfigurationPolicy) bool {
 	return !reflect.DeepEqual(policy.Spec, currentPolicy.Spec)
 }
 
+func onCreate(operation admissionv1.Operation, policy nmstatev1beta1.NodeNetworkConfigurationPolicy, currentPolicy nmstatev1beta1.NodeNetworkConfigurationPolicy) bool {
+	return operation == admissionv1.Create
+}
+
 func validatePolicyNotInProgressHook(policy nmstatev1beta1.NodeNetworkConfigurationPolicy, currentPolicy nmstatev1beta1.NodeNetworkConfigurationPolicy) []metav1.StatusCause {
 	causes := []metav1.StatusCause{}
 	currentPolicyAvailableCondition := currentPolicy.Status.Conditions.Find(shared.NodeNetworkConfigurationPolicyConditionAvailable)
@@ -60,15 +65,33 @@ func validatePolicyNodeSelector(policy nmstatev1beta1.NodeNetworkConfigurationPo
 	return causes
 }
 
+func validatePolicyName(policy nmstatev1beta1.NodeNetworkConfigurationPolicy, currentPolicy nmstatev1beta1.NodeNetworkConfigurationPolicy) []metav1.StatusCause {
+	causes := []metav1.StatusCause{}
+	validationErrors := validation.IsValidLabelValue(policy.Name)
+	if len(validationErrors) > 0 {
+		causes = append(causes, metav1.StatusCause{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: fmt.Sprintf("invalid policy name: %q: %s", policy.Name, strings.Join(validationErrors, "; ")),
+			Field:   "name",
+		})
+	}
+	return causes
+}
+
 func validatePolicyUpdateHook(cli client.Client) *webhook.Admission {
 	return &webhook.Admission{
-		Handler: admission.HandlerFunc(
-			validatePolicyHandler(
+		Handler: admission.MultiValidatingHandler(
+			admission.HandlerFunc(validatePolicyHandler(
 				cli,
 				onPolicySpecChange,
 				validatePolicyNotInProgressHook,
 				validatePolicyNodeSelector,
-			),
+			)),
+			admission.HandlerFunc(validatePolicyHandler(
+				cli,
+				onCreate,
+				validatePolicyName,
+			)),
 		),
 	}
 }
diff --git a/pkg/webhook/nodenetworkconfigurationpolicy/validation_test.go b/pkg/webhook/nodenetworkconfigurationpolicy/validation_test.go
@@ -107,5 +107,23 @@ var _ = Describe("NNCP Conditions Validation Admission Webhook", func() {
 			validationFn:     validatePolicyNodeSelector,
 			validationResult: []metav1.StatusCause{},
 		}),
+		Entry("policy has name with length beyond the limit", ValidationWebhookCase{
+			policy:       nmstatev1beta1.NodeNetworkConfigurationPolicy{ObjectMeta: metav1.ObjectMeta{Name: "this-is-longer-than-sixty-three-characters-hostname-bar-bar-bar.foo.com"}},
+			validationFn: validatePolicyName,
+			validationResult: []metav1.StatusCause{{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: "invalid policy name: \"this-is-longer-than-sixty-three-characters-hostname-bar-bar-bar.foo.com\": must be no more than 63 characters",
+				Field:   "name",
+			}},
+		}),
+		Entry("policy has name with invalid format", ValidationWebhookCase{
+			policy:       nmstatev1beta1.NodeNetworkConfigurationPolicy{ObjectMeta: metav1.ObjectMeta{Name: "foo+bar"}},
+			validationFn: validatePolicyName,
+			validationResult: []metav1.StatusCause{{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: "invalid policy name: \"foo+bar\": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')",
+				Field:   "name",
+			}},
+		}),
 	)
 })