-
Notifications
You must be signed in to change notification settings - Fork 32
chore: enable pre-releases for subscriptions #556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Roming22 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
/retest |
1 similar comment
/retest |
3f3f400
to
73bdd47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question
This script should be useful to developers and QE. It might be useful to the occasional customer that would want to validate an upcoming fix in an ephemeral environment.
|
Is this still relevant? |
@lcarva While the specific versions might not be, I believe the process is. |
/lgtm |
WalkthroughA new Bash script, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant pre-release.sh
participant OpenShift Cluster
participant Helm Values File
User->>pre-release.sh: Run script with product options
pre-release.sh->>pre-release.sh: Parse arguments
pre-release.sh->>pre-release.sh: Init temp directory
loop For each product
pre-release.sh->>OpenShift Cluster: Apply product-specific configuration
pre-release.sh->>Helm Values File: Update subscription values
end
pre-release.sh->>User: Output progress and completion message
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
hack/pre-release.sh (1)
64-68
:DEBUG
variable is set but never used
parse_args
exports$DEBUG
, yet no other code reads it. Either hook it into verbose tooling or drop the dead code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hack/pre-release.sh
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Roming22
PR: redhat-appstudio/rhtap-cli#1000
File: installer/charts/tssc-gitops/Chart.yaml:6-7
Timestamp: 2025-07-07T15:41:17.668Z
Learning: For the rhtap-cli repository, chart version bumps in Helm charts are handled at release time, not when appVersion is updated during development. The team follows a workflow where appVersion can be updated in PRs but the chart version field is only incremented during the release process.
🪛 Gitleaks (8.27.2)
hack/pre-release.sh
174-174: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
init() { | ||
SHARED_DIR="$(mktemp -d)" | ||
cd "$SHARED_DIR" | ||
export SHARED_DIR | ||
trap cleanup EXIT | ||
} | ||
|
||
cleanup() { | ||
rm -rf "$SHARED_DIR" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Possible rm -rf
of current $PWD
cleanup
runs while the process is still inside $SHARED_DIR
; removing the directory you are cd
-ed into can fail or leave you in a deleted cwd. cd /
(or back to $PROJECT_DIR
) before deletion, or use trap 'cd …; cleanup'
.
🤖 Prompt for AI Agents
In hack/pre-release.sh around lines 83 to 92, the cleanup function removes the
directory stored in $SHARED_DIR while the current working directory is still
inside it, which can cause errors or leave the shell in a deleted directory. To
fix this, modify the trap to first change directory to a safe location like / or
$PROJECT_DIR before calling cleanup, or update the cleanup function to cd out of
$SHARED_DIR before running rm -rf on it.
configure_rhdh(){ | ||
RHDH_INSTALL_SCRIPT="https://raw.githubusercontent.com/redhat-developer/rhdh-operator/main/.rhdh/scripts/install-rhdh-catalog-source.sh" | ||
curl -sSLO $RHDH_INSTALL_SCRIPT | ||
chmod +x install-rhdh-catalog-source.sh | ||
|
||
./install-rhdh-catalog-source.sh --latest --install-operator rhdh | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Blind execution of remote script – add integrity / safety checks
curl … | bash
-style patterns are dangerous. At minimum, verify the SHA256 of the downloaded script or pin to a commit hash so supply-chain attacks don’t silently own the cluster.
🤖 Prompt for AI Agents
In hack/pre-release.sh around lines 119 to 125, the script blindly downloads and
executes a remote install script without verifying its integrity, which is a
security risk. To fix this, modify the script to download the install script
first, then verify its SHA256 checksum against a known good value before making
it executable and running it. Alternatively, pin the URL to a specific commit
hash to ensure the script content is fixed and trusted. This prevents
supply-chain attacks by ensuring only verified code is executed.
DOCKERCONFIGJSON=$( | ||
oc get secrets \ | ||
-n openshift-config pull-secret \ | ||
-o jsonpath='{.data.\.dockerconfigjson}' \ | ||
| base64 -d | ||
) | ||
|
||
DOCKERCONFIGJSON=$( | ||
echo "$DOCKERCONFIGJSON" \ | ||
| jq -c '.auths."registry.stage.redhat.io" = { | ||
"auth": "MTEwMDkxMDN8cmhhZHM6ZXlKaGJHY2lPaUpTVXpVeE1pSjkuZXlKemRXSWlPaUk0T0RsaE1XWTJPR0ZsWVdVMFptTTNZVE13WWpBNVlXUmpOR1JsWVRneVlpSjkuSXZrVDZqQ001ZU9wUnJQeDVzOVk4NFlaY1NfcWpDX0RpZ0s1bmwtd3NCQ0Y4cGFVdG13b25RbE5GTm9sYXBQTGZUeFFUaWxUUDluYllIRGV4TGxGWnZUWGNRWmg4YllqMmFGOUhZaHR2bFpCbmxpS0tyakNVN24tUE5rb09DY296QmdfN21fZFU4Z1lIUGt4Q3pISC1obG53Y2dneklwMG5jV0xQTDhFTkM4R0lJV2pQZ0t4bm1vTDQ2TUNUUDl6amhNNG5LYUlnT1RuNzdCZGVqRHY4WGRoZVNKRHFJUEEyTjA0TnUwMWhaSE95U3NpX3NCdjR2OThZYllOdFhNLUtHSXdtYWtMcWNZTzFONFUwa0RJaUc5OWdMZ2V0ZGVYT1ZPWkl3ajVpRGFkZXd2cnoxdFlucGZvZ1JhT2pWU3pDcnVBTDkyM1IzdEV2M3FvMjNPWXdXS1hGZlBTMU9YbzBwNFE3YldKRUFmMnEtM1gxM0lOQ19UTmJ3MGZCRHdMS05XN1k2TEVoQmJZVFNZd1BET20tTTJSVXpMQzdKYjV1c2pOZjYxM01XZWt5RmVRVVlJSTNUc0xLeUExQ0s1cE9SdXNFS256MERjZ1lya0V0ZE9ENkVtYjBmdUdNbElhN0JiX1UwZjVxUmszdFFXbWdTejRkQ1VDMXhlM0NDT25kVGU4bDA2RUVzSGVVU2I5NUd0bWtudUVkME03eERZaURjUURRVXlQblhmRFl5WF9xcF9BZ2l1M19meExLcl9LOGplYzJlcHlIUFBFN3FhR2hHeEViVVlUc0dsZ1B0Z0ExUFRlS3h1dklyM0d6ajg4SF85bGVYU1lwR0d5Ujl6dXh1TXVNaFZFS19Jc25OYTl3bHBaTG1RdW0xM1lRelN5ajMtMFVWUEVma3M=", | ||
"email": "" | ||
}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded registry credentials leak – remove immediately
The base64 string injected into .auth
is a real credential (Gitleaks flagged it). Committing secrets makes the token irretrievable and opens the registry to anyone who reads the repo. Parameterise or source the value from a secure location (env-var, oc registry login
, or a vault) and make the script fail fast if it is missing.
- | jq -c '.auths."registry.stage.redhat.io" = {
- "auth": "MTEwMDkxMDN8cmhhZHM6ZXlKaGJHY2l...",
- "email": ""
- }'
+ | jq -c --arg auth "${RHTAS_REGISTRY_AUTH:?Unset RHTAS_REGISTRY_AUTH}" \
+ '.auths."registry.stage.redhat.io" = {auth:$auth, email:""}'
📝 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.
DOCKERCONFIGJSON=$( | |
oc get secrets \ | |
-n openshift-config pull-secret \ | |
-o jsonpath='{.data.\.dockerconfigjson}' \ | |
| base64 -d | |
) | |
DOCKERCONFIGJSON=$( | |
echo "$DOCKERCONFIGJSON" \ | |
| jq -c '.auths."registry.stage.redhat.io" = { | |
"auth": "MTEwMDkxMDN8cmhhZHM6ZXlKaGJHY2lPaUpTVXpVeE1pSjkuZXlKemRXSWlPaUk0T0RsaE1XWTJPR0ZsWVdVMFptTTNZVE13WWpBNVlXUmpOR1JsWVRneVlpSjkuSXZrVDZqQ001ZU9wUnJQeDVzOVk4NFlaY1NfcWpDX0RpZ0s1bmwtd3NCQ0Y4cGFVdG13b25RbE5GTm9sYXBQTGZUeFFUaWxUUDluYllIRGV4TGxGWnZUWGNRWmg4YllqMmFGOUhZaHR2bFpCbmxpS0tyakNVN24tUE5rb09DY296QmdfN21fZFU4Z1lIUGt4Q3pISC1obG53Y2dneklwMG5jV0xQTDhFTkM4R0lJV2pQZ0t4bm1vTDQ2TUNUUDl6amhNNG5LYUlnT1RuNzdCZGVqRHY4WGRoZVNKRHFJUEEyTjA0TnUwMWhaSE95U3NpX3NCdjR2OThZYllOdFhNLUtHSXdtYWtMcWNZTzFONFUwa0RJaUc5OWdMZ2V0ZGVYT1ZPWkl3ajVpRGFkZXd2cnoxdFlucGZvZ1JhT2pWU3pDcnVBTDkyM1IzdEV2M3FvMjNPWXdXS1hGZlBTMU9YbzBwNFE3YldKRUFmMnEtM1gxM0lOQ19UTmJ3MGZCRHdMS05XN1k2TEVoQmJZVFNZd1BET20tTTJSVXpMQzdKYjV1c2pOZjYxM01XZWt5RmVRVVlJSTNUc0xLeUExQ0s1cE9SdXNFS256MERjZ1lya0V0ZE9ENkVtYjBmdUdNbElhN0JiX1UwZjVxUmszdFFXbWdTejRkQ1VDMXhlM0NDT25kVGU4bDA2RUVzSGVVU2I5NUd0bWtudUVkME03eERZaURjUURRVXlQblhmRFl5WF9xcF9BZ2l1M19meExLcl9LOGplYzJlcHlIUFBFN3FhR2hHeEViVVlUc0dsZ1B0Z0ExUFRlS3h1dklyM0d6ajg4SF85bGVYU1lwR0d5Ujl6dXh1TXVNaFZFS19Jc25OYTl3bHBaTG1RdW0xM1lRelN5ajMtMFVWUEVma3M=", | |
"email": "" | |
}' | |
DOCKERCONFIGJSON=$( | |
oc get secrets \ | |
-n openshift-config pull-secret \ | |
-o jsonpath='{.data.\.dockerconfigjson}' \ | |
| base64 -d | |
) | |
DOCKERCONFIGJSON=$( | |
echo "$DOCKERCONFIGJSON" \ | |
| jq -c --arg auth "${RHTAS_REGISTRY_AUTH:?Unset RHTAS_REGISTRY_AUTH}" \ | |
'.auths."registry.stage.redhat.io" = {auth:$auth, email:""}' | |
) |
🧰 Tools
🪛 Gitleaks (8.27.2)
174-174: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In hack/pre-release.sh around lines 164 to 176, the script contains a hard-coded
base64-encoded registry credential string inside the jq command, which leaks
sensitive information. Remove this hard-coded credential and instead modify the
script to source the registry authentication token securely from an environment
variable, an external secret manager, or by running a secure login command like
`oc registry login`. Add error handling to make the script fail immediately if
the credential is not available to prevent unauthorized access.
main() { | ||
parse_args "$@" | ||
init | ||
for PRODUCT in $(echo "${PRODUCT_LIST[@]}" | tr " " "\n" | sort); do | ||
h1 "Configuring $PRODUCT" | ||
"configure_$PRODUCT" | ||
configure_subscription | ||
echo | ||
done | ||
echo "Done" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
No product selected results in silent no-op
If the user forgets --product
, the script exits successfully without doing anything. Fail fast to avoid false confidence:
main() {
parse_args "$@"
+ if [[ ${#PRODUCT_LIST[@]} -eq 0 ]]; then
+ echo "[ERROR] No product specified." >&2
+ usage
+ exit 1
+ fi
init
📝 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.
main() { | |
parse_args "$@" | |
init | |
for PRODUCT in $(echo "${PRODUCT_LIST[@]}" | tr " " "\n" | sort); do | |
h1 "Configuring $PRODUCT" | |
"configure_$PRODUCT" | |
configure_subscription | |
echo | |
done | |
echo "Done" | |
main() { | |
parse_args "$@" | |
if [[ ${#PRODUCT_LIST[@]} -eq 0 ]]; then | |
echo "[ERROR] No product specified." >&2 | |
usage | |
exit 1 | |
fi | |
init | |
for PRODUCT in $(echo "${PRODUCT_LIST[@]}" | tr " " "\n" | sort); do | |
h1 "Configuring $PRODUCT" | |
"configure_$PRODUCT" | |
configure_subscription | |
echo | |
done | |
echo "Done" | |
} |
🤖 Prompt for AI Agents
In hack/pre-release.sh around lines 207 to 216, the script silently does nothing
and exits successfully if no product is selected via --product. Add a check
after parsing arguments to verify that PRODUCT_LIST is not empty; if it is,
print an error message and exit with a non-zero status to fail fast and avoid
false confidence.
84b2860
into
redhat-appstudio:main
This script should be useful to developers and QE. It might be useful to the occasional customer that would want to validate an upcoming fix in an ephemeral environment.
Summary by CodeRabbit