Skip to content

OSAC-4913,OSAC-4915: fix bundled OpenBao OIDC issuer + vault endpoint namespace mismatch on OpenShift - #752

Closed
ygalblum wants to merge 2 commits into
osac-project:mainfrom
ygalblum:fix/OSAC-4913-openbao-oidc-issuer
Closed

OSAC-4913,OSAC-4915: fix bundled OpenBao OIDC issuer + vault endpoint namespace mismatch on OpenShift#752
ygalblum wants to merge 2 commits into
osac-project:mainfrom
ygalblum:fix/OSAC-4913-openbao-oidc-issuer

Conversation

@ygalblum

@ygalblum ygalblum commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • On OpenShift, Keycloak is deliberately configured to always report the external route hostname as its OIDC issuer (portable tokens), and install-osac's install-osac target already overrides service.auth.issuerUrl/service.idp.url at install time to match. However service.vault.keycloakIssuerUrl — the OIDC issuer URL the bundled OpenBao's init.sh uses for auth/jwt/config oidc_discovery_url=... — was missing from that same $(DOMAIN)-based override list, so it stayed pinned to the static internal cluster-local hostname from each CI profile's instance.yaml. This mismatched Keycloak's actual discovery-document issuer and made OpenBao's OIDC discovery validation fail forever (issuer did not match the returned issuer), so the lifecycle JWT auth role never got created on any OpenShift-deployed cluster.
  • Fix: extend the existing $(if $(DOMAIN), ...) --set block in osac-installer/Makefile's install-osac target to also set service.vault.keycloakIssuerUrl=https://keycloak-keycloak.$(DOMAIN)/realms/osac, matching the pattern already used for service.auth.issuerUrl. The static per-profile instance.yaml values become dead defaults at install time, same as service.auth.issuerUrl today — left as-is for consistency/fallback.

Test plan

  • yamllint --strict -c .yamllint.yaml osac-installer passes
  • pre-commit run --all-files passes
  • make helm-lint / make helm-validate fail only with the pre-existing, unrelated service.externalHostname/service.internalHostname schema error already present on main (confirmed before this change)
  • helm template charts/osac -f values/vmaas-ci/instance.yaml with the same --set flags install-osac would pass (mocked DOMAIN) confirms KEYCLOAK_ISSUER_URL in bundled-openbao.yaml's init container now renders as the external route URL (https://keycloak-keycloak.<domain>/realms/osac), matching Keycloak's actual discovery-document issuer

Fixes: https://redhat.atlassian.net/browse/OSAC-4913

Assisted-by: Claude Code noreply@anthropic.com

Deployment and authentication

  • Updated install-osac Helm overrides.
  • OpenBao now uses the external Keycloak issuer on OpenShift.
  • OpenBao now uses the service endpoint in the install namespace through $(NS).
  • This fixes OIDC discovery validation and fulfillment-controller login failures.

Tests and CI

  • YAML linting and pre-commit checks pass.
  • Helm template testing confirms the issuer override.
  • Helm lint and validation still report a pre-existing schema error.

API surface, controllers, database, and documentation

  • No API, controller, database, or documentation changes.

Backward compatibility

  • The overrides apply only when DOMAIN is available.
  • Other deployment paths keep their existing configuration.

Risk classification

  • risk:ship — The change is limited to deployment configuration and has focused template validation.
  • It does not qualify as risk:show because it adds no broad user-visible feature.
  • It does not qualify as risk:ask because it changes no application logic or data handling.

Keycloak reports the external route hostname as its OIDC issuer
regardless of the network path used to reach it, so the bundled
OpenBao JWT auth backend's oidc_discovery_url must match that same
external hostname. Add service.vault.keycloakIssuerUrl to the
existing DOMAIN-based --set override list in install-osac, alongside
service.auth.issuerUrl and service.idp.url.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
@openshift-ci-robot

openshift-ci-robot commented Sep 4, 2026

Copy link
Copy Markdown

@ygalblum: This pull request references OSAC-4913 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 bug to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Summary

  • On OpenShift, Keycloak is deliberately configured to always report the external route hostname as its OIDC issuer (portable tokens), and install-osac's install-osac target already overrides service.auth.issuerUrl/service.idp.url at install time to match. However service.vault.keycloakIssuerUrl — the OIDC issuer URL the bundled OpenBao's init.sh uses for auth/jwt/config oidc_discovery_url=... — was missing from that same $(DOMAIN)-based override list, so it stayed pinned to the static internal cluster-local hostname from each CI profile's instance.yaml. This mismatched Keycloak's actual discovery-document issuer and made OpenBao's OIDC discovery validation fail forever (issuer did not match the returned issuer), so the lifecycle JWT auth role never got created on any OpenShift-deployed cluster.
  • Fix: extend the existing $(if $(DOMAIN), ...) --set block in osac-installer/Makefile's install-osac target to also set service.vault.keycloakIssuerUrl=https://keycloak-keycloak.$(DOMAIN)/realms/osac, matching the pattern already used for service.auth.issuerUrl. The static per-profile instance.yaml values become dead defaults at install time, same as service.auth.issuerUrl today — left as-is for consistency/fallback.

Test plan

  • yamllint --strict -c .yamllint.yaml osac-installer passes
  • pre-commit run --all-files passes
  • make helm-lint / make helm-validate fail only with the pre-existing, unrelated service.externalHostname/service.internalHostname schema error already present on main (confirmed before this change)
  • helm template charts/osac -f values/vmaas-ci/instance.yaml with the same --set flags install-osac would pass (mocked DOMAIN) confirms KEYCLOAK_ISSUER_URL in bundled-openbao.yaml's init container now renders as the external route URL (https://keycloak-keycloak.<domain>/realms/osac), matching Keycloak's actual discovery-document issuer

Fixes: https://redhat.atlassian.net/browse/OSAC-4913

Assisted-by: Claude Code noreply@anthropic.com

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

openshift-ci Bot commented Sep 4, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ygalblum

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

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 617c9aea-8070-46cf-829d-39698e3f7a4d

📥 Commits

Reviewing files that changed from the base of the PR and between dfd381d and 2131d1f.

📒 Files selected for processing (1)
  • osac-installer/Makefile

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


Walkthrough

When a cluster domain is available, the OSAC Helm install command sets service.vault.endpoint to the OpenBao service URL for the target namespace.

Changes

OSAC installation configuration

Layer / File(s) Summary
Add OpenBao endpoint argument
osac-installer/Makefile
The conditional Helm arguments now include service.vault.endpoint alongside the existing issuer and service URL settings.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 2131d

The installation configuration now supplies the OpenBao endpoint for the target namespace, with no remaining identified merge-readiness risk.

Suggested labels: risk:ask


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error The pull request adds service.vault.endpoint=https://openbao.$(NS).svc.cluster.local:8200 to the helm upgrade recipe in osac-installer/Makefile. The recipe line has no @ prefix, so Make prints… Prevent the Helm command from being printed with its internal endpoint. Prefix the recipe with @ and ensure shell tracing is disabled, or construct the Helm arguments in a helper that does not log them. Verify CI output does not contain `…
✅ Passed checks (10 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 The pull request adds only two variable-derived service URLs in osac-installer/Makefile: the Keycloak issuer URL and the OpenBao endpoint. Neither URL contains embedded credentials, and the added li…
No-Weak-Crypto ✅ Passed PASS. The two feature commits only add Helm URL overrides in osac-installer/Makefile: the HTTPS Keycloak issuer URL and the HTTPS OpenBao service endpoint. The added lines contain no MD5, SHA1, DES,…
No-Injection-Vectors ✅ Passed The pull-request diff only extends a Makefile Helm --set argument list. The added URL interpolation uses Make variables, but it does not introduce SQL concatenation, shell=True, eval/exec, `pi…
Container-Privileges ✅ Passed PASS. The PR changes only osac-installer/Makefile. The added Helm overrides set service.vault.keycloakIssuerUrl and service.vault.endpoint; they do not add or change a container/Kubernetes manif…
Ai-Attribution ✅ Passed AI use is disclosed in the PR and both changed commits. Each commit has an Assisted-by: Claude Code <noreply@anthropic.com> trailer. Neither changed commit uses a Co-Authored-By trailer for the AI…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the OpenBao OIDC issuer and vault endpoint namespace mismatch on OpenShift, which matches the pull request objectives and changes.
Full details: No-Sensitive-Data-In-Logs

Explanation

The pull request adds service.vault.endpoint=https://openbao.$(NS).svc.cluster.local:8200 to the helm upgrade recipe in osac-installer/Makefile. The recipe line has no @ prefix, so Make prints the expanded command to CI or terminal output. This exposes the internal hostname openbao.&lt;namespace&gt;.svc.cluster.local, which matches the custom check's explicit internal-hostname condition. The diff confirms this logging exposure was introduced by the pull request.

Resolution

Prevent the Helm command from being printed with its internal endpoint. Prefix the recipe with @ and ensure shell tracing is disabled, or construct the Helm arguments in a helper that does not log them. Verify CI output does not contain svc.cluster.local or other internal hostnames.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

E2E on CodeRabbit approval

CodeRabbit APPROVED — starting expensive e2e (PR run replay).

  • Started: 3/3

fulfillment-controller failed to log in to the bundled OpenBao vault
because service.vault.endpoint was hardcoded to
openbao.osac.svc.cluster.local in every CI profile's instance.yaml,
while CI actually installs OSAC into the osac-e2e-ci namespace.

Extend the same DOMAIN-conditional --set block in install-osac (already
overriding service.auth.issuerUrl and service.vault.keycloakIssuerUrl
per OSAC-4913) to also override service.vault.endpoint using the
Makefile's existing $(NS) variable.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
@ygalblum ygalblum changed the title OSAC-4913: fix bundled OpenBao JWT OIDC issuer mismatch on OpenShift OSAC-4913,OSAC-4915: fix bundled OpenBao OIDC issuer + vault endpoint namespace mismatch on OpenShift Sep 4, 2026
@openshift-ci-robot

openshift-ci-robot commented Sep 4, 2026

Copy link
Copy Markdown

@ygalblum: This pull request references OSAC-4913 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 bug to target the "5.1.0" version, but no target version was set.

This pull request references OSAC-4915 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 bug to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Summary

  • On OpenShift, Keycloak is deliberately configured to always report the external route hostname as its OIDC issuer (portable tokens), and install-osac's install-osac target already overrides service.auth.issuerUrl/service.idp.url at install time to match. However service.vault.keycloakIssuerUrl — the OIDC issuer URL the bundled OpenBao's init.sh uses for auth/jwt/config oidc_discovery_url=... — was missing from that same $(DOMAIN)-based override list, so it stayed pinned to the static internal cluster-local hostname from each CI profile's instance.yaml. This mismatched Keycloak's actual discovery-document issuer and made OpenBao's OIDC discovery validation fail forever (issuer did not match the returned issuer), so the lifecycle JWT auth role never got created on any OpenShift-deployed cluster.
  • Fix: extend the existing $(if $(DOMAIN), ...) --set block in osac-installer/Makefile's install-osac target to also set service.vault.keycloakIssuerUrl=https://keycloak-keycloak.$(DOMAIN)/realms/osac, matching the pattern already used for service.auth.issuerUrl. The static per-profile instance.yaml values become dead defaults at install time, same as service.auth.issuerUrl today — left as-is for consistency/fallback.

Test plan

  • yamllint --strict -c .yamllint.yaml osac-installer passes
  • pre-commit run --all-files passes
  • make helm-lint / make helm-validate fail only with the pre-existing, unrelated service.externalHostname/service.internalHostname schema error already present on main (confirmed before this change)
  • helm template charts/osac -f values/vmaas-ci/instance.yaml with the same --set flags install-osac would pass (mocked DOMAIN) confirms KEYCLOAK_ISSUER_URL in bundled-openbao.yaml's init container now renders as the external route URL (https://keycloak-keycloak.<domain>/realms/osac), matching Keycloak's actual discovery-document issuer

Fixes: https://redhat.atlassian.net/browse/OSAC-4913

Assisted-by: Claude Code noreply@anthropic.com

Deployment and authentication

  • Updated the install-osac Helm overrides.
  • When DOMAIN is set, OpenBao now uses the external Keycloak issuer:
    https://keycloak-keycloak.$(DOMAIN)/realms/osac.
  • This fixes OIDC discovery validation on OpenShift and allows creation of the lifecycle JWT auth role.

Tests and CI

  • YAML linting and pre-commit checks pass.
  • Helm template validation confirms that the OpenBao init container receives the external issuer URL.
  • Helm lint and validation still report a pre-existing schema error.

API surface, controllers, database, and documentation

  • No API, controller, database, or documentation changes.

Backward compatibility

  • No change applies when the existing Helm override is not used.
  • Deployments that depend on the static internal issuer value may receive a corrected external issuer value when DOMAIN is set.

Risk classification

  • risk:ship — The change is limited to one Helm deployment override, fixes an authentication configuration mismatch, and has focused template validation.
  • It does not qualify as risk:show because it does not introduce a broad user-visible feature or require staged rollout.
  • It does not qualify as risk:ask because the change does not modify application logic, data handling, or security policy beyond correcting the configured OIDC issuer.

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.

@ygalblum

ygalblum commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Additional fix: OSAC-4915

Same root cause category and same Makefile line — service.vault.endpoint was hardcoded to https://openbao.osac.svc.cluster.local:8200 in every CI profile's instance.yaml, but CI actually installs OSAC into the osac-e2e-ci namespace, not osac. This made fulfillment-controller fail to log into vault entirely:

Failed to obtain vault token: failed to login to vault with JWT: vault login request failed:
Post "https://openbao.osac.svc.cluster.local:8200/v1/auth/jwt/login":
dial tcp: lookup openbao.osac.svc.cluster.local on 10.128.0.10:53: no such host

Fixed by extending the same $(DOMAIN)-conditional --set block in install-osac to also set service.vault.endpoint=https://openbao.$(NS).svc.cluster.local:8200, using the Makefile's existing $(NS) variable.

Verified via helm template (renders --vault-endpoint=https://openbao.osac-e2e-ci.svc.cluster.local:8200 correctly) and live on a running OpenShift dev cluster (fulfillment-controller's vault login errors disappeared after applying).

Fixes: https://redhat.atlassian.net/browse/OSAC-4915

Assisted-by: Claude Code noreply@anthropic.com

@coderabbitai coderabbitai Bot added risk:ask and removed risk:show labels Sep 4, 2026
@ygalblum

ygalblum commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing as it is addressed in #730

@ygalblum ygalblum closed this Sep 4, 2026
auto-merge was automatically disabled September 4, 2026 18:18

Pull request was closed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants