From 65b49daa5f23d8762db25d94d721dc0b27cfdfec Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Fri, 25 Mar 2022 18:49:30 +0000 Subject: [PATCH] stop checking enablePreviewPolicies when validating a policy --- internal/k8s/controller.go | 8 +- pkg/apis/configuration/validation/policy.go | 2 +- .../configuration/validation/policy_test.go | 171 ++++-------------- 3 files changed, 42 insertions(+), 139 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 0eb0170bf0..0672114832 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -877,7 +877,7 @@ func (lbc *LoadBalancerController) syncPolicy(task task) { if polExists && lbc.HasCorrectIngressClass(obj) { pol := obj.(*conf_v1.Policy) - err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled) + err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.appProtectEnabled) if err != nil { msg := fmt.Sprintf("Policy %v/%v is invalid and was rejected: %v", pol.Namespace, pol.Name, err) lbc.recorder.Eventf(pol, api_v1.EventTypeWarning, "Rejected", msg) @@ -2077,7 +2077,7 @@ func (lbc *LoadBalancerController) updatePoliciesStatus() error { for _, obj := range lbc.policyLister.List() { pol := obj.(*conf_v1.Policy) - err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled) + err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.appProtectEnabled) if err != nil { msg := fmt.Sprintf("Policy %v/%v is invalid and was rejected: %v", pol.Namespace, pol.Name, err) err = lbc.statusUpdater.UpdatePolicyStatus(pol, conf_v1.StateInvalid, "Rejected", msg) @@ -2625,7 +2625,7 @@ func (lbc *LoadBalancerController) getAllPolicies() []*conf_v1.Policy { for _, obj := range lbc.policyLister.List() { pol := obj.(*conf_v1.Policy) - err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled) + err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.appProtectEnabled) if err != nil { glog.V(3).Infof("Skipping invalid Policy %s/%s: %v", pol.Namespace, pol.Name, err) continue @@ -2667,7 +2667,7 @@ func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReferenc continue } - err = validation.ValidatePolicy(policy, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled) + err = validation.ValidatePolicy(policy, lbc.isNginxPlus, lbc.appProtectEnabled) if err != nil { errors = append(errors, fmt.Errorf("Policy %s is invalid: %w", policyKey, err)) continue diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 5ec6a1065f..07ea8c34fa 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -14,7 +14,7 @@ import ( ) // ValidatePolicy validates a Policy. -func ValidatePolicy(policy *v1.Policy, isPlus, enablePreviewPolicies, enableAppProtect bool) error { +func ValidatePolicy(policy *v1.Policy, isPlus, enableAppProtect bool) error { allErrs := validatePolicySpec(&policy.Spec, field.NewPath("spec"), isPlus, enableAppProtect) return allErrs.ToAggregate() } diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index c46c809cee..a7bd598c7f 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -10,11 +10,10 @@ import ( func TestValidatePolicy(t *testing.T) { t.Parallel() tests := []struct { - policy *v1.Policy - isPlus bool - enablePreviewPolicies bool - enableAppProtect bool - msg string + policy *v1.Policy + isPlus bool + enableAppProtect bool + msg string }{ { policy: &v1.Policy{ @@ -24,9 +23,8 @@ func TestValidatePolicy(t *testing.T) { }, }, }, - isPlus: false, - enablePreviewPolicies: false, - enableAppProtect: false, + isPlus: false, + enableAppProtect: false, }, { policy: &v1.Policy{ @@ -37,10 +35,9 @@ func TestValidatePolicy(t *testing.T) { }, }, }, - isPlus: true, - enablePreviewPolicies: true, - enableAppProtect: false, - msg: "use jwt(plus only) policy", + isPlus: true, + enableAppProtect: false, + msg: "use jwt(plus only) policy", }, { policy: &v1.Policy{ @@ -55,9 +52,8 @@ func TestValidatePolicy(t *testing.T) { }, }, }, - isPlus: true, - enablePreviewPolicies: true, - msg: "use OIDC (plus only)", + isPlus: true, + msg: "use OIDC (plus only)", }, { policy: &v1.Policy{ @@ -67,99 +63,13 @@ func TestValidatePolicy(t *testing.T) { }, }, }, - isPlus: true, - enablePreviewPolicies: true, - enableAppProtect: true, - msg: "use WAF(plus only) policy", - }, - { - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - WAF: &v1.WAF{ - Enable: true, - }, - }, - }, - isPlus: true, - enablePreviewPolicies: false, - enableAppProtect: true, - msg: "WAF policy with preview policies disabled", - }, - { - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - RateLimit: &v1.RateLimit{ - Rate: "10r/s", - ZoneSize: "10M", - Key: "${request_uri}", - }, - }, - }, - isPlus: false, - enablePreviewPolicies: false, - enableAppProtect: false, - msg: "rateLimit policy with preview policies disabled", - }, - { - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - JWTAuth: &v1.JWTAuth{ - Realm: "My Product API", - Secret: "my-jwk", - }, - }, - }, - isPlus: true, - enablePreviewPolicies: false, - enableAppProtect: false, - msg: "jwt policy with preview policies disabled", - }, - { - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - IngressMTLS: &v1.IngressMTLS{ - ClientCertSecret: "mtls-secret", - }, - }, - }, - isPlus: false, - enablePreviewPolicies: false, - enableAppProtect: false, - msg: "ingressMTLS policy with preview policies disabled", - }, - { - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - EgressMTLS: &v1.EgressMTLS{ - TLSSecret: "mtls-secret", - }, - }, - }, - isPlus: false, - enablePreviewPolicies: false, - enableAppProtect: false, - msg: "egressMTLS policy with preview policies disabled", - }, - { - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - OIDC: &v1.OIDC{ - AuthEndpoint: "https://foo.bar/auth", - TokenEndpoint: "https://foo.bar/token", - JWKSURI: "https://foo.bar/certs", - ClientID: "random-string", - ClientSecret: "random-secret", - Scope: "openid", - }, - }, - }, - isPlus: true, - enablePreviewPolicies: false, - msg: "OIDC policy with preview policies disabled", + isPlus: true, + enableAppProtect: true, + msg: "use WAF(plus only) policy", }, } for _, test := range tests { - err := ValidatePolicy(test.policy, test.isPlus, test.enablePreviewPolicies, test.enableAppProtect) + err := ValidatePolicy(test.policy, test.isPlus, test.enableAppProtect) if err != nil { t.Errorf("ValidatePolicy() returned error %v for valid input for the case of %v", err, test.msg) } @@ -169,20 +79,18 @@ func TestValidatePolicy(t *testing.T) { func TestValidatePolicyFails(t *testing.T) { t.Parallel() tests := []struct { - policy *v1.Policy - isPlus bool - enablePreviewPolicies bool - enableAppProtect bool - msg string + policy *v1.Policy + isPlus bool + enableAppProtect bool + msg string }{ { policy: &v1.Policy{ Spec: v1.PolicySpec{}, }, - isPlus: false, - enablePreviewPolicies: false, - enableAppProtect: false, - msg: "empty policy spec", + isPlus: false, + enableAppProtect: false, + msg: "empty policy spec", }, { policy: &v1.Policy{ @@ -197,10 +105,9 @@ func TestValidatePolicyFails(t *testing.T) { }, }, }, - isPlus: true, - enablePreviewPolicies: true, - enableAppProtect: false, - msg: "multiple policies in spec", + isPlus: true, + enableAppProtect: false, + msg: "multiple policies in spec", }, { policy: &v1.Policy{ @@ -211,10 +118,9 @@ func TestValidatePolicyFails(t *testing.T) { }, }, }, - isPlus: false, - enablePreviewPolicies: true, - enableAppProtect: false, - msg: "jwt(plus only) policy on OSS", + isPlus: false, + enableAppProtect: false, + msg: "jwt(plus only) policy on OSS", }, { policy: &v1.Policy{ @@ -224,10 +130,9 @@ func TestValidatePolicyFails(t *testing.T) { }, }, }, - isPlus: false, - enablePreviewPolicies: true, - enableAppProtect: false, - msg: "WAF(plus only) policy on OSS", + isPlus: false, + enableAppProtect: false, + msg: "WAF(plus only) policy on OSS", }, { policy: &v1.Policy{ @@ -242,9 +147,8 @@ func TestValidatePolicyFails(t *testing.T) { }, }, }, - isPlus: false, - enablePreviewPolicies: true, - msg: "OIDC policy in OSS", + isPlus: false, + msg: "OIDC policy in OSS", }, { policy: &v1.Policy{ @@ -254,14 +158,13 @@ func TestValidatePolicyFails(t *testing.T) { }, }, }, - isPlus: true, - enablePreviewPolicies: true, - enableAppProtect: false, - msg: "WAF policy with AP disabled", + isPlus: true, + enableAppProtect: false, + msg: "WAF policy with AP disabled", }, } for _, test := range tests { - err := ValidatePolicy(test.policy, test.isPlus, test.enablePreviewPolicies, test.enableAppProtect) + err := ValidatePolicy(test.policy, test.isPlus, test.enableAppProtect) if err == nil { t.Errorf("ValidatePolicy() returned no error for invalid input") }