Skip to content

CORS-4428: gcp: reject PSC endpoint overrides for sovereign clouds#10708

Open
barbacbd wants to merge 1 commit into
openshift:mainfrom
barbacbd:CORS-4428
Open

CORS-4428: gcp: reject PSC endpoint overrides for sovereign clouds#10708
barbacbd wants to merge 1 commit into
openshift:mainfrom
barbacbd:CORS-4428

Conversation

@barbacbd

@barbacbd barbacbd commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Sovereign cloud project IDs contain a colon delimiter that distinguishes them from standard GCP projects. PSC endpoint overrides generate URLs targeting the googleapis.com universe, which is incompatible with sovereign cloud environments.

Summary by CodeRabbit

  • Bug Fixes
    • PSC endpoint overrides are now rejected for sovereign cloud projects with a clear validation error.
    • Standard GCP projects continue to validate configured endpoints, including detecting invalid endpoint names.
    • Projects without a PSC endpoint override remain valid.
  • Tests
    • Added coverage for service endpoint override validation across sovereign and standard cloud scenarios.

@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

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

openshift-ci-robot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

@barbacbd: This pull request references CORS-4428 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Sovereign cloud project IDs contain a colon delimiter that distinguishes them from standard GCP projects. PSC endpoint overrides generate URLs targeting the googleapis.com universe, which is incompatible with sovereign cloud environments.

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.

@coderabbitai

coderabbitai Bot commented Jul 23, 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: Enterprise

Run ID: 7cc1ce37-0d47-47bb-a094-1060761152f5

📥 Commits

Reviewing files that changed from the base of the PR and between a793af3 and 9647199.

📒 Files selected for processing (2)
  • pkg/asset/installconfig/gcp/validation.go
  • pkg/asset/installconfig/gcp/validation_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/asset/installconfig/gcp/validation_test.go
  • pkg/asset/installconfig/gcp/validation.go

📝 Walkthrough

Walkthrough

GCP validation now rejects PSC endpoint overrides for sovereign-cloud projects. Tests cover sovereign and regular projects with absent, valid, and invalid endpoint overrides.

Changes

GCP PSC validation

Layer / File(s) Summary
Sovereign-cloud guard and scenario coverage
pkg/asset/installconfig/gcp/validation.go, pkg/asset/installconfig/gcp/validation_test.go
validateServiceEndpointOverride returns a forbidden error for sovereign-cloud endpoint overrides and skips endpoint lookup; table-driven tests cover sovereign and regular projects with absent, valid, and invalid endpoints.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 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 main change: rejecting PSC endpoint overrides for sovereign GCP clouds.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The added test uses static t.Run titles like "Sovereign cloud project with endpoint"; no dynamic or generated values appear in test names.
Test Structure And Quality ✅ Passed Isolated table-driven unit test with deferred gomock cleanup; no cluster resources, waits, or cleanup gaps, and it matches existing repo patterns.
Microshift Test Compatibility ✅ Passed The new test is a plain unit test (testing.T), not Ginkgo/e2e, and it doesn't reference any MicroShift-unsupported OpenShift APIs or namespaces.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo/e2e tests were added; the change is a unit test in pkg/asset/installconfig/gcp with no multi-node or SNO assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed Only GCP install-config validation changed; no manifests, controllers, replicas, affinities, node selectors, or topology-aware scheduling logic were added.
Ote Binary Stdout Contract ✅ Passed PR only changes GCP validation logic and a unit test; no main/init/TestMain/suite setup stdout writes were added.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the change is a unit test in validation_test.go with gomock and no external connectivity.
No-Weak-Crypto ✅ Passed Touched code only adds PSC endpoint validation and tests; no weak crypto, custom crypto, or secret comparison logic was introduced.
Container-Privileges ✅ Passed PR only changes GCP validation logic and tests; no container/K8s manifests were modified and no privileged settings were introduced.
No-Sensitive-Data-In-Logs ✅ Passed No new logging was added in the touched code; the PSC endpoint change only returns validation errors and the test contains no log output.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 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.

@tthvo tthvo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e-azure-nat-gateway-single-zone

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-gcp-custom-dns
/test e2e-gcp-custom-endpoints
/test e2e-gcp-default-config
/test e2e-gcp-ovn
/test e2e-gcp-ovn-byo-vpc
/test e2e-gcp-ovn-xpn
/test e2e-gcp-secureboot
/test e2e-gcp-xpn-custom-dns
/test e2e-gcp-xpn-dedicated-dns-project
/test e2e-aws-ovn

return nil
}

if strings.Contains(ic.GCP.ProjectID, ":") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This works, but we probably can reuse the existing func

func GetCloudEnvironment(projectID string) string {
// Check if project ID has a known sovereign cloud prefix
parts := strings.SplitN(projectID, ":", 2)
if len(parts) == 2 && sets.New(sovereignCloudProjectPrefixes...).Has(parts[0]) {
// Known sovereign prefix is definitive - this IS a sovereign cloud project
return CloudEnvironmentSovereign
}
// No known sovereign prefix found
return ""
}

Sovereign cloud project IDs contain a colon delimiter that distinguishes
them from standard GCP projects. PSC endpoint overrides generate URLs
targeting the googleapis.com universe, which is incompatible with
sovereign cloud environments.
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2026
@barbacbd
barbacbd requested a review from tthvo July 24, 2026 11:19

@tthvo tthvo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e-azure-nat-gateway-single-zone

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-gcp-custom-dns
/test e2e-gcp-custom-endpoints
/test e2e-gcp-default-config
/test e2e-gcp-ovn
/test e2e-gcp-ovn-byo-vpc
/test e2e-gcp-ovn-xpn
/test e2e-gcp-secureboot
/test e2e-gcp-xpn-custom-dns
/test e2e-gcp-xpn-dedicated-dns-project
/test e2e-aws-ovn

@tthvo

tthvo commented Jul 24, 2026

Copy link
Copy Markdown
Member

/test artifacts-images

@tthvo

tthvo commented Jul 24, 2026

Copy link
Copy Markdown
Member

/test e2e-gcp-custom-endpoints

@patrickdillon

Copy link
Copy Markdown
Contributor

/approve
/verified by unit

@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@patrickdillon: This PR has been marked as verified by unit.

Details

In response to this:

/approve
/verified by unit

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.

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 24, 2026
@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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:

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD d8f6a96 and 2 for PR HEAD 9647199 in total

@tthvo

tthvo commented Jul 25, 2026

Copy link
Copy Markdown
Member

/retest-required

1 similar comment
@tthvo

tthvo commented Jul 26, 2026

Copy link
Copy Markdown
Member

/retest-required

@openshift-ci

openshift-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@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/gcp-custom-endpoints-proxy-wif 9647199 link false /test gcp-custom-endpoints-proxy-wif
ci/prow/e2e-gcp-ovn-byo-vpc 9647199 link false /test e2e-gcp-ovn-byo-vpc
ci/prow/gcp-private 9647199 link false /test gcp-private
ci/prow/e2e-gcp-custom-endpoints 9647199 link false /test e2e-gcp-custom-endpoints
ci/prow/e2e-gcp-xpn-custom-dns 9647199 link false /test e2e-gcp-xpn-custom-dns
ci/prow/e2e-gcp-custom-dns 9647199 link false /test e2e-gcp-custom-dns
ci/prow/e2e-gcp-default-config 9647199 link false /test e2e-gcp-default-config
ci/prow/e2e-gcp-secureboot 9647199 link false /test e2e-gcp-secureboot
ci/prow/e2e-gcp-xpn-dedicated-dns-project 9647199 link false /test e2e-gcp-xpn-dedicated-dns-project
ci/prow/e2e-gcp-ovn-xpn 9647199 link false /test e2e-gcp-ovn-xpn
ci/prow/e2e-gcp-ovn 9647199 link true /test e2e-gcp-ovn

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

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants