Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add CRD feature and add KNP tests #1470

Merged
merged 5 commits into from
Apr 12, 2023
Merged

Add CRD feature and add KNP tests #1470

merged 5 commits into from
Apr 12, 2023

Conversation

doniacld
Copy link
Contributor

@doniacld doniacld commented Mar 24, 2023 •
edited by joestringer
Loading

Motivation

Adding Kubernetes Network Policy tests coverage.

Changes

  • Add K8S extension client to retrieve CRD
  • Add CRD feature and CNP enabled requirement
  • Add Kubernetes Network Policies
  • Add Kubernetes Network Policies tests scenarios

List of scenarios added for Kubernetes Network Policy

Scenario CNP KNP Comment
no-policies - -
no-policies-extra - -
allow-all-except-world manifests/allow-all-except-world.yaml ❌ no entities in KNP
client-ingress manifests/client-ingress-from-client2.yaml manifests/client-ingress-from-client2-knp.yaml
all-ingress-deny manifests/deny-all-ingress.yaml manifests/deny-all-ingress-knp.yaml
all-egress-deny manifests/deny-all-egress.yaml manifests/deny-all-egress-knp.yaml
all-entities-deny manifests/deny-all-entities.yaml ❌ no entities in KNP
cluster-entity manifests/allow-cluster-entity.yaml ❌ no entities in KNP
host-entity manifests/allow-host-entity.yaml ❌ no entities in KNP
echo-ingress manifests/echo-ingress-from-other-client.yaml manifests/echo-ingress-from-other-client-knp.yaml
client-ingress-icmp manifests/echo-ingress-icmp.yaml ❌ no icmp in KNP
client-egress manifests/client-egress-to-echo.yaml manifests/client-egress-to-echo-knp.yaml
client-egress-expression manifests/client-egress-to-echo-expression.yaml manifests/client-egress-to-echo-expression-knp.yaml
client-egress-to-echo-service-account manifests/client-egress-to-echo-service-account.yaml ❌ no Cilium identity labels in KNP
to-entities-world manifests/client-egress-to-entities-world.yaml ❌ no entities in KNP
to-cidr-1111 manifests/client-egress-to-cidr-1111.yaml manifests/client-egress-to-cidr-1111-knp.yaml ipblock supported by Cilium for external IPs
echo-ingress-from-other-client-deny manifests/echo-ingress-from-other-client-deny.yaml ❌ deny not supported by KNP
client-ingress-from-other-client-icmp-deny manifests/echo-ingress-icmp-deny.yaml ❌ no icmp in KNP
client-egress-to-echo-deny manifests/client-egress-to-echo-deny.yaml ❌ deny not supported by KNP
client-ingress-to-echo-named-port-deny manifests/client-egress-to-echo-named-port-deny.yaml ❌ deny not supported by KNP
client-egress-to-echo-expression-deny manifests/client-egress-to-echo-expression-deny.yaml ❌ deny not supported by KNP
client-egress-to-echo-service-account-deny manifests/echo-ingress-from-other-client-deny.yaml ❌ no Cilium identity labels in KNP
client-egress-to-cidr-deny manifests/client-egress-to-cidr-1111-deny.yaml ❌ ideny not supported by KNP
client-egress-to-cidr-deny-default manifests/client-egress-to-cidr-1111-deny.yaml ❌ deny not supported by KNP
echo-ingress-l7 manifests/echo-ingress-l7-http.yaml ❌ L7 not supported by KNP
echo-ingress-l7-named-port manifests/echo-ingress-l7-http-named-port.yaml ❌ L7 not supported by KNP
client-egress-l7-method manifests/client-egress-only-dns.yaml, manifests/client-egress-l7-http-method.yaml ❌ L7 not supported by KNP
client-egress-l7 manifests/client-egress-l7-http.yaml ❌ L7 not supported by KNP
client-egress-l7-named-port manifests/client-egress-l7-http-named-port.yaml ❌ L7 not supported by KNP
client-egress-l7-tls-deny-without-headers manifests/client-egress-l7-tls.yaml ❌ L7 not supported by KNP
client-egress-l7-tls-headers manifests/client-egress-l7-tls.yaml ❌ L7 not supported by KNP
client-egress-l7-set-header manifests/client-egress-l7-http-matchheader-secret.yaml ❌ L7 not supported by KNP
dns-only manifests/client-egress-only-dns.yaml ❌ L7 (dns) not supported by KNP
to-fqdns manifests/client-egress-to-fqdns-one-one-one-one.yaml ❌ fqdns not supported by KNP

Fixes: #1484

@doniacld doniacld temporarily deployed to ci March 24, 2023 19:18 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci March 24, 2023 19:48 — with GitHub Actions Inactive
@doniacld doniacld changed the title Add CRD feature and CNP enabled requirement WIP: Add CRD feature and CNP enabled requirement Mar 24, 2023
@doniacld doniacld temporarily deployed to ci March 27, 2023 13:26 — with GitHub Actions Inactive
@doniacld doniacld marked this pull request as ready for review March 27, 2023 13:27
@doniacld doniacld requested review from a team as code owners March 27, 2023 13:27
@doniacld doniacld requested review from tklauser and youngnick March 27, 2023 13:27
@doniacld doniacld changed the title WIP: Add CRD feature and CNP enabled requirement Add CRD feature and CNP enabled requirement Mar 27, 2023
@doniacld doniacld temporarily deployed to ci March 27, 2023 13:36 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci March 27, 2023 14:16 — with GitHub Actions Inactive
@doniacld doniacld changed the title Add CRD feature and CNP enabled requirement Add CRD feature and add KNP tests Mar 28, 2023
@doniacld doniacld temporarily deployed to ci March 28, 2023 13:02 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci March 29, 2023 07:34 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci March 29, 2023 07:41 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci March 29, 2023 09:49 — with GitHub Actions Inactive
@gandro gandro added the needs-rebase This PR needs to be rebased because it has merge conflicts. label Mar 30, 2023
@doniacld doniacld temporarily deployed to ci April 1, 2023 17:38 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci April 1, 2023 17:58 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci April 1, 2023 18:04 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci April 3, 2023 07:54 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci April 3, 2023 08:00 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci April 3, 2023 08:08 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci April 3, 2023 16:42 — with GitHub Actions Inactive
@doniacld doniacld temporarily deployed to ci April 5, 2023 09:12 — with GitHub Actions Inactive
@gandro gandro mentioned this pull request Apr 5, 2023
Closed
joestringer
joestringer approved these changes Apr 5, 2023
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mainly looked at the network policy manifests, LGTM. Will leave the remaining review to codeowners.

@doniacld doniacld temporarily deployed to ci April 6, 2023 13:23 — with GitHub Actions Inactive
@michi-covalent
Copy link
Contributor

hey @sayboras could you review this? @youngnick and @tklauser might not be around.

@michi-covalent michi-covalent requested a review from sayboras April 6, 2023 14:54
@doniacld doniacld temporarily deployed to ci April 11, 2023 08:59 — with GitHub Actions Inactive
sayboras
sayboras approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work ✔️

@maintainer-s-little-helper
Copy link

Commit 8320090 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit 8320090 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@doniacld doniacld temporarily deployed to ci April 12, 2023 08:20 — with GitHub Actions Inactive
doniacld added 5 commits April 12, 2023 10:24
@doniacld
Add CRD feature and CNP enabled requirement
5157ea8 
Signed-off-by: Donia Chaiehloudj <donia.cld@isovalent.com>
@doniacld
Add KNP feature
9fea2e6 
Signed-off-by: Donia Chaiehloudj <donia.cld@isovalent.com>
@doniacld
Add KNPs utils methods
37cd79a 
Signed-off-by: Donia Chaiehloudj <donia.cld@isovalent.com>
@doniacld
Add KNPs tests scenarios
cd11419 
Signed-off-by: Donia Chaiehloudj <donia.cld@isovalent.com>
@doniacld
Refactor generic policy to Cilium policy
5a85037 
Signed-off-by: Donia Chaiehloudj <donia.cld@isovalent.com>
@doniacld doniacld temporarily deployed to ci April 12, 2023 08:25 — with GitHub Actions Inactive
@doniacld doniacld self-assigned this Apr 12, 2023
@gandro gandro removed request for tklauser and youngnick April 12, 2023 10:33
@doniacld doniacld added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 12, 2023
@michi-covalent michi-covalent merged commit 2c20333 into cilium:master Apr 12, 2023
jrajahalme added a commit that referenced this pull request Apr 13, 2023
@jrajahalme
connectivity: Make detected Cilium Agent version available for featur…
00a0dd9 
…e detection

Make detected Cilium Agent version available for feature detection and
use it instead of trying to detect Cilium version from the agent image
reference. This helps avoid failures like this when the image tag is not
a semantic version (in this case image tag is "test"):

  connectivity test failed: unable to determine if KNP feature is enabled: failed to parse Cilium version test to semver

Cilium agent version was already detected, this PR moves the detection a
bit earlier to make it available for feature detection.

Fixes: #1470

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme mentioned this pull request Apr 13, 2023
Merged
@tklauser tklauser mentioned this pull request Apr 13, 2023
Merged
tklauser pushed a commit that referenced this pull request Apr 13, 2023
@jrajahalme @tklauser
connectivity: Make detected Cilium Agent version available for featur…
682f8aa 
…e detection

Make detected Cilium Agent version available for feature detection and
use it instead of trying to detect Cilium version from the agent image
reference. This helps avoid failures like this when the image tag is not
a semantic version (in this case image tag is "test"):

  connectivity test failed: unable to determine if KNP feature is enabled: failed to parse Cilium version test to semver

Cilium agent version was already detected, this PR moves the detection a
bit earlier to make it available for feature detection.

Fixes: #1470

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@ZhongRuoyu ZhongRuoyu mentioned this pull request Apr 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@sayboras sayboras sayboras approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

Assignees

@doniacld doniacld

Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for cilium connectivity test policy tests with NetworkPolicy
5 participants
@doniacld @michi-covalent @joestringer @sayboras @gandro