Files that changed from the base of the PR and between 0fee3db and a4ecef6.

• internal/manifest/charts.go (1 hunks)
• internal/manifest/manifest.go (1 hunks)

internal/manifest/charts.go (1)

50-56: Implementation looks good, but consider edge cases.

The solution effectively handles the OCI Artifacts limitation with plus signs by attempting a fallback lookup with underscore-to-plus conversion. However, there are some edge cases to consider:

1. The current implementation might incorrectly convert legitimate underscores to plus signs
2. As noted in the PR description, there could be ambiguity when a tag contains both underscore and plus sign (converted to underscore by Helm)

Let's verify the potential impact of these edge cases:

Consider adding a more specific validation:

 if strings.Contains(reference, "_") {
+    // Only replace underscores that are likely to be plus sign replacements
+    // e.g., in version patterns like "1.2.3_4" or "v1.2.3_beta.4"
+    if strings.Contains(reference, ".") && strings.Count(reference, "_") == 1 {
         reference = strings.ReplaceAll(reference, "_", "+")
+    }
 }

✅ Verification successful

The underscore-to-plus conversion is consistent with existing codebase patterns

The search results reveal that this conversion pattern is already established in the codebase:

• In internal/manifest/manifest.go, there's a similar conversion: target = strings.ReplaceAll(target, "_", "+")
• No instances of version strings containing underscores were found in the yaml files
• The implementation follows the same error-handling pattern as other index.Get calls

The original review's concerns about edge cases are less critical given:

1. The conversion is consistent with existing codebase behavior
2. The conversion only happens after an initial lookup fails, preserving any legitimate underscore usage
3. No conflicting version patterns were found in the codebase

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing chart versions containing underscores or plus signs
# to identify potential conflicts

# Look for version patterns in yaml files
rg -g '*.yaml' '(?:version|tag):\s*["\']?[^"'\s]*[_+][^"'\s]*["\']?' -A 2

# Look for direct version string assignments in go files
rg 'Version\s*=\s*["\']?[^"'\s]*[_+][^"'\s]*["\']?' -A 2

Length of output: 93

---

Script:

#!/bin/bash
# Let's try a different approach to search for version patterns

# Search for version patterns in yaml files
rg -g '*.yaml' '(?:version|tag):\s*["\']?[^"'\s]*[_+][^"'\s]*["\']?'

# Search for direct version string assignments in go files
rg 'Version\s*=\s*["\']?[^"'\s]*[_+][^"'\s]*["\']?'

# Look for chart version handling in the codebase
ast-grep --pattern 'chartVer, err = index.Get($_, $_)'

# Search for other underscore to plus conversions
rg 'ReplaceAll.*"_".*"+"'

Length of output: 94

---

Script:

#!/bin/bash

# Look for version patterns in yaml files
rg -g "*.yaml" "version:|tag:" -A 1

# Look for chart version handling in the codebase
rg "index\.Get\(" -A 2

# Search for underscore to plus conversions
rg "ReplaceAll.*_.*\+" -A 2

# Look for version-related error handling
rg "chartVer.*err.*index\.Get" -A 3

Length of output: 1825

internal/manifest/manifest.go (1)

201-212: Verify the impact on existing underscore usage.

We should verify that this change doesn't affect charts that legitimately use underscores in their version tags.

Let's check for any existing usage of underscores in chart versions:

✅ Verification successful

The underscore replacement is a safe fallback mechanism

The code changes are safe because:

1. The underscore replacement only occurs as a fallback mechanism when the initial chart lookup fails
2. The codebase shows no existing usage of underscores in version tags
3. The replacement is specifically designed to handle cases where + was encoded as _ in URLs, which is a common URL encoding scenario

The search results confirm there are no legitimate uses of underscores in chart versions that could be affected by this change. The only underscore handling in the codebase is this specific fallback mechanism in internal/manifest/charts.go and internal/manifest/manifest.go.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for chart versions containing underscores to assess impact
# Expected: Should find any legitimate uses of underscores in version tags

# Look for version fields in Chart.yaml files
echo "Searching for Chart.yaml version fields with underscores..."
fd Chart.yaml --exec rg -l 'version:\s+[^#\n]*_'

# Look for version references in values.yaml files
echo "Searching for version references with underscores in values.yaml..."
fd values.yaml --exec rg -l 'version:\s+[^#\n]*_'

Length of output: 376

---

Script:

#!/bin/bash
# Let's try a different approach to find version patterns with underscores
# Search for any version-like patterns with underscores in all files

# Look for version patterns like v1_2, 1_2_3, etc.
echo "Searching for version-like patterns with underscores..."
rg -l '[vV]?\d+_\d+(?:_\d+)?'

# Look specifically for chart references with underscores
echo "Searching for chart references with underscores..."
rg -l 'chart:.*_'

# Look for any version field followed by underscore pattern
echo "Searching for version fields with underscores..."
rg -l 'version:.*_'

# Check the manifest code for any other underscore handling
echo "Checking manifest code for underscore handling..."
rg -A 5 -B 5 '_' internal/manifest/

Length of output: 11994