feat(ci): allow for multiple artifacthub-values files#1419
Conversation
this allows to extract images from mutually exclusive options
WalkthroughThe pull request updates the image extraction script by enhancing its functions. The Changes
Sequence Diagram(s)sequenceDiagram
participant S as Script
participant GI as getImages
participant UT as updateChartYaml
participant A as ArtifactHub YAML Files
participant C as Chart.yaml
S->>GI: Call getImages(valuesFile)
GI->>GI: Check if existingValuesDir is a directory
GI->>S: Return image data (allImages)
S->>UT: Call updateChartYaml(allImages)
UT->>A: Loop through artifacthub-values YAML files
A-->>UT: Return values data
UT->>C: Update annotations with aggregated images data
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/scripts/extract-artifacthub-images.sh (1)
75-77: 🛠️ Refactor suggestionExistence Check for Artifacthub-Values File in Main Block:
The script verifies the existence of a single file:if ! [[ -f "$1/ci/artifacthub-values.yaml" ]]; thenGiven the PR’s aim to support multiple artifacthub-values files (using a glob pattern later in the script), consider updating this check to either:
- Allow for multiple matching files, or
- Clarify in documentation that one file with the name
artifacthub-values.yamlis mandatory even when additional files may be processed.
🧹 Nitpick comments (5)
.github/scripts/extract-artifacthub-images.sh (5)
19-25: Conditional Handling of Existing Directory vs. Helm Chart Generation:
WithingetImages, the conditional block checks if an existing directory (derived from parameter 2) is present. If it exists, the script copies it; otherwise, it falls back to generating a helm release YAML (usingtemplateLocalHelmChartandsplitYamlIntoDir). This approach effectively supports both scenarios.One suggestion: consider adding a brief comment or error message for cases where
$2is provided butexistingValuesDirisn’t a directory, to improve clarity for future debugging.
30-33: Grep Pipeline for Image Extraction:
Inside the subshell, the script changes into the temporary helm release directory and uses a grep pipeline to extract lines containing an image reference while filtering out lines with"artifacthub-ignore". The added error handler|| { if [[ "$?" == 2 ]]; then exit 2; fi; }aims to catch a specific
greperror. However, this construct can be brittle since it relies on the exit status of the pipeline; consider capturing and checking the exit status explicitly with a comment clarifying the intent.
40-51: Iterating Over Multiple Artifacthub-Values Files inupdateChartYaml:
The function now creates a temporary file (all_images.yaml) and iterates over files matching the glob"$chart/ci/artifacthub-values"*.yamlto process multiple values files. This is a step toward more flexible configuration. A couple of points to verify:
- Confirm that the shell’s globbing behavior (or options like
nullglob, if set) won’t cause unexpected iteration over the literal pattern when no files match.- Ensure that users are aware of the naming convention (filenames starting with
artifacthub-values) required for the new functionality.
51-60: Consolidation Pipeline for Image Data:
The output fromgetImagesis piped through a series of commands—awk,tr,sed,sort,uniq,column,jq, andyq—to aggregate image data into a single file. Please verify that:
- The
awkcommand correctly parses the expected fields from the output ofgetImages.- The successive transformations (especially filtering and formatting) match the intended image structure for annotations.
Adding inline comments to break down this pipeline might help improve maintainability and ease troubleshooting in the future.
85-95: Processing Multiple Charts in the Absence of Direct Parameters:
In theelsebranch, the script iterates over all charts in thecharts/directory and processes those with anartifacthub-values.yamlfile. Given the support for multiple values files, please confirm that this branch and its file existence check remain consistent with the new multi-file handling logic across all charts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/scripts/extract-artifacthub-images.sh(3 hunks)
🔇 Additional comments (3)
.github/scripts/extract-artifacthub-images.sh (3)
13-17: Parameter Handling and Computation ofexistingValuesDiringetImages:
The function now extracts bothexistingDir(from parameter 2) andvaluesFile(from parameter 3) and computesexistingValuesDirasexistingValuesDir="$2/$(basename --suffix=.yaml "$valuesFile")"Please ensure that:
valuesFilealways ends with a.yamlsuffix so thatbasenamestrips it correctly.- The case when
$2is empty is handled gracefully downstream.
62-63: UpdatingChart.yamlwith Aggregated Image Annotations:
The finalyqcommand updates theChart.yamlfile by injecting a new annotation (artifacthub.io/images) with the contents ofallImages. Double-check that thesub("\\s*$"; "")expression properly trims trailing whitespace as intended and that the raw file content is correctly interpreted byyq.
79-81: Validation of Provided Artifacthub-Values Directory:
The checkif [[ -v 2 && ! -d "$2/artifacthub-values" ]]; thenensures that if a second parameter is provided, it must include an
artifacthub-valuessubdirectory. This validation appears sound; just ensure that the expected directory structure is well documented for users.
2 participants
this allows to extract images from mutually exclusive options
Summary by CodeRabbit