Skip to content

gcp: allow global access on the internal load balancer - #10828

Open
kchawlani19 wants to merge 1 commit into
openshift:mainfrom
kchawlani19:gcp-10744-lb-global-access
Open

gcp: allow global access on the internal load balancer#10828
kchawlani19 wants to merge 1 commit into
openshift:mainfrom
kchawlani19:gcp-10744-lb-global-access

Conversation

@kchawlani19

@kchawlani19 kchawlani19 commented Aug 29, 2026

Copy link
Copy Markdown

Summary

  • Adds platform.gcp.loadBalancer.clientAccess (Global | Local) so private GCP clusters can enable ILB global access from install-config.
  • Wires Global through CAPG (InternalAccess=Global / allowGlobalAccess) so a cross-region installer (for example ACM) can reach the API server.
  • Also sets the default IngressController gcp.clientAccess: Global when ingress is internal.

Fixes #10744

Test plan

  • go test ./pkg/types/gcp/ ./pkg/types/gcp/validation/ ./pkg/asset/manifests/gcp/ ./pkg/asset/manifests/
  • Confirm invalid clientAccess values are rejected at install-config validation
  • GCP private install (publish: Internal) with platform.gcp.loadBalancer.clientAccess: Global from a different region completes and can reach https://api.<cluster>:6443
  • Default (unset / Local) still creates a regional ILB

Summary by CodeRabbit

  • New Features
    • Added GCP load balancer client-access settings for global or local regional access.
    • Applied configured access settings to internal load balancers and default ingress controllers.
  • Bug Fixes
    • Improved GCP ingress configuration so global client access is correctly preserved.
  • Validation
    • Added validation to reject unsupported GCP load balancer access values.
  • Tests
    • Added coverage for global, local, missing, and invalid load balancer configurations.

Cross-region clients such as ACM cannot reach a private GCP API server
because internal load balancers are regional by default. Expose
platform.gcp.loadBalancer.clientAccess in install-config and set CAPG
InternalAccess=Global when requested.

Fixes openshift#10744
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: ab0dcb80-88f3-4a26-9346-d6342eac3103

📥 Commits

Reviewing files that changed from the base of the PR and between c8d2999 and 8c92816.

⛔ Files ignored due to path filters (2)
  • data/data/install.openshift.io_installconfigs.yaml is excluded by !data/data/install.openshift.io_installconfigs.yaml
  • pkg/types/gcp/zz_generated.deepcopy.go is excluded by !**/zz_generated*
📒 Files selected for processing (9)
  • pkg/asset/manifests/gcp/cluster.go
  • pkg/asset/manifests/gcp/cluster_test.go
  • pkg/asset/manifests/infrastructure_test.go
  • pkg/asset/manifests/ingress.go
  • pkg/asset/manifests/ingress_test.go
  • pkg/types/gcp/platform.go
  • pkg/types/gcp/platform_test.go
  • pkg/types/gcp/validation/platform.go
  • pkg/types/gcp/validation/platform_test.go

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The GCP platform now supports global and local load balancer client access. Validation enforces these values. Cluster and ingress manifest generation maps global access to the corresponding CAPG and GCP provider settings.

Changes

GCP load balancer client access

Layer / File(s) Summary
Platform contract and validation
pkg/types/gcp/platform.go, pkg/types/gcp/validation/platform.go, pkg/types/gcp/*_test.go
Adds GCP load balancer client-access types, global-access detection, validation, and tests for supported values.
Cluster load balancer mapping
pkg/asset/manifests/gcp/cluster.go, pkg/asset/manifests/gcp/cluster_test.go, pkg/asset/manifests/infrastructure_test.go
Maps install-config client access to the CAPG internal load balancer specification and tests global, local, and unset configurations.
Ingress provider wiring
pkg/asset/manifests/ingress.go, pkg/asset/manifests/ingress_test.go
Sets GCP global-access provider parameters for internal default ingress controllers and validates the generated manifest.
Estimated code review effort: 3 (Moderate) ~20 minutes

Merge Risk: ⚪ Minimal · up to 8c928

This change is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 9 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive The reviewable changes implement issue #10744: they add GCP client-access configuration, validate Global and Local values, map Global access to CAPG, configure internal ingress, and preserve regional … Review the excluded data/data/install.openshift.io_installconfigs.yaml and pkg/types/gcp/zz_generated.deepcopy.go files. Confirm that the new GCP load balancer configuration is present in the install-config schema and that generated deepcop…
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: enabling global access on the GCP internal load balancer.
Out of Scope Changes check ✅ Passed The changes are within scope for issue #10744. They implement GCP internal load-balancer client access, related ingress behavior, validation, and focused tests. No unrelated production changes are sho…
Stable And Deterministic Test Names ✅ Passed PASS — The pull request adds only standard Go t.Run subtests, not Ginkgo It, Describe, Context, or When declarations. All added subtest names are fixed descriptive strings, such as `nil plat…
Test Structure And Quality ✅ Passed PASS: The pull request adds and modifies standard Go unit tests, not Ginkgo tests. The changed files use testing.T, t.Run, and testify/assert; they contain no Ginkgo It blocks, BeforeEach/`A…
Microshift Test Compatibility ✅ Passed PASS — the PR adds only standard Go unit tests (func Test...) using testing and testify/assert. The changed tests do not add Ginkgo constructs such as It, Describe, Context, or When, so …
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS: The pull request adds only Go unit tests using testing.T and testify/assert. The changed files contain no Ginkgo declarations or Ginkgo imports, and no files under e2e test locations changed…
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The pull request changes GCP internal load-balancer configuration and validation only. The manifest changes add CAPG InternalLoadBalancer and IngressController provider parameters; they do not…
Ote Binary Stdout Contract ✅ Passed PASS: The pull request changes only GCP manifest generation, types, validation, deepcopy code, and ordinary test functions. The added lines contain no fmt.Print*, os.Stdout, log/klog output, Ginkgo su…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS: The pull request adds only standard Go unit tests using testing.T and t.Run. It adds no Ginkgo It, Describe, Context, or When e2e tests. The changed test code contains no hardcoded I…
No-Weak-Crypto ✅ Passed PASS: The pull request adds GCP load-balancer configuration and manifest mapping only. The actual commit diff introduces no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/tok…
Container-Privileges ✅ Passed PASS: The pull request adds GCP load-balancer configuration and ingress provider parameters only. The diff introduces no privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, `allowPri…
No-Sensitive-Data-In-Logs ✅ Passed PASS: The pull-request diff adds configuration types, validation, manifest mapping, tests, and schema/deep-copy updates. It adds no logging, printing, event, or diagnostic output code. The new values …
Full details: Linked Issues check

Explanation

The reviewable changes implement issue #10744: they add GCP client-access configuration, validate Global and Local values, map Global access to CAPG, configure internal ingress, and preserve regional behavior when unset or Local. Verification is incomplete because the install-config schema file data/data/install.openshift.io_installconfigs.yaml was excluded by the !data/data/install.openshift.io_installconfigs.yaml path filter, and generated deepcopy code was excluded by !**/zz_generated*; these files may be required to complete install-config support.

Resolution

Review the excluded data/data/install.openshift.io_installconfigs.yaml and pkg/types/gcp/zz_generated.deepcopy.go files. Confirm that the new GCP load balancer configuration is present in the install-config schema and that generated deepcopy support is valid.

Full details: Out of Scope Changes check

Explanation

The changes are within scope for issue #10744. They implement GCP internal load-balancer client access, related ingress behavior, validation, and focused tests. No unrelated production changes are shown.

Full details: Stable And Deterministic Test Names

Explanation

PASS — The pull request adds only standard Go t.Run subtests, not Ginkgo It, Describe, Context, or When declarations. All added subtest names are fixed descriptive strings, such as nil platform, global client access, and gcp platform, private ingress, global client access. No timestamps, generated identifiers, node or namespace names, IP addresses, or runtime formatting appear in the test titles.

Full details: Test Structure And Quality

Explanation

PASS: The pull request adds and modifies standard Go unit tests, not Ginkgo tests. The changed files use testing.T, t.Run, and testify/assert; they contain no Ginkgo It blocks, BeforeEach/AfterEach, Eventually, or Consistently calls. The tests construct in-memory install-config and manifest data and do not create cluster-scoped resources or perform cluster waits. The table subtests each cover one related behavior, and the changes follow the existing repository pattern.

Full details: Microshift Test Compatibility

Explanation

PASS — the PR adds only standard Go unit tests (func Test...) using testing and testify/assert. The changed tests do not add Ginkgo constructs such as It, Describe, Context, or When, so the MicroShift Ginkgo e2e compatibility check does not apply. No MicroShift compatibility notice is required.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request adds only Go unit tests using testing.T and testify/assert. The changed files contain no Ginkgo declarations or Ginkgo imports, and no files under e2e test locations changed. The SNO multi-node compatibility check is therefore not applicable.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes GCP internal load-balancer configuration and validation only. The manifest changes add CAPG InternalLoadBalancer and IngressController provider parameters; they do not add or change replicas, anti-affinity, topology spread, node selectors or affinity, tolerations, or PodDisruptionBudgets. The added schema, helpers, tests, and deepcopy code also contain no topology-dependent scheduling constraints.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only GCP manifest generation, types, validation, deepcopy code, and ordinary test functions. The added lines contain no fmt.Print*, os.Stdout, log/klog output, Ginkgo suite setup, init, main, or TestMain code. No OTE process-level stdout path is introduced.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS: The pull request adds only standard Go unit tests using testing.T and t.Run. It adds no Ginkgo It, Describe, Context, or When e2e tests. The changed test code contains no hardcoded IPv4 addresses, IPv4-only parsing, IPv6-unsafe URL construction, or external connectivity calls. Existing IPv4 test constants were not added by this pull request.

Full details: No-Weak-Crypto

Explanation

PASS: The pull request adds GCP load-balancer configuration and manifest mapping only. The actual commit diff introduces no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/token comparisons. The changed code contains no crypto imports or cryptographic implementation.

Full details: Container-Privileges

Explanation

PASS: The pull request adds GCP load-balancer configuration and ingress provider parameters only. The diff introduces no privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation: true, root execution setting, or security-context fields in container or Kubernetes manifests.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS: The pull-request diff adds configuration types, validation, manifest mapping, tests, and schema/deep-copy updates. It adds no logging, printing, event, or diagnostic output code. The new values are non-sensitive enum settings (Global/Local) and are serialized into manifests, not logged. Existing error formatting in the touched files is unchanged.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

[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 vr4manta 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:

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

@openshift-ci
openshift-ci Bot requested review from andfasano and barbacbd August 29, 2026 15:20
@kchawlani19

Copy link
Copy Markdown
Author

/retest

@openshift-ci

openshift-ci Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

@kchawlani19: 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/gcp-private 8c92816 link false /test gcp-private
ci/prow/gcp-custom-endpoints-proxy-wif 8c92816 link false /test gcp-custom-endpoints-proxy-wif

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GCP allow to set global access on loadbalancer

1 participant