Deny firewall for external services by TortillaZHawaii · Pull Request #958 · kubernetes/cloud-provider-gcp

TortillaZHawaii · 2026-01-26T14:27:53Z

An adapted copy of similar changes done to ingress-gce:

two new GCE flags enable-l4-deny-firewall and enable-l4-deny-firewall-rollback-cleanup,
adds deny firewall functionality with correct order for provisioning/cleanup,
exports metrics for netlb including firewall deny state,
vendors in cmpopts for easier testing.

k8s-ci-robot · 2026-01-26T14:28:02Z

This issue is currently awaiting triage.

If the repository mantainers determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

TortillaZHawaii · 2026-01-26T14:28:06Z

/hold

/assign @mmamczur

TortillaZHawaii · 2026-01-26T14:29:16Z

/uncc elmiko cici37

cmd/cloud-controller-manager/main.go

mmamczur · 2026-01-27T11:39:22Z

providers/gce/gce_loadbalancer_external.go

+	return nil
+}
+
+func (g *Cloud) deleteFirewallWithoutAuditLogs(fwName string) error {


this name suggests we do something bad while I assume it's to avoid a NOT_FOUND error log

cmd/cloud-controller-manager/main.go

providers/gce/gce_loadbalancer_external_test.go

08volt · 2026-01-28T14:14:51Z

providers/gce/gce_loadbalancer_external.go

+
+type protocol string
+
+func portsPerProtocol[T compute.FirewallAllowed | compute.FirewallDenied](a []*T) (map[protocol]sets.Set[int], error) {


You are expanding every interval in integer sets, could we sort/merge them instead?

Do you mean like comparing string slices directly? I'm not sure about this, the GCE may return "10-15", and we can ask for "10", "11", "12, "13", "14", "15". They're the same firewall but differently written.

Do you know if we can have such guarantees?

Generated example:

type portInterval struct { Start uint16 End uint16 } func mergeIntervals(intervals []portInterval) []portInterval { if len(intervals) < 2 { return intervals } // Sort by Start, then End. slices.SortFunc(intervals, func(a, b portInterval) int { if n := cmp.Compare(a.Start, b.Start); n != 0 { return n } return cmp.Compare(a.End, b.End) }) w := 0 for r := 1; r < len(intervals); r++ { // If current interval overlaps or is adjacent to the next one. if intervals[r].Start <= intervals[w].End+1 { if intervals[r].End > intervals[w].End { intervals[w].End = intervals[r].End } } else { w++ intervals[w] = intervals[r] } } return intervals[:w+1] }

It sounds to me like yours implementation would be quicker, but given that the most time is spent on the API calls I don't think the ROI is worth it.

I don't want to rewrite this for this PR, I might revisit it later.

providers/gce/gce_loadbalancer_external.go

TortillaZHawaii · 2026-01-30T10:15:40Z

this test is super flaky https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/cloud-provider-gcp/958/cloud-provider-gcp-tests/2017177332799770624

gkenetworkparamset_controller_test.go:1461: TestPopulateDesiredDefaultParamSet diff (-want +got): 
        &v1.GKENetworkParamSet{
         TypeMeta:   {},
         ObjectMeta: {Name: "default", Labels: {"addonmanager.kubernetes.io/mode": "EnsureExists"}, Annotations: {"components.gke.io/component-name": "cloud-controller-manager", "components.gke.io/layer": "addon"}},
         Spec: v1.GKENetworkParamSetSpec{
      - 		VPC:        "default-network",
      + 		VPC:        "random-vpc",
          VPCSubnet:  "default-subnetwork",
          DeviceMode: "",
          ... // 2 identical fields
         },
         Status: {},
        }

/retest

TortillaZHawaii · 2026-01-30T10:24:25Z

This wasn't introduced by this PR, it's caused by something already merged on master. Here's another PR that had the same test failing #955. 🙄

/test cloud-provider-gcp-tests

08volt · 2026-01-30T11:03:56Z

/lgtm

An adapted copy of similar changes done to ingress-gce: * two new GCE flags `enable-l4-deny-firewall` and `enable-l4-deny-firewall-rollback-cleanup`, * adds deny firewall functionality with correct order for provisioning/cleanup, the new firewall is following the previous naming scheme and adds "-deny" suffix at the end, * exports metric "number_of_l4_netlbs" including firewall deny state and general status, * vendors in `cmpopts` for easier testing.

TortillaZHawaii · 2026-01-30T11:28:43Z

/unhold

as said before, the commits were squashed

mmamczur · 2026-01-30T11:55:20Z

/lgtm
/approve

k8s-ci-robot · 2026-01-30T11:55:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mmamczur, TortillaZHawaii

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mmamczur]

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

[Cherrypick #958] Add deny firewall for external service LB

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 26, 2026

k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 26, 2026

k8s-ci-robot requested review from cici37 and elmiko January 26, 2026 14:28

k8s-ci-robot assigned mmamczur Jan 26, 2026

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 26, 2026

k8s-ci-robot removed request for cici37 and elmiko January 26, 2026 14:29

TortillaZHawaii force-pushed the deny-flag branch from 2f4424c to 509ce17 Compare January 26, 2026 14:37

mmamczur reviewed Jan 27, 2026

View reviewed changes

cmd/cloud-controller-manager/main.go Show resolved Hide resolved

mmamczur reviewed Jan 27, 2026

View reviewed changes

TortillaZHawaii commented Jan 27, 2026

View reviewed changes

cmd/cloud-controller-manager/main.go Outdated Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 27, 2026

TortillaZHawaii force-pushed the deny-flag branch from 509ce17 to b92e3c8 Compare January 28, 2026 11:42

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 28, 2026

TortillaZHawaii force-pushed the deny-flag branch 2 times, most recently from a055934 to 52bffa9 Compare January 28, 2026 12:07

anyush reviewed Jan 28, 2026

View reviewed changes

providers/gce/gce_loadbalancer_external_test.go Show resolved Hide resolved

08volt reviewed Jan 28, 2026

View reviewed changes

TortillaZHawaii requested review from 08volt, anyush and mmamczur January 30, 2026 10:29

k8s-ci-robot assigned 08volt Jan 30, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2026

TortillaZHawaii force-pushed the deny-flag branch from d136671 to 164cd53 Compare January 30, 2026 11:27

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2026

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2026

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 30, 2026

k8s-ci-robot merged commit 04bd3fb into kubernetes:master Jan 30, 2026
7 checks passed

k8s-ci-robot added a commit that referenced this pull request Feb 3, 2026

Merge pull request #964 from TortillaZHawaii/release-1.35

1684496

[Cherrypick #958] Add deny firewall for external service LB


		type protocol string

		func portsPerProtocol[T compute.FirewallAllowed \| compute.FirewallDenied](a []*T) (map[protocol]sets.Set[int], error) {

Conversation

TortillaZHawaii commented Jan 26, 2026

Uh oh!

k8s-ci-robot commented Jan 26, 2026

Uh oh!

TortillaZHawaii commented Jan 26, 2026

Uh oh!

TortillaZHawaii commented Jan 26, 2026

Uh oh!

Uh oh!

mmamczur Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

08volt Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

TortillaZHawaii Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

08volt Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

TortillaZHawaii Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TortillaZHawaii Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TortillaZHawaii commented Jan 30, 2026

Uh oh!

TortillaZHawaii commented Jan 30, 2026

Uh oh!

08volt commented Jan 30, 2026

Uh oh!

TortillaZHawaii commented Jan 30, 2026

Uh oh!

mmamczur commented Jan 30, 2026

Uh oh!

k8s-ci-robot commented Jan 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

TortillaZHawaii Jan 30, 2026 •

edited

Loading