Skip to content

Fixing release-5 prefix bug for OCP 5.0 #728

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 1 commit into from
Feb 26, 2026
Merged

Fixing release-5 prefix bug for OCP 5.0 #728

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion cmd/release-controller/sync_verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (c *Controller) resolveUpgradeRelease(upgradeRelease *releasecontroller.Upg
} else if upgradeRelease.Candidate != nil {
// create blank semver.Range
var constraint semver.Range
stream := fmt.Sprintf("%s.0-0.%s%s", upgradeRelease.Candidate.Version, upgradeRelease.Candidate.Stream, strings.TrimPrefix(release.Config.To, "release"))
stream := fmt.Sprintf("%s.0-0.%s%s", upgradeRelease.Candidate.Version, upgradeRelease.Candidate.Stream, TrimPrefixes(release.Config.To, "release-5", "release"))
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n cmd/release-controller/sync_verify.go | sed -n '215,255p'

Repository: openshift/release-controller

Length of output: 1850

🏁 Script executed:

rg "TrimPrefixes" --type go

Repository: openshift/release-controller

Length of output: 367

🏁 Script executed:

rg "Config\.To" --type go -A 2 -B 2 | head -100

Repository: openshift/release-controller

Length of output: 678

🏁 Script executed:

rg "release-\d" --type go | head -50

Repository: openshift/release-controller

Length of output: 1805

🏁 Script executed:

rg "type.*Config struct" --type go -A 30 | grep -A 30 "To.*string"

Repository: openshift/release-controller

Length of output: 50384

🏁 Script executed:

rg "Config.*struct" --type go cmd/release-controller/ -A 20 | head -80

Repository: openshift/release-controller

Length of output: 54

🏁 Script executed:

rg "\.To\s*=" --type go cmd/release-controller/ -B 5 -A 2 | head -80

Repository: openshift/release-controller

Length of output: 54

🏁 Script executed:

fd -t f "\.go$" cmd/release-controller/ | xargs grep -l "type Config struct" | head -5

Repository: openshift/release-controller

Length of output: 54

🏁 Script executed:

find cmd/release-controller -name "*.go" -exec grep -l "\.To\s" {} \; | head -5

Repository: openshift/release-controller

Length of output: 109

🏁 Script executed:

rg "type.*Config\s+struct" cmd/release-controller/ -A 20

Repository: openshift/release-controller

Length of output: 54

🏁 Script executed:

cat -n cmd/release-controller/sync_verify.go | sed -n '190,225p'

Repository: openshift/release-controller

Length of output: 2402

🏁 Script executed:

rg "release\.Config\.To" cmd/release-controller/ -B 5 -A 5

Repository: openshift/release-controller

Length of output: 1162

🏁 Script executed:

rg "Config struct" cmd/release-controller/ -A 15

Repository: openshift/release-controller

Length of output: 54

🏁 Script executed:

rg "type Release struct" --type go -A 20

Repository: openshift/release-controller

Length of output: 6184

🏁 Script executed:

rg "Config\s+struct" --type go cmd/release-controller/ -A 15

Repository: openshift/release-controller

Length of output: 54

🏁 Script executed:

# Find where release parameter comes from in the function signature
rg "func.*resolveUpgradeRelease" --type go -A 2

Repository: openshift/release-controller

Length of output: 468

🏁 Script executed:

grep -n "^type ReleaseConfig struct" pkg/release-controller/types.go -A 50

Repository: openshift/release-controller

Length of output: 2863

🏁 Script executed:

python3 << 'EOF'
def trim_prefixes(s, *prefixes):
    for p in prefixes:
        if s.startswith(p):
            return s[len(p):]
    return s

# Test cases from the review and realistic scenarios
cases = [
    "release-5.0",
    "release-50",
    "release-50.1",
    "release-4.19",
    "release-5",
    "release-5-edge",
    "release-500",
]

for c in cases:
    result = trim_prefixes(c, "release-5", "release")
    print(f"{c:20} -> {result:10} (after prefix trim)")
EOF

Repository: openshift/release-controller

Length of output: 456

🏁 Script executed:

# Test the proposed fix
python3 << 'EOF'
def trim_prefixes_fixed(s, *prefixes):
    for p in prefixes:
        if s.startswith(p):
            after = s[len(p):]
            # Proposed boundary check
            if after == "" or after.startswith(".") or after.startswith("-"):
                return after
    return s

cases = [
    "release-5.0",
    "release-50",
    "release-50.1",
    "release-4.19",
    "release-5",
    "release-5-edge",
    "release-500",
]

print("Original vs Fixed:")
for c in cases:
    orig = "release-5" if c.startswith("release-5") else "release" if c.startswith("release") else c
    result_orig = c[len("release-5"):] if c.startswith("release-5") else c[len("release"):] if c.startswith("release") else c
    result_fixed = trim_prefixes_fixed(c, "release-5", "release")
    print(f"{c:20} -> orig: {result_orig:10} fixed: {result_fixed:10} {'✓ SAME' if result_orig == result_fixed else '✗ DIFF'}")
EOF

Repository: openshift/release-controller

Length of output: 552

🏁 Script executed:

# Look for actual values of Config.To in release configs
find . -name "*.yaml" -o -name "*.json" | xargs grep -l "to:" 2>/dev/null | head -5

Repository: openshift/release-controller

Length of output: 570

🏁 Script executed:

# Check if there are tests or examples showing expected Config.To values
rg "Config.*To" pkg/release-controller/ cmd/release-controller/ -B 3 -A 3

Repository: openshift/release-controller

Length of output: 1344

🏁 Script executed:

# Look for documentation about Config.To field values
rg "To.*image stream" --type go -B 5 -A 5

Repository: openshift/release-controller

Length of output: 7144

🏁 Script executed:

# Check actual release controller documentation or comments
rg "\"to\"" pkg/release-controller/types.go -B 5 -A 5

Repository: openshift/release-controller

Length of output: 2092

🏁 Script executed:

# See if there are examples in the codebase
find . -path ./vendor -prune -o -name "*.md" -type f -exec grep -l "release-5\|release-4" {} \; 2>/dev/null | head -5

Repository: openshift/release-controller

Length of output: 54

🏁 Script executed:

# Verify the concern is realistic - check if "release-5" prefix can cause partial matches
python3 << 'EOF'
import re

# Check strings.CutPrefix behavior for problematic cases
def cut_prefix(s, prefix):
    if s.startswith(prefix):
        return s[len(prefix):]
    return None

test_cases = [
    ("release-5.0", "release-5"),   # Intended match
    ("release-50.1", "release-5"),  # Unintended match (partial numeric)
    ("release-500", "release-5"),   # Unintended match (partial numeric)
    ("release-5", "release-5"),     # Intended match (exact)
    ("release-5-edge", "release-5"), # Intended match (boundary marker)
]

print("Problematic cases where release-5 causes partial numeric matches:")
for s, prefix in test_cases:
    result = cut_prefix(s, prefix)
    if result is not None:
        # Check if result looks like it was partially matched
        if result and result[0].isdigit():  # Result starts with a digit
            print(f"  {s:20} → {result:10} (starts with digit - possible unintended match)")
        else:
            print(f"  {s:20} → {result:10}")
EOF

Repository: openshift/release-controller

Length of output: 413

Add boundary-aware prefix trimming to prevent silent malformation of stream names

The TrimPrefixes function at lines 243-250 performs raw text prefix matching via strings.CutPrefix, which can accidentally match partial version numbers. For example, if release.Config.To contains "release-50" or "release-50.1", the prefix "release-5" will match and produce "0" or "0.1" respectively, creating malformed stream names passed to line 223.

The proposed boundary check (allowing only results that are empty or start with "." or "-") prevents silent generation of invalid stream suffixes.

🔧 Proposed fix 
func TrimPrefixes(s string, prefixes ...string) string {
-  for _, prefix := range prefixes {
-	  if after, found := strings.CutPrefix(s, prefix); found {
-		  return after
-	  }
-  }
-  return s
+	for _, prefix := range prefixes {
+		if after, found := strings.CutPrefix(s, prefix); found {
+			// Avoid partial token matches like release-50.x matching release-5
+			if after == "" || strings.HasPrefix(after, ".") || strings.HasPrefix(after, "-") {
+				return after
+			}
+		}
+	}
+	return s
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/release-controller/sync_verify.go` at line 223, The TrimPrefixes usage
when building the stream string (see stream := fmt.Sprintf(...) using
TrimPrefixes) can produce malformed suffixes by naively cutting prefixes like
"release-5" from values such as "release-50"; update TrimPrefixes (function
TrimPrefixes) to perform boundary-aware trimming: after a candidate prefix is
removed only accept the trimmed result if it is empty or its first character is
'.' or '-' (otherwise keep the original input). Replace the existing
strings.CutPrefix-based behavior with this boundary check so stream construction
uses only safe suffixes.

r, latest, err := releasecontroller.LatestForStream(c.parsedReleaseConfigCache, c.eventRecorder, c.releaseLister, stream, constraint, upgradeRelease.Candidate.Relative, "")
if err != nil {
return "", "", fmt.Errorf("failed to get latest tag for stream %s: %w", stream, err)
Expand All @@ -239,3 +239,12 @@ func (c *Controller) resolveUpgradeRelease(upgradeRelease *releasecontroller.Upg
}
return "", "", fmt.Errorf("upgradeRelease fields must be set if upgradeRelease is set")
}

func TrimPrefixes(s string, prefixes ...string) string {
for _, prefix := range prefixes {
if after, found := strings.CutPrefix(s, prefix); found {
return after
}
}
return s
}