Skip to content

Commit

Permalink
stop checking enablePreviewPolicies when validating a policy
Browse files Browse the repository at this point in the history
  • Loading branch information
@haywoodsh
haywoodsh committed Mar 30, 2022
1 parent 9baa51a commit 65b49da
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 139 deletions.
8 changes: 4 additions & 4 deletions internal/k8s/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/configuration/validation/policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
171 changes: 37 additions & 134 deletions pkg/apis/configuration/validation/policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -24,9 +23,8 @@ func TestValidatePolicy(t *testing.T) {
},
},
},
isPlus: false,
enablePreviewPolicies: false,
enableAppProtect: false,
isPlus: false,
enableAppProtect: false,
},
{
policy: &v1.Policy{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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)
}
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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")
}
Expand Down

0 comments on commit 65b49da

Please sign in to comment.