Merge pull request #132 from vshn/resourceNames

block too long db names early
vshn · Feb 15, 2024 · 97e7166 · 97e7166
2 parents e242edf + d5eed3a
commit 97e7166
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 12 deletions.
diff --git a/crds/stackgres.io_sgpoolingconfigs.yaml b/crds/stackgres.io_sgpoolingconfigs.yaml
@@ -32,7 +32,7 @@ spec:
           metadata:
             type: object
           spec:
-            description: Spec defines the desired state of a VSHNPostgreSQL.
+            description: Spec contains the custom configurations for the pgbouncer.
             properties:
               pgBouncer:
                 description: Connection pooling configuration based on PgBouncer.
@@ -72,7 +72,7 @@ spec:
                 type: object
             type: object
           status:
-            description: Status reflects the observed state of a VSHNPostgreSQL.
+            description: Status contains the default settings for the pgbouncer.
             properties:
               pgBouncer:
                 description: Connection pooling configuration status based on PgBouncer.

diff --git a/pkg/controller/webhooks/postgresql.go b/pkg/controller/webhooks/postgresql.go
@@ -82,6 +82,17 @@ func (p *PostgreSQLWebhookHandler) ValidateCreate(ctx context.Context, obj runti
 
 	allErrs = append(allErrs, instancesError...)
 
+	err := p.validateResourceNameLength(pg.GetName())
+	if err != nil {
+		allErrs = append(allErrs, &field.Error{
+			Field: ".metadata.name",
+			Detail: fmt.Sprintf("Please shorten PostgreSQL name, currently it is: %s",
+				err.Error()),
+			BadValue: pg.GetName(),
+			Type:     field.ErrorTypeTooLong,
+		})
+	}
+
 	if len(allErrs) != 0 {
 		return nil, apierrors.NewInvalid(
 			pgGK,
@@ -123,6 +134,17 @@ func (p *PostgreSQLWebhookHandler) ValidateUpdate(ctx context.Context, oldObj, n
 
 	allErrs = append(allErrs, instancesError...)
 
+	err := p.validateResourceNameLength(pg.GetName())
+	if err != nil {
+		allErrs = append(allErrs, &field.Error{
+			Field: ".metadata.name",
+			Detail: fmt.Sprintf("Please shorten PostgreSQL name, currently it is: %s",
+				err.Error()),
+			BadValue: pg.GetName(),
+			Type:     field.ErrorTypeTooLong,
+		})
+	}
+
 	// We aggregate and return all errors at the same time.
 	// So the user is aware of all broken parameters.
 	// But at the same time, if any of these fail we cannot do proper quota checks anymore.
@@ -251,3 +273,12 @@ func (p *PostgreSQLWebhookHandler) checkGuaranteedAvailability(ctx context.Conte
 	}
 	return fieldErrs
 }
+
+// k8s limitation is 52 characters, our longest postfix we add is 15 character, therefore 37 chracters is the maximum length
+// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
+func (r *PostgreSQLWebhookHandler) validateResourceNameLength(name string) error {
+	if len(name) > 37 {
+		return fmt.Errorf("name is too long: %d. We add various postfixes and CronJob name length has it's own limitations: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names", len(name))
+	}
+	return nil
+}
diff --git a/pkg/controller/webhooks/redis.go b/pkg/controller/webhooks/redis.go
@@ -47,41 +47,96 @@ func SetupRedisWebhookHandlerWithManager(mgr ctrl.Manager, withQuota bool) error
 
 // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type
 func (r *RedisWebhookHandler) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
-
+	allErrs := field.ErrorList{}
 	redis, ok := obj.(*vshnv1.VSHNRedis)
 	if !ok {
-		return nil, fmt.Errorf("Provided manifest is not a valid VSHNRedis object")
+		return nil, fmt.Errorf("provided manifest is not a valid VSHNRedis object")
 	}
 
 	if r.withQuota {
-		err := r.checkQuotas(ctx, redis, true)
-		if err != nil {
-			return nil, err
+		quotaErrs := r.checkQuotas(ctx, redis, true)
+		if quotaErrs != nil {
+			allErrs = append(allErrs, &field.Error{
+				Field: "quota",
+				Detail: fmt.Sprintf("quota check failed: %s",
+					quotaErrs.Error()),
+				BadValue: "*your namespace quota*",
+				Type:     field.ErrorTypeForbidden,
+			})
 		}
 	}
+	err := r.validateResourceNameLength(redis.GetName())
+	if err != nil {
+		allErrs = append(allErrs, &field.Error{
+			Field: ".metadata.name",
+			Detail: fmt.Sprintf("Please shorten Redis name, currently it is: %s",
+				err.Error()),
+			BadValue: redis.GetName(),
+			Type:     field.ErrorTypeTooLong,
+		})
+	}
+
+	// We aggregate and return all errors at the same time.
+	// So the user is aware of all broken parameters.
+	// But at the same time, if any of these fail we cannot do proper quota checks anymore.
+	if len(allErrs) != 0 {
+		return nil, apierrors.NewInvalid(
+			redisGK,
+			redis.GetName(),
+			allErrs,
+		)
+	}
 
 	return nil, nil
 }
 
 // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type
 func (r *RedisWebhookHandler) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
-
+	allErrs := field.ErrorList{}
 	redis, ok := newObj.(*vshnv1.VSHNRedis)
 	if !ok {
-		return nil, fmt.Errorf("Provided manifest is not a valid VSHNRedis object")
+		return nil, fmt.Errorf("provided manifest is not a valid VSHNRedis object")
 	}
 
 	if redis.DeletionTimestamp != nil {
 		return nil, nil
 	}
 
 	if r.withQuota {
-		err := r.checkQuotas(ctx, redis, false)
-		if err != nil {
-			return nil, err
+		quotaErrs := r.checkQuotas(ctx, redis, true)
+		if quotaErrs != nil {
+			allErrs = append(allErrs, &field.Error{
+				Field: "quota",
+				Detail: fmt.Sprintf("quota check failed: %s",
+					quotaErrs.Error()),
+				BadValue: "*your namespace quota*",
+				Type:     field.ErrorTypeForbidden,
+			})
 		}
 	}
 
+	err := r.validateResourceNameLength(redis.GetName())
+	if err != nil {
+		allErrs = append(allErrs, &field.Error{
+			Field: ".metadata.name",
+			Detail: fmt.Sprintf("Please shorten Redis name, currently it is: %s",
+				err.Error()),
+			BadValue: redis.GetName(),
+			Type:     field.ErrorTypeTooLong,
+		})
+	}
+
+	// We aggregate and return all errors at the same time.
+	// So the user is aware of all broken parameters.
+	// But at the same time, if any of these fail we cannot do proper quota checks anymore.
+	if len(allErrs) != 0 {
+		return nil, apierrors.NewInvalid(
+			redisGK,
+			redis.GetName(),
+			allErrs,
+		)
+	}
+
 	return nil, nil
 }
 
@@ -183,3 +238,12 @@ func (r *RedisWebhookHandler) addPathsToResources(res *utils.Resources) {
 	res.MemoryRequestsPath = basePath.Child("memoryLimits")
 	res.DiskPath = basePath.Child("disk")
 }
+
+// k8s limitation is 52 characters, our longest postfix we add is 15 character, therefore 37 chracters is the maximum length
+// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
+func (r *RedisWebhookHandler) validateResourceNameLength(name string) error {
+	if len(name) > 37 {
+		return fmt.Errorf("%d/37 chars.\n\tWe add various postfixes and CronJob name length has it's own limitations: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names", len(name))
+	}
+	return nil
+}