Prod Auth for Lightspeed Exporter#121
Prod Auth for Lightspeed Exporter#121openshift-merge-bot[bot] merged 1 commit intorh-ecosystem-edge:mainfrom
Conversation
WalkthroughAdds parameters to template.yaml to support configurable LIGHTSPEED_EXPORTER_AUTH_MODE (manual|sso|openshift) and SSO_CLIENT_SECRET_NAME, wires the mode into the exporter --mode arg, makes ingress auth token secret optional, and adds optional SSO client env vars for the exporter. Changes
Sequence Diagram(s)sequenceDiagram
participant Deployer as Deployer
participant Template as template.yaml
participant K8s as Kubernetes API
participant Exporter as lightspeed-to-dataverse-exporter
participant SSOSecret as SSO secret (optional)
participant IngressSecret as INSIGHTS_INGRESS_SECRET (optional)
Deployer->>Template: set LIGHTSPEED_EXPORTER_AUTH_MODE & SSO_CLIENT_SECRET_NAME
Template->>K8s: create/update Deployment with --mode=${LIGHTSPEED_EXPORTER_AUTH_MODE} and env refs
K8s->>Exporter: start container
alt mode == manual
Exporter->>IngressSecret: read INGRESS_SERVER_AUTH_TOKEN (expected present)
else mode == sso
Exporter->>SSOSecret: read CLIENT_ID / CLIENT_SECRET (optional)
Exporter->>IngressSecret: read INGRESS_SERVER_AUTH_TOKEN (optional)
else mode == openshift
Exporter->>IngressSecret: treat token as optional (use platform auth)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Skipping CI for Draft Pull Request. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
template.yaml (2)
77-79: Clarify secret behavior across auth modes; tighten wordingThe current wording is a bit ambiguous. Explicitly call out when the Secret is optional vs required and the expected key name.
Apply this diff to improve clarity:
- description: | - The name of a secret containing the auth_token for the insights ingress server. Will be ignored - if it doesn't exist but LIGHTSPEED_EXPORTER_AUTH_MODE should be set to 'openshift' in that case. + description: | + Name of the Secret that contains the insights ingress auth token (data key: auth_token). + In 'openshift' auth mode, this Secret is optional and will be ignored if missing. + In 'manual' auth mode, this Secret is required; if absent, the exporter will fail to authenticate.
89-95: Document default, valid values, and failure mode; ensure stage overrideThe parameter is a solid addition. Recommend tightening the description and confirming that stage explicitly sets 'manual' and has the Secret present.
Suggested description tweak for precision:
- description: | - The type of authentication to use for the lightspeed data exporter. Valid values are 'manual' - and 'openshift'. If 'manual' is specified the INSIGHTS_INGRESS_SECRET_NAME secret should contain - a valid auth token in the data item `auth_token`. + description: | + Authentication mode for the lightspeed data exporter. Valid values: 'manual' or 'openshift'. + Default: 'openshift' (uses the pod's in-cluster service account token). In this mode the + ingress Secret may be absent and will be ignored. + When set to 'manual', the ${INSIGHTS_INGRESS_SECRET_NAME} Secret must exist and contain + the key 'auth_token' with a valid token; otherwise the exporter will fail to authenticate.Verification: Please confirm the stage environment explicitly sets LIGHTSPEED_EXPORTER_AUTH_MODE=manual and that the ${INSIGHTS_INGRESS_SECRET_NAME} Secret exists with data.auth_token there. Also confirm the exporter image tag in stage/prod includes support for '--mode openshift'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
template.yaml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
🔇 Additional comments (2)
template.yaml (2)
490-490: Wires exporter mode to parameter (LGTM)Passing the mode via parameter is correct and aligns with the new default behavior.
Please verify that the deployed lightspeed-to-dataverse-exporter image version supports the '--mode openshift' flag and behaves correctly when INGRESS_SERVER_AUTH_TOKEN is unset.
501-501: Optional Secret is appropriate; be aware of manual mode failure semanticsMarking the Secret as optional is correct for 'openshift' mode. In 'manual' mode, this will lead to startup/auth failures if the Secret is missing, which is acceptable but should be expected. The description updates above help set that expectation.
Please ensure environments that set LIGHTSPEED_EXPORTER_AUTH_MODE=manual also provision the ${INSIGHTS_INGRESS_SECRET_NAME} Secret with key 'auth_token'.
This uses sso auth in prod and makes the manual auth secret optional so we don't have to have it in prod.
keitwb
force-pushed
the
exporter-prod-auth
branch
from
158f76d to
dbd8ce4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
template.yaml (2)
80-81: Clarify expected keys for SSO secretDocument the required keys to reduce deploy-time guesswork and misconfigurations.
- description: "Name of the K8s secret that contains the SSO client credentials" + description: "Name of the K8s secret that contains the SSO client credentials (keys: client_id, client_secret)"
521-527: Token secret made optional: ensure manual mode fails fast and observably when missingMaking INGRESS_SERVER_AUTH_TOKEN optional is necessary for 'sso'/'openshift', but in 'manual' mode its absence should result in a clear, immediate failure (not a silent no-op). Ensure the exporter emits actionable logs and exits non-zero, or expose a health/readiness signal.
If the exporter exposes a health endpoint, consider adding a basic readinessProbe for this sidecar to surface misconfiguration:
- name: lightspeed-to-dataverse-exporter image: quay.io/lightspeed-core/lightspeed-to-dataverse-exporter:${LIGHTSPEED_EXPORTER_IMAGE_TAG} imagePullPolicy: Always + readinessProbe: + httpGet: + path: /healthz + port: 8080 + initialDelaySeconds: 10 + periodSeconds: 10If there’s no HTTP health endpoint, we can switch to an exec probe that checks for a recent successful send or a mode-specific config check. Happy to draft once we confirm exporter capabilities.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
template.yaml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
🔇 Additional comments (3)
template.yaml (3)
503-503: LGTM: mode is now parameterizedSwitching from a hard-coded mode to "${LIGHTSPEED_EXPORTER_AUTH_MODE}" is correct and aligns the deployment with environment-specific configuration.
509-520: Verify SSO mode env configuration and config injection
- The template already defines the
SSO_BASE_URLparameter (lines 111–113) and injects it into thejwk_tokenmodule’s JWKS URL in the generatedconfig.yaml(line 164). Ensure that the renderedconfig.yamlcontains the correct URL (${SSO_BASE_URL}/protocol/openid-connect/certs).- Confirm that the exporter binary actually consumes the
CLIENT_IDandCLIENT_SECRETenvironment variables when invoked with--mode "sso". The grep scan did not reveal any code paths reading these vars directly, so please verify in the exporter source.- Only if the exporter also reads
SSO_BASE_URLfrom its environment (rather than from the config file), consider adding this env var to the container:env: - name: CLIENT_ID valueFrom: secretKeyRef: name: ${SSO_CLIENT_SECRET_NAME} key: client_id optional: true - name: CLIENT_SECRET valueFrom: secretKeyRef: name: ${SSO_CLIENT_SECRET_NAME} key: client_secret optional: true + - name: SSO_BASE_URL + value: ${SSO_BASE_URL}Verification command used:
rg -n -C2 -i 'CLIENT_(ID|SECRET)|SSO_BASE_URL|LIGHTSPEED_EXPORTER_AUTH_MODE|--mode|openshift|sso|manual'
91-98: Set default LIGHTSPEED_EXPORTER_AUTH_MODE to 'openshift'Verified that in template.yaml (lines 91–98) the default remains
"sso", which contradicts the PR’s intent to default to OpenShift pull-secret authentication in production.Locations needing change:
- template.yaml:91–92 (current default)
- template.yaml:503–504 (where
--modeis passed to the exporter)Proposed patch:
- name: LIGHTSPEED_EXPORTER_AUTH_MODE - value: "sso" + name: LIGHTSPEED_EXPORTER_AUTH_MODE + value: "openshift"Follow-up: please confirm how the exporter resolves credentials in ‘openshift’ mode (e.g., via the Pod’s service-account imagePullSecrets or a mounted cluster pull-secret) and update the chart with any required mounts or RBAC.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: keitwb, maorfr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 0da6efb
into
rh-ecosystem-edge:main
3 participants
This uses sso auth in prod and makes the manual auth secret optional so
we don't have to have it in prod.
Summary by CodeRabbit
New Features
Improvements
Documentation