ingress: Create FromGroups resource #30708

Alex-Waring · 2024-02-12T09:28:06Z

Duplicates the structures inplace to evaluate the toGroups resource into the ingress section, allowing the creation of FromGroups. This means AWS SG groups can be included as ingress resources and directly translated into fromCIDR rules.

Example usage:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "l4-rule"
spec:
  endpointSelector:
    matchLabels:
      role: backend
  ingress:
  - toPorts:
    - ports:
      - port: '80'
        protocol: TCP
    fromGroups:
    - aws:
        securityGroupsIds:
        - 'sg-0123456789abcdefg'

Fixes: #30032

Allows for using AWS SGs in the ingress section of rules.

christarazi

Thanks for the PR, initial approach LGTM. A few comments below. I'd also suggest making the rename of ToGroups to Groups a separate commit, just to help scope down the review.

pkg/policy/api/ingress.go

christarazi

Thanks, looking good! Could you take care of the two comments below and it's good to go from my perspective?

pkg/policy/groups/helpers.go

pkg/k8s/apis/cilium.io/client/crds/v2/ciliumclusterwidenetworkpolicies.yaml

christarazi · 2024-02-29T22:07:34Z

/test

Alex-Waring · 2024-03-01T08:22:01Z

Hey @christarazi, I'm happy to do more investigation but these errors don't seem to be related to my change. I can rebase onto master?

christarazi · 2024-03-01T19:26:13Z

Sure, rebase and I will retrigger the tests.

In preparation for making the groups resource applicable to both ingress and egress rules, this commit changes the name of the ToGroups struct to Groups. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

As I am going to be copying the Create Derivative functions, this commit abstracts out some of the logic into a helper function, to make the code more DRY. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

Duplicates the structures inplace to evaluate the toGroups resource into the ingress section, allowing the creation of FromGroups. This means AWS SG groups can be included as ingress resources and directly translated into fromCIDR rules. Fixes: cilium#30032 Signed-off-by: Alex Waring <ajmwaring@gmail.com>

nathanjsweet · 2024-03-04T20:06:14Z

Looks good!

nathanjsweet · 2024-03-04T20:06:23Z

/test

Edit:

CI: Ingress Conformance Test: An Ingress with mixed path rules should send traffic to the matching backend service where Exact is preferred #30281
CI: (ci-ipsec-upgrade) v1.14 -> v1.15: client-egress-l7-named-port #29856

joestringer · 2024-04-15T23:23:40Z

Hi @Alex-Waring , is the example in the PR description correct? The indentation seems odd, and it's not referring to fromGroups.

Alex-Waring · 2024-04-16T07:51:54Z

Hi @joestringer no it was very much not correct, apologies. I've updated it now.

Currently fromGroups, which was added in cilium#30708, is correctly represented in the YAML but not being converted to a rule. This commit fixes that by: - Reporting that a rule requires derivation when an ingress component does, and making sure to clear ingress rules when creating that derivative. - Adding FromGroups to parseToCiliumIngressCommonRule logic - During validation make sure that fromGroups is not combined with other L3 rules. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

In the previous commit and cilium#30708 the fromGroups resource was added, but testing was not covered properly. This commit validates that we are working as expected.

Currently fromGroups, which was added in cilium#30708, is correctly represented in the YAML but not being converted to a rule. This commit fixes that by: - Reporting that a rule requires derivation when an ingress component does, and making sure to clear ingress rules when creating that derivative. - Adding FromGroups to parseToCiliumIngressCommonRule logic - During validation make sure that fromGroups is not combined with other L3 rules.

In the previous commit and cilium#30708 the fromGroups resource was added, but testing was not covered properly. This commit validates that we are working as expected.

Currently fromGroups, which was added in cilium#30708, is correctly represented in the YAML but not being converted to a rule. This commit fixes that by: - Reporting that a rule requires derivation when an ingress component does, and making sure to clear ingress rules when creating that derivative. - Adding FromGroups to parseToCiliumIngressCommonRule logic - During validation make sure that fromGroups is not combined with other L3 rules. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

In the previous commit and cilium#30708 the fromGroups resource was added, but testing was not covered properly. This commit validates that we are working as expected. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

Currently fromGroups, which was added in cilium#30708, is correctly represented in the YAML but not being converted to a rule. This commit fixes that by: - Reporting that a rule requires derivation when an ingress component does, and making sure to clear ingress rules when creating that derivative. - Adding FromGroups to parseToCiliumIngressCommonRule logic - During validation make sure that fromGroups is not combined with other L3 rules. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

In the previous commit and cilium#30708 the fromGroups resource was added, but testing was not covered properly. This commit validates that we are working as expected. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

Currently fromGroups, which was added in #30708, is correctly represented in the YAML but not being converted to a rule. This commit fixes that by: - Reporting that a rule requires derivation when an ingress component does, and making sure to clear ingress rules when creating that derivative. - Adding FromGroups to parseToCiliumIngressCommonRule logic - During validation make sure that fromGroups is not combined with other L3 rules. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

In the previous commit and #30708 the fromGroups resource was added, but testing was not covered properly. This commit validates that we are working as expected. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 12, 2024

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Feb 12, 2024

Alex-Waring mentioned this pull request Feb 12, 2024

Support AWS Security Groups with security group rules ingress from another security group for Network Policies #30032

Closed

Alex-Waring marked this pull request as ready for review February 13, 2024 19:59

Alex-Waring requested review from a team as code owners February 13, 2024 19:59

Alex-Waring requested a review from nathanjsweet February 13, 2024 19:59

Alex-Waring force-pushed the AWar/groups/ingress branch from 7f8c38b to d53f7eb Compare February 13, 2024 20:02

ti-mo added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Feb 22, 2024

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 22, 2024

christarazi reviewed Feb 26, 2024

View reviewed changes

pkg/policy/api/ingress.go Outdated Show resolved Hide resolved

pkg/policy/api/ingress.go Outdated Show resolved Hide resolved

christarazi added kind/feature This introduces new functionality. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. labels Feb 26, 2024

Alex-Waring force-pushed the AWar/groups/ingress branch 3 times, most recently from f3ddfba to a08ac45 Compare February 28, 2024 09:35

Alex-Waring requested a review from christarazi February 28, 2024 12:31

ldelossa assigned Alex-Waring and unassigned Alex-Waring Feb 28, 2024

christarazi approved these changes Feb 28, 2024

View reviewed changes

pkg/policy/groups/helpers.go Outdated Show resolved Hide resolved

pkg/k8s/apis/cilium.io/client/crds/v2/ciliumclusterwidenetworkpolicies.yaml Outdated Show resolved Hide resolved

Alex-Waring force-pushed the AWar/groups/ingress branch 3 times, most recently from 7a7c60f to 0e39308 Compare February 28, 2024 21:34

Alex-Waring added 2 commits March 2, 2024 11:39

ingress: Rename FromGroups resource

9fe893d

In preparation for making the groups resource applicable to both ingress and egress rules, this commit changes the name of the ToGroups struct to Groups. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

abstract out ExtractCidrSet

8a3cca8

As I am going to be copying the Create Derivative functions, this commit abstracts out some of the logic into a helper function, to make the code more DRY. Signed-off-by: Alex Waring <ajmwaring@gmail.com>

Alex-Waring force-pushed the AWar/groups/ingress branch from 0e39308 to 8c68fdb Compare March 2, 2024 11:45

nathanjsweet approved these changes Mar 4, 2024

View reviewed changes

nathanjsweet enabled auto-merge March 4, 2024 20:08

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 12, 2024

nathanjsweet added this pull request to the merge queue Mar 12, 2024

Merged via the queue into cilium:main with commit cc82eff Mar 12, 2024
62 checks passed

JonasKs mentioned this pull request Apr 4, 2024

CFP: External endpoints #31768

Open

Alex-Waring mentioned this pull request Apr 29, 2024

ingress: Correct FromGroups rule Parsing #32231

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ingress: Create FromGroups resource #30708

ingress: Create FromGroups resource #30708

Alex-Waring commented Feb 12, 2024 •

edited

christarazi left a comment

christarazi left a comment

christarazi commented Feb 29, 2024

Alex-Waring commented Mar 1, 2024

christarazi commented Mar 1, 2024

nathanjsweet commented Mar 4, 2024

nathanjsweet commented Mar 4, 2024 •

edited by christarazi

joestringer commented Apr 15, 2024

Alex-Waring commented Apr 16, 2024

ingress: Create FromGroups resource #30708

ingress: Create FromGroups resource #30708

Conversation

Alex-Waring commented Feb 12, 2024 • edited

christarazi left a comment

Choose a reason for hiding this comment

christarazi left a comment

Choose a reason for hiding this comment

christarazi commented Feb 29, 2024

Alex-Waring commented Mar 1, 2024

christarazi commented Mar 1, 2024

nathanjsweet commented Mar 4, 2024

nathanjsweet commented Mar 4, 2024 • edited by christarazi

joestringer commented Apr 15, 2024

Alex-Waring commented Apr 16, 2024

Alex-Waring commented Feb 12, 2024 •

edited

nathanjsweet commented Mar 4, 2024 •

edited by christarazi