fix(kuma-cp): allow names of the resource to be longer and validate t…

…he length (#6123)

Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.com>
Co-authored-by: Bart Smykla <bartek@smykla.com>

UPGRADE.md

@@ -14,6 +14,11 @@ We now support version `v0.6.0` of the Gateway API. See the [upstream API
 changes](https://github.com/kubernetes-sigs/gateway-api/releases/tag/v0.6.0) for
 more info.
 
+### Longer name of the resource in postgres
+
+Kuma now permits the creation of a resource with a name of up to 253 characters, which is an increase from the previous limit of 100 characters. This adjustment brings our system in line with the naming convention supported by Kubernetes.
+This change requires to run `kuma-cp migrate up` to apply changes to the postgres database.
+
 ### Auth configuration of DP server in Kuma CP
 
 `dpServer.auth` configuration of Kuma CP was deprecated. You can still set config in this section, but it will be removed in the future.

pkg/api-server/resource_endpoints.go

@@ -34,6 +34,7 @@ const (
 
 type resourceEndpoints struct {
 	mode           config_core.CpMode
+	zoneName       string
 	resManager     manager.ResourceManager
 	descriptor     model.ResourceTypeDescriptor
 	resourceAccess access.ResourceAccess
@@ -259,10 +260,12 @@ func (r *resourceEndpoints) validateResourceRequest(request *restful.Request, re
 	var err validators.ValidationError
 	name := request.PathParameter("name")
 	meshName := r.meshFromRequest(request)
-
 	if name != resourceMeta.Name {
 		err.AddViolation("name", "name from the URL has to be the same as in body")
 	}
+	if r.mode == config_core.Zone && !r.doesNameLengthFitsGlobal(name) {
+		err.AddViolation("name", "the length of the name must be shorter")
+	}
 	if string(r.descriptor.Name) != resourceMeta.Type {
 		err.AddViolation("type", "type from the URL has to be the same as in body")
 	}
@@ -273,6 +276,16 @@ func (r *resourceEndpoints) validateResourceRequest(request *restful.Request, re
 	return err.OrNil()
 }
 
+// The resource is prefixed with the zone name and suffixed with the namespace
+// (if the K8s store is used in the global control-plane) when it is synchronized
+// to global control-plane. It is important to notice that the zone is unaware
+// of the type of the store used by the global control-plane, so we must prepare
+// for the worst-case scenario. We don't have to check other plugabble policies
+// because zone doesn't allow to create policies on the zone.
+func (r *resourceEndpoints) doesNameLengthFitsGlobal(name string) bool {
+	return len(fmt.Sprintf("%s.%s.%s", r.zoneName, name, "default")) < 253
+}
+
 func (r *resourceEndpoints) meshFromRequest(request *restful.Request) string {
 	if r.descriptor.Scope == model.ScopeMesh {
 		return request.PathParameter("mesh")

pkg/api-server/resource_endpoints_test.go

@@ -22,6 +22,122 @@ import (
 	test_metrics "github.com/kumahq/kuma/pkg/test/metrics"
 )
 
+var _ = Describe("Resource Endpoints Zone", func() {
+	var apiServer *api_server.ApiServer
+	var resourceStore store.ResourceStore
+	var client resourceApiClient
+	var stop = func() {}
+
+	const mesh = "default"
+
+	BeforeEach(func() {
+		resourceStore = store.NewPaginationStore(memory.NewStore())
+		apiServer, _, stop = StartApiServer(
+			NewTestApiServerConfigurer().
+				WithStore(resourceStore).
+				WithZone("custom-long-zone-name-to-check-if-name-length-check-works"),
+		)
+		client = resourceApiClient{
+			address: apiServer.Address(),
+			path:    "/meshes/" + mesh + "/dataplanes",
+		}
+	})
+
+	AfterEach(func() {
+		stop()
+	})
+
+	BeforeEach(func() {
+		// create default mesh
+		err := resourceStore.Create(context.Background(), core_mesh.NewMeshResource(), store.CreateByKey(mesh, model.NoMesh))
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	Describe("On PUT", func() {
+		It("should create a resource when one does not exist", func() {
+			// given
+			res := &unversioned.Resource{
+				Meta: rest_v1alpha1.ResourceMeta{
+					Name: "new-resource",
+					Mesh: mesh,
+					Type: string(core_mesh.DataplaneType),
+				},
+				Spec: &mesh_proto.Dataplane{
+					Networking: &mesh_proto.Dataplane_Networking{
+						Address:           "192.168.1.1",
+						AdvertisedAddress: "192.168.1.1",
+						Inbound: []*mesh_proto.Dataplane_Networking_Inbound{
+							{
+								Port:        8000,
+								ServicePort: 8080,
+								Tags: map[string]string{
+									mesh_proto.ServiceTag: "service-1",
+								},
+							},
+						},
+					},
+				},
+			}
+
+			// when
+			response := client.put(res)
+
+			// then
+			Expect(response.StatusCode).To(Equal(201))
+		})
+
+		It("should return 400 when resource name is too long", func() {
+			// given
+			res := &unversioned.Resource{
+				Meta: rest_v1alpha1.ResourceMeta{
+					Name: "super-long-resource-name-that-exceed-the-length-of-the-resource" +
+						"-that-can-be-created-on-the-global-control-plane" +
+						"we-want-to-check-if-resource-prefixed-with-zone-and-suffixed-with-namespace" +
+						"can-fit-into-global-control-plane",
+					Mesh: mesh,
+					Type: string(core_mesh.DataplaneType),
+				},
+				Spec: &mesh_proto.Dataplane{
+					Networking: &mesh_proto.Dataplane_Networking{
+						Address:           "192.168.1.1",
+						AdvertisedAddress: "192.168.1.1",
+						Inbound: []*mesh_proto.Dataplane_Networking_Inbound{
+							{
+								Port:        8000,
+								ServicePort: 8080,
+								Tags: map[string]string{
+									mesh_proto.ServiceTag: "service-1",
+								},
+							},
+						},
+					},
+				},
+			}
+
+			// when
+			response := client.put(res)
+
+			// then
+			Expect(response.StatusCode).To(Equal(400))
+
+			// and
+			bytes, err := io.ReadAll(response.Body)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(bytes).To(MatchJSON(`
+			{
+				"title": "Could not process a resource",
+				"details": "Resource is not valid",
+				"causes": [
+					{
+						"field": "name",
+						"message": "the length of the name must be shorter"
+					}
+				]
+			  }`))
+		})
+	})
+})
+
 var _ = Describe("Resource Endpoints", func() {
 	var apiServer *api_server.ApiServer
 	var resourceStore store.ResourceStore

pkg/api-server/server.go

@@ -242,6 +242,9 @@ func addResourcesEndpoints(ws *restful.WebService, defs []model.ResourceTypeDesc
 			descriptor:     definition,
 			resourceAccess: resourceAccess,
 		}
+		if cfg.Mode == config_core.Zone && cfg.Multizone != nil && cfg.Multizone.Zone != nil {
+			endpoints.zoneName = cfg.Multizone.Zone.Name
+		}
 		switch defType {
 		case mesh.ServiceInsightType:
 			// ServiceInsight is a bit different

pkg/plugins/resources/postgres/migrate_test.go

@@ -26,14 +26,14 @@ var _ = Describe("Migrate", func() {
 
 		// then
 		Expect(err).ToNot(HaveOccurred())
-		Expect(ver).To(Equal(plugins.DbVersion(1610445956)))
+		Expect(ver).To(Equal(plugins.DbVersion(1677096751)))
 
 		// and when migrating again
 		ver, err = migrateDb(cfg)
 
 		// then
 		Expect(err).To(Equal(plugins.AlreadyMigrated))
-		Expect(ver).To(Equal(plugins.DbVersion(1610445956)))
+		Expect(ver).To(Equal(plugins.DbVersion(1677096751)))
 	})
 
 	It("should throw an error when trying to run migrations on newer migration version of DB than in Kuma", func() {
@@ -51,7 +51,7 @@ var _ = Describe("Migrate", func() {
 		_, err = migrateDb(cfg)
 
 		// then
-		Expect(err).To(MatchError("DB is migrated to newer version than Kuma. DB migration version 9999999999. Kuma migration version 1610445956. Run newer version of Kuma"))
+		Expect(err).To(MatchError("DB is migrated to newer version than Kuma. DB migration version 9999999999. Kuma migration version 1677096751. Run newer version of Kuma"))
 	})
 
 	It("should indicate if db is migrated", func() {

pkg/plugins/resources/postgres/migrations/data/1677096751_update_resources_name_limit.up.sql

@@ -0,0 +1,2 @@
+ALTER TABLE resources 
+    ALTER COLUMN name TYPE varchar(253);