Skip to content

Commit

Permalink
Rework from review
Browse files Browse the repository at this point in the history
  • Loading branch information
@soneillf5
soneillf5 committed Jul 15, 2021
1 parent 55f97c2 commit f1ebca1
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 53 deletions.
10 changes: 5 additions & 5 deletions internal/k8s/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -792,11 +792,6 @@ func (lbc *LoadBalancerController) syncPolicy(task task) {
return
}

if !lbc.HasCorrectIngressClass(obj) {
glog.V(3).Infof("Ignoring a Policy with non-matching ingressClassName")
return
}

glog.V(2).Infof("Adding, Updating or Deleting Policy: %v\n", key)

if polExists {
Expand Down Expand Up @@ -2492,6 +2487,11 @@ func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReferenc

policy := policyObj.(*conf_v1.Policy)

if !lbc.HasCorrectIngressClass(policy) {
errors = append(errors, fmt.Errorf("referenced policy %s has incorrect ingress class: %s (controller ingress class: %s)", policyKey, policy.Spec.IngressClass, lbc.ingressClass))
continue
}

err = validation.ValidatePolicy(policy, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled)
if err != nil {
errors = append(errors, fmt.Errorf("Policy %s is invalid: %w", policyKey, err))
Expand Down
20 changes: 20 additions & 0 deletions internal/k8s/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,19 @@ func TestGetPolicies(t *testing.T) {
},
}

validPolicyIngressClass := &conf_v1.Policy{
ObjectMeta: meta_v1.ObjectMeta{
Name: "valid-policy-ingress-class",
Namespace: "default",
},
Spec: conf_v1.PolicySpec{
IngressClass: "test-class",
AccessControl: &conf_v1.AccessControl{
Allow: []string{"127.0.0.1"},
},
},
}

invalidPolicy := &conf_v1.Policy{
ObjectMeta: meta_v1.ObjectMeta{
Name: "invalid-policy",
Expand All @@ -726,6 +739,8 @@ func TestGetPolicies(t *testing.T) {
switch key {
case "default/valid-policy":
return validPolicy, true, nil
case "default/valid-policy-ingress-class":
return validPolicyIngressClass, true, nil
case "default/invalid-policy":
return invalidPolicy, true, nil
case "nginx-ingress/valid-policy":
Expand Down Expand Up @@ -754,13 +769,18 @@ func TestGetPolicies(t *testing.T) {
Name: "some-policy", // will make lister return error
Namespace: "nginx-ingress",
},
{
Name: "valid-policy-ingress-class",
Namespace: "default",
},
}

expectedPolicies := []*conf_v1.Policy{validPolicy}
expectedErrors := []error{
errors.New("Policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `jwt`, `oidc`, `waf`"),
errors.New("Policy nginx-ingress/valid-policy doesn't exist"),
errors.New("Failed to get policy nginx-ingress/some-policy: GetByKey error"),
errors.New("referenced policy default/valid-policy-ingress-class has incorrect ingress class: test-class (controller ingress class: )"),
}

result, errors := lbc.getPolicies(policyRefs, "default")
Expand Down
101 changes: 53 additions & 48 deletions tests/suite/test_policy_ingress_class.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@
policy_other_ingress_class_src = f"{TEST_DATA}/policy-ingress-class/policy-other-ingress-class.yaml"


@pytest.mark.sean
@pytest.mark.policies
@pytest.mark.parametrize(
"crd_ingress_controller, virtual_server_setup",
[
(
{
"type": "complete",
"extra_args": [
"-ingress-class=nginx",
f"-enable-custom-resources",
f"-enable-preview-policies",
f"-enable-leader-election=false",
Expand Down Expand Up @@ -59,104 +58,110 @@ def test_policy_empty_ingress_class(
self, kube_apis, crd_ingress_controller, virtual_server_setup, test_namespace, src,
):
"""
Test if rate-limiting policy is working with 1 rps
Test if policy with no ingress class is applied to vs
"""
print(f"Create rl policy")
pol_name = create_policy_from_yaml(kube_apis.custom_objects, policy_src, test_namespace)

wait_before_test()
policy_info = read_custom_resource(kube_apis.custom_objects, test_namespace, "policies", pol_name)
assert (
policy_info["status"]
and policy_info["status"]["reason"] == "AddedOrUpdated"
and policy_info["status"]["state"] == "Valid"
)

print(f"Patch vs with policy: {src}")
patch_virtual_server_from_yaml(
kube_apis.custom_objects,
virtual_server_setup.vs_name,
src,
virtual_server_setup.namespace,
)

wait_before_test()
policy_info = read_custom_resource(kube_apis.custom_objects, test_namespace, "policies", pol_name)
occur = []
t_end = time.perf_counter() + 1
resp = requests.get(
virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host},

vs_info = read_custom_resource(kube_apis.custom_objects, virtual_server_setup.namespace, "virtualservers", virtual_server_setup.vs_name)
assert (
vs_info["status"]
and vs_info["status"]["reason"] == "AddedOrUpdated"
and vs_info["status"]["state"] == "Valid"
)
print(resp.status_code)
assert resp.status_code == 200
while time.perf_counter() < t_end:
resp = requests.get(
virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host},
)
occur.append(resp.status_code)

delete_policy(kube_apis.custom_objects, pol_name, test_namespace)
self.restore_default_vs(kube_apis, virtual_server_setup)
assert (
policy_info["status"]
and policy_info["status"]["reason"] == "AddedOrUpdated"
and policy_info["status"]["state"] == "Valid"
)
assert occur.count(200) <= 1

@pytest.mark.parametrize("src", [vs_policy_src])
def test_policy_matching_ingress_class(
self, kube_apis, crd_ingress_controller, virtual_server_setup, test_namespace, src,
):
"""
Test if rate-limiting policy is working with 1 rps
Test if policy with matching ingress class is applied to vs
"""
print(f"Create rl policy")
pol_name = create_policy_from_yaml(kube_apis.custom_objects, policy_ingress_class_src, test_namespace)

wait_before_test()
policy_info = read_custom_resource(kube_apis.custom_objects, test_namespace, "policies", pol_name)
assert (
policy_info["status"]
and policy_info["status"]["reason"] == "AddedOrUpdated"
and policy_info["status"]["state"] == "Valid"
)

print(f"Patch vs with policy: {src}")
patch_virtual_server_from_yaml(
kube_apis.custom_objects,
virtual_server_setup.vs_name,
src,
virtual_server_setup.namespace,
)

wait_before_test()
policy_info = read_custom_resource(kube_apis.custom_objects, test_namespace, "policies", pol_name)
occur = []
t_end = time.perf_counter() + 1
resp = requests.get(
virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host},

vs_info = read_custom_resource(kube_apis.custom_objects, virtual_server_setup.namespace, "virtualservers", virtual_server_setup.vs_name)
assert (
vs_info["status"]
and vs_info["status"]["reason"] == "AddedOrUpdated"
and vs_info["status"]["state"] == "Valid"
)
print(resp.status_code)
assert resp.status_code == 200
while time.perf_counter() < t_end:
resp = requests.get(
virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host},
)
occur.append(resp.status_code)

delete_policy(kube_apis.custom_objects, pol_name, test_namespace)
self.restore_default_vs(kube_apis, virtual_server_setup)
assert (
policy_info["status"]
and policy_info["status"]["reason"] == "AddedOrUpdated"
and policy_info["status"]["state"] == "Valid"
)
assert occur.count(200) <= 1

@pytest.mark.parametrize("src", [vs_policy_src])
def test_policy_non_matching_ingress_class(
self, kube_apis, crd_ingress_controller, virtual_server_setup, test_namespace, src,
):
"""
Test if rate-limiting policy is working with 1 rps
Test if non matching policy gets caught by vc validation
"""
print(f"Create rl policy")
pol_name = create_policy_from_yaml(kube_apis.custom_objects, policy_other_ingress_class_src, test_namespace)

wait_before_test()
policy_info = read_custom_resource(kube_apis.custom_objects, test_namespace, "policies", pol_name)
assert (
policy_info["status"]
and policy_info["status"]["reason"] == "AddedOrUpdated"
and policy_info["status"]["state"] == "Valid"
)

print(f"Patch vs with policy: {src}")
patch_virtual_server_from_yaml(
kube_apis.custom_objects,
virtual_server_setup.vs_name,
src,
virtual_server_setup.namespace,
)

wait_before_test()
policy_info = read_custom_resource(kube_apis.custom_objects, test_namespace, "policies", pol_name)

delete_policy(kube_apis.custom_objects, pol_name, test_namespace)
self.restore_default_vs(kube_apis, virtual_server_setup)
vs_info = read_custom_resource(kube_apis.custom_objects, virtual_server_setup.namespace, "virtualservers", virtual_server_setup.vs_name)
assert (
"status" not in policy_info
vs_info["status"]
and "rate-limit-primary is missing or invalid" in vs_info["status"]["message"]
and vs_info["status"]["reason"] == "AddedOrUpdatedWithWarning"
and vs_info["status"]["state"] == "Warning"
)

delete_policy(kube_apis.custom_objects, pol_name, test_namespace)
self.restore_default_vs(kube_apis, virtual_server_setup)

0 comments on commit f1ebca1

Please sign in to comment.