From 994aa4b1d33a0392d723d4da2f4b0cd3e32f84ee Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Tue, 14 Oct 2025 15:57:20 -0400 Subject: [PATCH 1/2] wip: add test for api admission Signed-off-by: Bryce Palmer --- test/extended/apiserver/api_admission.go | 180 +++++++++++++++++++++++ test/extended/util/client.go | 18 ++- 2 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 test/extended/apiserver/api_admission.go diff --git a/test/extended/apiserver/api_admission.go b/test/extended/apiserver/api_admission.go new file mode 100644 index 000000000000..a5f5d809c4e9 --- /dev/null +++ b/test/extended/apiserver/api_admission.go @@ -0,0 +1,180 @@ +package apiserver + +import ( + "context" + "errors" + "fmt" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + + v1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" + "k8s.io/utils/ptr" + + oauthv1 "github.com/openshift/api/oauth/v1" + exutil "github.com/openshift/origin/test/extended/util" +) + +// TODO: remove this in favor of a better registration approach +func init() { + MustRegisterGVRStub(oauthv1.SchemeGroupVersion.WithResource("useroauthaccesstokens"), &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": oauthv1.SchemeGroupVersion.String(), + "kind": "UserOAuthAccessToken", + "metadata": map[string]interface{}{ + "name": "sha256~tokenneedstobelongenoughelseitwontworkg", + }, + "clientName": "testclient", + "userName": "testuser", + "userUID": "notempty", + "scopes": []string{ + "user:info", + }, + "redirectURI": "https://something.com/", + }, + }) +} + +// TODO: create a generic structure for stubbing out GVRs. +// TODO: Use a validating admission policy with a response warning for simulating the webhook behavior + +type GVRStubRegistry map[schema.GroupVersionResource]*unstructured.Unstructured + +var stubs = make(GVRStubRegistry) + +func MustRegisterGVRStub(gvr schema.GroupVersionResource, stub *unstructured.Unstructured) { + if _, ok := stubs[gvr]; ok { + panic(fmt.Sprintf("gvr %v is already registered", gvr)) + } + + if stub == nil { + panic("stub should not be nil") + } + + stubs[gvr] = stub +} + +type resourceFunc func(context.Context, schema.GroupVersionResource, dynamic.Interface, *unstructured.Unstructured) error + +// TODO: opt-out of these operations for apis that don't support it? +var defaultResourceFuncs = map[string]resourceFunc{ + "create": testCreateFunc, + "update": testUpdateFunc, + "patch": testPatchFunc, + "delete": testDeleteFunc, + "deletecollection": testDeleteCollectionFunc, +} + +var _ = g.Describe("[Jira:\"kube-apiserver\"] Admission behaves correctly for OpenShift APIs", func() { + defer g.GinkgoRecover() + oc := exutil.NewCLI("apiserver") + + // TODO: should this be created per GVR with a label selection matcher that only matches resources created by this test? + vap := &v1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-admission-testing.openshift.io", + }, + Spec: v1.ValidatingAdmissionPolicySpec{ + FailurePolicy: ptr.To(v1.Fail), + MatchConstraints: &v1.MatchResources{ + ResourceRules: []v1.NamedRuleWithOperations{ + { + RuleWithOperations: v1.RuleWithOperations{ + Operations: []v1.OperationType{ + v1.OperationAll, + }, + Rule: v1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*"}, + }, + }, + }, + }, + }, + Validations: []v1.Validation{ + { + Expression: "1 != 1", + Message: "oat-boom", + }, + }, + }, + } + + _, err := oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(context.TODO(), vap, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error creating VAP") + + vapBinding := &v1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-admission-testing-binding.openshift.io", + }, + Spec: v1.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "openshift-admission-testing.openshift.io", + ValidationActions: []v1.ValidationAction{ + v1.Warn, + }, + }, + } + + _, err = oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Create(context.TODO(), vapBinding, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error creating VAP Binding") + + warnHandler := &warningHandler{} + dynamicClient := oc.DynamicClient(func(c *rest.Config) { + c.WarningHandler = warnHandler + }) + + failures := []error{} + for verb, action := range defaultResourceFuncs { + for gvr, stub := range stubs { + g.By(fmt.Sprintf("testing admission for verb %s with resource %s", verb, gvr.String())) + + err := action(context.TODO(), gvr, dynamicClient, stub) + if err != nil { + failures = append(failures, fmt.Errorf("%s-ing resource %s : %w", verb, gvr.String(), err)) + } + + if warnHandler.warning != "todo" { + failures = append(failures, fmt.Errorf("admission warning did not match the expected warning when %s-ing resource %s", verb, gvr.String())) + } + } + } + + o.Expect(failures).To(o.BeEmpty(), "should not have admission failures") +}) + +type warningHandler struct { + warning string +} + +func (w *warningHandler) HandleWarningHeader(code int, agent string, text string) { + w.warning = text +} + +func testCreateFunc(ctx context.Context, gvr schema.GroupVersionResource, client dynamic.Interface, obj *unstructured.Unstructured) error { + _, err := client.Resource(gvr).Create(ctx, obj, metav1.CreateOptions{}) + return err +} + +func testUpdateFunc(ctx context.Context, gvr schema.GroupVersionResource, client dynamic.Interface, obj *unstructured.Unstructured) error { + _, err := client.Resource(gvr).Update(ctx, obj, metav1.UpdateOptions{}) + return err +} + +func testPatchFunc(ctx context.Context, gvr schema.GroupVersionResource, client dynamic.Interface, obj *unstructured.Unstructured) error { + return errors.New("patch testing not yet implemented") +} + +func testDeleteFunc(ctx context.Context, gvr schema.GroupVersionResource, client dynamic.Interface, obj *unstructured.Unstructured) error { + return client.Resource(gvr).Delete(ctx, obj.GetName(), metav1.DeleteOptions{}) +} + +func testDeleteCollectionFunc(ctx context.Context, gvr schema.GroupVersionResource, client dynamic.Interface, obj *unstructured.Unstructured) error { + // TODO: first do an update to set labels for the resource to delete + return client.Resource(gvr).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{}) +} diff --git a/test/extended/util/client.go b/test/extended/util/client.go index 5e034dd6261e..4c30280df828 100644 --- a/test/extended/util/client.go +++ b/test/extended/util/client.go @@ -813,8 +813,10 @@ func (c *CLI) KubeClient() kubernetes.Interface { return kubernetes.NewForConfigOrDie(c.UserConfig()) } -func (c *CLI) DynamicClient() dynamic.Interface { - return dynamic.NewForConfigOrDie(c.UserConfig()) +type ClientOption func(*rest.Config) + +func (c *CLI) DynamicClient(clientOpts ...ClientOption) dynamic.Interface { + return dynamic.NewForConfigOrDie(c.UserConfig(clientOpts...)) } // AdminKubeClient provides a Kubernetes client for the cluster admin user. @@ -861,13 +863,17 @@ func (c *CLI) NewPrometheusClient(ctx context.Context) prometheusv1.API { return prometheusClient } -func (c *CLI) UserConfig() *rest.Config { +func (c *CLI) UserConfig(clientOpts ...ClientOption) *rest.Config { if c.token != "" { clientConfig, err := GetClientConfig(c.adminConfigPath) if err != nil { FatalErr(err) } + for _, clientOpt := range clientOpts { + clientOpt(clientConfig) + } + anon := rest.AnonymousClientConfig(clientConfig) anon.BearerToken = c.token return anon @@ -877,6 +883,11 @@ func (c *CLI) UserConfig() *rest.Config { if err != nil { FatalErr(err) } + + for _, clientOpt := range clientOpts { + clientOpt(clientConfig) + } + return clientConfig } @@ -1137,7 +1148,6 @@ func (c *CLI) CreateUser(prefix string) *userv1.User { } func (c *CLI) GetClientConfigForUser(username string) *rest.Config { - userAPIExists, err := DoesApiResourceExist(c.AdminConfig(), "users", "user.openshift.io") if err != nil { FatalErr(err) From dcf10be339211c3687a851c90a8a3dbfb08378f2 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Mon, 20 Oct 2025 15:43:58 -0400 Subject: [PATCH 2/2] wip: more work Signed-off-by: Bryce Palmer --- test/extended/apiserver/api_admission.go | 196 ++++++++++++++--------- test/extended/util/client.go | 11 +- 2 files changed, 129 insertions(+), 78 deletions(-) diff --git a/test/extended/apiserver/api_admission.go b/test/extended/apiserver/api_admission.go index a5f5d809c4e9..2092d9381904 100644 --- a/test/extended/apiserver/api_admission.go +++ b/test/extended/apiserver/api_admission.go @@ -12,6 +12,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" "k8s.io/utils/ptr" @@ -22,20 +23,43 @@ import ( // TODO: remove this in favor of a better registration approach func init() { - MustRegisterGVRStub(oauthv1.SchemeGroupVersion.WithResource("useroauthaccesstokens"), &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": oauthv1.SchemeGroupVersion.String(), - "kind": "UserOAuthAccessToken", - "metadata": map[string]interface{}{ - "name": "sha256~tokenneedstobelongenoughelseitwontworkg", + MustRegisterGVRStub(oauthv1.SchemeGroupVersion.WithResource("useroauthaccesstokens"), Stub{ + Object: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": oauthv1.SchemeGroupVersion.String(), + "kind": "UserOAuthAccessToken", + "metadata": map[string]interface{}{ + "name": "sha256~tokenneedstobelongenoughelseitwontworkg", + }, + "clientName": "testclient", + "userName": "admin", + "userUID": "notempty", + "scopes": []string{ + "user:info", + }, + "redirectURI": "https://something.com/", }, - "clientName": "testclient", - "userName": "testuser", - "userUID": "notempty", - "scopes": []string{ - "user:info", + }, + UnsupportedVerbs: sets.New("create", "update", "patch", "deletecollection"), + DependentResources: map[schema.GroupVersionResource][]unstructured.Unstructured{ + oauthv1.SchemeGroupVersion.WithResource("oauthaccesstokens"): { + { + Object: map[string]interface{}{ + "apiVersion": oauthv1.SchemeGroupVersion.String(), + "kind": "OAuthAccessToken", + "metadata": map[string]interface{}{ + "name": "sha256~tokenneedstobelongenoughelseitwontworkg", + }, + "clientName": "openshift-challenging-client", + "userName": "testuser", + "userUID": "notempty", + "scopes": []string{ + "user:info", + }, + "redirectURI": "https://something.com/", + }, + }, }, - "redirectURI": "https://something.com/", }, }) } @@ -43,19 +67,21 @@ func init() { // TODO: create a generic structure for stubbing out GVRs. // TODO: Use a validating admission policy with a response warning for simulating the webhook behavior -type GVRStubRegistry map[schema.GroupVersionResource]*unstructured.Unstructured +type Stub struct { + Object *unstructured.Unstructured + UnsupportedVerbs sets.Set[string] + DependentResources map[schema.GroupVersionResource][]unstructured.Unstructured +} + +type GVRStubRegistry map[schema.GroupVersionResource]Stub var stubs = make(GVRStubRegistry) -func MustRegisterGVRStub(gvr schema.GroupVersionResource, stub *unstructured.Unstructured) { +func MustRegisterGVRStub(gvr schema.GroupVersionResource, stub Stub) { if _, ok := stubs[gvr]; ok { panic(fmt.Sprintf("gvr %v is already registered", gvr)) } - if stub == nil { - panic("stub should not be nil") - } - stubs[gvr] = stub } @@ -70,82 +96,102 @@ var defaultResourceFuncs = map[string]resourceFunc{ "deletecollection": testDeleteCollectionFunc, } -var _ = g.Describe("[Jira:\"kube-apiserver\"] Admission behaves correctly for OpenShift APIs", func() { +var _ = g.Describe("[sig-api-machinery][kube-apiserver] Admission behaves correctly for OpenShift APIs", func() { defer g.GinkgoRecover() oc := exutil.NewCLI("apiserver") - // TODO: should this be created per GVR with a label selection matcher that only matches resources created by this test? - vap := &v1.ValidatingAdmissionPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "openshift-admission-testing.openshift.io", - }, - Spec: v1.ValidatingAdmissionPolicySpec{ - FailurePolicy: ptr.To(v1.Fail), - MatchConstraints: &v1.MatchResources{ - ResourceRules: []v1.NamedRuleWithOperations{ - { - RuleWithOperations: v1.RuleWithOperations{ - Operations: []v1.OperationType{ - v1.OperationAll, - }, - Rule: v1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{"*"}, - Resources: []string{"*"}, + g.BeforeEach(func() { + vap := &v1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-admission-testing.openshift.io", + }, + Spec: v1.ValidatingAdmissionPolicySpec{ + FailurePolicy: ptr.To(v1.Fail), + MatchConstraints: &v1.MatchResources{ + ResourceRules: []v1.NamedRuleWithOperations{ + { + RuleWithOperations: v1.RuleWithOperations{ + Operations: []v1.OperationType{ + v1.OperationAll, + }, + Rule: v1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*"}, + }, }, }, }, }, - }, - Validations: []v1.Validation{ - { - Expression: "1 != 1", - Message: "oat-boom", + Validations: []v1.Validation{ + { + Expression: "1 != 1", + Message: "oat-boom", + }, }, }, - }, - } + } - _, err := oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(context.TODO(), vap, metav1.CreateOptions{}) - o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error creating VAP") + _, err := oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(context.TODO(), vap, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error creating VAP") - vapBinding := &v1.ValidatingAdmissionPolicyBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "openshift-admission-testing-binding.openshift.io", - }, - Spec: v1.ValidatingAdmissionPolicyBindingSpec{ - PolicyName: "openshift-admission-testing.openshift.io", - ValidationActions: []v1.ValidationAction{ - v1.Warn, + vapBinding := &v1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-admission-testing-binding.openshift.io", }, - }, - } - - _, err = oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Create(context.TODO(), vapBinding, metav1.CreateOptions{}) - o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error creating VAP Binding") + Spec: v1.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: "openshift-admission-testing.openshift.io", + ValidationActions: []v1.ValidationAction{ + v1.Warn, + }, + }, + } - warnHandler := &warningHandler{} - dynamicClient := oc.DynamicClient(func(c *rest.Config) { - c.WarningHandler = warnHandler + _, err = oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Create(context.TODO(), vapBinding, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error creating VAP Binding") }) - failures := []error{} - for verb, action := range defaultResourceFuncs { - for gvr, stub := range stubs { - g.By(fmt.Sprintf("testing admission for verb %s with resource %s", verb, gvr.String())) + g.AfterEach(func() { + err := oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicies().Delete(context.TODO(), "openshift-admission-testing.openshift.io", metav1.DeleteOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error cleaning up VAP") - err := action(context.TODO(), gvr, dynamicClient, stub) - if err != nil { - failures = append(failures, fmt.Errorf("%s-ing resource %s : %w", verb, gvr.String(), err)) - } + err = oc.AdminKubeClient().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Delete(context.TODO(), "openshift-admission-testing-binding.openshift.io", metav1.DeleteOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error cleaning up VAP binding") + }) - if warnHandler.warning != "todo" { - failures = append(failures, fmt.Errorf("admission warning did not match the expected warning when %s-ing resource %s", verb, gvr.String())) + g.It("should work", func() { + // TODO: should this be created per GVR with a label selection matcher that only matches resources created by this test? + warnHandler := &warningHandler{} + dynamicClient := oc.AdminDynamicClient(func(c *rest.Config) { + c.WarningHandler = warnHandler + }) + + for verb, action := range defaultResourceFuncs { + for gvr, stub := range stubs { + if stub.UnsupportedVerbs.Has(verb) { + continue + } + + for depGVR, dependentResource := range stub.DependentResources { + for _, depRes := range dependentResource { + _, err := dynamicClient.Resource(depGVR).Create(context.TODO(), &depRes, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + } + } + + err := action(context.TODO(), gvr, dynamicClient, stub.Object) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(warnHandler.warning).To(o.Equal("oat-boom")) + + for depGVR, dependentResource := range stub.DependentResources { + for _, depRes := range dependentResource { + err := dynamicClient.Resource(depGVR).Delete(context.TODO(), depRes.GetName(), metav1.DeleteOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + } + } } } - } - - o.Expect(failures).To(o.BeEmpty(), "should not have admission failures") + }) }) type warningHandler struct { diff --git a/test/extended/util/client.go b/test/extended/util/client.go index 4c30280df828..a7d7fac6928b 100644 --- a/test/extended/util/client.go +++ b/test/extended/util/client.go @@ -824,8 +824,8 @@ func (c *CLI) AdminKubeClient() kubernetes.Interface { return kubernetes.NewForConfigOrDie(c.AdminConfig()) } -func (c *CLI) AdminDynamicClient() dynamic.Interface { - return dynamic.NewForConfigOrDie(c.AdminConfig()) +func (c *CLI) AdminDynamicClient(clientOpts ...ClientOption) dynamic.Interface { + return dynamic.NewForConfigOrDie(c.AdminConfig(clientOpts...)) } func (c *CLI) NewPrometheusClient(ctx context.Context) prometheusv1.API { @@ -891,11 +891,16 @@ func (c *CLI) UserConfig(clientOpts ...ClientOption) *rest.Config { return clientConfig } -func (c *CLI) AdminConfig() *rest.Config { +func (c *CLI) AdminConfig(clientOpts ...ClientOption) *rest.Config { clientConfig, err := GetClientConfig(c.adminConfigPath) if err != nil { FatalErr(err) } + + for _, opt := range clientOpts { + opt(clientConfig) + } + return clientConfig }