test proxy in deploy-konflux-operator#80783
Conversation
Signed-off-by: Anvesh J <ajaggapa@redhat.com>
WalkthroughIn ChangesDeploy Konflux Operator Script
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ajaggapa 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-ci
Bot
added
the
approved
|
[REHEARSALNOTIFIER]
A total of 287 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@ci-operator/step-registry/deploy-konflux-operator/deploy-konflux-operator-commands.sh`:
- Around line 138-139: The `install_deps` and `set_proxy` function calls are in
the wrong order. The `set_proxy` function must be called before `install_deps`
to ensure that proxy environment variables are configured before any dependency
installation commands are executed. Swap the order of these two function calls
so that `set_proxy` executes first, followed by `install_deps`.
- Around line 131-136: The echo statements for HTTP_PROXY, HTTPS_PROXY,
NO_PROXY, http_proxy, https_proxy, and no_proxy variables expose sensitive
infrastructure details including URLs and embedded credentials to CI logs.
Replace these raw variable echo statements with checks that only indicate
whether each proxy variable is set (without printing the actual value), such as
using conditional tests or redacted confirmation messages that show presence but
not content.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 90e64513-c30b-450d-be71-fd42a2a43eaf
📒 Files selected for processing (1)
ci-operator/step-registry/deploy-konflux-operator/deploy-konflux-operator-commands.sh
| echo "HTTP_PROXY=${HTTP_PROXY:-}" | ||
| echo "HTTPS_PROXY=${HTTPS_PROXY:-}" | ||
| echo "NO_PROXY=${NO_PROXY:-}" | ||
| echo "http_proxy=${http_proxy:-}" | ||
| echo "https_proxy=${https_proxy:-}" | ||
| echo "no_proxy=${no_proxy:-}" |
There was a problem hiding this comment.
Do not print raw proxy environment variables to CI logs.
HTTP_PROXY/HTTPS_PROXY/http_proxy/https_proxy can contain full URLs and embedded credentials; echoing them leaks sensitive infrastructure details. Log presence only (or redact values) instead of printing raw content.
Suggested fix
-echo "HTTP_PROXY=${HTTP_PROXY:-}"
-echo "HTTPS_PROXY=${HTTPS_PROXY:-}"
-echo "NO_PROXY=${NO_PROXY:-}"
-echo "http_proxy=${http_proxy:-}"
-echo "https_proxy=${https_proxy:-}"
-echo "no_proxy=${no_proxy:-}"
+echo "HTTP_PROXY set: $([[ -n ${HTTP_PROXY:-} ]] && echo yes || echo no)"
+echo "HTTPS_PROXY set: $([[ -n ${HTTPS_PROXY:-} ]] && echo yes || echo no)"
+echo "NO_PROXY set: $([[ -n ${NO_PROXY:-} ]] && echo yes || echo no)"
+echo "http_proxy set: $([[ -n ${http_proxy:-} ]] && echo yes || echo no)"
+echo "https_proxy set: $([[ -n ${https_proxy:-} ]] && echo yes || echo no)"
+echo "no_proxy set: $([[ -n ${no_proxy:-} ]] && echo yes || echo no)"As per coding guidelines, step-registry command scripts must never print sensitive data (including URLs/credential-bearing values) to logs.
🤖 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
`@ci-operator/step-registry/deploy-konflux-operator/deploy-konflux-operator-commands.sh`
around lines 131 - 136, The echo statements for HTTP_PROXY, HTTPS_PROXY,
NO_PROXY, http_proxy, https_proxy, and no_proxy variables expose sensitive
infrastructure details including URLs and embedded credentials to CI logs.
Replace these raw variable echo statements with checks that only indicate
whether each proxy variable is set (without printing the actual value), such as
using conditional tests or redacted confirmation messages that show presence but
not content.
Source: Coding guidelines
| install_deps | ||
| set_proxy |
There was a problem hiding this comment.
Proxy setup must happen before dependency installation.
install_deps now runs before set_proxy, so oc/opm installs may execute without required proxy env and fail in restricted networks. The order should be set_proxy first, then install_deps.
Suggested fix
-install_deps
-set_proxy
+set_proxy
+install_deps📝 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.
| install_deps | |
| set_proxy | |
| set_proxy | |
| install_deps |
🤖 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
`@ci-operator/step-registry/deploy-konflux-operator/deploy-konflux-operator-commands.sh`
around lines 138 - 139, The `install_deps` and `set_proxy` function calls are in
the wrong order. The `set_proxy` function must be called before `install_deps`
to ensure that proxy environment variables are configured before any dependency
installation commands are executed. Swap the order of these two function calls
so that `set_proxy` executes first, followed by `install_deps`.
|
/pj-rehearse pull-ci-openshift-ovn-kubernetes-main-e2e-metal-ipi-ovn-dualstack-bgp-local-gw |
|
@ajaggapa: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
1 participant
Testing proxy dependency for installing oc & opm
Summary by CodeRabbit
This PR modifies the
deploy-konflux-operatorworkflow script to improve testing of proxy dependency functionality for the OpenShift CI infrastructure.Changes
The script now includes:
Proxy environment variable logging (lines 131-136): Added diagnostic logging to output the current values of proxy-related environment variables (
HTTP_PROXY,HTTPS_PROXY,NO_PROXY, and their lowercase equivalents) before proceeding with dependency installation.Reordered execution sequence (lines 138-139): Changed the execution order so that
install_deps(which installsopmandoctools via curl) runs beforeset_proxy, meaning proxy configuration is applied after the initial tools are downloaded rather than before.Impact
These changes enable better testing of the proxy configuration in the deploy-konflux-operator workflow step, allowing visibility into the proxy environment variables and ensuring that dependency installation can be verified independently of proxy settings before proxy configuration is applied.