Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion cmd/release-controller-api/http_changelog.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (c *Controller) getChangeLog(ctx context.Context, ch chan renderResult, chN
return
}

out, err = rhcos.TransformMarkDownOutput(out, fromTag, toTag, architecture, archExtension)
out, err = rhcos.TransformMarkDownOutput(out, fromTag, toTag, architecture, archExtension, c.releaseInfo, toImage.GenerateDigestPullSpec())
if err != nil {
ch <- renderResult{err: err}
return
Expand Down
2 changes: 1 addition & 1 deletion hack/changelog-preview/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func main() {
fmt.Fprintf(os.Stderr, "ChangeLog: %v\n", err)
os.Exit(1)
}
out, err = rhcos.TransformMarkDownOutput(out, *fromTag, *toTag, archName, archExt)
out, err = rhcos.TransformMarkDownOutput(out, *fromTag, *toTag, archName, archExt, info, *to)
if err != nil {
fmt.Fprintf(os.Stderr, "TransformMarkDownOutput: %v\n", err)
os.Exit(1)
Expand Down
18 changes: 17 additions & 1 deletion pkg/release-controller/semver.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,5 +172,21 @@ func ReleaseTagIsDualRHCOS(toTag string) bool {
if err != nil {
return false
}
return v.Major == 4 && v.Minor >= 21
return (v.Major == 4 && v.Minor >= 21) || v.Major >= 5
}

// PreferredMachineOSTag returns the machine-OS tag that should be displayed first
// in the Components section based on the OpenShift major version.
// - 4.Y releases prefer rhel-coreos (RHCOS 9)
// - 5.Y+ releases prefer rhel-coreos-10 (RHCOS 10)
// Returns empty string if the version can't be parsed.
func PreferredMachineOSTag(releaseTag string) string {
v, err := SemverParseTolerant(releaseTag)
if err != nil {
return ""
}
if v.Major >= 5 {
return "rhel-coreos-10"
}
return "rhel-coreos"
}
34 changes: 34 additions & 0 deletions pkg/release-controller/semver_dual_rhcos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ func TestReleaseTagIsDualRHCOS(t *testing.T) {
{"4.20.0", false},
{"4.20.0-ec.0", false},
{"not-a-version", false},
{"5.0.0", true},
{"5.0.0-ec.1", true},
{"5.1.0", true},
{"5.2.3", true},
}
for _, tt := range tests {
t.Run(tt.tag, func(t *testing.T) {
Expand All @@ -22,3 +26,33 @@ func TestReleaseTagIsDualRHCOS(t *testing.T) {
})
}
}

func TestPreferredMachineOSTag(t *testing.T) {
tests := []struct {
tag string
want string
}{
// 4.Y releases prefer rhel-coreos (RHCOS 9)
{"4.21.0-ec.1", "rhel-coreos"},
{"4.21.0", "rhel-coreos"},
{"4.22.1", "rhel-coreos"},
{"4.20.0", "rhel-coreos"},
{"4.20.0-ec.0", "rhel-coreos"},
{"4.30.0", "rhel-coreos"},
// 5.Y+ releases prefer rhel-coreos-10 (RHCOS 10)
{"5.0.0", "rhel-coreos-10"},
{"5.0.0-ec.1", "rhel-coreos-10"},
{"5.1.0", "rhel-coreos-10"},
{"5.2.3", "rhel-coreos-10"},
{"6.0.0", "rhel-coreos-10"},
// Invalid versions return empty string
{"not-a-version", ""},
}
for _, tt := range tests {
t.Run(tt.tag, func(t *testing.T) {
if got := PreferredMachineOSTag(tt.tag); got != tt.want {
t.Errorf("PreferredMachineOSTag(%q) = %q, want %q", tt.tag, got, tt.want)
}
})
}
}
189 changes: 182 additions & 7 deletions pkg/rhcos/rhcos.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"maps"
"net/url"
"os"
"regexp"
"slices"
"sort"
Expand Down Expand Up @@ -42,8 +43,9 @@ var (
reMdRHCoSVersion = regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS(?: \d+\.\d+)? ((\d+)\.[\w\.\-]+)\n`)

// RHEL 10 node image (rhel-coreos-10); match before generic RHCOS regex (longer prefix first).
reMdRHCoS10Diff = regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS 10(?: \d+\.\d+)? upgraded from ((\d+)\.[\w\.\-]+) to ((\d+)\.[\w\.\-]+)\n`)
reMdRHCoS10Version = regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS 10(?: \d+\.\d+)? ((\d+)\.[\w\.\-]+)\n`)
// [. ] allows either "CoreOS 10.2 ..." (period) or "CoreOS 10 10.2 ..." (space) after "10".
reMdRHCoS10Diff = regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS 10(?:[. ]\d[\d.]*)? upgraded from ((\d+)\.[\w\.\-]+) to ((\d+)\.[\w\.\-]+)\n`)
reMdRHCoS10Version = regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS 10(?:[. ]\d[\d.]*)? ((\d+)\.[\w\.\-]+)\n`)

reMdCentOSCoSDiff = regexp.MustCompile(`\* CentOS Stream CoreOS upgraded from ((\d+)\.[\w\.\-]+) to ((\d+)\.[\w\.\-]+)\n`)
reMdCentOSCoSVersion = regexp.MustCompile(`\* CentOS Stream CoreOS ((\d+)\.[\w\.\-]+)\n`)
Expand All @@ -58,7 +60,161 @@ var (
reRhelCoreOsVersion = regexp.MustCompile(`(\d+)\.(\d+)\.(\d+)-(\d+)`)
)

func TransformMarkDownOutput(markdown, fromTag, toTag, architecture, architectureExtension string) (string, error) {
// swapRHCOSComponentIfNeeded replaces the RHCOS component shown in the ### Components section
// with the preferred version based on the OpenShift major version (4.Y prefers RHCOS 9, 5.Y+ prefers RHCOS 10).
func swapRHCOSComponentIfNeeded(markdown, toTag, architecture, architectureExtension string, releaseInfo releasecontroller.ReleaseInfo, toImage string) (string, error) {
// Determine preferred machine-OS tag based on release version
preferredTag := releasecontroller.PreferredMachineOSTag(toTag)
if preferredTag == "" {
// Can't parse version, skip swap
return markdown, nil
}

// Query which machine-OS streams exist in the release
streams, err := releaseInfo.ListMachineOSStreams(toImage)
if err != nil || len(streams) == 0 {
// Can't determine streams, skip swap
return markdown, nil
}

// Check if both rhel-coreos and rhel-coreos-10 exist
var hasRHCOS9, hasRHCOS10 bool
var rhcos9Info, rhcos10Info releasecontroller.MachineOSStreamInfo
for _, s := range streams {
if s.Tag == "rhel-coreos" {
hasRHCOS9 = true
rhcos9Info = s
} else if s.Tag == "rhel-coreos-10" {
hasRHCOS10 = true
rhcos10Info = s
}
}

if !hasRHCOS9 || !hasRHCOS10 {
// Only one RHCOS version exists, no need to swap
return markdown, nil
}

// Parse the Components section to find which RHCOS is currently shown
reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)\n\n###`)
componentsMatch := reComponentsSection.FindStringSubmatch(markdown)
if componentsMatch == nil {
// Can't find Components section
return markdown, nil
}
Comment on lines +99 to +104

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle terminal Components sections in the matcher.

At Line 99, the regex requires a following \n\n###, so swap is skipped when ### Components is the last section in the markdown.

Suggested fix
-	reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)\n\n###`)
+	reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)(?:\n\n###|\z)`)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)\n\n###`)
componentsMatch := reComponentsSection.FindStringSubmatch(markdown)
if componentsMatch == nil {
// Can't find Components section
return markdown, nil
}
reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)(?:\n\n###|\z)`)
componentsMatch := reComponentsSection.FindStringSubmatch(markdown)
if componentsMatch == nil {
// Can't find Components section
return markdown, nil
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/rhcos/rhcos.go` around lines 99 - 104, The regex pattern in the
reComponentsSection variable uses a required `\n\n###` lookahead that prevents
matching when the Components section is the last section in the markdown file.
Modify the regex pattern to make the trailing `\n\n###` optional or use an
alternation pattern that matches either the trailing `\n\n###` or the end of the
string, ensuring the Components section is captured regardless of whether it
appears at the end of the document.


componentsSection := componentsMatch[1]

// Determine which RHCOS is currently shown and what we want to show
var currentlyShown, wantToShow string
var desiredInfo releasecontroller.MachineOSStreamInfo

if strings.Contains(componentsSection, rhelCoreOs10) {
currentlyShown = "rhel-coreos-10"
} else if strings.Contains(componentsSection, rhelCoreOs) {
currentlyShown = "rhel-coreos"
} else {
// No RHCOS component found in Components section
return markdown, nil
}

// Determine what we want to show
if preferredTag == "rhel-coreos-10" {
wantToShow = "rhel-coreos-10"
desiredInfo = rhcos10Info
} else {
wantToShow = "rhel-coreos"
desiredInfo = rhcos9Info
}

if currentlyShown == wantToShow {
// Already showing the preferred version
return markdown, nil
}

// Need to swap: fetch the version info for the desired RHCOS
releaseJSON, err := releaseInfo.ReleaseInfo(toImage)
if err != nil {
return markdown, fmt.Errorf("failed to get release info: %w", err)
}

// Parse the release JSON to get the machine-os component version
var relInfo struct {
References struct {
Spec struct {
Tags []struct {
Name string `json:"name"`
Annotations map[string]string `json:"annotations"`
} `json:"tags"`
} `json:"spec"`
} `json:"references"`
}

if err := json.Unmarshal([]byte(releaseJSON), &relInfo); err != nil {
return markdown, fmt.Errorf("failed to parse release JSON: %w", err)
}

// Find the version annotation for the desired machine-OS tag
var desiredVersion string
for _, tag := range relInfo.References.Spec.Tags {
if tag.Name == wantToShow {
if versionAnnotation, ok := tag.Annotations["io.openshift.build.versions"]; ok {
// Parse the version from the annotation, format: "machine-os=X.Y.Z"
for _, part := range strings.Split(versionAnnotation, ",") {
part = strings.TrimSpace(part)
if strings.HasPrefix(part, "machine-os=") {
desiredVersion = strings.TrimPrefix(part, "machine-os=")
break
}
}
}
break
}
}

if desiredVersion == "" {
// Couldn't find version, skip swap
return markdown, nil
}

// Build the replacement RHCOS component line with proper formatting
displayName := desiredInfo.DisplayName
if displayName == "" {
displayName = releasecontroller.MachineOSTitle(desiredInfo)
}

// Build the URL to the RHCOS release browser
stream, ok := getRHCoSReleaseStream(desiredVersion, architectureExtension)
if !ok {
// Can't determine stream, skip enrichment
return markdown, nil
}

rhcosURL := url.URL{
Scheme: serviceScheme,
Host: serviceUrl,
Path: "/",
Fragment: desiredVersion,
RawQuery: (url.Values{
"stream": []string{stream},
"arch": []string{architecture},
"release": []string{desiredVersion},
}).Encode(),
}

// Create the new component line with enriched link and alert box
enrichedComponent := fmt.Sprintf("* %s [%s](%s) %s", displayName, desiredVersion, rhcosURL.String(), baseLayerAlertBox)

// Find and replace the old RHCOS component line in the Components section
reComponentLine := regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS[^\n]+`)

newComponentsSection := reComponentLine.ReplaceAllString(componentsSection, enrichedComponent)
markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1)
Comment on lines +209 to +212

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Replace only the targeted RHCOS line, not every match.

At Line 211, ReplaceAllString rewrites all Red Hat Enterprise Linux CoreOS... lines. If both RHCOS 9 and 10 lines are present, both can be replaced with the same component.

Suggested fix
-	reComponentLine := regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS[^\n]+`)
-
-	newComponentsSection := reComponentLine.ReplaceAllString(componentsSection, enrichedComponent)
+	lines := strings.Split(componentsSection, "\n")
+	for i, line := range lines {
+		if !strings.HasPrefix(line, "* Red Hat Enterprise Linux CoreOS") {
+			continue
+		}
+		isRHCOS10 := strings.Contains(line, rhelCoreOs10)
+		if (currentlyShown == "rhel-coreos-10" && isRHCOS10) || (currentlyShown == "rhel-coreos" && !isRHCOS10) {
+			lines[i] = enrichedComponent
+			break
+		}
+	}
+	newComponentsSection := strings.Join(lines, "\n")
 	markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
reComponentLine := regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS[^\n]+`)
newComponentsSection := reComponentLine.ReplaceAllString(componentsSection, enrichedComponent)
markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1)
lines := strings.Split(componentsSection, "\n")
for i, line := range lines {
if !strings.HasPrefix(line, "* Red Hat Enterprise Linux CoreOS") {
continue
}
isRHCOS10 := strings.Contains(line, rhelCoreOs10)
if (currentlyShown == "rhel-coreos-10" && isRHCOS10) || (currentlyShown == "rhel-coreos" && !isRHCOS10) {
lines[i] = enrichedComponent
break
}
}
newComponentsSection := strings.Join(lines, "\n")
markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/rhcos/rhcos.go` around lines 209 - 212, The
reComponentLine.ReplaceAllString call on line 211 is replacing all occurrences
of Red Hat Enterprise Linux CoreOS lines in the componentsSection, which causes
multiple RHCOS version lines to be replaced with the same enriched component.
Instead of using ReplaceAllString, use a method that replaces only the first
occurrence or the specific targeted line, such as replacing only the exact match
of the componentsSection variable itself rather than using a pattern-based
replacement that could match multiple lines. This ensures only the intended
RHCOS component line is updated, not all matching lines in the section.


return markdown, nil
}

func TransformMarkDownOutput(markdown, fromTag, toTag, architecture, architectureExtension string, releaseInfo releasecontroller.ReleaseInfo, toImage string) (string, error) {
// replace references to the previous version with links
rePrevious, err := regexp.Compile(fmt.Sprintf(`([^\w:])%s(\W)`, regexp.QuoteMeta(fromTag)))
if err != nil {
Expand All @@ -75,6 +231,15 @@ func TransformMarkDownOutput(markdown, fromTag, toTag, architecture, architectur
// add link to tag from which current version promoted from
markdown = reMdPromotedFrom.ReplaceAllString(markdown, fmt.Sprintf("Release %s was created from [$1:$2](/releasetag/$2)", toTag))

// Swap RHCOS component in Components section if needed (4.Y prefers RHCOS 9, 5.Y+ prefers RHCOS 10)
if releaseInfo != nil && toImage != "" {
markdown, err = swapRHCOSComponentIfNeeded(markdown, toTag, architecture, architectureExtension, releaseInfo, toImage)
if err != nil {
// Log but don't fail - this is a best-effort improvement
fmt.Fprintf(os.Stderr, "Warning: failed to swap RHCOS component: %v\n", err)
}
}

// Apply CoreOS link transforms for every matching line (OpenShift 4.21+ may list RHCOS 9 and 10 separately).
for {
var m []string
Expand All @@ -85,7 +250,11 @@ func TransformMarkDownOutput(markdown, fromTag, toTag, architecture, architectur
name = rhelCoreOs10
case reMdRHCoSDiff.MatchString(markdown):
m = reMdRHCoSDiff.FindStringSubmatch(markdown)
name = rhelCoreOs
if fromMajor, err := strconv.Atoi(m[2]); err == nil && fromMajor >= 10 && fromMajor < 100 {
name = rhelCoreOs10
} else {
name = rhelCoreOs
}
case reMdCentOSCoSDiff.MatchString(markdown):
m = reMdCentOSCoSDiff.FindStringSubmatch(markdown)
name = centosStreamCoreOs
Expand All @@ -106,7 +275,11 @@ func TransformMarkDownOutput(markdown, fromTag, toTag, architecture, architectur
name = rhelCoreOs10
case reMdRHCoSVersion.MatchString(markdown):
m = reMdRHCoSVersion.FindStringSubmatch(markdown)
name = rhelCoreOs
if vMajor, err := strconv.Atoi(m[2]); err == nil && vMajor >= 10 && vMajor < 100 {
name = rhelCoreOs10
} else {
name = rhelCoreOs
}
case reMdCentOSCoSVersion.MatchString(markdown):
m = reMdCentOSCoSVersion.FindStringSubmatch(markdown)
name = centosStreamCoreOs
Expand All @@ -129,8 +302,10 @@ func TransformJsonOutput(output, architecture, architectureExtension string) (st
}

for i, component := range changeLogJson.Components {
switch component.Name {
case rhelCoreOs, rhelCoreOs10, centosStreamCoreOs:
switch {
case strings.HasPrefix(component.Name, rhelCoreOs10):
changeLogJson.Components[i] = enrichCoreOSComponentJSON(component, architecture, architectureExtension)
case component.Name == rhelCoreOs || component.Name == centosStreamCoreOs:
changeLogJson.Components[i] = enrichCoreOSComponentJSON(component, architecture, architectureExtension)
}
}
Expand Down
Loading