First commit for upgrade #1368
First commit for upgrade #1368
Conversation
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @msajidmansoori12. Thanks for your PR. I'm waiting for a konveyor member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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 kubernetes-sigs/prow repository. |
msajidmansoori12
force-pushed
the
test-upgradeci
branch
16 times, most recently
from
7f3afd6 to
3967901
Compare
Signed-off-by: M Sajid Mansoori <[email protected]>
msajidmansoori12
force-pushed
the
test-upgradeci
branch
10 times, most recently
from
2063694 to
345bbd4
Compare
Signed-off-by: M Sajid Mansoori <[email protected]>
msajidmansoori12
force-pushed
the
test-upgradeci
branch
from
61435c3 to
c8baf28
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
.github/workflows/upgrade-ci.yml (7)
15-18: Migrate toactions/checkout@v4
v2is no longer maintained and can fail on newer runners.
Replace it with the current major release.
33-35: Add SHA-256 verification when downloadingoperator-sdkBlindly installing a network-fetched binary is a supply-chain risk.
Verify the checksum beforesudo install.
43-45:kubectldownload also lacks checksum validationApply the same integrity check you plan for
operator-sdk.
61-65: Avoid echoing secrets into logsWriting
QUAY_USERNAME/QUAY_PASSWORDto$GITHUB_ENVleaks them in plain text logs.
Use theenv:map to inject secrets without log exposure.
78-80: Fix YAML syntax – remove extra space beforerun:
run :is invalid YAML and will break the workflow parser.
95-110: Ingress-wait loop still fragile
- Uses invalid
--templateflag; replace with-o jsonpath=….- Unquoted vars (SC2086) and no timeout → possible infinite loop.
- You poll
external_ipthen ignore it (endpoint=$(minikube ip)).Re-apply the earlier fix or adopt
minikube kubectl -- …with a bounded wait.
122-127: Password value drifts between steps
login.test.tsusespassword, while subsequent tests useDog8code.
Pick one value to prevent flakey auth failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 61435c3125a4eedd08b3325c89c4977dd117a39e and c8baf28.
📒 Files selected for processing (1)
.github/workflows/upgrade-ci.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 141-141: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
⏰ 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: e2e-main / e2e-ui-integration-tests
| export OPM_VERSION=$(curl -s https://api.github.com/repos/operator-framework/operator-registry/releases/latest | grep tag_name | cut -d '"' -f 4) | ||
|
|
||
| # Download the binary | ||
| curl -Lo opm-linux-amd64 "https://github.com/operator-framework/operator-registry/releases/download/${OPM_VERSION}/linux-amd64-opm" | ||
| chmod +x opm-linux-amd64 | ||
| sudo mv opm-linux-amd64 /usr/local/bin/opm | ||
| opm version |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extend checksum/sig verification to the opm binary
The OPM CLI is fetched from GitHub and installed with root privileges, yet no integrity check is performed.
Add a SHA-256 (or GPG) verification step before mv.
🤖 Prompt for AI Agents
In .github/workflows/upgrade-ci.yml around lines 53 to 59, the script downloads
the opm binary and installs it with root privileges but lacks integrity
verification. To fix this, add a step to download the corresponding SHA-256
checksum or GPG signature file for the opm binary, then verify the downloaded
binary against this checksum or signature before moving it to /usr/local/bin. If
the verification fails, the script should abort to prevent installing a
potentially tampered binary.
99f07d1 to
3430885
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
.github/workflows/upgrade-ci.yml (8)
14-18: Update deprecatedactions/checkout@v2to @v4
v2is no longer maintained and triggers the “runner too old” warning. Switch toactions/checkout@v4for future-proofing.- - name: Checkout tackle - uses: actions/checkout@v2 + - name: Checkout tackle + uses: actions/checkout@v4
26-35: Add SHA-256 verification for downloaded operator-sdk binary
Fetching a root-installed binary over HTTP without integrity checking is a supply-chain risk.curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/operator-sdk_linux_amd64 +echo "f0b0e… operator-sdk_linux_amd64" | sha256sum -c - # ← insert real checksum +test $? -eq 0 || { echo "::error::operator-sdk checksum mismatch"; exit 1; } sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdk
46-58: Extend integrity checks to theopmdownload
opmis fetched and moved to/usr/local/binwith root privileges but no verification. Apply SHA-256/GPG verification beforemv.
61-65: Avoid echoing secrets into logs – useenv:
Writing secrets to$GITHUB_ENVviaechoexposes them in step logs. Prefer:- - name: Set Quay credentials - run: | - echo "QUAY_USERNAME=${{ secrets.QUAY_USERNAME }}" >> $GITHUB_ENV - echo "QUAY_PASSWORD=${{ secrets.QUAY_PASSWORD }}" >> $GITHUB_ENV + - name: Set Quay credentials + env: + QUAY_USERNAME: ${{ secrets.QUAY_USERNAME }} + QUAY_PASSWORD: ${{ secrets.QUAY_PASSWORD }} + run: echo "Quay creds injected via environment"
78-80: YAML syntax error: extra space before:breaks parsing- run : minikube addons enable ingress + run: minikube addons enable ingress
85-88:head -n 2still risks embedding a newline – usehead -n1and trim- konveyor_version=$(kubectl get csv ... | head -n 2 | tr -d '[:space:]') + konveyor_version=$(kubectl get csv ... | head -n1 | tr -d '[:space:]')A single newline breaks later string comparisons.
101-116: Ingress-wait loop: invalid flag, quoting, no timeout, wrong IP used
--templateis not a valid flag; use-o jsonpath=….- Unquoted vars → SC2086.
- No timeout ⇒ possible infinite loop.
- After resolving
external_ip, the script ignores it and usesminikube ip.- external_ip=$(kubectl get ingress tackle --template="{{range.status.loadBalancer.ingress}}{{.ip}}{{end}}" -n konveyor-tackle) + external_ip=$(kubectl get ingress tackle -n konveyor-tackle \ + -o jsonpath='{.status.loadBalancer.ingress[0].ip}' 2>/dev/null) || trueWrap the loop with a
timeout(e.g., 300 s) and finally:- export endpoint=$(minikube ip); + endpoint="$external_ip"
128-132: Password value drifts between steps – pick one
login.test.tsusespassword, while pre-upgrade tests useDog8code. Use a single value to avoid flaky auth.- npx cypress run ... --env user="admin",pass="password",tackleUrl=... + npx cypress run ... --env user="admin",pass="Dog8code",tackleUrl=...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 99f07d120cac9a81dbb41980d74a8b116d751e7c and 343088596f309994c014be9e55d2f691f0ae6fe1.
📒 Files selected for processing (1)
.github/workflows/upgrade-ci.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 106-106: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 138-138: trailing spaces
(trailing-spaces)
[error] 143-143: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 146-146: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
⏰ 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: e2e-main / e2e-ui-integration-tests
.github/workflows/upgrade-ci.yml
Outdated
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mirror the same checksum guard for kubectl
Add integrity verification before sudo install, exactly as recommended for operator-sdk.
🤖 Prompt for AI Agents
In .github/workflows/upgrade-ci.yml around lines 36 to 45, the kubectl
installation step lacks a checksum verification before installing the binary. To
fix this, add a checksum download and verification step similar to the
operator-sdk installation, ensuring the kubectl binary's integrity before
running the sudo install command. This involves downloading the checksum file,
verifying the kubectl binary against it, and only proceeding with installation
if the checksum matches.
3430885 to
6d4d14e
Compare
msajidmansoori12
force-pushed
the
test-upgradeci
branch
from
6d4d14e to
068dc16
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
.github/workflows/upgrade-ci.yml (7)
15-16: Useactions/checkout@v4– v2 runner is deprecated
actions/checkout@v2often fails with “runner too old”. Bump to v4 for compatibility.
26-45: Add SHA-256 verification for downloaded binaries
operator-sdk,kubectl, andopmare fetched over HTTPS and installed with root privileges without any integrity check – a supply-chain risk. Verify checksums beforeinstall/mv.Also applies to: 46-60
61-65: Don’t echo secrets into logs – use theenv:map
Writing${{ secrets.QUAY_* }}to$GITHUB_ENVleaks masked values into build output and is unnecessary. Inject them via theenv:block instead.
78-80:run :is invalid YAML syntax
The extra space before:breaks parsing. Change torun:.
103-118: Ingress-poll loop can spin forever & uses invalid flags
--templateis not a validkubectl get ingressflag, variables are unquoted, and there’s no timeout. Adopt the earlier suggested bounded loop.
117-118: You fetch$external_ipbut then ignore it
After resolving the ingress IP the script overwritesendpointwithminikube ip, defeating the whole poll.
135-139: Password value drifts between steps – pick one
login.test.tsusespassword, other tests useDog8code. Use a single value to avoid flaky auth.Also applies to: 174-175
🧹 Nitpick comments (1)
.github/workflows/upgrade-ci.yml (1)
85-88: Grab only the first CSV line
head -n 2can still leave an empty/extra line; use one line only.- konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version | head -n 2 | tr -d '[:space:]') + konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version | head -n1 | tr -d '[:space:]')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/upgrade-ci.yml(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
⏰ 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: e2e-main / e2e-ui-integration-tests
msajidmansoori12
force-pushed
the
test-upgradeci
branch
from
068dc16 to
a2c7e8c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (11)
hack/install-postupgrade-konveyor.sh (1)
24-30: Guard against emptyPREUPGRADE_UI_PODbefore entering the loop
Same issue was raised previously and is still unresolved.-echo "Previous UI Pod: $PREUPGRADE_UI_POD" +echo "Previous UI Pod: $PREUPGRADE_UI_POD" +if [[ -z "${PREUPGRADE_UI_POD:-}" ]]; then + echo "ERROR: PREUPGRADE_UI_POD not set – cannot verify pod replacement." + exit 1 +fi.github/workflows/upgrade-ci.yml (10)
15-18: Upgrade toactions/checkout@v4
v2is deprecated and flagged by action-lint.- - name: Checkout tackle - uses: actions/checkout@v2 + - name: Checkout tackle + uses: actions/checkout@v4
26-35: Download of operator-sdk lacks integrity verificationSupply-chain best practice: verify SHA256 (same applies to the other binaries).
curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/operator-sdk_linux_amd64 -sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdk +echo "EXPECTED_SHA256 operator-sdk_linux_amd64" | sha256sum -c - && \ +sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdk
36-45: Mirror the same checksum guard forkubectlIdentical risk as above – verify before installing.
53-58: Extend checksum verification to theopmbinaryThe script still installs
opmwithout validating its integrity.
61-65: Avoid echoing secrets into build logsUse the
env:block instead of writing secrets to$GITHUB_ENV.- - name: Set Environment Variables - run: | - echo "QUAY_USERNAME=${{ secrets.QUAY_USERNAME }}" >> $GITHUB_ENV - echo "QUAY_PASSWORD=${{ secrets.QUAY_PASSWORD }}" >> $GITHUB_ENV + - name: Set Environment Variables + env: + QUAY_USERNAME: ${{ secrets.QUAY_USERNAME }} + QUAY_PASSWORD: ${{ secrets.QUAY_PASSWORD }} + run: echo "Quay credentials injected via environment"
79-80: YAML syntax error – extra space before:
run :will cause the workflow to fail parsing.- run : minikube addons enable ingress + run: minikube addons enable ingress
85-88: Trim to a single version line
head -n 2can still include a newline and break env usage; usehead -n1.- konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version | head -n 2 | tr -d '[:space:]') + konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version \ + | head -n1 | tr -d '[:space:]')
103-118: Ingress-wait loop can spin forever & uses invalid--templateflagSame issues previously reported:
kubectl --templateis invalid ➜ exits non-zero.- No timeout ➜ infinite loop.
- Unquoted vars.
Consider the earlier suggested bounded loop:
set -eo pipefail timeout=300; end=$((SECONDS+timeout)) while [[ -z "${external_ip:-}" && $SECONDS -lt $end ]]; do echo "Waiting for ingress…" external_ip=$(kubectl get ingress tackle -n konveyor-tackle \ -o jsonpath='{.status.loadBalancer.ingress[0].ip}' 2>/dev/null) || true sleep 10 done [[ -z $external_ip ]] && { echo "::error::Ingress not ready"; exit 1; } echo "CYPRESS_tackleUrl=https://$external_ip" >>"$GITHUB_ENV"
130-136: Mismatched quoting & inconsistent passwordsShellCheck (SC2140) flags the unescaped quote; also the login test still uses
passwordwhereas later steps useDog8code.- run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" --env user="admin",pass="password",tackleUrl=${{ env.CYPRESS_tackleUrl }} + run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" \ + --env user=admin,pass=Dog8code,tackleUrl=${{ env.CYPRESS_tackleUrl }}
174-178: Undefined git cred env-vars & wildcard glob warning
cypress_git_user/cypress_git_passwordare never set, so the tests will
receive empty strings. Also**/*.test.tstriggers SC2035 when a file starts
with-.- CYPRESS_INCLUDE_TAGS=@post-upgrade npx cypress run --spec **/*.test.ts --env user=admin,pass=Dog8code,tackleUrl=${{ env.CYPRESS_tackleUrl }},git_user=${{ env.cypress_git_user }},git_password=${{ env.cypress_git_password }} + CYPRESS_INCLUDE_TAGS=@post-upgrade \ + npx cypress run --spec "./cypress/e2e/tests/**/*.test.ts" \ + --env user=admin,pass=Dog8code,tackleUrl=${{ env.CYPRESS_tackleUrl }},\ + git_user=${{ secrets.KONVEYOR_GIT_USER }},\ + git_password=${{ secrets.KONVEYOR_GIT_PASSWORD }}
🧹 Nitpick comments (1)
hack/install-postupgrade-konveyor.sh (1)
26-44: Quote variable expansions inside the polling loopUnquoted variables may expand to multiple words and break the test; also
kubectl get … -o namereturnspod/<name>– consider-o jsonpathto get the
plain pod name.- UI_POD_AFTER=$(kubectl get pods -n "${NAMESPACE}" -l app.kubernetes.io/name=tackle-ui -o name) + UI_POD_AFTER="$(kubectl get pods -n "${NAMESPACE}" \ + -l app.kubernetes.io/name=tackle-ui \ + -o jsonpath='{.items[0].metadata.name}')" ... - if [[ "$PREUPGRADE_UI_POD" != "$UI_POD_AFTER" ]]; then + if [[ "${PREUPGRADE_UI_POD}" != "${UI_POD_AFTER}" ]]; then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/upgrade-ci.yml(1 hunks)hack/install-postupgrade-konveyor.sh(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
85-85: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
130-130: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
148-148: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
165-165: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
174-174: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
177-177: shellcheck reported issue in this script: SC2035:info:1:59: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
85-85: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
130-130: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
148-148: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
165-165: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
174-174: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
177-177: shellcheck reported issue in this script: SC2035:info:1:59: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
⏰ 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: e2e-main / e2e-ui-integration-tests
| NAMESPACE="${NAMESPACE:-konveyor-tackle}" | ||
| TEMPLATE_DIR="${TEMPLATE_DIR:-hack/templates}" | ||
| KONVEYOR_UPGRADE_INDEX="${KONVEYOR_UPGRADE_INDEX:-quay.io/migqe/tackle2-operator-upgrade-index:latest}" |
There was a problem hiding this comment.
Add a proper she-bang and fail-fast flags
Without a she-bang the script inherits /bin/sh on many systems and ShellCheck (SC2148) complains.
set -euo pipefail prevents silent errors and unbound variables.
+#!/usr/bin/env bash
+set -euo pipefail
NAMESPACE="${NAMESPACE:-konveyor-tackle}"
TEMPLATE_DIR="${TEMPLATE_DIR:-hack/templates}"
KONVEYOR_UPGRADE_INDEX="${KONVEYOR_UPGRADE_INDEX:-quay.io/migqe/tackle2-operator-upgrade-index:latest}"🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🤖 Prompt for AI Agents
In hack/install-postupgrade-konveyor.sh at the beginning of the file (lines 1 to
3), add a proper she-bang line such as #!/bin/bash to explicitly specify the
shell interpreter. Also, add the line set -euo pipefail after the she-bang to
enable fail-fast behavior, causing the script to exit on errors, unset
variables, or pipeline failures. This will improve script robustness and silence
ShellCheck SC2148 warnings.
| kubectl patch installplan $(kubectl get installplan -n "${NAMESPACE}" | egrep "$POSTUPGRADE_VERSION"|awk '{print $1}') -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote command substitution to avoid word-splitting
If kubectl get installplan … returns multiple items the unquoted substitution
will be split and only the first token passed to patch, breaking the upgrade.
-kubectl patch installplan $(kubectl get installplan -n "${NAMESPACE}" | egrep "$POSTUPGRADE_VERSION"|awk '{print $1}') -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}'
+kubectl patch installplan "$(kubectl get installplan -n "${NAMESPACE}" \
+ | grep -E "$POSTUPGRADE_VERSION" | awk '{print $1}')" \
+ -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}'🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
In hack/install-postupgrade-konveyor.sh at lines 17 to 18, the command
substitution for getting the installplan names is unquoted, which can cause word
splitting if multiple installplans are returned. To fix this, enclose the
command substitution in double quotes to ensure the entire output is treated as
a single string, preventing incorrect splitting and ensuring all intended
installplans are passed correctly to the kubectl patch command.
93e4232 to
c66182f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (10)
hack/install-postupgrade-konveyor.sh (3)
1-3: Add a proper she-bang and fail-fast flags
#!/usr/bin/env bash+set -euo pipefailare still missing, so the script may run under/bin/shand ignore errors.
17-18: Quote command substitution to prevent word-splittingThe unquoted
$(kubectl …)output will be split if multiple installplans match, breaking the patch command.-kubectl patch installplan $(kubectl get installplan -n "${NAMESPACE}" | egrep "$POSTUPGRADE_VERSION"|awk '{print $1}') -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}' +kubectl patch installplan "$(kubectl get installplan -n "${NAMESPACE}" \ + | grep -E "$POSTUPGRADE_VERSION" | awk '{print $1}')" \ + -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}'
26-34: Guard against empty PREUPGRADE_UI_PODIf
PREUPGRADE_UI_PODis unset/empty the loop exits immediately, giving a false positive.+[[ -z "${PREUPGRADE_UI_POD:-}" ]] && { + echo "ERROR: PREUPGRADE_UI_POD not set"; exit 1; +}Insert this check before the loop.
.github/workflows/upgrade-ci.yml (7)
15-16: Upgrade toactions/checkout@v4
v2is deprecated and triggers “runner too old” warnings.- - name: Checkout tackle - uses: actions/checkout@v2 + - name: Checkout tackle + uses: actions/checkout@v4
61-65: Avoid echoing secrets to logsWriting
QUAY_USERNAME/PASSWORDinto$GITHUB_ENVechoes them in plain text. Pass them via theenv:map instead.
79-80: Fix invalid YAML keyExtra space before
:makesrun :invalid YAML. Remove the space.- run : minikube addons enable ingress + run: minikube addons enable ingress
103-118: Ingress-wait loop uses wrong flag and can hang forever
--templateis invalid forkubectl get ingress; the loop lacks a timeout and variables are unquoted.Replace with:
timeout=300; end=$((SECONDS+timeout)) while [[ -z "${external_ip:-}" && $SECONDS -lt $end ]]; do external_ip=$(kubectl get ingress tackle -n konveyor-tackle \ -o jsonpath='{.status.loadBalancer.ingress[0].ip}' 2>/dev/null) || true sleep 10 done [[ -z $external_ip ]] && { echo "::error::Ingress not ready"; exit 1; } echo "CYPRESS_tackleUrl=https://$external_ip" >>"$GITHUB_ENV"
117-118: Use resolved ingress IP instead of node IPAfter polling
external_ipthe workflow overwrites it withminikube ip, defeating the purpose.-export endpoint=$(minikube ip); -echo "CYPRESS_tackleUrl=https://$endpoint" >>$GITHUB_ENV' +echo "CYPRESS_tackleUrl=https://$external_ip" >>$GITHUB_ENV'
135-135: Password mismatch causes flaky loginLogin test uses
pass="password"whereas other steps useDog8code. Use one consistent value.-run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" --env user="admin",pass="password",tackleUrl=${{ env.CYPRESS_tackleUrl }} +run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" --env user="admin",pass="Dog8code",tackleUrl=${{ env.CYPRESS_tackleUrl }}
33-45: Add checksum verification when installing binaries
operator-sdk,kubectl, andopmare downloaded and installed with root privileges without any integrity check, posing a supply-chain risk. Fetch the official SHA-256 and verify beforeinstall/mv.
🧹 Nitpick comments (1)
hack/install-postupgrade-konveyor.sh (1)
46-47: Long blind sleep degrades feedback
sleep 200s(~3 min 20 s) after pod replacement unnecessarily delays the workflow. Replace with a readiness check on the new pod.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/upgrade-ci.yml(1 hunks)hack/install-postupgrade-konveyor.sh(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
85-85: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
130-130: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
148-148: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
165-165: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
174-174: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
177-177: shellcheck reported issue in this script: SC2035:info:1:59: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
85-85: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
130-130: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
148-148: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
165-165: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
174-174: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
177-177: shellcheck reported issue in this script: SC2035:info:1:59: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
⏰ 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: e2e-main / e2e-ui-integration-tests
msajidmansoori12
force-pushed
the
test-upgradeci
branch
2 times, most recently
from
373184a to
25b3389
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (10)
hack/install-postupgrade-konveyor.sh (3)
17-18: Unquoted command-substitution can break on spaces / multiple installplans
Previous feedback covered this – the issue is still present.-kubectl patch installplan $(kubectl get installplan -n "${NAMESPACE}" | egrep "$POSTUPGRADE_VERSION"|awk '{print $1}') -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}' +kubectl patch installplan "$(kubectl get installplan -n "${NAMESPACE}" \ + | grep -E "$POSTUPGRADE_VERSION" | awk '{print $1}')" \ + -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}'
24-34: Loop can succeed with empty$PREUPGRADE_UI_POD– add a guard
If the env-var is unset the first comparison succeeds immediately, giving a false positive.+[[ -z "${PREUPGRADE_UI_POD:-}" ]] && { echo "PREUPGRADE_UI_POD not set"; exit 1; }Insert before the
whileloop.
1-3: Add she-bang & fail-fast flags (already requested earlier)
The script still starts with variable assignments; without a she-bang it is executed by/bin/shon many systems and Shell-Check SC2148 keeps flagging it. Add#!/usr/bin/env bashandset -euo pipefailto avoid silent failures and unbound variables.+#!/usr/bin/env bash +set -euo pipefail.github/workflows/upgrade-ci.yml (7)
14-18:actions/checkout@v2is deprecated – upgrade to v4
The runner for v2 is no longer maintained and triggers warnings.- - name: Checkout tackle - uses: actions/checkout@v2 + - name: Checkout tackle + uses: actions/checkout@v4
79-80: YAML syntax error – extra space before colon
run :is invalid YAML and will fail to parse.- run : minikube addons enable ingress + run: minikube addons enable ingress
61-65: Stop echoing secrets into logs – inject viaenv:
Secrets are written to the build log; GitHub masks them but better not emit at all.- - name: Set Environment Variables - run: | - echo "QUAY_USERNAME=${{ secrets.QUAY_USERNAME }}" >> $GITHUB_ENV - echo "QUAY_PASSWORD=${{ secrets.QUAY_PASSWORD }}" >> $GITHUB_ENV + - name: Set Environment Variables + env: + QUAY_USERNAME: ${{ secrets.QUAY_USERNAME }} + QUAY_PASSWORD: ${{ secrets.QUAY_PASSWORD }} + run: echo "Quay creds injected via environment"
100-118: Ingress-wait loop still uses invalid flag & can run forever
--templateis not a valid kubectl flag, variables are unquoted and there’s no timeout – previously flagged.- external_ip=$(kubectl get ingress tackle --template="{{range.status.loadBalancer.ingress}}{{.ip}}{{end}}" -n konveyor-tackle);[[ -z $external_ip ]] && +end=$((SECONDS+300)) +while [[ -z "${external_ip:-}" && $SECONDS -lt $end ]]; do + external_ip=$(kubectl get ingress tackle -n konveyor-tackle \ + -o jsonpath='{.status.loadBalancer.ingress[0].ip}' 2>/dev/null) || true + sleep 10 +done +[[ -z "$external_ip" ]] && { echo "::error::Ingress not ready"; exit 1; }Also switch
CYPRESS_tackleUrlto use$external_ipinstead ofminikube ip.
85-88: Usehead -n1, nothead -n 2when extracting version
Multiple CSVs can exist; grabbing two lines re-introduces newline issues.- konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version | head -n 2 | tr -d '[:space:]') + konveyor_version=$(kubectl get csv -n konveyor-tackle \ + -o=custom-columns=:spec.version | head -n1 | tr -d '[:space:]')
135-139: Password mismatch between pre- and post-upgrade tests
login.test.tsstill usespasswordwhile the rest of the suite usesDog8code, causing flaky auth failures. Standardise on one value.- run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" --env user="admin",pass="password",tackleUrl=${{ env.CYPRESS_tackleUrl }} + run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" --env user="admin",pass="Dog8code",tackleUrl=${{ env.CYPRESS_tackleUrl }}
28-35: Binary downloads lack checksum verification (supply-chain risk)
operator-sdk,kubectlandopmare installed with root privileges without integrity checks. Add SHA-256 or GPG verification beforeinstall/mv.
Example for operator-sdk:curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/operator-sdk_linux_amd64 +echo "f0b0e… operator-sdk_linux_amd64" | sha256sum -c - sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdkApply the same pattern to the other binaries.
🧹 Nitpick comments (1)
hack/install-postupgrade-konveyor.sh (1)
46-47: Replace blindsleep 200swith a readiness wait
A fixed 200-second pause slows fast clusters and may still be too short on slow ones. Prefer waiting on the new pod’s Ready condition:-sleep 200s +kubectl wait --for=condition=Ready pod/"${UI_POD_AFTER#pod/}" \ + -n "${NAMESPACE}" --timeout=200s
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/upgrade-ci.yml(1 hunks)hack/install-postupgrade-konveyor.sh(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
85-85: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
130-130: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
148-148: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
165-165: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
174-174: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
62-62: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
85-85: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
130-130: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
148-148: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
165-165: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
174-174: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[warning] 79-79: too many spaces before colon
(colons)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
| kubectl patch sub konveyor-operator -n konveyor-tackle --type=merge -p "{\"spec\":{\"channel\":\"${SUBSCRIPTION_CHANNEL}\"}}" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded namespace ignores the configurable $NAMESPACE
You export NAMESPACE at the top yet this line patches konveyor-tackle directly.
-kubectl patch sub konveyor-operator -n konveyor-tackle --type=merge -p "{\"spec\":{\"channel\":\"${SUBSCRIPTION_CHANNEL}\"}}"
+kubectl patch sub konveyor-operator -n "${NAMESPACE}" --type=merge \
+ -p "{\"spec\":{\"channel\":\"${SUBSCRIPTION_CHANNEL}\"}}"🤖 Prompt for AI Agents
In hack/install-postupgrade-konveyor.sh at lines 13 to 14, the kubectl patch
command uses a hard-coded namespace 'konveyor-tackle' instead of the
configurable $NAMESPACE variable. Replace the hard-coded namespace with the
$NAMESPACE variable to respect the exported configuration and allow flexibility.
25b3389 to
3ec4348
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (10)
hack/install-postupgrade-konveyor.sh (4)
1-1: Add she-bang and fail-fast flags
The script still has no interpreter line orset -euo pipefail, so it silently ignores errors and unread variables.+#!/usr/bin/env bash +set -euo pipefail NAMESPACE="${NAMESPACE:-konveyor-tackle}"
13-14: Hard-coded namespace again bypasses$NAMESPACE
kubectl patch sub … -n konveyor-tackleignores your configurable variable.-kubectl patch sub konveyor-operator -n konveyor-tackle --type=merge -p "{\"spec\":{\"channel\":\"${SUBSCRIPTION_CHANNEL}\"}}" +kubectl patch sub konveyor-operator -n "${NAMESPACE}" \ + --type merge \ + -p "{\"spec\":{\"channel\":\"${SUBSCRIPTION_CHANNEL}\"}}"
17-18: Unquoted command substitution → word-splitting risk
If the install-plan list ever returns more than one token you’ll patch only the first. Quote it.-kubectl patch installplan $(kubectl get installplan -n "${NAMESPACE}" | egrep "$POSTUPGRADE_VERSION"|awk '{print $1}') -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}' +kubectl patch installplan "$(kubectl get installplan -n "${NAMESPACE}" \ + | grep -E "$POSTUPGRADE_VERSION" | awk '{print $1}')" \ + -n "${NAMESPACE}" --type merge \ + --patch '{"spec":{"approved":true}}'
26-34: Loop succeeds ifPREUPGRADE_UI_PODis empty
Add an early guard to avoid the false positive noted previously.+[[ -z "${PREUPGRADE_UI_POD:-}" ]] && { + echo "PREUPGRADE_UI_POD not set – aborting"; exit 1; +} while true; do.github/workflows/upgrade-ci.yml (6)
146-151: Password mismatch between steps causes flaky auth
Login test usespassword; later tests useDog8code. Consolidate.Would you confirm which password is canonical so we can update both invocations?
33-45: No checksum verification for downloaded binaries
operator-sdk,kubectl, andopmare installed with root privileges without integrity checks—supply-chain risk.Consider fetching the official SHA-256 files and running
sha256sum -cbeforeinstall.
15-18: Upgrade toactions/checkout@v4
v2 runners are deprecated and flagged by action-lint.- - name: Checkout tackle - uses: actions/checkout@v2 + - name: Checkout tackle + uses: actions/checkout@v4
90-92:run :breaks YAML parsing
The space before the colon makes the key invalid.- run : minikube addons enable ingress + run: minikube addons enable ingress
73-77: Stop echoing secrets into logs
Writing Quay creds viaecholeaks them in plain text. Useenv:instead.
118-130: Ingress wait loop: invalid flag, no timeout, unquoted vars
--templateis wrong, and the loop can spin forever. Previous fix still not applied.- external_ip=$(kubectl get ingress tackle --template="{{range.status.loadBalancer.ingress}}{{.ip}}{{end}}" -n konveyor-tackle);[[ -z $external_ip ]] && +end=$((SECONDS+300)) +while [[ -z "${external_ip:-}" && $SECONDS -lt $end ]]; do + external_ip=$(kubectl get ingress tackle -n konveyor-tackle \ + -o jsonpath='{.status.loadBalancer.ingress[0].ip}' 2>/dev/null) || true + [[ -z "${external_ip}" ]] && { echo "Waiting for ingress…"; sleep 10; } +done +[[ -z "${external_ip}" ]] && { echo "::error::Ingress not ready in 5 min"; exit 1; }
🧹 Nitpick comments (1)
hack/install-postupgrade-konveyor.sh (1)
42-44: Quote variables insidesleep/ arithmetic
Minor, but quoting avoids shellcheck SC2086 and future surprises.- sleep $INTERVAL - ((ELAPSED+=INTERVAL)) + sleep "${INTERVAL}" + ((ELAPSED+=INTERVAL))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/upgrade-ci.yml(1 hunks)hack/install-postupgrade-konveyor.sh(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
97-97: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
147-147: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
172-172: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
177-177: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
186-186: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
97-97: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
147-147: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
172-172: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
177-177: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
186-186: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
⏰ 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: e2e-main / e2e-ui-integration-tests
msajidmansoori12
force-pushed
the
test-upgradeci
branch
2 times, most recently
from
378af0f to
36d7892
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (9)
hack/install-postupgrade-konveyor.sh (4)
1-1: Add a proper she-bang andset -euo pipefail
Without a she-bang the script may run under/bin/sh; fail-fast flags avoid silent errors.
13-14: Hard-coded namespace breaks configurability
Use the already-defined$NAMESPACEinstead of literalkonveyor-tackle.
17-18: Unquoted command-substitution can split when multiple installplans exist
Quote the substitution to avoid passing only the first token tokubectl patch.
26-40: Loop exits immediately ifPREUPGRADE_UI_PODis empty
Insert an early guard that fails when the variable is unset to prevent false positives..github/workflows/upgrade-ci.yml (5)
15-16:actions/checkout@v2is deprecated – bump to@v4
Avoid “runner too old” warnings and get bug-fixes.
73-77: Stop echoing secrets to the build log
Use theenv:map instead of writing credentials to$GITHUB_ENV.
90-91:run :is invalid YAML
Remove the space before the colon or the step will fail to parse.
115-130: Ingress wait loop still has multiple issues
• Uses invalid--templateflag
• Variables unquoted (SC2086)
• No real timeout guard → can spin forever
• Ignores the resolvedexternal_ipin favour ofminikube ipPlease apply the earlier suggested refactor to make this robust.
147-152: Password value drifts between steps
login.test.tsusespassword, other tests useDog8code; choose one to prevent flaky auth failures.
🧹 Nitpick comments (1)
hack/install-postupgrade-konveyor.sh (1)
28-33: Make pod comparison deterministic when multiple UI pods exist
kubectl get pods … -o namemay return several lines, causing a never-true comparison or unexpected break.-UI_POD_AFTER=$(kubectl get pods -n "${NAMESPACE}" -l app.kubernetes.io/name=tackle-ui -o name) +UI_POD_AFTER=$(kubectl get pod -n "${NAMESPACE}" \ + -l app.kubernetes.io/name=tackle-ui \ + -o jsonpath='{.items[0].metadata.name}')This captures exactly one pod name for reliable string comparison.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/upgrade-ci.yml(1 hunks)hack/install-postupgrade-konveyor.sh(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
97-97: shellcheck reported issue in this script: SC2086:info:2:48: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
147-147: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
172-172: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
177-177: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
186-186: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
97-97: shellcheck reported issue in this script: SC2086:info:2:48: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
147-147: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
172-172: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
177-177: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
186-186: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
⏰ 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: e2e-main / e2e-ui-integration-tests
Signed-off-by: M Sajid Mansoori <[email protected]>
msajidmansoori12
force-pushed
the
test-upgradeci
branch
from
36d7892 to
0775550
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (13)
hack/install-postupgrade-konveyor.sh (4)
1-3: Add she-bang and fail-fast flagsWithout an explicit interpreter and
set -euo pipefail, the script may silently continue on errors and ShellCheck SC2148 is triggered.+#!/usr/bin/env bash +set -euo pipefail
12-14: Stop hard-coding the namespaceThe patch command ignores the configurable
$NAMESPACEand defeats the purpose of the variable.-kubectl patch sub konveyor-operator -n konveyor-tackle --type=merge -p "{\"spec\":{\"channel\":\"${SUBSCRIPTION_CHANNEL}\"}}" +kubectl patch sub konveyor-operator -n "${NAMESPACE}" --type=merge \ + -p "{\"spec\":{\"channel\":\"${SUBSCRIPTION_CHANNEL}\"}}"
17-18: Quote command substitution to prevent word-splittingUnquoted output from
kubectl get installplan …breaks if multiple rows are returned.-kubectl patch installplan $(kubectl get installplan -n "${NAMESPACE}" | egrep "$POSTUPGRADE_VERSION"|awk '{print $1}') -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}' +kubectl patch installplan "$(kubectl get installplan -n "${NAMESPACE}" \ + | grep -E "$POSTUPGRADE_VERSION" | awk '{print $1}')" \ + -n "${NAMESPACE}" --type merge --patch '{"spec":{"approved":true}}'
26-38: Guard against emptyPREUPGRADE_UI_PODto avoid false positivesIf the variable is unset/empty the loop exits immediately. Add an early check.
+[[ -z "${PREUPGRADE_UI_POD:-}" ]] && { echo "PREUPGRADE_UI_POD not set"; exit 1; } while true; do.github/workflows/upgrade-ci.yml (9)
14-17: Useactions/checkout@v4
v2images are deprecated and flagged by action-lint.- - name: Checkout tackle - uses: actions/checkout@v2 + - name: Checkout tackle + uses: actions/checkout@v4
26-35: Verify the operator-sdk download with a checksumInstalling a network-fetched binary as root without integrity verification is a supply-chain risk. Fetch the official SHA-256 and verify before
install.- curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/operator-sdk_linux_amd64 - sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdk + curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/operator-sdk_linux_amd64 + echo "c0d3... operator-sdk_linux_amd64" | sha256sum -c - + sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdk
36-45: Apply the same checksum guard forkubectlRepeat the integrity check pattern before installing the
kubectlbinary.
65-70: Add checksum verification for theopmCLI
opmis downloaded and installed with root privileges but no integrity check is performed.
73-77: Avoid echoing secrets into logs – useenv:insteadWriting credentials to
$GITHUB_ENVleaks them (even if masked). Pass them via theenv:map on the step instead.
90-92: Fix YAML syntax – remove extra space beforerun:
run :is invalid YAML and will fail the workflow.- - name: Enable ingress addon - run : minikube addons enable ingress + - name: Enable ingress addon + run: minikube addons enable ingress
97-100: Select a single CSV line & quote variables
head -n 2can still leave newlines. Usehead -n1and quote expansions.- konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version | head -n 2 | tr -d '[:space:]') + konveyor_version=$(kubectl get csv -n konveyor-tackle \ + -o=custom-columns=:spec.version | head -n1 | tr -d '[:space:]')
115-131: Ingress wait loop can spin forever & uses wrong flag
--templateis invalid, variables are unquoted, and no timeout is enforced. Suggested bounded loop:- while [[ -z $external_ip ]] + timeout=300; end=$((SECONDS+timeout)) + while [[ -z "${external_ip:-}" && $SECONDS -lt $end ]]; do ... - external_ip=$(kubectl get ingress tackle --template="{{range.status.loadBalancer.ingress}}{{.ip}}{{end}}" -n konveyor-tackle);[[ -z $external_ip ]] && + external_ip=$(kubectl get ingress tackle -n konveyor-tackle \ + -o jsonpath='{.status.loadBalancer.ingress[0].ip}' 2>/dev/null) || true ... - export endpoint=$(minikube ip); - echo "CYPRESS_tackleUrl=https://$endpoint" >>$GITHUB_ENV' + echo "CYPRESS_tackleUrl=https://$external_ip" >>"$GITHUB_ENV"
147-148: Password mismatch between pre- and post-upgrade testsFirst login test uses
password, later steps useDog8code; pick one to avoid flaky auth failures.
🧹 Nitpick comments (1)
hack/install-postupgrade-konveyor.sh (1)
46-47: Trim the unconditional 200 s sleepAfter the loop has already verified pod readiness, a fixed 200-second pause needlessly inflates CI time. Consider replacing with a second readiness check or removing it entirely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/upgrade-ci.yml(1 hunks)hack/install-postupgrade-konveyor.sh(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
97-97: shellcheck reported issue in this script: SC2086:info:2:48: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
147-147: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
172-172: shellcheck reported issue in this script: SC2086:info:2:48: Double quote to prevent globbing and word splitting
(shellcheck)
177-177: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
186-186: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: shellcheck reported issue in this script: SC2086:info:1:56: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2155:warning:5:8: Declare and assign separately to avoid masking return values
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:1:54: Double quote to prevent globbing and word splitting
(shellcheck)
74-74: shellcheck reported issue in this script: SC2086:info:2:54: Double quote to prevent globbing and word splitting
(shellcheck)
97-97: shellcheck reported issue in this script: SC2086:info:2:48: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
147-147: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
160-160: shellcheck reported issue in this script: SC2086:info:1:14: Double quote to prevent globbing and word splitting
(shellcheck)
172-172: shellcheck reported issue in this script: SC2086:info:2:48: Double quote to prevent globbing and word splitting
(shellcheck)
177-177: shellcheck reported issue in this script: SC2086:info:2:60: Double quote to prevent globbing and word splitting
(shellcheck)
186-186: shellcheck reported issue in this script: SC2140:warning:1:76: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
hack/install-postupgrade-konveyor.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 17-17: Quote this to prevent word splitting.
(SC2046)
⏰ 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: e2e-main / e2e-ui-integration-tests
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
.github/workflows/upgrade-ci.yml (9)
15-16: Updateactions/checkoutto v4
v2runners are deprecated and will soon be removed. Bump to v4 for compatibility and faster fetches.- - name: Checkout tackle - uses: actions/checkout@v2 + - name: Checkout tackle + uses: actions/checkout@v4
26-35: Add checksum verification foroperator-sdkdownloadInstalling a network-fetched binary with root privileges without integrity checks is a supply-chain risk. Fetch the SHA-256 file and verify before
install.- curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/operator-sdk_linux_amd64 - sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdk + curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/operator-sdk_linux_amd64 + curl -LO https://github.com/operator-framework/operator-sdk/releases/download/v1.35.0/checksums.txt + grep 'operator-sdk_linux_amd64$' checksums.txt | sha256sum -c - + sudo install -o root -g root -m 0755 operator-sdk_linux_amd64 /usr/local/bin/operator-sdk
43-44: Verify thekubectlbinary before installSame integrity concern applies here—download the checksum and abort on mismatch.
58-70: Add checksum verification for theopmCLI
opmgets installed with root perms; verify the hash first to close the supply-chain hole.
73-77: Avoid echoing secrets into logsWrite secrets via the
env:map instead ofecho >> $GITHUB_ENVto keep logs clean and reduce accidental leakage.- name: Set Quay credentials env: QUAY_USERNAME: ${{ secrets.QUAY_USERNAME }} QUAY_PASSWORD: ${{ secrets.QUAY_PASSWORD }} run: echo "Quay creds injected via environment"
90-91: Fix invalid YAML keyThe extra space before
:makes the workflow invalid.- - name: Enable ingress addon - run : minikube addons enable ingress + - name: Enable ingress addon + run: minikube addons enable ingress
97-100: Select only the first CSV version line
head -n 2still keeps a newline in many cases. Trim to a single line.- konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version | head -n 2 | tr -d '[:space:]') + konveyor_version=$(kubectl get csv -n konveyor-tackle -o=custom-columns=:spec.version | head -n 1 | tr -d '[:space:]')
146-147: Use a single password value across testsPre-upgrade login uses
password, whereas the rest of the suite usesDog8code, leading to flaky auth failures.- run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" --env user="admin",pass="password",tackleUrl=${{ env.CYPRESS_tackleUrl }} + run: npx cypress run --spec "cypress/e2e/tests/login.test.ts" --env user="admin",pass="Dog8code",tackleUrl=${{ env.CYPRESS_tackleUrl }}
115-131: Ingress-wait loop can spin forever & uses invalid flags
--templateis not a valid kubectl flag.- No timeout → endless loop.
- You ignore the resolved
$external_ipand fall back tominikube ip.- external_ip=$(kubectl get ingress tackle --template="{{range.status.loadBalancer.ingress}}{{.ip}}{{end}}" -n konveyor-tackle);[[ -z $external_ip ]] && + end=$((SECONDS+300)) # 5-min timeout + external_ip="" + while [[ -z "${external_ip}" && $SECONDS -lt $end ]]; do + echo "Waiting for ingress…" + external_ip=$(kubectl get ingress tackle -n konveyor-tackle \ + -o jsonpath='{.status.loadBalancer.ingress[0].ip}' 2>/dev/null) || true + sleep 10 + done + [[ -z "${external_ip}" ]] && { echo "::error::Ingress not ready"; exit 1; } echo $external_ip; - export endpoint=$(minikube ip); + endpoint="$external_ip"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/upgrade-ci.yml(1 hunks)
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/upgrade-ci.yml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/upgrade-ci.yml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[warning] 91-91: too many spaces before colon
(colons)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
⏰ 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: e2e-main / e2e-ui-integration-tests
| OC_VERSION=$(curl -s https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/release.txt | grep 'Name:' | awk '{print $2}') | ||
| curl -LO "https://mirror.openshift.com/pub/openshift-v4/clients/ocp/${OC_VERSION}/openshift-client-linux.tar.gz" | ||
| tar -xzf openshift-client-linux.tar.gz oc | ||
| sudo mv oc /usr/local/bin/oc | ||
| oc version --client |
There was a problem hiding this comment.
oc binary also lacks checksum verification
You extract and install oc with root privileges but never validate the archive.
Add SHA-256 (or GPG) verification to avoid executing a tampered binary.
🤖 Prompt for AI Agents
In .github/workflows/upgrade-ci.yml around lines 52 to 56, the script downloads
and installs the 'oc' binary without verifying its integrity. To fix this, add a
step to download the corresponding checksum file or signature, then verify the
downloaded archive's SHA-256 checksum or GPG signature before extracting and
installing it. This ensures the binary is authentic and untampered before
execution.
Signed-off-by: M Sajid Mansoori <[email protected]>
msajidmansoori12
force-pushed
the
test-upgradeci
branch
from
8509889 to
ab6e2be
Compare
2 participants
Summary by CodeRabbit
New Features
Chores