no-jira: Allow CAPG to create firewall Rules by barbacbd · Pull Request #10318 · openshift/installer

barbacbd · 2026-02-16T19:07:22Z

*** Includes unmerged CAPG commits:
kubernetes-sigs/cluster-api-provider-gcp#1538

** Code Changes:

pkg/asset/manifests/gcp/cluster.go:

Migrated the creation of firewall rules to CAPG. These rules are now formed
in the manifest and passed to CAPG for creation.

pkg/infrastructure/gcp/clusterapi:

Remove the creation of firewall rules.

barbacbd · 2026-02-16T19:08:04Z

/cc @patrickdillon
/cc @damdo
/cc @tthvo
/cc @sadasu
/cc @JoelSpeed

openshift-ci · 2026-02-16T19:08:27Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mtulio for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

tthvo · 2026-02-17T22:55:57Z

pkg/asset/manifests/gcp/cluster.go

+	if installConfig.Config.Publish == types.InternalPublishingStrategy {
+		healthCheckSrcRanges = append(healthCheckSrcRanges, []string{"209.85.152.0/22", "209.85.204.0/22"}...)
+	}


if installConfig.Config.Publish == types.InternalPublishingStrategy { healthCheckSrcRanges = append(healthCheckSrcRanges, []string{"209.85.152.0/22", "209.85.204.0/22"}...) }

I think the condition should be installConfig.Config.Publish == types.ExternalPublishingStrategy since these 209.x.x.x/22 subnets are applicable to public clusters, right?

nit: Can we use installConfig.Config.PublicAPI()?

This is a case of don't fix what isn't broken. These were taken directly from the infrastructure creation.

Sorry, but this part said health check IPs "209.85.152.0/22", "209.85.204.0/22" are for public install, right?

installer/pkg/infrastructure/gcp/clusterapi/firewallrules.go

Lines 249 to 253 in b0514c8

srcRanges = []string{"35.191.0.0/16", "130.211.0.0/22"}

if in.InstallConfig.Config.Publish == types.ExternalPublishingStrategy {

// public installs require additional google ip addresses for health checks

srcRanges = append(srcRanges, []string{"209.85.152.0/22", "209.85.204.0/22"}...)

}

pkg/asset/manifests/gcp/cluster.go

tthvo · 2026-02-17T23:11:03Z

pkg/asset/manifests/gcp/cluster.go

-				Firewall: capg.FirewallSpec{
-					DefaultRulesManagement: firewallRulesManagementPolicy,
+				FirewallSpec: capg.FirewallSpec{
+					DefaultRulesManagement: capg.RulesManagementManaged,


We still need to consider user-provider options for rule management mode, right?

firewallRulesManagementPolicy := capg.RulesManagementManaged if installConfig.Config.GCP.FirewallRulesManagement == gcp.UnmanagedFirewallRules { firewallRulesManagementPolicy = capg.RulesManagementUnmanaged }

Yes, this was just a proof on concept. It was more to assist getting the firewall rules merged in CAPG.

tthvo · 2026-02-17T23:14:19Z

pkg/asset/manifests/gcp/cluster.go

+	firewallRules := createBootstrapFirewallRuleForCAPG(installConfig, clusterID)
+	firewallRules = append(firewallRules, createFirewallRulesForCAPG(installConfig, clusterID)...)


When the user defines platform.gcp.networkProjectID, we should use that project to create the firewall rules, right?

With the latest change, I don't see it being checked anyway... Maybe CAPG handles it?

CAPG should handle this now. We attach the firewall rules spec to a network spec.

…the changes to allow users to specify the firewall rules to be created by CAPG.

…o the top level vendor in the installer.

Migrated the creation of firewall rules to CAPG. These rules are now formed in the manifest and passed to CAPG for creation. pkg/infrastructure/gcp/clusterapi: Remove the creation of firewall rules.

openshift-ci-robot · 2026-02-18T15:38:59Z

@barbacbd: This pull request explicitly references no jira issue.

Details

In response to this:

*** Includes unmerged CAPG commits:
kubernetes-sigs/cluster-api-provider-gcp#1538

** Code Changes:

pkg/asset/manifests/gcp/cluster.go:

Migrated the creation of firewall rules to CAPG. These rules are now formed
in the manifest and passed to CAPG for creation.

pkg/infrastructure/gcp/clusterapi:

Remove the creation of firewall rules.

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 openshift-eng/jira-lifecycle-plugin repository.

barbacbd · 2026-02-18T15:39:04Z

/hold

openshift-ci · 2026-02-18T19:35:52Z

@barbacbd: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/images	`8942e19`	link	true	`/test images`
ci/prow/e2e-gcp-ovn-xpn	`8942e19`	link	false	`/test e2e-gcp-ovn-xpn`
ci/prow/e2e-gcp-ovn-byo-vpc	`8942e19`	link	false	`/test e2e-gcp-ovn-byo-vpc`
ci/prow/e2e-gcp-xpn-custom-dns	`8942e19`	link	false	`/test e2e-gcp-xpn-custom-dns`
ci/prow/e2e-gcp-custom-dns	`8942e19`	link	false	`/test e2e-gcp-custom-dns`
ci/prow/e2e-aws-ovn-heterogeneous	`8942e19`	link	false	`/test e2e-aws-ovn-heterogeneous`
ci/prow/e2e-aws-ovn-dualstack-ipv6-primary-techpreview	`8942e19`	link	false	`/test e2e-aws-ovn-dualstack-ipv6-primary-techpreview`
ci/prow/gcp-private	`8942e19`	link	false	`/test gcp-private`
ci/prow/e2e-aws-default-config	`8942e19`	link	false	`/test e2e-aws-default-config`
ci/prow/e2e-aws-ovn-dualstack-ipv4-primary-techpreview	`8942e19`	link	false	`/test e2e-aws-ovn-dualstack-ipv4-primary-techpreview`
ci/prow/e2e-aws-ovn-shared-vpc-custom-security-groups	`8942e19`	link	false	`/test e2e-aws-ovn-shared-vpc-custom-security-groups`
ci/prow/e2e-gcp-secureboot	`8942e19`	link	false	`/test e2e-gcp-secureboot`
ci/prow/e2e-aws-ovn-single-node	`8942e19`	link	false	`/test e2e-aws-ovn-single-node`
ci/prow/e2e-gcp-custom-endpoints	`8942e19`	link	false	`/test e2e-gcp-custom-endpoints`
ci/prow/e2e-gcp-default-config	`8942e19`	link	false	`/test e2e-gcp-default-config`
ci/prow/e2e-gcp-xpn-dedicated-dns-project	`8942e19`	link	false	`/test e2e-gcp-xpn-dedicated-dns-project`
ci/prow/e2e-gcp-ovn	`8942e19`	link	true	`/test e2e-gcp-ovn`
ci/prow/e2e-aws-ovn-shared-vpc-edge-zones	`8942e19`	link	false	`/test e2e-aws-ovn-shared-vpc-edge-zones`
ci/prow/e2e-aws-ovn-fips	`8942e19`	link	false	`/test e2e-aws-ovn-fips`
ci/prow/e2e-aws-ovn-edge-zones	`8942e19`	link	false	`/test e2e-aws-ovn-edge-zones`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

tthvo · 2026-02-18T22:13:03Z

pkg/asset/manifests/gcp/cluster.go

+		firewallRules = append(firewallRules, createBootstrapFirewallRuleForCAPG(installConfig, clusterID)...)
+		firewallRules = append(firewallRules, createFirewallRulesForCAPG(installConfig, clusterID)...)


Since CAPG manages the firewall rules now, I wonder how we should handle bootstrap firewall rule destroy...? Currently, we have below code in DestroyBootstrap phase.

installer/pkg/infrastructure/gcp/clusterapi/clusterapi.go

Lines 294 to 296 in b0514c8

if err := removeBootstrapFirewallRules(ctx, in.Metadata.InfraID, projectID, in.Metadata.GCP.Endpoint); err != nil {

return fmt.Errorf("failed to remove bootstrap firewall rules: %w", err)

}

My question is: will there a "fight" between the installer and CAPG? The installer deletes the firewall rule, but CAPG tries to re-create it as the rule is still defined in spec? If so, we should follow AWS here to edit the firewall rule spec and wait till its gone right?

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 16, 2026

openshift-ci bot requested review from JoelSpeed, andfasano, damdo, patrickdillon, sadasu and tthvo February 16, 2026 19:08

barbacbd changed the title ~~Allow CAPG to create firewall Rules~~ WIP: Allow CAPG to create firewall Rules Feb 16, 2026

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 16, 2026

tthvo reviewed Feb 17, 2026

View reviewed changes

barbacbd force-pushed the test-firewall-rules-capg branch 2 times, most recently from 75c3fb8 to ac0a050 Compare February 18, 2026 14:34

barbacbd added 4 commits February 18, 2026 09:37

Adding the infrastructure manifest for capg firewall rules addition.

526c004

Updating the cluster api/providers so that the gcp provider includes …

8198ac8

…the changes to allow users to specify the firewall rules to be created by CAPG.

Add the CAPg changes for the firewall spec to create firewall rules t…

f1a5e28

…o the top level vendor in the installer.

pkg/asset/manifests/gcp/cluster.go:

8942e19

Migrated the creation of firewall rules to CAPG. These rules are now formed in the manifest and passed to CAPG for creation. pkg/infrastructure/gcp/clusterapi: Remove the creation of firewall rules.

barbacbd force-pushed the test-firewall-rules-capg branch from ac0a050 to 8942e19 Compare February 18, 2026 15:26

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 18, 2026

barbacbd changed the title ~~WIP: Allow CAPG to create firewall Rules~~ Allow CAPG to create firewall Rules Feb 18, 2026

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 18, 2026

barbacbd changed the title ~~Allow CAPG to create firewall Rules~~ no-jira: Allow CAPG to create firewall Rules Feb 18, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 18, 2026

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 18, 2026

tthvo reviewed Feb 18, 2026

View reviewed changes

	srcRanges = []string{"35.191.0.0/16", "130.211.0.0/22"}
	if in.InstallConfig.Config.Publish == types.ExternalPublishingStrategy {
	// public installs require additional google ip addresses for health checks
	srcRanges = append(srcRanges, []string{"209.85.152.0/22", "209.85.204.0/22"}...)
	}

		firewallRules := createBootstrapFirewallRuleForCAPG(installConfig, clusterID)
		firewallRules = append(firewallRules, createFirewallRulesForCAPG(installConfig, clusterID)...)

	if err := removeBootstrapFirewallRules(ctx, in.Metadata.InfraID, projectID, in.Metadata.GCP.Endpoint); err != nil {
	return fmt.Errorf("failed to remove bootstrap firewall rules: %w", err)
	}

Conversation

barbacbd commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

*** Includes unmerged CAPG commits: kubernetes-sigs/cluster-api-provider-gcp#1538

Uh oh!

barbacbd commented Feb 16, 2026

Uh oh!

openshift-ci bot commented Feb 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Feb 18, 2026

*** Includes unmerged CAPG commits: kubernetes-sigs/cluster-api-provider-gcp#1538

Uh oh!

barbacbd commented Feb 18, 2026

Uh oh!

openshift-ci bot commented Feb 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

barbacbd commented Feb 16, 2026 •

edited

Loading

*** Includes unmerged CAPG commits:
kubernetes-sigs/cluster-api-provider-gcp#1538

*** Includes unmerged CAPG commits:
kubernetes-sigs/cluster-api-provider-gcp#1538