Skip to content

Commit

Permalink
Add validation for CR name length (#2375)
Browse files Browse the repository at this point in the history
  • Loading branch information
0sewa0 authored Nov 17, 2023
1 parent 3618ca6 commit 58bf939
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 4 deletions.
8 changes: 7 additions & 1 deletion pkg/api/v1alpha1/edgeconnect/properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/v1beta1/dynakube/properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-<some-hash>)
// 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"
Expand Down
1 change: 1 addition & 0 deletions pkg/webhook/validation/dynakube/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ var validators = []validator{
conflictingOneAgentVolumeStorageSettings,
invalidSyntheticNodeType,
nameViolatesDNS1035,
nameTooLong,
namespaceSelectorMatchLabelsViolateLabelSpec,
}

Expand Down
14 changes: 13 additions & 1 deletion pkg/webhook/validation/dynakube/dynakube_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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-<some-hash>)
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 != "" {
Expand All @@ -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 ""
}
52 changes: 52 additions & 0 deletions pkg/webhook/validation/dynakube/dynakube_name_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package dynakube

import (
"fmt"
"strings"
"testing"

dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta1/dynakube"
Expand All @@ -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)
}
})
}
}
2 changes: 1 addition & 1 deletion pkg/webhook/validation/edgeconnect/api_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion pkg/webhook/validation/edgeconnect/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
21 changes: 21 additions & 0 deletions pkg/webhook/validation/edgeconnect/name.go
Original file line number Diff line number Diff line change
@@ -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 ""
}
60 changes: 60 additions & 0 deletions pkg/webhook/validation/edgeconnect/name_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit 58bf939

Please sign in to comment.