From 8afc18ddcc4184f1fc4d97e8a8640c0c4150731f Mon Sep 17 00:00:00 2001 From: pranshul gupta Date: Wed, 28 Jan 2026 22:57:05 +0530 Subject: [PATCH] Refactor Declarative validation test file for role and clusterrole to be more clearner --- .../declarative_validation_test.go | 52 ++++++----------- .../rbac/role/declarative_validation_test.go | 57 ++++++------------- 2 files changed, 32 insertions(+), 77 deletions(-) diff --git a/pkg/registry/rbac/clusterrole/declarative_validation_test.go b/pkg/registry/rbac/clusterrole/declarative_validation_test.go index 799b355560b..6dfd2311ad7 100644 --- a/pkg/registry/rbac/clusterrole/declarative_validation_test.go +++ b/pkg/registry/rbac/clusterrole/declarative_validation_test.go @@ -47,15 +47,15 @@ func testDeclarativeValidateForDeclarative(t *testing.T, apiVersion string) { input: mkValidClusterRole(), }, "invalid ClusterRole missing verbs": { - input: mkValidClusterRole(tweakVerbs(0, nil)), + input: mkValidClusterRole(tweakVerbs(nil)), expectedErrs: field.ErrorList{ field.Required(field.NewPath("rules").Index(0).Child("verbs"), ""), }, }, - "invalid ClusterRole missing resources": { - input: mkValidClusterRole(tweakResources(0, nil)), + "invalid ClusterRole empty verbs": { + input: mkValidClusterRole(tweakVerbs([]string{})), expectedErrs: field.ErrorList{ - field.Required(field.NewPath("rules").Index(0).Child("resources"), ""), + field.Required(field.NewPath("rules").Index(0).Child("verbs"), ""), }, }, // TODO: Add more test cases @@ -83,21 +83,23 @@ func testValidateUpdateForDeclarative(t *testing.T, apiVersion string) { update rbac.ClusterRole expectedErrs field.ErrorList }{ - "valid update adding resources": { - old: mkValidClusterRole(tweakResourceVersion("1")), - update: mkValidClusterRole(tweakResourceVersion("2"), tweakAddResource(0, "services")), + "valid update": { + old: mkValidClusterRole(), + update: mkValidClusterRole(), }, - "invalid update clearing resources": { - old: mkValidClusterRole(tweakResourceVersion("1")), - update: mkValidClusterRole(tweakResourceVersion("2"), tweakResources(0, []string{})), + "invalid update clearing verbs": { + old: mkValidClusterRole(), + update: mkValidClusterRole(tweakVerbs([]string{})), expectedErrs: field.ErrorList{ - field.Required(field.NewPath("rules").Index(0).Child("resources"), ""), + field.Required(field.NewPath("rules").Index(0).Child("verbs"), ""), }, }, // TODO: Add more test cases } for k, tc := range testCases { t.Run(k, func(t *testing.T) { + tc.old.ResourceVersion = "1" + tc.update.ResourceVersion = "2" apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, Strategy.ValidateUpdate, tc.expectedErrs) }) } @@ -121,32 +123,10 @@ func mkValidClusterRole(tweaks ...func(*rbac.ClusterRole)) rbac.ClusterRole { return cr } -func tweakResourceVersion(rv string) func(*rbac.ClusterRole) { +func tweakVerbs(verbs []string) func(*rbac.ClusterRole) { return func(cr *rbac.ClusterRole) { - cr.ResourceVersion = rv - } -} - -func tweakVerbs(ruleIndex int, verbs []string) func(*rbac.ClusterRole) { - return func(cr *rbac.ClusterRole) { - if len(cr.Rules) > ruleIndex { - cr.Rules[ruleIndex].Verbs = verbs - } - } -} - -func tweakResources(ruleIndex int, resources []string) func(*rbac.ClusterRole) { - return func(cr *rbac.ClusterRole) { - if len(cr.Rules) > ruleIndex { - cr.Rules[ruleIndex].Resources = resources - } - } -} - -func tweakAddResource(ruleIndex int, resource string) func(*rbac.ClusterRole) { - return func(cr *rbac.ClusterRole) { - if len(cr.Rules) > ruleIndex { - cr.Rules[ruleIndex].Resources = append(cr.Rules[ruleIndex].Resources, resource) + if len(cr.Rules) > 0 { + cr.Rules[0].Verbs = verbs } } } diff --git a/pkg/registry/rbac/role/declarative_validation_test.go b/pkg/registry/rbac/role/declarative_validation_test.go index f80afbb16f6..5a5755f1fb9 100644 --- a/pkg/registry/rbac/role/declarative_validation_test.go +++ b/pkg/registry/rbac/role/declarative_validation_test.go @@ -53,17 +53,18 @@ func testDeclarativeValidateForDeclarative(t *testing.T, apiVersion string) { input: mkValidRole(), }, "invalid Role missing verbs": { - input: mkValidRole(tweakRoleVerbs(0, nil)), + input: mkValidRole(tweakVerbs(nil)), expectedErrs: field.ErrorList{ field.Required(field.NewPath("rules").Index(0).Child("verbs"), ""), }, }, - "invalid Role missing resources": { - input: mkValidRole(tweakRoleResources(0, nil)), + "invalid Role empty verbs": { + input: mkValidRole(tweakVerbs([]string{})), expectedErrs: field.ErrorList{ - field.Required(field.NewPath("rules").Index(0).Child("resources"), ""), + field.Required(field.NewPath("rules").Index(0).Child("verbs"), ""), }, }, + // TODO: Add more test cases } for name, tc := range testCases { @@ -94,28 +95,24 @@ func testValidateUpdateForDeclarative(t *testing.T, apiVersion string) { old, update rbac.Role expectedErrs field.ErrorList }{ - "valid update adding resources": { - old: mkValidRole(tweakRoleResourceVersion("1")), - update: mkValidRole(tweakRoleResourceVersion("2"), tweakRoleAddResource(0, "services")), + "valid update": { + old: mkValidRole(), + update: mkValidRole(), }, "invalid update clearing verbs": { - old: mkValidRole(tweakRoleResourceVersion("1")), - update: mkValidRole(tweakRoleResourceVersion("2"), tweakRoleVerbs(0, []string{})), + old: mkValidRole(), + update: mkValidRole(tweakVerbs([]string{})), expectedErrs: field.ErrorList{ field.Required(field.NewPath("rules").Index(0).Child("verbs"), ""), }, }, - "invalid update clearing resources": { - old: mkValidRole(tweakRoleResourceVersion("1")), - update: mkValidRole(tweakRoleResourceVersion("2"), tweakRoleResources(0, []string{})), - expectedErrs: field.ErrorList{ - field.Required(field.NewPath("rules").Index(0).Child("resources"), ""), - }, - }, + // TODO: Add more test cases } for name, tc := range testCases { t.Run(name, func(t *testing.T) { + tc.old.ResourceVersion = "1" + tc.update.ResourceVersion = "2" apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, Strategy.ValidateUpdate, tc.expectedErrs) }) } @@ -140,32 +137,10 @@ func mkValidRole(tweaks ...func(*rbac.Role)) rbac.Role { return r } -func tweakRoleResourceVersion(rv string) func(*rbac.Role) { +func tweakVerbs(verbs []string) func(*rbac.Role) { return func(r *rbac.Role) { - r.ResourceVersion = rv - } -} - -func tweakRoleVerbs(ruleIndex int, verbs []string) func(*rbac.Role) { - return func(r *rbac.Role) { - if len(r.Rules) > ruleIndex { - r.Rules[ruleIndex].Verbs = verbs - } - } -} - -func tweakRoleResources(ruleIndex int, resources []string) func(*rbac.Role) { - return func(r *rbac.Role) { - if len(r.Rules) > ruleIndex { - r.Rules[ruleIndex].Resources = resources - } - } -} - -func tweakRoleAddResource(ruleIndex int, resource string) func(*rbac.Role) { - return func(r *rbac.Role) { - if len(r.Rules) > ruleIndex { - r.Rules[ruleIndex].Resources = append(r.Rules[ruleIndex].Resources, resource) + if len(r.Rules) > 0 { + r.Rules[0].Verbs = verbs } } }