fix(kuma-cp): meta validation compatible with Kubernetes naming rules (…

…#7976)

* allow a dot in the name
* disallow underscore in new resource names
* check the length of the name in the API (not in the store)

Signed-off-by: Ilya Lobkov <[email protected]>

UPGRADE.md

@@ -6,6 +6,20 @@ with `x.y.z` being the version you are planning to upgrade to.
 If such a section does not exist, the upgrade you want to perform
 does not have any particular instructions.
 
+## Upgrade to `2.5.x`
+
+#### More strict validation rules for resource names
+
+In order to be compatible with Kubernetes naming policy we updated the validation rules. Old rule:
+
+> Valid characters are numbers, lowercase latin letters and '-', '_' symbols.
+
+New rule:
+
+> A lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character
+
+New rule is applied for CREATE operations. The old rule is still applied for UPDATE, but this is going to change in Kuma 2.7.x or later.
+
 ## Upgrade to `2.4.x`
 
 ### Configuration change

app/kuma-dp/cmd/dataplane.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"fmt"
 	"io"
 	"os"
 
@@ -46,8 +47,12 @@ func readResource(cmd *cobra.Command, r *kuma_dp.DataplaneRuntime) (model.Resour
 		return nil, err
 	}
 
-	if err := core_mesh.ValidateMeta(res.GetMeta().GetName(), res.GetMeta().GetMesh(), res.Descriptor().Scope); err.HasViolations() {
+	if err, msg := core_mesh.ValidateMetaBackwardsCompatible(res.GetMeta(), res.Descriptor().Scope); err.HasViolations() {
 		return nil, &err
+	} else if msg != "" {
+		if _, printErr := fmt.Fprintln(cmd.ErrOrStderr(), msg); printErr != nil {
+			return nil, printErr
+		}
 	}
 
 	return res, nil

app/kumactl/cmd/apply/apply.go

@@ -116,8 +116,12 @@ $ kumactl apply -f https://example.com/resource.yaml
 				if err != nil {
 					return errors.Wrap(err, "YAML contains invalid resource")
 				}
-				if err := mesh.ValidateMeta(res.GetMeta().GetName(), res.GetMeta().GetMesh(), res.Descriptor().Scope); err.HasViolations() {
+				if err, msg := mesh.ValidateMetaBackwardsCompatible(res.GetMeta(), res.Descriptor().Scope); err.HasViolations() {
 					return err.OrNil()
+				} else if msg != "" {
+					if _, printErr := fmt.Fprintln(cmd.ErrOrStderr(), msg); printErr != nil {
+						return printErr
+					}
 				}
 				resources = append(resources, res)
 			}

pkg/api-server/resource_endpoints.go

@@ -10,7 +10,6 @@ import (
 	"github.com/emicklei/go-restful/v3"
 
 	config_core "github.com/kumahq/kuma/pkg/config/core"
-	"github.com/kumahq/kuma/pkg/core"
 	"github.com/kumahq/kuma/pkg/core/resources/access"
 	"github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	"github.com/kumahq/kuma/pkg/core/resources/apis/system"
@@ -137,7 +136,7 @@ func (r *resourceEndpoints) findResource(withInsight bool) func(request *restful
 			rest_errors.HandleError(request.Request.Context(), response, err.OrNil(), "invalid format")
 		}
 		if err := response.WriteAsJson(res); err != nil {
-			core.Log.Error(err, "Could not write the response")
+			log.Error(err, "Could not write the response")
 		}
 	}
 }
@@ -276,18 +275,21 @@ func (r *resourceEndpoints) createOrUpdateResource(request *restful.Request, res
 		return
 	}
 
-	if err := r.validateResourceRequest(request, resourceRest.GetMeta()); err != nil {
+	create := false
+	resource := r.descriptor.NewObject()
+	if err := r.resManager.Get(request.Request.Context(), resource, store.GetByKey(name, meshName)); err != nil && store.IsResourceNotFound(err) {
+		create = true
+	} else if err != nil {
+		rest_errors.HandleError(request.Request.Context(), response, err, "Could not find a resource")
+	}
+
+	if err := r.validateResourceRequest(request, resourceRest.GetMeta(), create); err != nil {
 		rest_errors.HandleError(request.Request.Context(), response, err, "Could not process a resource")
 		return
 	}
 
-	resource := r.descriptor.NewObject()
-	if err := r.resManager.Get(request.Request.Context(), resource, store.GetByKey(name, meshName)); err != nil {
-		if store.IsResourceNotFound(err) {
-			r.createResource(request.Request.Context(), name, meshName, resourceRest.GetSpec(), response)
-		} else {
-			rest_errors.HandleError(request.Request.Context(), response, err, "Could not find a resource")
-		}
+	if create {
+		r.createResource(request.Request.Context(), name, meshName, resourceRest.GetSpec(), response)
 	} else {
 		r.updateResource(request.Request.Context(), resource, resourceRest.GetSpec(), response)
 	}
@@ -382,7 +384,7 @@ func (r *resourceEndpoints) deleteResource(request *restful.Request, response *r
 	}
 }
 
-func (r *resourceEndpoints) validateResourceRequest(request *restful.Request, resourceMeta rest_v1alpha1.ResourceMeta) error {
+func (r *resourceEndpoints) validateResourceRequest(request *restful.Request, resourceMeta rest_v1alpha1.ResourceMeta, create bool) error {
 	var err validators.ValidationError
 	name := request.PathParameter("name")
 	meshName := r.meshFromRequest(request)
@@ -398,7 +400,15 @@ func (r *resourceEndpoints) validateResourceRequest(request *restful.Request, re
 	if r.descriptor.Scope == model.ScopeMesh && meshName != resourceMeta.Mesh {
 		err.AddViolation("mesh", "mesh from the URL has to be the same as in body")
 	}
-	err.AddError("", mesh.ValidateMeta(name, meshName, r.descriptor.Scope))
+	if create {
+		err.AddError("", mesh.ValidateMeta(resourceMeta, r.descriptor.Scope))
+	} else {
+		if verr, msg := mesh.ValidateMetaBackwardsCompatible(resourceMeta, r.descriptor.Scope); verr.HasViolations() {
+			err.AddError("", verr)
+		} else if msg != "" {
+			log.Info(msg, "type", r.descriptor.Name, "mesh", resourceMeta.Mesh, "name", resourceMeta.Name)
+		}
+	}
 	return err.OrNil()
 }
 

pkg/api-server/testdata/resource_name-mesh.golden.json

@@ -6,22 +6,22 @@
  "invalid_parameters": [
   {
    "field": "name",
-   "reason": "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols."
+   "reason": "invalid characters. A lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
   },
   {
    "field": "mesh",
-   "reason": "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols."
+   "reason": "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_', '.' symbols."
   }
  ],
  "details": "Resource is not valid",
  "causes": [
   {
    "field": "name",
-   "message": "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols."
+   "message": "invalid characters. A lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
   },
   {
    "field": "mesh",
-   "message": "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols."
+   "message": "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_', '.' symbols."
   }
  ]
 }

pkg/core/resources/apis/mesh/meta_validator.go

@@ -3,33 +3,73 @@ package mesh
 import (
 	"regexp"
 
-	"github.com/kumahq/kuma/pkg/core/resources/model"
+	core_model "github.com/kumahq/kuma/pkg/core/resources/model"
 	"github.com/kumahq/kuma/pkg/core/validators"
 )
 
-var nameMeshRegexp = regexp.MustCompile("^[0-9a-z-_]*$")
+var (
+	backwardCompatRegexp = regexp.MustCompile(`^[0-9a-z-_.]*$`)
+	backwardCompatErrMsg = "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_', '.' symbols."
+)
+
+var (
+	identifierRegexp = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
+	identifierErrMsg = "invalid characters. A lowercase RFC 1123 subdomain must consist of lower case alphanumeric " +
+		"characters, '-' or '.', and must start and end with an alphanumeric character"
+)
+
+func ValidateMeta(m core_model.ResourceMeta, scope core_model.ResourceScope) validators.ValidationError {
+	var err validators.ValidationError
+	err.AddError("name", validateIdentifier(m.GetName(), identifierRegexp, identifierErrMsg))
+	err.Add(ValidateMesh(m.GetMesh(), scope))
+	return err
+}
+
+// ValidateMetaBackwardsCompatible allows 'name' and 'mesh' to contain '_'. Should be called when updating existing
+// resources or on the clients (like 'kumactl' or 'kuma-dp') when it's not clear if it's create or update operation.
+//
+// We're going to remove this function in Kuma 2.7.x
+func ValidateMetaBackwardsCompatible(m core_model.ResourceMeta, scope core_model.ResourceScope) (validators.ValidationError, string) {
+	// trying to validate meta against the new (more strict) rules
+	if verr := ValidateMeta(m, scope); !verr.HasViolations() {
+		return validators.ValidationError{}, ""
+	}
 
-func ValidateMeta(name, mesh string, scope model.ResourceScope) validators.ValidationError {
 	var err validators.ValidationError
-	if name == "" {
-		err.AddViolation("name", "cannot be empty")
+	err.AddError("name", validateIdentifier(m.GetName(), backwardCompatRegexp, backwardCompatErrMsg))
+	err.Add(ValidateMesh(m.GetMesh(), scope))
+
+	if !err.HasViolations() {
+		// ResourceMeta doesn't pass new validation, but passes old validation. In that case we'd like to warn user about
+		// the new validation rules
+		return validators.ValidationError{}, "[WARNING] Kuma 2.5.x introduces more strict validation rules for resource name, it must consist of " +
+			"lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character. You can't " +
+			"CREATE resources with invalid names anymore, but it's possible to UPDATE such resources until Kuma 2.7.x is released."
 	}
-	if !nameMeshRegexp.MatchString(name) {
-		err.AddViolation("name", "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols.")
+
+	return err, ""
+}
+
+// ValidateMesh checks that resource's mesh matches the old regex (with '_'). Even if user creates entirely new resource,
+// we can't check resource's mesh against the new regex, because Mesh resource itself can be old and contain '_' in its name.
+// All new Mesh resources will have their name validated against new regex.
+func ValidateMesh(mesh string, scope core_model.ResourceScope) validators.ValidationError {
+	var err validators.ValidationError
+	if scope == core_model.ScopeMesh {
+		err.AddError("mesh", validateIdentifier(mesh, backwardCompatRegexp, backwardCompatErrMsg))
 	}
-	err.Add(ValidateMesh(mesh, scope))
 	return err
 }
 
-func ValidateMesh(mesh string, scope model.ResourceScope) validators.ValidationError {
+func validateIdentifier(identifier string, r *regexp.Regexp, errMsg string) validators.ValidationError {
 	var err validators.ValidationError
-	if scope == model.ScopeMesh {
-		if mesh == "" {
-			err.AddViolation("mesh", "cannot be empty")
-		}
-		if !nameMeshRegexp.MatchString(mesh) {
-			err.AddViolation("mesh", "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols.")
-		}
+	switch {
+	case identifier == "":
+		err.AddViolation("", "cannot be empty")
+	case len(identifier) > 253:
+		err.AddViolation("", "value length must less or equal 253")
+	case !r.MatchString(identifier):
+		err.AddViolation("", errMsg)
 	}
 	return err
 }