-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Update MariaDB RPM links #14
Conversation
Issue aws#409 , if available: Description of changes: Update MariaDB RPM links By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
WalkthroughThe pull request updates the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
🧹 Nitpick comments (1)
docker/script/bootstrap.sh (1)
56-62
: Consider using variables for version and distributionThe MariaDB version and Fedora version are hardcoded in multiple URLs. Consider extracting these as variables for easier maintenance.
+ MARIADB_VERSION="11.4.2" + FEDORA_VERSION="38" + BASE_URL="https://mirror.mariadb.org/yum/${MARIADB_VERSION}/fedora${FEDORA_VERSION}" if [[ $(uname -p) == "aarch64" ]]; then - wget https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-common-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm + wget "${BASE_URL}-aarch64/rpms/MariaDB-common-${MARIADB_VERSION}-1.fc${FEDORA_VERSION}.$(uname -p).rpm" -P /mariadb_rpm🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 56-56: Quote this to prevent word splitting.
(SC2046)
[warning] 57-57: Quote this to prevent word splitting.
(SC2046)
[warning] 58-58: Quote this to prevent word splitting.
(SC2046)
[warning] 60-60: Quote this to prevent word splitting.
(SC2046)
[warning] 61-61: Quote this to prevent word splitting.
(SC2046)
[warning] 62-62: Quote this to prevent word splitting.
(SC2046)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker/script/bootstrap.sh
(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
docker/script/bootstrap.sh
[warning] 56-56: Quote this to prevent word splitting.
(SC2046)
[warning] 57-57: Quote this to prevent word splitting.
(SC2046)
[warning] 58-58: Quote this to prevent word splitting.
(SC2046)
[warning] 60-60: Quote this to prevent word splitting.
(SC2046)
[warning] 61-61: Quote this to prevent word splitting.
(SC2046)
[warning] 62-62: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (1)
docker/script/bootstrap.sh (1)
56-62
: Verify compatibility with Amazon Linux 2
The MariaDB packages are being sourced from Fedora 38 repositories. While the version upgrade from 11.1.2 to 11.4.2 is good for security, we should verify compatibility with Amazon Linux 2 base image.
Also, the command can be improved to handle word splitting safely:
- wget https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-common-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm
+ wget "https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-common-11.4.2-1.fc38.$(uname -p).rpm" -P /mariadb_rpm
Run this script to verify package compatibility:
✅ Verification successful
Using Fedora 38 MariaDB packages is compatible with Amazon Linux 2023
The review comment's concern about Amazon Linux 2 compatibility is no longer relevant as the Dockerfile shows the base image has been updated to amazonlinux:2023
. Fedora 38 packages are compatible with Amazon Linux 2023 as both use modern versions of glibc and other system libraries. The MariaDB RPM installation is part of a well-structured bootstrap process that:
- Uses the appropriate package manager (dnf) for AL2023
- Installs necessary dependencies before MariaDB
- Handles architecture-specific packages correctly (aarch64/amd64)
The only suggested improvement is to quote the wget URLs to prevent potential word splitting:
- wget https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-common-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm
+ wget "https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-common-11.4.2-1.fc38.$(uname -p).rpm" -P /mariadb_rpm
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the MariaDB packages have any known compatibility issues with AL2
# and verify if there are any reported issues in the repository
# Check AL2 base image version
cat /etc/system-release
# List current MariaDB packages
rpm -qa | grep -i mariadb
# Check for any conflicts
rpm -qpR /mariadb_rpm/MariaDB-*.rpm
Length of output: 334
Script:
#!/bin/bash
# Let's examine the Dockerfile and bootstrap script to understand the base image and package management system
# Check Dockerfile for base image
fd Dockerfile --type f --exec cat {}
# Check bootstrap script for package installation commands
cat docker/script/bootstrap.sh
# Check if there are any package management related files
fd "yum|dnf|apt" --type f --exec head -n 20 {}
Length of output: 6439
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 56-56: Quote this to prevent word splitting.
(SC2046)
[warning] 57-57: Quote this to prevent word splitting.
(SC2046)
[warning] 58-58: Quote this to prevent word splitting.
(SC2046)
[warning] 60-60: Quote this to prevent word splitting.
(SC2046)
[warning] 61-61: Quote this to prevent word splitting.
(SC2046)
[warning] 62-62: Quote this to prevent word splitting.
(SC2046)
wget https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-common-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
wget https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-shared-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
wget https://mirror.mariadb.org/yum/11.4/fedora38-aarch64/rpms/MariaDB-devel-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
else | ||
wget https://mirror.mariadb.org/yum/11.1/fedora38-amd64/rpms/MariaDB-common-11.1.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
wget https://mirror.mariadb.org/yum/11.1/fedora38-amd64/rpms/MariaDB-shared-11.1.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
wget https://mirror.mariadb.org/yum/11.1/fedora38-amd64/rpms/MariaDB-devel-11.1.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
wget https://mirror.mariadb.org/yum/11.4/fedora38-amd64/rpms/MariaDB-common-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
wget https://mirror.mariadb.org/yum/11.4/fedora38-amd64/rpms/MariaDB-shared-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm | ||
wget https://mirror.mariadb.org/yum/11.4/fedora38-amd64/rpms/MariaDB-devel-11.4.2-1.fc38.$(uname -p).rpm -P /mariadb_rpm |
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.
💡 Codebase verification
Update MariaDB version to 11.4.3
The code currently uses MariaDB 11.4.2, but version 11.4.3 is available in the same repository. For security best practices, it's recommended to use the latest stable version within the same major release.
- Update the version number from 11.4.2 to 11.4.3 in all wget commands in docker/script/bootstrap.sh
🔗 Analysis chain
Verify MariaDB version security status
While upgrading to 11.4.2 is good, let's verify it's the latest stable version and check for any known security vulnerabilities.
Let me try a different approach to verify the MariaDB version information.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest MariaDB version and security advisories
# Get latest version from MariaDB repository
curl -s "https://downloads.mariadb.org/rest-api/mariadb/" | jq -r '.[] | select(.release_status=="stable") | .release_id' | sort -V | tail -n1
# Check for CVEs
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "mariadb-java-client") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 571
Script:
#!/bin/bash
# Check MariaDB version availability and related files
# First, verify if this version exists in the mirror
curl -s -I "https://mirror.mariadb.org/yum/11.4/fedora38-amd64/rpms/MariaDB-common-11.4.2-1.fc38.x86_64.rpm" | head -n 1
# Check if there's a newer version in the same series
curl -s "https://mirror.mariadb.org/yum/11.4/fedora38-amd64/rpms/" | grep -o 'MariaDB-common-11.4.[0-9]*-[0-9].fc38' | sort -V | tail -n 1
# Look for any version-related comments or documentation in the repository
rg -i "mariadb.*version|mariadb.*security" --type sh
Length of output: 363
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 56-56: Quote this to prevent word splitting.
(SC2046)
[warning] 57-57: Quote this to prevent word splitting.
(SC2046)
[warning] 58-58: Quote this to prevent word splitting.
(SC2046)
[warning] 60-60: Quote this to prevent word splitting.
(SC2046)
[warning] 61-61: Quote this to prevent word splitting.
(SC2046)
[warning] 62-62: Quote this to prevent word splitting.
(SC2046)
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.
lgtm. Thanks!
As an alternative to #13, this PR just cherry-picks the single commit we need from the OSS upstream.
Summary by CodeRabbit