From 5c03c14b25ada948b0bb61e8a22abe5c7ca55dcd Mon Sep 17 00:00:00 2001 From: Prem Saraswat Date: Thu, 10 Oct 2024 20:33:18 +0530 Subject: [PATCH] resource-api: Loosen name validation to match K8s requirements (#93404) * resource-api: Loosen name validation to match K8s requirements This patch modifies some of the requirements for name validation of objects in Resource API to match Kubernetes. The limit we have on characters in name is 64, but some resources allow upto 253 characters. Similarly we also include `:` in the regex, as many objects in default K8s setup use it in the name (the group `system:masters` for example) Signed-off-by: Prem Kumar * Update the name column length in migrator and update e2e test to verify --------- Signed-off-by: Prem Kumar --- pkg/storage/unified/resource/validation.go | 4 ++-- .../unified/resource/validation_test.go | 8 ++++---- .../unified/sql/db/migrations/resource_mig.go | 4 ++-- pkg/tests/apis/scopes/scopes_test.go | 20 +++++++++++++++++++ .../apis/scopes/testdata/example-scope3.yaml | 14 +++++++++++++ 5 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 pkg/tests/apis/scopes/testdata/example-scope3.yaml diff --git a/pkg/storage/unified/resource/validation.go b/pkg/storage/unified/resource/validation.go index dce25a1b8fe..066ac42c2af 100644 --- a/pkg/storage/unified/resource/validation.go +++ b/pkg/storage/unified/resource/validation.go @@ -4,14 +4,14 @@ import ( "regexp" ) -var validNameCharPattern = `a-zA-Z0-9\-\_\.` +var validNameCharPattern = `a-zA-Z0-9:\-\_\.` var validNamePattern = regexp.MustCompile(`^[` + validNameCharPattern + `]*$`).MatchString func validateName(name string) *ErrorResult { if len(name) == 0 { return NewBadRequestError("name is too short") } - if len(name) > 64 { + if len(name) > 253 { return NewBadRequestError("name is too long") } if !validNamePattern(name) { diff --git a/pkg/storage/unified/resource/validation_test.go b/pkg/storage/unified/resource/validation_test.go index b9f9a51d7b5..bc0e4bd8bda 100644 --- a/pkg/storage/unified/resource/validation_test.go +++ b/pkg/storage/unified/resource/validation_test.go @@ -1,22 +1,22 @@ package resource import ( + "strings" "testing" "github.com/stretchr/testify/require" ) func TestNameValidation(t *testing.T) { - require.NotNil(t, validateName("")) // too short - require.NotNil(t, validateName( // too long (max 64) - "0123456789012345678901234567890123456789012345678901234567890123456789", - )) + require.NotNil(t, validateName("")) // too short + require.NotNil(t, validateName(strings.Repeat("0", 254))) // too long (max 253) // OK require.Nil(t, validateName("a")) require.Nil(t, validateName("hello-world")) require.Nil(t, validateName("hello.world")) require.Nil(t, validateName("hello_world")) + require.Nil(t, validateName("hello:world")) // Bad characters require.NotNil(t, validateName("hello world")) diff --git a/pkg/storage/unified/sql/db/migrations/resource_mig.go b/pkg/storage/unified/sql/db/migrations/resource_mig.go index 15e13c08a38..fcaddaacc80 100644 --- a/pkg/storage/unified/sql/db/migrations/resource_mig.go +++ b/pkg/storage/unified/sql/db/migrations/resource_mig.go @@ -22,7 +22,7 @@ func initResourceTables(mg *migrator.Migrator) string { {Name: "group", Type: migrator.DB_NVarchar, Length: 190, Nullable: false}, {Name: "resource", Type: migrator.DB_NVarchar, Length: 190, Nullable: false}, {Name: "namespace", Type: migrator.DB_NVarchar, Length: 63, Nullable: false}, - {Name: "name", Type: migrator.DB_NVarchar, Length: 190, Nullable: false}, + {Name: "name", Type: migrator.DB_NVarchar, Length: 253, Nullable: false}, {Name: "value", Type: migrator.DB_LongText, Nullable: true}, {Name: "action", Type: migrator.DB_Int, Nullable: false}, // 1: create, 2: update, 3: delete @@ -44,7 +44,7 @@ func initResourceTables(mg *migrator.Migrator) string { {Name: "group", Type: migrator.DB_NVarchar, Length: 190, Nullable: false}, {Name: "resource", Type: migrator.DB_NVarchar, Length: 190, Nullable: false}, {Name: "namespace", Type: migrator.DB_NVarchar, Length: 63, Nullable: false}, - {Name: "name", Type: migrator.DB_NVarchar, Length: 190, Nullable: false}, + {Name: "name", Type: migrator.DB_NVarchar, Length: 253, Nullable: false}, {Name: "value", Type: migrator.DB_LongText, Nullable: true}, {Name: "action", Type: migrator.DB_Int, Nullable: false}, // 1: create, 2: update, 3: delete diff --git a/pkg/tests/apis/scopes/scopes_test.go b/pkg/tests/apis/scopes/scopes_test.go index 87e8abb1318..eb380937dff 100644 --- a/pkg/tests/apis/scopes/scopes_test.go +++ b/pkg/tests/apis/scopes/scopes_test.go @@ -3,6 +3,7 @@ package scopes import ( "context" "encoding/json" + "strings" "testing" "github.com/stretchr/testify/require" @@ -159,6 +160,25 @@ func TestIntegrationScopes(t *testing.T) { ) require.NoError(t, err) + // Name length test + scope3 := helper.LoadYAMLOrJSONFile("testdata/example-scope3.yaml") + + // Name too long (>253) + scope3.SetName(strings.Repeat("0", 254)) + _, err = scopeClient.Resource.Create(ctx, + scope3, + createOptions, + ) + require.Error(t, err) + + // Maximum allowed length for name (253) + scope3.SetName(strings.Repeat("0", 253)) + _, err = scopeClient.Resource.Create(ctx, + scope3, + createOptions, + ) + require.NoError(t, err) + // Field Selector test found, err := scopeClient.Resource.List(ctx, metav1.ListOptions{ FieldSelector: "spec.title=foo-scope", diff --git a/pkg/tests/apis/scopes/testdata/example-scope3.yaml b/pkg/tests/apis/scopes/testdata/example-scope3.yaml new file mode 100644 index 00000000000..e1f36c39d87 --- /dev/null +++ b/pkg/tests/apis/scopes/testdata/example-scope3.yaml @@ -0,0 +1,14 @@ +apiVersion: scope.grafana.app/v0alpha1 +kind: Scope +metadata: + name: example-long +spec: + title: baz-scope + description: Longer description for a scope + filters: + - key: aaa + operator: equals + value: eee + - key: ccc + operator: not-equals + value: fff