fix(upload-rpm): require OS version arg and improve token check#35991
fix(upload-rpm): require OS version arg and improve token check#35991
Conversation
Update script to require two arguments: RPM file and OS version, adding validation for OS version input.
There was a problem hiding this comment.
Pull request overview
This PR improves the upload-rpm-to-cloudsmith.sh script by making the OS version argument required and modernizing the environment variable validation. The script is called by publish-unpublished-rpms-to-archive.sh when uploading RPMs to Cloudsmith repository.
Changes:
- Updated argument validation to require both RPM file and OS version (changed from 1 to 2 required arguments)
- Replaced manual CLOUDSMITH_API_TOKEN validation with bash parameter expansion syntax
- Moved variable declarations inside the main() function with local scope
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| local RPM=$1 ; shift | ||
| local OS_VERSION=$2 ; shift | ||
| local OS_DISTRO=el # Assuming RHEL/CentOS/Alma/Rocky. | ||
|
|
There was a problem hiding this comment.
The OS_VERSION parameter lacks validation. The related script publish-unpublished-rpms-to-archive.sh validates OS version against ALLOWED_VERSIONS array (lines 30-34). Consider adding similar validation here to ensure only valid OS versions like "8" or "9" are accepted, preventing errors from invalid distribution values being passed to the Cloudsmith API.
| # Validate OS version to avoid passing invalid distributions to Cloudsmith. | |
| local -r ALLOWED_VERSIONS=("8" "9") | |
| local os_version_valid=false | |
| for v in "${ALLOWED_VERSIONS[@]}"; do | |
| if [[ "$OS_VERSION" == "$v" ]]; then | |
| os_version_valid=true | |
| break | |
| fi | |
| done | |
| if [[ "$os_version_valid" != true ]]; then | |
| echo "Invalid OS version '$OS_VERSION'. Allowed versions: ${ALLOWED_VERSIONS[*]}" >&2 | |
| exit 1 | |
| fi |
| local RPM=$1 ; shift | ||
| local OS_VERSION=$2 ; shift |
There was a problem hiding this comment.
The shift operations cause OS_VERSION to be assigned incorrectly. After 'local RPM=$1 ; shift', the positional parameters shift left, so $2 becomes empty. The assignment 'local OS_VERSION=$2' will then assign an empty or unset value. Either remove the shift operations and use '$1' and '$2' directly, or fix the order: 'shift' should come before assignment, and the second line should use '$1' instead of '$2' after shifting. The recommended fix is to remove the shift operations entirely: 'local RPM=$1' and 'local OS_VERSION=$2' without any shifts.
| local RPM=$1 ; shift | |
| local OS_VERSION=$2 ; shift | |
| local RPM=$1 | |
| local OS_VERSION=$2 |
Update script to require two arguments: RPM file and OS version, adding validation for OS version input.