search

openshift / origin

Conformance test suite for OpenShift
http://www.openshift.org
Apache License 2.0
8.44k stars 4.69k forks source link

Add new E2E test for [SDN-1364] ACL audit logging #26071

Closed astoycos closed 2 years ago

astoycos commented 3 years ago

This works adds a networking extended test for the new network policy audit logging feature

The name of the new test is:

"[sig-network] acl-logging should activate acl-logging [Suite:openshift/conformance/parallel]"

It first activates the audit logging by applying new annotations to the acl-logging ns 

ns.Annotations["k8s.ovn.org/acl-logging"] = `{ "deny": "alert", "allow": "alert" }`

Next it make's three pods, two(pod[0] and pod[1]) in the ns acl-logging and one(pod[3]) in the ns acl-logging-off

Then it makes two networkPolicies in the acl-logging ns ->

  1. allow-same-namespace
  2. default-deny-all

Finally it sends packets from pods 1 and 2 to pod 0..... The packet from pod[1] will be allowed by the allow-same-namespace policy and the packet from pod[2] will be dropped by the default-deny-all policy. This info will be captured by the acl-audit-logs.

The logs are then collected and verified with an oc adm node-logs

openshift-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: astoycos To complete the pull request process, please assign smarterclayton after the PR has been reviewed. You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[test/extended/networking/OWNERS](https://github.com/openshift/origin/blob/master/test/extended/networking/OWNERS)** - **[test/extended/util/annotate/OWNERS](https://github.com/openshift/origin/blob/master/test/extended/util/annotate/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
astoycos commented 3 years ago

/hold Since we need to skip this test for the OCP-SDN network plugin in the rules defined in openshift/kubernetes and then re-vendor once https://github.com/openshift/kubernetes/pull/664 merges

astoycos commented 3 years ago

/assign @squeed

Did I do this workflow right? (i.e opening the PR in openshift/k8s in order to ensure we don't run the test for the default openshift-sdn plugin)

squeed commented 3 years ago

The test seems reasonable as written; just needs to be structured more like an Openshift / kube test.

squeed commented 3 years ago

Easiest way to skip this is in code; just retrieve the network.config.openshift.io object and check the configured plugin.

astoycos commented 2 years ago

@danwinship If you could give this another look I'd appreciate it

astoycos commented 2 years ago

/retest

squeed commented 2 years ago

The test is good, but needs to be structured a bit differently to ensure it tolerates delays in ovn-kubernetes (and thus doesn't become a flake-fest). ovn sadly sometimes falls behind during load or restarts / upgrades.

I'd recommend doing two things:

  1. looping until an expected-to-fail connection does indeed fail
  2. looping the ping-and-check-logs step until the expected log line shows up.

Those should work around any asynchronicity.

astoycos commented 2 years ago

I fixed this concern by ensuring we retest both pings (allow and deny) until the results are as expected, I don't think we need to provide a retry mechanism for the log parsing, since if the traffic is flowing as expected (i.e the OVN ACLs are there) then we should always be seeing the correct logs

I also added a mechanism that collects all created pod logs on failure which should make any potential bugs much easier to fix

astoycos commented 2 years ago

/retest

astoycos commented 2 years ago

/unhold

astoycos commented 2 years ago

/test e2e-aws-ovn

astoycos commented 2 years ago

/test e2e-gcp-ovn

openshift-ci[bot] commented 2 years ago

@astoycos: The specified target(s) for /test were not found. The following commands are available to trigger jobs:

Use /test all to run the following jobs:

In response to [this](https://github.com/openshift/origin/pull/26071#issuecomment-856935316): >/test e2e-gcp-ovn > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
astoycos commented 2 years ago

/test e2e-metal-ipi-ovn-dualstack

astoycos commented 2 years ago

/test e2e-metal-ipi-ovn-dualstack

astoycos commented 2 years ago

/test e2e-aws-ovn

astoycos commented 2 years ago

[sig-network][Feature:Network Policy Audit logging] when using openshift ovn-kubernetes should ensure acl logs are created and correct [Suite:openshift/conformance/parallel]

Is now passing on

ci/prow/e2e-metal-ipi-ovn-dualstack ci/prow/e2e-metal-ipi-ovn-ipv6 ci/prow/e2e-aws-ovn

astoycos commented 2 years ago

retesting those tests for further verification

astoycos commented 2 years ago

/test e2e-metal-ipi-ovn-dualstack

astoycos commented 2 years ago

/test e2e-metal-ipi-ovn-ipv6

openshift-ci[bot] commented 2 years ago

@astoycos: The specified target(s) for /test were not found. The following commands are available to trigger jobs:

Use /test all to run the following jobs:

In response to [this](https://github.com/openshift/origin/pull/26071#issuecomment-857363544): >/test ci/prow/e2e-aws-ovn Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
astoycos commented 2 years ago

/test e2e-aws-ovn

astoycos commented 2 years ago

Otherwise this test should be ready to go

astoycos commented 2 years ago

/test e2e-aws-ovn

astoycos commented 2 years ago

/test e2e-metal-ipi-ovn-dualstack

squeed commented 2 years ago

/approve /lgtm

squeed commented 2 years ago

@knobunc could you approve this (it touches some helper code)

astoycos commented 2 years ago

/test e2e-aws-ovn

squeed commented 2 years ago

/lgtm

astoycos commented 2 years ago

@squeed I lost the lgtm because I saw a flake on the recent run, and added some better "on failure" logic to see what was going on

astoycos commented 2 years ago

Most recent changes should fix the flake (sorry I lost the LGTM again)

astoycos commented 2 years ago

/test e2e-aws-ovn

astoycos commented 2 years ago

/test e2e-metal-ipi-ovn-dualstack

astoycos commented 2 years ago

Letting this curn in CI for a bit, I will revisit late this afternoon (also looks like I need an accompanying bug for this)

astoycos commented 2 years ago

/test e2e-aws-serial

astoycos commented 2 years ago

I am seeing the test pass without flakes in 

ci/prow/e2e-metal-ipi-ovn-dualstack
ci/prow/e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-aws-ovn
astoycos commented 2 years ago

/test e2e-aws-serial

knobunc commented 2 years ago

/approve /lgtm

openshift-ci[bot] commented 2 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astoycos, knobunc, squeed

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[test/extended/networking/OWNERS](https://github.com/openshift/origin/blob/master/test/extended/networking/OWNERS)~~ [knobunc,squeed] - ~~[test/extended/util/annotate/OWNERS](https://github.com/openshift/origin/blob/master/test/extended/util/annotate/OWNERS)~~ [knobunc] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot commented 2 years ago

/retest

Please review the full test history for this PR and help us cut down flakes.