From 58bf9398a780e8d3365585952807000b68949602 Mon Sep 17 00:00:00 2001 From: Marcell Sevcsik <31651557+0sewa0@users.noreply.github.com> Date: Fri, 17 Nov 2023 09:17:49 +0100 Subject: [PATCH] Add validation for CR name length (#2375) --- pkg/api/v1alpha1/edgeconnect/properties.go | 8 ++- pkg/api/v1beta1/dynakube/properties.go | 4 ++ pkg/webhook/validation/dynakube/config.go | 1 + .../validation/dynakube/dynakube_name.go | 14 ++++- .../validation/dynakube/dynakube_name_test.go | 52 ++++++++++++++++ .../validation/edgeconnect/api_server.go | 2 +- pkg/webhook/validation/edgeconnect/config.go | 3 +- pkg/webhook/validation/edgeconnect/name.go | 21 +++++++ .../validation/edgeconnect/name_test.go | 60 +++++++++++++++++++ 9 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 pkg/webhook/validation/edgeconnect/name.go create mode 100644 pkg/webhook/validation/edgeconnect/name_test.go diff --git a/pkg/api/v1alpha1/edgeconnect/properties.go b/pkg/api/v1alpha1/edgeconnect/properties.go index 1d82a8d1f5..e6ec48db39 100644 --- a/pkg/api/v1alpha1/edgeconnect/properties.go +++ b/pkg/api/v1alpha1/edgeconnect/properties.go @@ -8,7 +8,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const defaultEdgeConnectRepository = "docker.io/dynatrace/edgeconnect" +const ( + // MaxNameLength is the maximum length of a EdgeConnect's name, we tend to add suffixes to the name to avoid name collisions for resources related to the EdgeConnect. + // The limit is necessary because kubernetes uses the name of some resources for the label value, which has a limit of 63 characters. (see https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set) + MaxNameLength = 40 + + defaultEdgeConnectRepository = "docker.io/dynatrace/edgeconnect" +) func (edgeConnect *EdgeConnect) Image() string { repository := defaultEdgeConnectRepository diff --git a/pkg/api/v1beta1/dynakube/properties.go b/pkg/api/v1beta1/dynakube/properties.go index a7de3fdec7..0804f44294 100644 --- a/pkg/api/v1beta1/dynakube/properties.go +++ b/pkg/api/v1beta1/dynakube/properties.go @@ -29,6 +29,10 @@ import ( ) const ( + // MaxNameLength is the maximum length of a DynaKube's name, we tend to add suffixes to the name to avoid name collisions for resources related to the DynaKube. (example: dkName-activegate-) + // The limit is necessary because kubernetes uses the name of some resources (ActiveGate StatefulSet) for the label value, which has a limit of 63 characters. (see https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set) + MaxNameLength = 40 + // PullSecretSuffix is the suffix appended to the DynaKube name to n. PullSecretSuffix = "-pull-secret" ActiveGateTenantSecretSuffix = "-activegate-tenant-secret" diff --git a/pkg/webhook/validation/dynakube/config.go b/pkg/webhook/validation/dynakube/config.go index 08baa1bf9f..c4483d635b 100644 --- a/pkg/webhook/validation/dynakube/config.go +++ b/pkg/webhook/validation/dynakube/config.go @@ -34,6 +34,7 @@ var validators = []validator{ conflictingOneAgentVolumeStorageSettings, invalidSyntheticNodeType, nameViolatesDNS1035, + nameTooLong, namespaceSelectorMatchLabelsViolateLabelSpec, } diff --git a/pkg/webhook/validation/dynakube/dynakube_name.go b/pkg/webhook/validation/dynakube/dynakube_name.go index f3f1adb4ff..bfe8d3302d 100644 --- a/pkg/webhook/validation/dynakube/dynakube_name.go +++ b/pkg/webhook/validation/dynakube/dynakube_name.go @@ -2,6 +2,7 @@ package dynakube import ( "context" + "fmt" dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta1/dynakube" "k8s.io/apimachinery/pkg/util/validation" @@ -11,9 +12,12 @@ const ( errorNoDNS1053Label = `The DynaKube's specification violates DNS-1035. [a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')] ` + + errorNameTooLong = `The length limit for the name of a DynaKube is %d, because it is the base for the name of resources related to the DynaKube. (example: dkName-activegate-) + The limit is necessary because kubernetes uses the name of some resources (example: StatefulSet) for the label value, which has a limit of 63 characters. (see https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set)` ) -func nameViolatesDNS1035(_ context.Context, dv *dynakubeValidator, dynakube *dynatracev1beta1.DynaKube) string { +func nameViolatesDNS1035(_ context.Context, _ *dynakubeValidator, dynakube *dynatracev1beta1.DynaKube) string { dynakubeName := dynakube.Name var errs []string if dynakubeName != "" { @@ -25,3 +29,11 @@ func nameViolatesDNS1035(_ context.Context, dv *dynakubeValidator, dynakube *dyn } return errorNoDNS1053Label } + +func nameTooLong(_ context.Context, _ *dynakubeValidator, dynakube *dynatracev1beta1.DynaKube) string { + dynakubeName := dynakube.Name + if dynakubeName != "" && len(dynakubeName) > dynatracev1beta1.MaxNameLength { + return fmt.Sprintf(errorNameTooLong, dynatracev1beta1.MaxNameLength) + } + return "" +} diff --git a/pkg/webhook/validation/dynakube/dynakube_name_test.go b/pkg/webhook/validation/dynakube/dynakube_name_test.go index c8d61ef2a1..d9d6a31b75 100644 --- a/pkg/webhook/validation/dynakube/dynakube_name_test.go +++ b/pkg/webhook/validation/dynakube/dynakube_name_test.go @@ -1,6 +1,8 @@ package dynakube import ( + "fmt" + "strings" "testing" dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta1/dynakube" @@ -24,3 +26,53 @@ func TestNameStartsWithDigit(t *testing.T) { }) }) } + +func TestNameTooLong(t *testing.T) { + type testCase struct { + name string + crNameLength int + allow bool + } + + testCases := []testCase{ + { + name: "normal length", + crNameLength: 10, + allow: true, + }, + { + name: "max - 1 ", + crNameLength: dynatracev1beta1.MaxNameLength - 1, + allow: true, + }, + { + name: "max", + crNameLength: dynatracev1beta1.MaxNameLength, + allow: true, + }, + { + name: "max + 1 ", + crNameLength: dynatracev1beta1.MaxNameLength + 1, + allow: false, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + dk := &dynatracev1beta1.DynaKube{ + ObjectMeta: metav1.ObjectMeta{ + Name: strings.Repeat("a", test.crNameLength), + }, + Spec: dynatracev1beta1.DynaKubeSpec{ + APIURL: "https://tenantid.doma.in/api", + }, + } + if test.allow { + assertAllowedResponse(t, dk) + } else { + errorMessage := fmt.Sprintf(errorNameTooLong, dynatracev1beta1.MaxNameLength) + assertDeniedResponse(t, []string{errorMessage}, dk) + } + }) + } +} diff --git a/pkg/webhook/validation/edgeconnect/api_server.go b/pkg/webhook/validation/edgeconnect/api_server.go index 294c548711..bb053163ef 100644 --- a/pkg/webhook/validation/edgeconnect/api_server.go +++ b/pkg/webhook/validation/edgeconnect/api_server.go @@ -21,7 +21,7 @@ var ( } ) -func IsInvalidApiServer(_ context.Context, _ *edgeconnectValidator, edgeConnect *edgeconnect.EdgeConnect) string { +func isInvalidApiServer(_ context.Context, _ *edgeconnectValidator, edgeConnect *edgeconnect.EdgeConnect) string { for _, suffix := range allowedSuffix { if strings.HasSuffix(edgeConnect.Spec.ApiServer, suffix) { hostnameWithDomains := strings.FieldsFunc(suffix, diff --git a/pkg/webhook/validation/edgeconnect/config.go b/pkg/webhook/validation/edgeconnect/config.go index cabb33f64a..1e49ccc413 100644 --- a/pkg/webhook/validation/edgeconnect/config.go +++ b/pkg/webhook/validation/edgeconnect/config.go @@ -12,5 +12,6 @@ var log = logger.Factory.GetLogger("edgeconnect-validation") type validator func(ctx context.Context, dv *edgeconnectValidator, edgeConnect *edgeconnect.EdgeConnect) string var validators = []validator{ - IsInvalidApiServer, + isInvalidApiServer, + nameTooLong, } diff --git a/pkg/webhook/validation/edgeconnect/name.go b/pkg/webhook/validation/edgeconnect/name.go new file mode 100644 index 0000000000..65311abe71 --- /dev/null +++ b/pkg/webhook/validation/edgeconnect/name.go @@ -0,0 +1,21 @@ +package edgeconnect + +import ( + "context" + "fmt" + + "github.com/Dynatrace/dynatrace-operator/pkg/api/v1alpha1/edgeconnect" +) + +const ( + errorNameTooLong = `The length limit for the name of a EdgeConnect is %d, because it is the base for the name of resources related to the EdgeConnect. + The limit is necessary because kubernetes uses the name of some resources for the label value, which has a limit of 63 characters. (see https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set)` +) + +func nameTooLong(_ context.Context, _ *edgeconnectValidator, edgeConnect *edgeconnect.EdgeConnect) string { + edgeConnectName := edgeConnect.Name + if edgeConnectName != "" && len(edgeConnectName) > edgeconnect.MaxNameLength { + return fmt.Sprintf(errorNameTooLong, edgeconnect.MaxNameLength) + } + return "" +} diff --git a/pkg/webhook/validation/edgeconnect/name_test.go b/pkg/webhook/validation/edgeconnect/name_test.go new file mode 100644 index 0000000000..13f611221f --- /dev/null +++ b/pkg/webhook/validation/edgeconnect/name_test.go @@ -0,0 +1,60 @@ +package edgeconnect + +import ( + "fmt" + "strings" + "testing" + + "github.com/Dynatrace/dynatrace-operator/pkg/api/v1alpha1/edgeconnect" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNameTooLong(t *testing.T) { + type testCase struct { + name string + crNameLength int + allow bool + } + + testCases := []testCase{ + { + name: "normal length", + crNameLength: 10, + allow: true, + }, + { + name: "max - 1 ", + crNameLength: edgeconnect.MaxNameLength - 1, + allow: true, + }, + { + name: "max", + crNameLength: edgeconnect.MaxNameLength, + allow: true, + }, + { + name: "max + 1 ", + crNameLength: edgeconnect.MaxNameLength + 1, + allow: false, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + ec := &edgeconnect.EdgeConnect{ + ObjectMeta: metav1.ObjectMeta{ + Name: strings.Repeat("a", test.crNameLength), + }, + Spec: edgeconnect.EdgeConnectSpec{ + ApiServer: "id." + allowedSuffix[0], + }, + } + if test.allow { + assertAllowedResponse(t, ec) + } else { + errorMessage := fmt.Sprintf(errorNameTooLong, edgeconnect.MaxNameLength) + assertDeniedResponse(t, []string{errorMessage}, ec) + } + }) + } +}