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.

Sign up for GitHub

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

Jump to bottom

Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress #2186

Merged
merged 1 commit into from
Jul 3, 2023
Merged

Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress #2186

merged 1 commit into from
Jul 3, 2023

Conversation

gauravkghildiyal
Copy link
Member

/assign @swetharepakula

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 3, 2023
@gauravkghildiyal
Copy link
Member Author

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2023
@aojea
Copy link
Member

aojea commented Jul 3, 2023

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jul 3, 2023
gauravkghildiyal added a commit to gauravkghildiyal/ingress-gce that referenced this pull request Jul 3, 2023
@gauravkghildiyal
Cherry-pick kubernetes#2186: Include proxy-only subnet with purpose=R…
33bf835 
…EGIONAL_MANAGED_PROXY when generating firewall rules for Ingress
@gauravkghildiyal
Copy link
Member Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2023
aojea
aojea reviewed Jul 3, 2023
View reviewed changes
pkg/loadbalancers/features/l7ilb.go
@@ -44,7 +44,7 @@ func ILBSubnetSourceRange(cloud *gce.Cloud, region string) (string, error) {
if err != nil {
return "", fmt.Errorf("error comparing subnets: %v", err)
}
if subnet.Role == "ACTIVE" && subnet.Purpose == "INTERNAL_HTTPS_LOAD_BALANCER" && sameNetwork {
if subnet.Role == "ACTIVE" && (subnet.Purpose == "INTERNAL_HTTPS_LOAD_BALANCER" || subnet.Purpose == "REGIONAL_MANAGED_PROXY") && sameNetwork {
Copy link
Member

Choose a reason for hiding this comment

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

is it possible that these two subnets exist at the same time and we have to return multiple subnet.IpCidrRange?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a great question!

I don't think that's possible. Source https://cloud.google.com/load-balancing/docs/proxy-only-subnets:

"You can't have both an INTERNAL_HTTPS_LOAD_BALANCER and a REGIONAL_MANAGED_PROXY subnet in the same network and region, in the same way you can't have two REGIONAL_MANAGED_PROXY proxies or two INTERNAL_HTTPS_LOAD_BALANCER proxies."

I anyways gave it a try and couldn't create two.

@aojea
Copy link
Member

aojea commented Jul 3, 2023

/lgtm

@swetharepakula for final approval

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 3, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, gauravkghildiyal

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 3, 2023
@k8s-ci-robot k8s-ci-robot merged commit f3646a0 into kubernetes:master Jul 3, 2023
k8s-ci-robot added a commit that referenced this pull request Jul 3, 2023
@k8s-ci-robot
Merge pull request #2187 from gauravkghildiyal/cherry-pick-#2186-to-1.20
6afc55a 
Cherry-pick #2186: Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress
@cezarygerard cezarygerard mentioned this pull request Jul 3, 2023
Merged
This was referenced Jul 3, 2023
Merged
Merged
k8s-ci-robot added a commit that referenced this pull request Jul 3, 2023
@k8s-ci-robot
Merge pull request #2190 from aojea/cherry-pick-2186-1.21
f136a69 
[Cherry-pick #2186] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when ge…
k8s-ci-robot added a commit that referenced this pull request Jul 3, 2023
@k8s-ci-robot
Merge pull request #2189 from aojea/cherry-pick-2186-1.22
a642ae7 
[Cherry-pick #2186] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when ge…
k8s-ci-robot added a commit that referenced this pull request Jul 3, 2023
@k8s-ci-robot
Merge pull request #2188 from aojea/cherry-pick-2186-1.23
2092c7c 
[Cherry-pick #2186] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when ge…
gauravkghildiyal added a commit to gauravkghildiyal/ingress-gce that referenced this pull request Jul 28, 2023
@gauravkghildiyal
Revert "[Cherry-pick kubernetes#2186] Include proxy-only subnet with …
ef5f568 
…purpose=REGIONAL_MANAGED_PROXY when ge…"
gauravkghildiyal added a commit to gauravkghildiyal/ingress-gce that referenced this pull request Jul 28, 2023
@gauravkghildiyal
Revert "[Cherry-pick kubernetes#2186] Include proxy-only subnet with …
361800d 
…purpose=REGIONAL_MANAGED_PROXY when ge…"
gauravkghildiyal added a commit to gauravkghildiyal/ingress-gce that referenced this pull request Jul 28, 2023
@gauravkghildiyal
Revert "Cherry-pick kubernetes#2186: Include proxy-only subnet with p…
6401e10 
…urpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress"
k8s-ci-robot added a commit that referenced this pull request Jul 28, 2023
@k8s-ci-robot
Merge pull request #2204 from gauravkghildiyal/revert-2189-cherry-pic…
f8be173 
…k-2186-1.22

Revert "[Cherry-pick #2186 -> 1.22] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY..."
k8s-ci-robot added a commit that referenced this pull request Jul 28, 2023
@k8s-ci-robot
Merge pull request #2206 from gauravkghildiyal/revert-2187-cherry-pic…
9167c84 
…k-#2186-to-1.20

Revert "Cherry-pick #2186 -> 1.20: Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress"
k8s-ci-robot added a commit that referenced this pull request Jul 28, 2023
@k8s-ci-robot
Merge pull request #2205 from gauravkghildiyal/revert-2190-cherry-pic…
ae1e455 
…k-2186-1.21

Revert "[Cherry-pick #2186 -> 1.21] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when ge…"
gauravkghildiyal added a commit to gauravkghildiyal/ingress-gce that referenced this pull request Aug 21, 2023
@gauravkghildiyal
Revert "Revert "Cherry-pick kubernetes#2186 -> 1.20: Include proxy-on…
a4592ad 
…ly subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress""
gauravkghildiyal added a commit to gauravkghildiyal/ingress-gce that referenced this pull request Aug 21, 2023
@gauravkghildiyal
Revert "Revert "[Cherry-pick kubernetes#2186 -> 1.21] Include proxy-o…
2abb322 
…nly subnet with purpose=REGIONAL_MANAGED_PROXY when ge…""
gauravkghildiyal added a commit to gauravkghildiyal/ingress-gce that referenced this pull request Aug 21, 2023
@gauravkghildiyal
Revert "Revert "[Cherry-pick kubernetes#2186 -> 1.22] Include proxy-o…
93b49d1 
…nly subnet with purpose=REGIONAL_MANAGED_PROXY...""
@gauravkghildiyal gauravkghildiyal mentioned this pull request Aug 21, 2023
Merged
k8s-ci-robot added a commit that referenced this pull request Aug 30, 2023
@k8s-ci-robot
Merge pull request #2239 from gauravkghildiyal/revert-2205-revert-219…
0b30b9a 
…0-cherry-pick-2186-1.21

[Cherry-pick #2186 -> 1.21] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress
k8s-ci-robot added a commit that referenced this pull request Aug 30, 2023
@k8s-ci-robot
Merge pull request #2241 from gauravkghildiyal/cherry-pick-2186-to-1.19
d17ab66 
[Cherry-pick #2186 -> 1.19] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress
k8s-ci-robot added a commit that referenced this pull request Aug 30, 2023
@k8s-ci-robot
Merge pull request #2238 from gauravkghildiyal/revert-2206-revert-218…
1a52c8b 
…7-cherry-pick-#2186-to-1.20

Cherry-pick #2186 -> 1.20: Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress
k8s-ci-robot added a commit that referenced this pull request Aug 30, 2023
@k8s-ci-robot
Merge pull request #2240 from gauravkghildiyal/revert-2204-revert-218…
3f7e1f1 
…9-cherry-pick-2186-1.22

[Cherry-pick #2186 -> 1.22] Include proxy-only subnet with purpose=REGIONAL_MANAGED_PROXY when generating firewall rules for Ingress
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aojea aojea aojea left review comments

@code-elinka code-elinka Awaiting requested review from code-elinka

Assignees

@swetharepakula swetharepakula

@aojea aojea

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gauravkghildiyal @aojea @k8s-ci-robot @swetharepakula