CORS-4230: Add a firewall spec and the ability to manage or unmanaged firewall rule creation #1532

barbacbd · 2025-09-18T19:38:04Z

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

Add API changes to Skip firewall rule creation. When unmanaged, the firewall rules will not be
created. When this is the case, the firewall rules should exist prior to creating the network.
This will allow ServiceAccounts to skip the rules:compute.firewalls.create.
Update the services and interfaces. The firewall service will no longer create firewall rules when
the firewall policy is set to unmanaged OR when a shared vpc is used during installation and resource creation.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

https://issues.redhat.com/browse/CORS-4230

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Add API changes to Skip firewall rule creation. When unmanaged, the firewall rules will not be
created. When this is the case, the firewall rules should exist prior to creating the network.
This will allow ServiceAccounts to skip the rules:compute.firewalls.create.

Update the services and interfaces. The firewall service will no longer create firewall rules when
the firewall policy is set to unmanaged OR when a shared vpc is used during installation and resource creation.

netlify · 2025-09-18T19:38:10Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-gcp ready!

Name	Link
🔨 Latest commit	`ea29531`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-gcp/deploys/68e79b8544829900084d8df4
😎 Deploy Preview	https://deploy-preview-1532--kubernetes-sigs-cluster-api-gcp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-09-18T19:38:15Z

Hi @barbacbd. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

damdo

/ok-to-test

barbacbd · 2025-09-22T10:55:08Z

/retest

damdo

Thanks @barbacbd

On the API side, I think we could make it a bit more "future proof" and adherent to the upstream API conventions: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md

api/v1beta1/types.go

damdo

Thanks!

/approve

/assign @justinsb @salasberryfin @cpanato

k8s-ci-robot · 2025-09-25T13:50:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: barbacbd, damdo

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:

~~OWNERS~~ [damdo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cloud/services/compute/firewalls/reconcile.go

api/v1beta1/types.go

justinsb · 2025-09-28T12:03:42Z

Generally LGTM but one quibble on rules vs rulesManagement (which would normally not be a blocker, except it is our API so it's painful to change later)

api/v1beta1/types.go

JoelSpeed · 2025-10-03T10:14:10Z

cloud/scope/managedcluster.go

+// SkipFirewallRuleCreation returns whether the spec indicates that firewall rules
+// should be created or not.
+func (s *ManagedClusterScope) SkipFirewallRuleCreation() bool {
+	return (s.GCPManagedCluster.Spec.Network.Firewall.RulesManagement == infrav1.RulesManagementUnmanaged) || s.IsSharedVpc()


How would you explain this relationship in the API documentation/validations?

There are no custom rules in this PR so this should be taken only with the information in this PR. If the installation includes a shared VPC or if the user has explicitly said firewall rules are unmanaged then the firewall rules should not be created. It is important to note that this will ONLY apply to the default rules.

I don't see anything in the API about not being compatible with shared VPC?

We should update he godoc of the API to mention this

… firewall rule creation api: Add API changes to Skip firewall rule creation. When unmanaged, the firewall rules will not be created. When this is the case, the firewall rules should exist prior to creating the network. This will allow ServiceAccounts to skip the rules: compute.firewalls.create cloud: Update the services and interfaces. The firewall service will no longer create firewall rules when the firewall policy is set to unmanaged OR when a shared vpc is used during installation and resource creation.

JoelSpeed · 2025-10-09T12:36:37Z

API LGTM

damdo · 2025-10-09T15:50:14Z

Looks good

But I'll leave the LGTM to other maintainers that have already reviewed.

/assign @justinsb @salasberryfin @cpanato

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 18, 2025

k8s-ci-robot requested review from damdo and dims September 18, 2025 19:38

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 18, 2025

damdo reviewed Sep 19, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 19, 2025

damdo reviewed Sep 23, 2025

View reviewed changes

api/v1beta1/types.go Outdated Show resolved Hide resolved

barbacbd force-pushed the CORS-4230 branch from 9b62d6a to 56d4280 Compare September 24, 2025 17:48

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 24, 2025

barbacbd changed the title ~~CORS-4230: Add SkipFirewallRuleCreation~~ CORS-4230: Add a firewall spec and the ability to manage or unmanaged firewall rule creation Sep 24, 2025

barbacbd requested review from JoelSpeed and damdo September 24, 2025 19:01

damdo reviewed Sep 25, 2025

View reviewed changes

k8s-ci-robot assigned cpanato, justinsb and salasberryfin Sep 25, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 25, 2025

barbacbd mentioned this pull request Sep 26, 2025

Firewallrules creation #1538

Open

3 tasks

justinsb reviewed Sep 28, 2025

View reviewed changes

cloud/services/compute/firewalls/reconcile.go Show resolved Hide resolved

api/v1beta1/types.go Outdated Show resolved Hide resolved

JoelSpeed reviewed Oct 3, 2025

View reviewed changes

barbacbd requested review from JoelSpeed, damdo and justinsb October 3, 2025 11:41

barbacbd force-pushed the CORS-4230 branch 2 times, most recently from 80e9953 to 0699e58 Compare October 8, 2025 18:51

barbacbd force-pushed the CORS-4230 branch from 0699e58 to ea29531 Compare October 9, 2025 11:24

CORS-4230: Add a firewall spec and the ability to manage or unmanaged firewall rule creation #1532

Are you sure you want to change the base?

CORS-4230: Add a firewall spec and the ability to manage or unmanaged firewall rule creation #1532

Conversation

barbacbd commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-cluster-api-gcp ready!

Uh oh!

k8s-ci-robot commented Sep 18, 2025

Uh oh!

damdo left a comment

Choose a reason for hiding this comment

Uh oh!

barbacbd commented Sep 22, 2025

Uh oh!

damdo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

damdo left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Sep 25, 2025

Uh oh!

Uh oh!

Uh oh!

justinsb commented Sep 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoelSpeed Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

barbacbd Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

JoelSpeed commented Oct 9, 2025

Uh oh!

damdo commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

barbacbd commented Sep 18, 2025 •

edited

Loading

netlify bot commented Sep 18, 2025 •

edited

Loading