Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

OSAC-420: Configure console-proxy token issuer, CORS, and cert SANs - #262

Merged
openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
sk-ilya:console
Jun 11, 2026
Merged

openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
sk-ilya:console

Conversation

@sk-ilya

@sk-ilya sk-ilya commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Align the fulfillment-service console-proxy auth and TLS hostnames with the actual FQDNs used by clients. This ensures the token iss claim, derived JWKS endpoint, CORS origin checks, and certificate SANs all point at hostnames that are actually reachable and covered by TLS.

Update the fulfillment-api certificate SANs across overlays to use the correct namespace-qualified service DNS names. For osac-integration, also add the external route hostname to the token issuer, CORS origin, and certificate SANs, and update setup.sh to auto-discover and apply the same route-based values at deploy time (covers development overlays as well).

Summary by CodeRabbit

  • Documentation

    • Updated configuration instructions with clearer guidance on cluster domain substitution and automatic configuration patching.
  • Bug Fixes

    • Improved certificate DNS naming to automatically support namespace-qualified domain names across deployment environments.
    • Enhanced token configuration and CORS settings discovery to properly use external ingress hostnames.

@openshift-ci-robot

openshift-ci-robot commented Jun 11, 2026

Copy link
Copy Markdown

@sk-ilya: This pull request references OSAC-420 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 task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

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 Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

This pull request automates the configuration of the fulfillment-api external hostname for token issuance, CORS policy, and TLS certificate coverage across all deployment overlays and runtime initialization. Changes update documentation, add kustomize namespace-injection rules to five overlay environments, extend the osac-integration configuration with explicit external hostname patches, and introduce dynamic runtime patching in the setup script.

Changes

External Hostname and Token Configuration Automation

Layer / File(s) Summary
Documentation of automated patching
README.md
README clarifies that setup.sh automatically discovers and patches external hostname values into token-issuer, CORS, and certificate configuration rather than requiring manual substitution.
Kustomize certificate SAN replacements across overlays
overlays/caas-ci/console-proxy-replacements.yaml, overlays/development/console-proxy-replacements.yaml, overlays/hypershift2/console-proxy-replacements.yaml, overlays/osac-integration/console-proxy-replacements.yaml, overlays/vmaas-ci/console-proxy-replacements.yaml
All five deployment overlays add identical kustomize replacement rules that propagate fulfillment-api namespace into certificate spec.dnsNames at indices 2, 3, 5, 6 using delimiter-based extraction, enabling in-cluster namespace-qualified FQDN coverage for JWKS fetches and in-cluster client access.
OSAC integration external hostname configuration
overlays/osac-integration/kustomization.yaml
Patches fulfillment-grpc-server and fulfillment-console-proxy deployments to set --token-issuer and --console-cors-allowed-origins to the external fulfillment-api-osac-integration hostname, and patches the fulfillment-api Certificate to add that external hostname to spec.dnsNames for TLS validation during external JWKS access.
Runtime token configuration script function
scripts/prepare-fulfillment-service.sh
Introduces patch_token_config function that queries the fulfillment-api ingress route, dynamically patches deployment command arguments for --token-issuer and --console-cors-allowed-origins, and idempotently adds the route hostname to Certificate SANs before concurrent hub creation and AAP project sync.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • osac-project/osac-installer#233: Modifies scripts/prepare-fulfillment-service.sh script orchestration and retry logic, overlapping with this PR's same-script additions.
  • osac-project/osac-installer#86: Updates overlays/osac-integration configuration and scripts/prepare-fulfillment-service.sh for fulfillment token and CORS behavior, overlapping with this PR's integration setup changes.
  • osac-project/osac-installer#87: Modifies scripts/prepare-fulfillment-service.sh timeout behavior, sharing the same script as this PR's patch_token_config addition.

Suggested reviewers

  • tzvatot
  • omer-vishlitzky
  • akshaynadkarni
  • adriengentil

Poem

🔐 Hostname harmony achieved at last,
Five overlays, one namespace cast,
Token-issuer and CORS aligned,
Certificates with SANs refined—
Setup discovers what once was signed.


Security Considerations

Risk Severity: Medium | Impact: Certificate validation and token authentication

This PR introduces critical security-relevant changes affecting TLS certificate coverage and token issuance configuration:

  1. Certificate SAN Coverage Risk: The kustomize replacement rules inject dynamic namespaces into certificate spec.dnsNames across five overlays. Misconfigured indices (2, 3, 5, 6) or delimiter parsing could create certificate SAN mismatches, causing TLS validation failures for console-proxy JWKS fetch operations. Verify that the .-delimited extraction correctly isolates the namespace component at index: 1.

  2. Token Issuer Authority: The script's dynamic patch_token_config function patches deployment arguments to set --token-issuer to the discovered external ingress hostname. If the route discovery logic is flawed or the external hostname is attacker-controlled, token validation could be redirected to a malicious issuer. Confirm that the route lookup is scoped to the correct namespace and cannot be spoofed.

  3. CORS Origin Whitelist: Patching --console-cors-allowed-origins to match the external hostname accepts WebSocket connections from that origin without additional verification. If the external hostname is not strictly controlled, an attacker could inject JavaScript into cross-origin clients and exfiltrate session tokens. Review that the external hostname derivation is deterministic and bound to a trusted ingress controller.

  4. Certificate Patch Idempotency: The script uses idempotent certificate SAN patching (checking for existence before adding), but concurrent execution of multiple patch attempts could cause race conditions if kubectl patch is not atomic. Verify serial execution or use server-side apply to prevent SANs duplication or loss during simultaneous setup runs.


🔕 Pre-merge checks override applied

The pre-merge checks have been overridden successfully. You can now proceed with the merge.

Overridden by @sk-ilya via checkbox on 2026-06-11T15:18:02.010Z.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error [IGNORED] PR logs sensitive internal hostnames: scripts/prepare-fulfillment-service.sh line 191 echoes token issuer URL to logs; overlays/osac-integration/kustomization.yaml exposes cluster internal FQDNs in... Remove echo statement at line 191; use environment variables for sensitive config instead of command arguments, or suppress logging of URLs that contain internal hostnames.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (9 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main changes: configuration of console-proxy token issuer, CORS, and certificate SANs across multiple overlays and scripts.
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.
No-Hardcoded-Secrets ✅ Passed No hardcoded secrets found. All tokens/credentials retrieved dynamically from Kubernetes secrets at runtime; only legitimate git SHA found, no API keys, passwords, or private keys embedded.
No-Weak-Crypto ✅ Passed No weak cryptography found. PR contains only configuration changes: kustomize YAML overlays for certificate SANs, token issuer URLs, CORS settings, and a shell script that patches deployments with...
No-Injection-Vectors ✅ Passed No instances of SQL concatenation, shell=True, eval/exec, pickle.loads, unsafe yaml.load, os.system, or dangerouslySetInnerHTML found in PR changes.
Container-Privileges ✅ Passed No container privilege escalation indicators found: no privileged mode, host access (PID/Network/IPC), SYS_ADMIN capabilities, or allowPrivilegeEscalation settings in any modified files. Changes on...
Ai-Attribution ✅ Passed Commit uses proper "Assisted-by: Claude Code" trailer with Red Hat author attribution, no improper Co-Authored-By trailers for AI tools detected.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/prepare-fulfillment-service.sh`:
- Around line 187-189: The script currently reads api_route_host once
(api_route_host=$(oc get route -n "${INSTALLER_NAMESPACE}" fulfillment-api -o
jsonpath='{.status.ingress[0].host}') ) and fails immediately if empty; change
this to perform a bounded retry loop that re-queries oc up to a configured max
attempts (e.g., MAX_RETRIES) with a short sleep between attempts, updating
api_route_host each iteration, and only exit with error after retries are
exhausted; once a non-empty api_route_host is obtained set
issuer_url="https://${api_route_host}" as before.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0d4beadd-fc8d-446c-b39b-0c1ad979aa87

📥 Commits

Reviewing files that changed from the base of the PR and between 0aea7ed and 8547bde.

📒 Files selected for processing (8)
  • README.md
  • overlays/caas-ci/console-proxy-replacements.yaml
  • overlays/development/console-proxy-replacements.yaml
  • overlays/hypershift2/console-proxy-replacements.yaml
  • overlays/osac-integration/console-proxy-replacements.yaml
  • overlays/osac-integration/kustomization.yaml
  • overlays/vmaas-ci/console-proxy-replacements.yaml
  • scripts/prepare-fulfillment-service.sh

Comment thread scripts/prepare-fulfillment-service.sh
… SANs

Propagate the fulfillment-api namespace into ingress-proxy certificate
dnsNames across all overlays so TLS covers namespace-qualified FQDNs.

Add token-signer, token-issuer, and CORS flags to osac-integration.
Auto-discover and patch these values at deploy time in setup.sh.
Update README with new overlay configuration steps.

Assisted-by: Claude Code <noreply@anthropic.com>
@omer-vishlitzky

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@openshift-ci

openshift-ci Bot commented Jun 11, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: omer-vishlitzky, sk-ilya

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-merge-bot
openshift-merge-bot Bot merged commit f4c2eb5 into osac-project:main Jun 11, 2026
9 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants