Skip to content

CNTRLPLANE-3020: Adopt coreos/stream-metadata-go upstream library #8673

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 2 commits into from
Jun 11, 2026
Merged

CNTRLPLANE-3020: Adopt coreos/stream-metadata-go upstream library #8673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
566a597
refactor(releaseinfo): adopt coreos/stream-metadata-go upstream library
sdminonne Jun 4, 2026
bc5f122
fix(nodepool): add nil-guard tests and split error messages for upstr…
sdminonne Jun 9, 2026
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
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ require (
github.com/clarketm/json v1.17.1
github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf
github.com/coreos/ignition/v2 v2.25.1
github.com/coreos/stream-metadata-go v0.4.11

@coderabbitai coderabbitai Bot Jun 4, 2026

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify stream-metadata-go v0.4.11 for CVEs, license, and stability

echo "=== Checking GitHub Security Advisories ==="
gh api graphql -f query='
{
  securityVulnerabilities(first: 10, ecosystem: GO, package: "github.com/coreos/stream-metadata-go") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

echo ""
echo "=== Checking Package Metadata from pkg.go.dev ==="
curl -s "https://pkg.go.dev/github.com/coreos/stream-metadata-go@v0.4.11" | grep -E "(License|Deprecated|Retracted)" || echo "No deprecation/retraction found"

echo ""
echo "=== Checking if v0.4.11 exists on GitHub releases ==="
gh api repos/coreos/stream-metadata-go/releases --jq '.[] | select(.tag_name == "v0.4.11") | {tag_name, prerelease, draft, published_at}'

Repository: openshift/hypershift

Length of output: 491

🏁 Script executed:

#!/bin/bash
set -euo pipefail

MOD="github.com/coreos/stream-metadata-go"
VER="v0.4.11"

echo "=== Checking OSV for known vulnerabilities ==="
curl -s "https://api.osv.dev/v1/query" \
  -H "content-type: application/json" \
  -d `@-` <<JSON | jq -c '.'
{
  "package": { "name": "'"$MOD"'" },
  "version": "'"$VER"'"
}
JSON

echo ""
echo "=== Extracting license(s) from pkg.go.dev ==="
# Fetch only the relevant part of the licenses tab (best-effort)
curl -s "https://pkg.go.dev/$MOD@$VER?tab=licenses" \
  | rg -o -m 5 'License:\s*</[^>]+>\s*<[^>]+>([^<]+)' || true

# Also output any "License:" lines to avoid missing formatting changes
curl -s "https://pkg.go.dev/$MOD@$VER?tab=licenses" \
  | rg -n "License:" | head -n 20 || true

Repository: openshift/hypershift

Length of output: 291

🏁 Script executed:

#!/bin/bash
set -euo pipefail

MOD="github.com/coreos/stream-metadata-go"
VER="v0.4.11"

echo "=== Checking OSV for known vulnerabilities (package + version) ==="
payload="$(jq -n --arg name "$MOD" --arg version "$VER" '{package: {name: $name}, version: $version}')"
curl -s "https://api.osv.dev/v1/query" \
  -H "content-type: application/json" \
  -d "$payload" | jq -c '.'

echo ""
echo "=== Checking pkg.go.dev license for v0.4.11 ==="
# Fetch HTML and extract license labels (best-effort; pkg.go.dev markup can vary)
curl -s -L -A "Mozilla/5.0" "https://pkg.go.dev/$MOD@$VER?tab=licenses" \
  | rg -n "License:" | head -n 20

curl -s -L -A "Mozilla/5.0" "https://pkg.go.dev/$MOD@$VER?tab=licenses" \
  | rg -o -m 10 "License:\s*[^<]+" || true

Repository: openshift/hypershift

Length of output: 183

🏁 Script executed:

#!/bin/bash
set -euo pipefail

MOD="github.com/coreos/stream-metadata-go"
VER="v0.4.11"

echo "=== OSV raw query response ==="
payload="$(jq -n --arg name "$MOD" --arg version "$VER" '{package: {name: $name}, version: $version}')"
curl -sS -D /tmp/osv_headers.txt -o /tmp/osv_body.json \
  -H "content-type: application/json" \
  --data "$payload" \
  https://api.osv.dev/v1/query || true

echo "--- HTTP ---"
cat /tmp/osv_headers.txt

echo "--- Body (first 500 chars) ---"
python3 - <<'PY'
import json, pathlib
p=pathlib.Path("/tmp/osv_body.json")
b=p.read_text(errors="ignore")
print(b[:500])
# try parse if possible
try:
    j=json.loads(b)
    print("\n--- Parsed keys ---")
    print(list(j.keys()))
    print("vulns_count=", len(j.get("vulns", [])) if isinstance(j.get("vulns"), list) else None)
except Exception:
    pass
PY

echo ""
echo "=== pkg.go.dev license tab (presence check) ==="
# Just check whether the page contains "License:" text to avoid brittle parsing
curl -sS -L -A "Mozilla/5.0" "https://pkg.go.dev/$MOD@$VER?tab=licenses" \
  | rg -n "License:" | head -n 20 || true

echo ""
echo "=== Check if module version is yanked via Go proxy ==="
curl -sS -D /tmp/proxy_headers.txt -o /tmp/proxy_info.json \
  "https://proxy.golang.org/$MOD/@v/$VER.info" || true
cat /tmp/proxy_headers.txt
python3 - <<'PY'
import pathlib, json
p=pathlib.Path("/tmp/proxy_info.json")
b=p.read_text(errors="ignore")
print("\n--- proxy info body (first 300 chars) ---")
print(b[:300])
try:
    j=json.loads(b)
    print("\nParsed:", j)
except Exception:
    pass
PY

echo ""
echo "=== Determine upstream license by scanning LICENSE files in module source ==="
tmpdir="$(mktemp -d)"
zip="/tmp/module_src.zip"
curl -sS -L -o "$zip" "https://proxy.golang.org/$MOD/@v/$VER.zip"

python3 - <<'PY'
import zipfile, re, pathlib, io, os, sys
zip_path="/tmp/module_src.zip"
z=zipfile.ZipFile(zip_path)

candidates=[]
for name in z.namelist():
    low=name.lower()
    if any(k in low for k in ["license", "copying", "licence"]):
        if low.endswith((".txt",".md",".rst",".html")) or "license" in low or "copying" in low:
            candidates.append(name)

# Prefer top-level / typical filenames
preferred=[]
for n in candidates:
    base=os.path.basename(n).lower()
    if base in ["license","license.txt","license.md","copying","copying.txt","licence","licence.txt"]:
        preferred.append(n)
for n in preferred + candidates:
    if n not in candidates[:10]:
        pass

top = (preferred + candidates)[:10]
print("License-like file candidates:", top)

keywords = [
    ("Apache License", re.compile(r"Apache License", re.I)),
    ("MIT License", re.compile(r"\bMIT License\b", re.I)),
    ("BSD 2-Clause", re.compile(r"\bRedistribution and use in source and binary forms\b.*\bBSD\b", re.I|re.S)),
    ("BSD 3-Clause", re.compile(r"\bNeither the name of\b.*\bmay be used to endorse\b", re.I|re.S)),
    ("GPLv2", re.compile(r"\bGNU GENERAL PUBLIC LICENSE\b.*Version 2", re.I|re.S)),
    ("GPLv3", re.compile(r"\bGNU GENERAL PUBLIC LICENSE\b.*Version 3", re.I|re.S)),
    ("MPL 2.0", re.compile(r"\bMozilla Public License\b.*2\.0", re.I|re.S)),
]

def classify(text):
    hits=[]
    for label, rx in keywords:
        if rx.search(text):
            hits.append(label)
    # also capture "SPDX-License-Identifier" if present
    spdx=re.findall(r"SPDX-License-Identifier:\s*([^\s]+)", text)
    return hits, spdx

for name in top:
    try:
        data=z.read(name)
    except Exception:
        continue
    # limit amount
    chunk=data[:200000]
    try:
        text=chunk.decode("utf-8", errors="ignore")
    except Exception:
        text=str(chunk)
    hits, spdx=classify(text)
    if hits or spdx:
        print(f"\n--- {name} ---")
        if spdx:
            print("SPDX:", spdx[:5])
        print("Matches:", hits[:10])
        # show first relevant line
        m=re.search(r"(SPDX-License-Identifier[^\n]*|Apache License[^\n]*|MIT License[^\n]*|Mozilla Public License[^\n]*|GNU GENERAL PUBLIC LICENSE[^\n]*)", text, re.I)
        if m:
            print("Example:", m.group(1).strip())
PY

Repository: openshift/hypershift

Length of output: 2296

🏁 Script executed:

#!/bin/bash
set -euo pipefail

MOD="github.com/coreos/stream-metadata-go"
VER="v0.4.11"

echo "=== License header snippet from module source ==="
zip="/tmp/module_src.zip"
curl -sS -L -o "$zip" "https://proxy.golang.org/$MOD/@v/$VER.zip"

python3 - <<'PY'
import zipfile, re
zip_path="/tmp/module_src.zip"
z=zipfile.ZipFile(zip_path)
# Prefer LICENSE at root
names=[n for n in z.namelist() if n.lower().endswith('/license') or n.lower().endswith('license')]
if not names:
    names=[n for n in z.namelist() if 'license' in n.lower()]
for n in names[:5]:
    data=z.read(n)
    text=data.decode('utf-8', errors='ignore')
    # show a tight snippet around "Apache License"
    m=re.search(r"(Apache License[^\n]{0,200})", text)
    print("\nFILE:", n)
    if m:
        print("SNIPPET:", m.group(1).strip())
    # SPDX line if present
    spdx=re.search(r"SPDX-License-Identifier:\s*([^\n]+)", text)
    if spdx:
        print("SPDX:", spdx.group(1).strip())
    # show first 30 lines (best effort)
    first="\n".join(text.splitlines()[:30])
    print("FIRST_LINES:\n", first)
    break
PY

echo ""
echo "=== Go proxy zip availability (yank check) ==="
curl -sS -I "https://proxy.golang.org/$MOD/@v/$VER.zip" | rg -i "HTTP/|Content-Type|x-goog-stored-content-length" || true

Repository: openshift/hypershift

Length of output: 1962

🏁 Script executed:

#!/bin/bash
set -euo pipefail
MOD="github.com/coreos/stream-metadata-go"
VER="v0.4.11"

payload="$(jq -n --arg name "$MOD" --arg version "$VER" \
  '{package:{name:$name, ecosystem:"Go"}, version:$version}')"

curl -sS -H "content-type: application/json" \
  -d "$payload" https://api.osv.dev/v1/query | jq -c '.vulns'

Repository: openshift/hypershift

Length of output: 1962

Supply-chain checks for github.com/coreos/stream-metadata-go v0.4.11

  • No CVEs reported for v0.4.11 (GitHub Security Advisories / OSV came back empty).
  • License: module root LICENSE indicates the Apache License.
  • Version stability: v0.4.11 is a published non-prerelease GitHub release and https://proxy.golang.org/.../@v/v0.4.11.zip is available (not yanked).

Confirm the Apache license fits your repo/org’s approved-license policy.

🤖 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 `@go.mod` at line 45, The dependency entry github.com/coreos/stream-metadata-go
v0.4.11 in go.mod has been reviewed for supply-chain and licensing; verify that
the Apache License in the module's LICENSE file complies with your org's
approved-license policy and then either (a) document approval by adding a note
to your dependency/security tracker referencing
github.com/coreos/stream-metadata-go v0.4.11 and the SPDX identifier Apache-2.0,
or (b) if it is not approved, replace or remove the dependency (update go.mod to
a compliant alternative or open an RFC/PR to remove usage in the codebase such
as functions/types referencing the module) and record the decision in your
dependency policy tracker.

github.com/distribution/reference v0.6.0
github.com/docker/distribution v2.8.3+incompatible
github.com/elazarl/goproxy v1.7.2
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ github.com/coreos/go-systemd/v22 v22.6.0 h1:aGVa/v8B7hpb0TKl0MWoAavPDmHvobFe5R5z
github.com/coreos/go-systemd/v22 v22.6.0/go.mod h1:iG+pp635Fo7ZmV/j14KUcmEyWF+0X7Lua8rrTWzYgWU=
github.com/coreos/ignition/v2 v2.25.1 h1:mrXOVwb4ZPlLG1tIki5em604Yiz5oRFlWkISrEYpHds=
github.com/coreos/ignition/v2 v2.25.1/go.mod h1:Px9MZK4oLhMUM3QMzzRhKbHowc5Hkf+VUu67i0gmsNw=
github.com/coreos/stream-metadata-go v0.4.11 h1:sQQOpI+v/eTyYO76r7jJeCmqZcm1oQYKdwirFJ7f0M0=
github.com/coreos/stream-metadata-go v0.4.11/go.mod h1:dTE8UEFgyUcrbdUg7vGT3uIP7S8a1IwUlmWLKlOp8G8=
github.com/coreos/vcontext v0.0.0-20231102161604-685dc7299dc5 h1:sMZSC2BW5LKCdvNbfN12SbKrNvtLBUNjfHZmMvI2ItY=
github.com/coreos/vcontext v0.0.0-20231102161604-685dc7299dc5/go.mod h1:Salmysdw7DAVuobBW/LwsKKgpyCPHUhjyJoMJD+ZJiI=
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
Expand Down
8 changes: 5 additions & 3 deletions hypershift-operator/controllers/nodepool/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,12 +437,14 @@ func getWindowsAMI(region string, specifiedArch string, releaseImage *releaseinf
}

// Access the rhel-coreos-extensions aws-winli data
winliData := archData.RHCOS.AWSWinLi
if winliData.Regions == nil {
if archData.RHELCoreOSExtensions == nil {
return "", fmt.Errorf("no rhel-coreos-extensions data found in release image metadata")
}
if archData.RHELCoreOSExtensions.AwsWinLi == nil || archData.RHELCoreOSExtensions.AwsWinLi.Regions == nil {
return "", fmt.Errorf("no aws-winli regions data found in release image metadata")
}

regionData, exists := winliData.Regions[region]

@jparrill jparrill Jun 8, 2026

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.

The triple nil-check is correct, but a single error message for three different failure modes makes debugging harder. Consider splitting:

if archData.RHELCoreOSExtensions == nil {
    return "", fmt.Errorf("no rhel-coreos-extensions data found in release image metadata")
}
if archData.RHELCoreOSExtensions.AwsWinLi == nil || archData.RHELCoreOSExtensions.AwsWinLi.Regions == nil {
    return "", fmt.Errorf("no aws-winli regions data found in release image metadata")
}

Also — could you add test cases for RHELCoreOSExtensions == nil and AwsWinLi == nil? The current tests only cover Regions == nil.

regionData, exists := archData.RHELCoreOSExtensions.AwsWinLi.Regions[region]
if !exists {
return "", fmt.Errorf("no Windows AMI found for region %s in release image metadata", region)
}
Expand Down
112 changes: 76 additions & 36 deletions hypershift-operator/controllers/nodepool/aws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/coreos/stream-metadata-go/stream"
"github.com/coreos/stream-metadata-go/stream/rhcos"
"github.com/google/go-cmp/cmp"
)

Expand Down Expand Up @@ -269,12 +271,12 @@ func TestAWSMachineTemplateSpec(t *testing.T) {
Name: "4.17.0",
},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHCOS: releaseinfo.CoreRHCOSImage{
AWSWinLi: releaseinfo.CoreAWSWinLi{
Regions: map[string]releaseinfo.CoreAWSWinLiRegion{
RHELCoreOSExtensions: &rhcos.Extensions{
AwsWinLi: &rhcos.ReplicatedImage{
Regions: map[string]rhcos.SingleImage{
"us-east-1": {
Release: "418.94.202410090804-0",
Image: "ami-0abcdef1234567890",
Expand Down Expand Up @@ -707,12 +709,50 @@ func TestGetWindowsAMI(t *testing.T) {
Name: "4.17.0",
},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{},
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{},
},
},
expectedError: "couldn't find OS metadata for architecture \"amd64\"",
},
{
name: "When RHELCoreOSExtensions is nil, it should return error",
region: "us-east-1",
arch: hyperv1.ArchitectureAMD64,
releaseImage: &releaseinfo.ReleaseImage{
ImageStream: &v1.ImageStream{
ObjectMeta: metav1.ObjectMeta{
Name: "4.17.0",
},
},
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {},
},
},
},
expectedError: "no rhel-coreos-extensions data found in release image metadata",
},
{
name: "When AwsWinLi is nil, it should return error",
region: "us-east-1",
arch: hyperv1.ArchitectureAMD64,
releaseImage: &releaseinfo.ReleaseImage{
ImageStream: &v1.ImageStream{
ObjectMeta: metav1.ObjectMeta{
Name: "4.17.0",
},
},
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHELCoreOSExtensions: &rhcos.Extensions{},
},
},
},
},
expectedError: "no aws-winli regions data found in release image metadata",
},
{
name: "no aws-winli regions data",
region: "us-east-1",
Expand All @@ -723,11 +763,11 @@ func TestGetWindowsAMI(t *testing.T) {
Name: "4.17.0",
},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHCOS: releaseinfo.CoreRHCOSImage{
AWSWinLi: releaseinfo.CoreAWSWinLi{
RHELCoreOSExtensions: &rhcos.Extensions{
AwsWinLi: &rhcos.ReplicatedImage{
Regions: nil,
},
},
Expand All @@ -747,12 +787,12 @@ func TestGetWindowsAMI(t *testing.T) {
Name: "4.17.0",
},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHCOS: releaseinfo.CoreRHCOSImage{
AWSWinLi: releaseinfo.CoreAWSWinLi{
Regions: map[string]releaseinfo.CoreAWSWinLiRegion{
RHELCoreOSExtensions: &rhcos.Extensions{
AwsWinLi: &rhcos.ReplicatedImage{
Regions: map[string]rhcos.SingleImage{
"us-east-1": {
Release: "418.94.202410090804-0",
Image: "ami-testimage",
Expand All @@ -776,12 +816,12 @@ func TestGetWindowsAMI(t *testing.T) {
Name: "4.17.0",
},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHCOS: releaseinfo.CoreRHCOSImage{
AWSWinLi: releaseinfo.CoreAWSWinLi{
Regions: map[string]releaseinfo.CoreAWSWinLiRegion{
RHELCoreOSExtensions: &rhcos.Extensions{
AwsWinLi: &rhcos.ReplicatedImage{
Regions: map[string]rhcos.SingleImage{
"us-east-1": {
Release: "418.94.202410090804-0",
Image: "",
Expand All @@ -805,12 +845,12 @@ func TestGetWindowsAMI(t *testing.T) {
Name: "4.17.0",
},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHCOS: releaseinfo.CoreRHCOSImage{
AWSWinLi: releaseinfo.CoreAWSWinLi{
Regions: map[string]releaseinfo.CoreAWSWinLiRegion{
RHELCoreOSExtensions: &rhcos.Extensions{
AwsWinLi: &rhcos.ReplicatedImage{
Regions: map[string]rhcos.SingleImage{
"us-east-1": {
Release: "418.94.202410090804-0",
Image: "ami-0abcdef1234567890",
Expand Down Expand Up @@ -838,12 +878,12 @@ func TestGetWindowsAMI(t *testing.T) {
Name: "4.17.0",
},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHCOS: releaseinfo.CoreRHCOSImage{
AWSWinLi: releaseinfo.CoreAWSWinLi{
Regions: map[string]releaseinfo.CoreAWSWinLiRegion{
RHELCoreOSExtensions: &rhcos.Extensions{
AwsWinLi: &rhcos.ReplicatedImage{
Regions: map[string]rhcos.SingleImage{
"us-east-1": {
Release: "418.94.202410090804-0",
Image: "ami-0abcdef1234567890",
Expand Down Expand Up @@ -1048,12 +1088,12 @@ func TestResolveAWSAMI(t *testing.T) {
ImageStream: &v1.ImageStream{
ObjectMeta: metav1.ObjectMeta{Name: "4.17.0"},
},
StreamMetadata: &releaseinfo.CoreOSStreamMetadata{
Architectures: map[string]releaseinfo.CoreOSArchitecture{
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
RHCOS: releaseinfo.CoreRHCOSImage{
AWSWinLi: releaseinfo.CoreAWSWinLi{
Regions: map[string]releaseinfo.CoreAWSWinLiRegion{
RHELCoreOSExtensions: &rhcos.Extensions{
AwsWinLi: &rhcos.ReplicatedImage{
Regions: map[string]rhcos.SingleImage{
"us-east-1": {
Release: "418.94.202410090804-0",
Image: "ami-windows-us-east-1",
Expand Down
32 changes: 19 additions & 13 deletions hypershift-operator/controllers/nodepool/azure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,33 +133,39 @@ func getAzureMarketplaceMetadata(releaseImage *releaseinfo.ReleaseImage, arch st
// Extract marketplace metadata from the RHCOS extensions
// Structure: .architectures.<arch>.rhel-coreos-extensions.marketplace.azure.no-purchase-plan
// Check for nil safety before accessing nested fields
if archData.RHCOS.Marketplace.Azure.NoPurchasePlan.HyperVGen1 == nil &&
archData.RHCOS.Marketplace.Azure.NoPurchasePlan.HyperVGen2 == nil {
if archData.RHELCoreOSExtensions == nil ||
archData.RHELCoreOSExtensions.Marketplace == nil ||
archData.RHELCoreOSExtensions.Marketplace.Azure == nil ||
archData.RHELCoreOSExtensions.Marketplace.Azure.NoPurchasePlan == nil {
return nil, nil // No marketplace data available

@jparrill jparrill Jun 8, 2026

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.

The cascading nil check is well done. One question: should RHELCoreOSExtensions == nil also be tested explicitly? The current mock helper (createMockReleaseImage) always sets it to non-nil.

}

azureMarketplace := archData.RHELCoreOSExtensions.Marketplace.Azure.NoPurchasePlan
if azureMarketplace.Gen1 == nil && azureMarketplace.Gen2 == nil {
return nil, nil // No marketplace data available
}
azureMarketplace := archData.RHCOS.Marketplace.Azure.NoPurchasePlan

// Convert from release info format to our internal format
result := &azureMarketplaceMetadata{
NoPurchasePlan: &azureMarketplaceImageInfo{},
}

if azureMarketplace.HyperVGen1 != nil {
if azureMarketplace.Gen1 != nil {
result.NoPurchasePlan.HyperVGen1 = &hyperv1.AzureMarketplaceImage{
Publisher: azureMarketplace.HyperVGen1.Publisher,
Offer: azureMarketplace.HyperVGen1.Offer,
SKU: azureMarketplace.HyperVGen1.SKU,
Version: azureMarketplace.HyperVGen1.Version,
Publisher: azureMarketplace.Gen1.Publisher,
Offer: azureMarketplace.Gen1.Offer,
SKU: azureMarketplace.Gen1.SKU,
Version: azureMarketplace.Gen1.Version,
ImageGeneration: ptr.To(hyperv1.Gen1),
}
}

if azureMarketplace.HyperVGen2 != nil {
if azureMarketplace.Gen2 != nil {
result.NoPurchasePlan.HyperVGen2 = &hyperv1.AzureMarketplaceImage{
Publisher: azureMarketplace.HyperVGen2.Publisher,
Offer: azureMarketplace.HyperVGen2.Offer,
SKU: azureMarketplace.HyperVGen2.SKU,
Version: azureMarketplace.HyperVGen2.Version,
Publisher: azureMarketplace.Gen2.Publisher,
Offer: azureMarketplace.Gen2.Offer,
SKU: azureMarketplace.Gen2.SKU,
Version: azureMarketplace.Gen2.Version,
ImageGeneration: ptr.To(hyperv1.Gen2),
}
}
Expand Down
55 changes: 43 additions & 12 deletions hypershift-operator/controllers/nodepool/azure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import (

capiazure "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1"

"github.com/coreos/stream-metadata-go/stream"
"github.com/coreos/stream-metadata-go/stream/rhcos"
)

func TestAzureMachineTemplateSpec(t *testing.T) {
Expand Down Expand Up @@ -946,6 +949,34 @@ func TestDefaultAzureNodePoolImage(t *testing.T) {
expectedImageType: "",
expectedMarketplaceImage: nil,
},
{
name: "skip defaulting when RHELCoreOSExtensions is nil",

@coderabbitai coderabbitai Bot Jun 9, 2026

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.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use "When ... it should ..." format for test case name.

The test case name should follow the "When ... it should ..." format as per coding guidelines for unit tests.

📝 Suggested rename 
-			name: "skip defaulting when RHELCoreOSExtensions is nil",
+			name: "When RHELCoreOSExtensions is nil, it should skip defaulting",
🤖 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 `@hypershift-operator/controllers/nodepool/azure_test.go` at line 953, Rename
the test case name string from "skip defaulting when RHELCoreOSExtensions is
nil" to follow the "When ... it should ..." convention (for example: "When
RHELCoreOSExtensions is nil it should skip defaulting") in the test table entry
used by the test function (the table element with name: "...") so the case in
the test (where RHELCoreOSExtensions is nil) reads as "When ... it should ...".

Source: Coding guidelines

nodePool: &hyperv1.NodePool{
Spec: hyperv1.NodePoolSpec{
Arch: hyperv1.ArchitectureAMD64,
Platform: hyperv1.NodePoolPlatform{
Type: hyperv1.AzurePlatform,
Azure: &hyperv1.AzureNodePoolPlatform{
Image: hyperv1.AzureVMImage{},
},
},
},
},
releaseImage: &releaseinfo.ReleaseImage{
ImageStream: &imageapi.ImageStream{
ObjectMeta: metav1.ObjectMeta{Name: "4.20.0"},
},
StreamMetadata: &stream.Stream{
Architectures: map[string]stream.Arch{
"x86_64": {
Images: stream.Images{},
},
},
},
},
expectedImageType: "",
expectedMarketplaceImage: nil,
},
{
name: "skip defaulting when no marketplace metadata available",
nodePool: &hyperv1.NodePool{
Expand Down Expand Up @@ -1113,28 +1144,28 @@ func TestDefaultAzureNodePoolImage(t *testing.T) {

// createMockReleaseImage creates a mock release image for testing
func createMockReleaseImage(version string, hasMarketplaceMetadata bool) *releaseinfo.ReleaseImage {
architecture := releaseinfo.CoreOSArchitecture{
Artifacts: map[string]releaseinfo.CoreOSArtifact{},
Images: releaseinfo.CoreOSImages{},
RHCOS: releaseinfo.CoreRHCOSImage{
AzureDisk: releaseinfo.CoreAzureDisk{
architecture := stream.Arch{
Artifacts: map[string]stream.PlatformArtifacts{},
Images: stream.Images{},
RHELCoreOSExtensions: &rhcos.Extensions{
AzureDisk: &rhcos.AzureDisk{
Release: "9.6.20250701-0",
URL: "https://rhcos.blob.core.windows.net/imagebucket/rhcos-9.6.20250701-0-azure.x86_64.vhd",
},
},
}

if hasMarketplaceMetadata {
architecture.RHCOS.Marketplace = releaseinfo.CoreMarketplace{
Azure: releaseinfo.CoreAzureMarketplace{
NoPurchasePlan: releaseinfo.CoreAzureMarketplaceNoPurchasePlan{
HyperVGen1: &releaseinfo.CoreAzureMarketplaceImage{
architecture.RHELCoreOSExtensions.Marketplace = &rhcos.Marketplace{
Azure: &rhcos.AzureMarketplace{
NoPurchasePlan: &rhcos.AzureMarketplaceImages{
Gen1: &rhcos.AzureMarketplaceImage{
Publisher: "azureopenshift",
Offer: "aro4",
SKU: "aro_419",
Version: "419.6.20250523",
},
HyperVGen2: &releaseinfo.CoreAzureMarketplaceImage{
Gen2: &rhcos.AzureMarketplaceImage{
Publisher: "azureopenshift",
Offer: "aro4",
SKU: "419-v2",
Expand All @@ -1145,12 +1176,12 @@ func createMockReleaseImage(version string, hasMarketplaceMetadata bool) *releas
}
}

architectures := map[string]releaseinfo.CoreOSArchitecture{
architectures := map[string]stream.Arch{
"x86_64": architecture,
"aarch64": architecture, // ARM64 uses the same marketplace metadata
}

streamMetadata := &releaseinfo.CoreOSStreamMetadata{
streamMetadata := &stream.Stream{
Stream: "test-stream",
Architectures: architectures,
}
Expand Down
Loading