Skip to content

WIP: poc: deploying upstream component #919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
everettraven wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

WIP: poc: deploying upstream component #919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0291194
poc: deploying upstream component
everettraven Jun 16, 2026
2660b69
fixup! change the right deployment generation method...
everettraven Jun 17, 2026
6662838
wip: changes to get padlok image to work
everettraven Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bindata/oauth-apiserver/externaloidc-deploy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ spec:
- name: oauth-apiserver
terminationMessagePolicy: FallbackToLogsOnError
image: ${IMAGE}
imagePullPolicy: IfNotPresent
imagePullPolicy: Always

@everettraven everettraven Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note: this change isn't actually necessary. Just helpful for PoC-ing.

command: ["/bin/bash", "-ec"]
args:
- |
if [ -s /var/run/configmaps/trusted-ca-bundle/tls-ca-bundle.pem ]; then
echo "Copying system trust bundle"
cp -f /var/run/configmaps/trusted-ca-bundle/tls-ca-bundle.pem /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem
fi
exec oauth-apiserver external-oidc \
exec padlok run \
--secure-port=8443 \
--tls-private-key-file=/var/run/secrets/serving-cert/tls.key \
--tls-cert-file=/var/run/secrets/serving-cert/tls.crt \
Expand Down
14 changes: 13 additions & 1 deletion pkg/operator/workload/sync_openshift_oauth_apiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"os"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -349,9 +350,20 @@ func (c *OAuthAPIServerWorkload) syncExternalOIDCDeployment(ctx context.Context,
// log level verbosity is taken from the spec always
args["v"] = []string{loglevelToKlog(operatorSpec.LogLevel)}

imageToUse := c.targetImagePullSpec
klog.Info("UPSTREAM_OIDC_COMPONENT_IMAGE", os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"))
if upstreamImage := os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"); len(upstreamImage) > 0 {
imageToUse = upstreamImage

klog.Info("using UPSTREAM_OIDC_COMPONENT_IMAGE", os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"))

Comment on lines +354 to +359

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid logging raw UPSTREAM_OIDC_COMPONENT_IMAGE values.

Line 354 and Line 358 emit the full env value; this can leak internal registry hostnames (and potentially credential-bearing image refs) into logs.

Suggested fix 
-	klog.Info("UPSTREAM_OIDC_COMPONENT_IMAGE", os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"))
-	if upstreamImage := os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"); len(upstreamImage) > 0 {
+	upstreamImage := os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE")
+	if len(upstreamImage) > 0 {
 		imageToUse = upstreamImage

-		klog.Info("using UPSTREAM_OIDC_COMPONENT_IMAGE", os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"))
+		klog.V(2).Info("using UPSTREAM_OIDC_COMPONENT_IMAGE override")
 
 		// configure the API endpoint on the upstream component
 		args["authenticate-path"] = []string{"/apis/oauth.openshift.io/v1/tokenreviews"}
 	}

As per coding guidelines, "Flag logging that may expose ... internal hostnames ..." and "Never log secrets or credentials."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
klog.Info("UPSTREAM_OIDC_COMPONENT_IMAGE", os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"))
if upstreamImage := os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"); len(upstreamImage) > 0 {
imageToUse = upstreamImage
klog.Info("using UPSTREAM_OIDC_COMPONENT_IMAGE", os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE"))
upstreamImage := os.Getenv("UPSTREAM_OIDC_COMPONENT_IMAGE")
if len(upstreamImage) > 0 {
imageToUse = upstreamImage
klog.V(2).Info("using UPSTREAM_OIDC_COMPONENT_IMAGE override")
// configure the API endpoint on the upstream component
args["authenticate-path"] = []string{"/apis/oauth.openshift.io/v1/tokenreviews"}
}
🤖 Prompt for 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.

In `@pkg/operator/workload/sync_openshift_oauth_apiserver.go` around lines 354 -
359, The code is logging the raw value of the UPSTREAM_OIDC_COMPONENT_IMAGE
environment variable in two places (the klog.Info calls on lines 354 and 358),
which can expose internal registry hostnames and credentials. Remove the
os.Getenv() calls from both klog.Info statements and replace them with generic
log messages that only indicate the variable was set or is being used, such as
"using upstream OIDC component image from environment" or similar, without
exposing the actual image reference value.

Source: Coding guidelines

// configure the API endpoint on the upstream component
args["authenticate-path"] = []string{"/apis/oauth.openshift.io/v1/tokenreviews"}
}
Comment on lines +353 to +362

@everettraven everettraven Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note: This isn't actually how we would do this, but this is sufficient for a PoC.

How we would really do this is likely another image pull spec environment variable that is injected by the CVO and used as the ${IMAGE} replacement.


// use string replacer for simple things
r := strings.NewReplacer(
"${IMAGE}", c.targetImagePullSpec,
"${IMAGE}", imageToUse,
"${REVISION}", strconv.Itoa(int(operatorStatus.LatestAvailableRevision)),
)

Expand Down