Repository navigation
Conversation
…dows tas-env-variables.ps1 always overwrote OIDC_ISSUER_URL with a value derived from the cluster, even when the caller had already set one. The derived value guesses the Keycloak realm path, and that guess only holds for RHBK, which serves under /realms; legacy RH-SSO serves under /auth/realms. A caller that knows its own issuer URL had it discarded on Windows while tas-env-variables.sh honoured it everywhere else, so Windows runs failed to authenticate for a reason unrelated to the test. Guard the derivation the same way the shell script already does, and read the route host from .status.ingress[0].host to match it. .spec.host is the requested hostname and is empty when the route lets OpenShift generate one; the status field always carries the admitted host. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 8ad7cf2)
… scripts Guarding OIDC_ISSUER_URL alone fixed one of five instances of the same bug, and only in one of the two Windows scripts. tas-env-variables.sh derives a value from `oc` only when the variable is unset, for all six variables. Both Windows scripts unconditionally overwrote TUF_URL, FULCIO_URL, REKOR_URL and REKOR_UI_URL. This matters because e2e.yml exports exactly those four from kubectl and then sources the env script: on Linux they survive, on Windows they were discarded and re-derived from a cluster that may answer differently. Guard all six in both scripts, and switch the .bat route lookup to .status.ingress[0].host to match. The .bat needs the flat goto form for OIDC_ISSUER_URL rather than a parenthesised if: cmd expands %OIDC_ROUTE% when it parses the block, so composing the URL inside one would read it before the for loop had set it. The single-step lookups are safe on one line because `set X=%%i` takes the for variable rather than an expanded %VAR%. Neither script can be executed on macOS, so this is reviewed, not run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit e2a7075)
PR Summary by QodoBackport Windows OIDC issuer and URL override fixes to release-1.4
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo 1. Caller URLs can run Windows commands
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of the Windows OIDC issuer and caller-supplied URL fixes from #110 to
release-1.4.Cherry-picked commits:
8ad7cf2fix: securesign-6182: honour a caller-supplied OIDC_ISSUER_URL on Windowse2a7075fix: securesign-6182: honour every caller-supplied URL in the Windows scripts