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.

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

Add support for BaselineAdminNetworkPolicy. #9464

Merged
merged 8 commits into from
Dec 5, 2024

Conversation

fasaxc
Copy link
Member

@fasaxc fasaxc commented Nov 12, 2024

Description

Add support for sig-network-policy-api's BaselineAdminNetworkPolicy.

  • Map the single BANP to a policy (with order=1000) in a new dedicated Tier with order=10,000,000.
  • Convert the policy as per AdminNetworkPolicy. Due to different datatypes, the logic has to be copied (I didn't fancy using reflection).
  • Add RBAC to allow Typha and Node to access the resources.

Related issues/PRs

CORE-10800

Builds on ANP work:

Tier support: #9085
Default action in tiers: #9232
AdminNetworkPolicy: #9206
AdminNetworkPolicy egress networks: #9276
AdminNetworkPolicy named port: #9254
Operator: tigera/operator#3634
BANP validation: #9633

Todos

  • Tests
  • Documentation
  • Release note

Release Note

Add support for Kubernetes BaselineAdminNetworkPolicy. https://network-policy-api.sigs.k8s.io/api-overview/

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@marvin-tigera marvin-tigera added this to the Calico v3.30.0 milestone Nov 12, 2024
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Nov 12, 2024
@fasaxc fasaxc force-pushed the banp branch 8 times, most recently from 526d489 to 568c1c2 Compare November 19, 2024 14:42
@fasaxc fasaxc changed the title [WIP] Initial pass at BaselineAdminNetworkPolicy. Add support for BaselineAdminNetworkPolicy. Nov 19, 2024
- Top level struct types are different but the "leaf" structs
  are the same in most cases.
- Extract the "leaf" structs at the top level and then share
  the remaining logic.
@fasaxc fasaxc marked this pull request as ready for review December 3, 2024 14:29
@fasaxc fasaxc requested a review from a team as a code owner December 3, 2024 14:29
@fasaxc fasaxc force-pushed the banp branch 4 times, most recently from 9d94a19 to 4b2f1b1 Compare December 4, 2024 09:58
Copy link
Member

@mazdakn mazdakn left a comment

Choose a reason for hiding this comment

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

LGTM. I just left a few nits.

libcalico-go/lib/clientv3/tier_e2e_test.go Outdated Show resolved Hide resolved
libcalico-go/lib/backend/k8s/conversion/conversion.go Outdated Show resolved Hide resolved
rules := []apiv3.Rule{}
func unpackANPPorts(k8sPorts *[]adminpolicy.AdminNetworkPolicyPort) (map[string][]numorstring.Port, []string, error) {
// If there are no ports, represent that as zero struct.
ports := []adminpolicy.AdminNetworkPolicyPort{{}}
Copy link
Member

Choose a reason for hiding this comment

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

You probably forgot to replace this section with the new unpackANPPorts function.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the new function?

Copy link
Member

Choose a reason for hiding this comment

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

clearly I left this comment in a wrong place. I meant this change: #9568

@fasaxc
Copy link
Member Author

fasaxc commented Dec 5, 2024

/merge-when-ready squash-commits

@marvin-tigera
Copy link
Contributor

OK, I will merge the pull request when it's ready, squash the commits when I merge it, and leave the branch after I've merged it.

@marvin-tigera marvin-tigera merged commit 20f048e into projectcalico:master Dec 5, 2024
5 checks passed
sridhartigera pushed a commit to sridhartigera/calico that referenced this pull request Dec 18, 2024
* Initial pass at BaselineAdminNetworkPolicy.

* Fix policy name prefixes.

* Combine ANP and BANP conversion logic.

- Top level struct types are different but the "leaf" structs
  are the same in most cases.
- Extract the "leaf" structs at the top level and then share
  the remaining logic.

* Fix felix syncer test.

* Ignore pprofs in gitignore.

* Add tests for BANP conversion.

* Markups.
tomastigera pushed a commit to tomastigera/project-calico-calico that referenced this pull request Dec 20, 2024
* Initial pass at BaselineAdminNetworkPolicy.

* Fix policy name prefixes.

* Combine ANP and BANP conversion logic.

- Top level struct types are different but the "leaf" structs
  are the same in most cases.
- Extract the "leaf" structs at the top level and then share
  the remaining logic.

* Fix felix syncer test.

* Ignore pprofs in gitignore.

* Add tests for BANP conversion.

* Markups.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-pr-required Change is not yet documented merge-when-ready release-note-required Change has user-facing impact (no matter how small) squash-commits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants