Implement e2e test for security policy #360
Conversation
k8s-ci-robot
added
the
size/L
k8s-ci-robot
added
the
cncf-cla: yes
MrHohn
force-pushed
the
e2e-security-policy
branch
from
f42f394
to
f082ca9
Compare
| Name: name, | ||
| Rules: []*computebeta.SecurityPolicyRule{ | ||
| &computebeta.SecurityPolicyRule{ | ||
| Action: "deny(403)", |
There was a problem hiding this comment.
Also, turned out it's possible to add check on the response code (can be explicitly configured on policy). I will add that to validator later.
There was a problem hiding this comment.
Will put that in a separate PR.
cmd/e2e-test/basic_test.go
Outdated
| if err := Framework.Clientset.Extensions().Ingresses(s.Namespace).Delete(tc.ing.Name, &metav1.DeleteOptions{}); err != nil { | ||
| t.Errorf("Delete(%q) = %v, want nil", tc.ing.Name, err) | ||
| if err := e2e.WaitForIngressDeletion(ctx, Framework.Cloud, gclb, s, ing, false); err != nil { | ||
| t.Errorf("Failed to wait for ingress deletion: %v", err) |
There was a problem hiding this comment.
e2e.WaitForIngressDeletion(..., %q, false) = %v, want nil", ing.Name, err
| Name: name, | ||
| Rules: []*computebeta.SecurityPolicyRule{ | ||
| &computebeta.SecurityPolicyRule{ | ||
| Action: "deny(403)", |
cmd/e2e-test/security_policy_test.go
Outdated
| t.Parallel() | ||
|
|
||
| Framework.RunWithSandbox("Security Policy Enable", t, func(t *testing.T, s *e2e.Sandbox) { | ||
| // ------ Step: Preparing test ------ |
There was a problem hiding this comment.
replace this with t.Logf("creating security policy") or something similar
"preparing test" doesn't tell us anything about what is going on here
There was a problem hiding this comment.
Removed comment since it isn't useful. Things like "creating security policy" are already logged below.
cmd/e2e-test/security_policy_test.go
Outdated
| t.Fatalf("Failed to prepare k8s resources: %v", err) | ||
| } | ||
|
|
||
| // ------ Step: Executing test ------ |
cmd/e2e-test/security_policy_test.go
Outdated
| } | ||
| defer func() { | ||
| if err := cleanupSecurityPolicies(ctx, Framework.Cloud, policies); err != nil { | ||
| t.Errorf("Failed to cleanup policies: %v", err) |
There was a problem hiding this comment.
don't capitalize t.Errorf strings
cmd/e2e-test/security_policy_test.go
Outdated
| }() | ||
| policies, err := prepareSecurityPolicies(ctx, Framework.Cloud, policies) | ||
| if err != nil { | ||
| t.Fatalf("Failed to prepare policies: %v", err) |
There was a problem hiding this comment.
prepareSecurityPolicies(ctx, Framework.Cloud, %+v) = _, %v; want _, nil, policies, err
cmd/e2e-test/security_policy_test.go
Outdated
| Framework.RunWithSandbox("Security Policy Transition", t, func(t *testing.T, s *e2e.Sandbox) { | ||
| // ------ Step: Preparing test ------ | ||
|
|
||
| ctx := context.Background() |
cmd/e2e-test/security_policy_test.go
Outdated
| } | ||
| } | ||
|
|
||
| // ------ Step: Cleaning up test ------ |
cmd/e2e-test/security_policy_test.go
Outdated
| glog.Infof("Deleting security policies...") | ||
| for _, policy := range policies { | ||
| if err := c.BetaSecurityPolicies().Delete(ctx, meta.GlobalKey(policy.Name)); err != nil { | ||
| return fmt.Errorf("failed to delete security policy %q: %v", policy.Name, err) |
There was a problem hiding this comment.
glog.Info("Security policy %q deleted
cmd/e2e-test/security_policy_test.go
Outdated
| glog.Infof("Deleting security policies...") | ||
| for _, policy := range policies { | ||
| if err := c.BetaSecurityPolicies().Delete(ctx, meta.GlobalKey(policy.Name)); err != nil { | ||
| return fmt.Errorf("failed to delete security policy %q: %v", policy.Name, err) |
There was a problem hiding this comment.
I feel like this should just try to delete all policies and then return the first error
There was a problem hiding this comment.
Sounds good, changed to delete all policies and capture all errors..
cmd/e2e-test/security_policy_test.go
Outdated
| } | ||
| glog.Infof("Backend config %s/%s created", testBackendConfig.Name, s.Namespace) | ||
|
|
||
| _, testSvc, err := e2e.CreateEchoService(s, "service-1", testBackendConfigAnnotation) |
There was a problem hiding this comment.
Why not just put this outside at the top of the test instead of doing it here?
MrHohn
force-pushed
the
e2e-security-policy
branch
2 times, most recently
from
bb0527d
to
cc2b099
Compare
MrHohn
force-pushed
the
e2e-security-policy
branch
from
cc2b099
to
707d8aa
Compare
pkg/e2e/helpers.go
Outdated
| @@ -58,11 +59,25 @@ func WaitForIngress(s *Sandbox, ing *v1beta1.Ingress) (*v1beta1.Ingress, error) | |||
| return ing, err | |||
| } | |||
|
|
|||
| // WaitForIngressDeletion deletes the given ingress and waits for the | |||
| // resources associated with it to be deleted. | |||
| func WaitForIngressDeletion(ctx context.Context, c cloud.Cloud, g *fuzz.GCLB, s *Sandbox, ing *v1beta1.Ingress, omitSystemDefaultBackend bool) error { | |||
There was a problem hiding this comment.
do you need to pass in cloud separately if you pass in sandbox?
pkg/e2e/helpers.go
Outdated
| @@ -58,11 +59,25 @@ func WaitForIngress(s *Sandbox, ing *v1beta1.Ingress) (*v1beta1.Ingress, error) | |||
| return ing, err | |||
| } | |||
|
|
|||
| // WaitForIngressDeletion deletes the given ingress and waits for the | |||
| // resources associated with it to be deleted. | |||
| func WaitForIngressDeletion(ctx context.Context, c cloud.Cloud, g *fuzz.GCLB, s *Sandbox, ing *v1beta1.Ingress, omitSystemDefaultBackend bool) error { | |||
There was a problem hiding this comment.
Create a struct
type IngressDeletionOptions struct {
skipDefaultBackend bool
}
d03b49c
to
203e047
Compare
This e2e test mostly checks if security policy is properly set on the relevant backend service resource upon ingress creation/update.
/assign @rramkumar1 @bowei