Skip to content
Closed
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
241 changes: 241 additions & 0 deletions cmd/release-controller-api/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ func (c *Controller) userInterfaceHandler() http.Handler {
mux.HandleFunc("/api/v1/releasestream/{release}/latest", c.apiReleaseLatest)
mux.HandleFunc("/api/v1/releasestream/{release}/candidate", c.apiReleaseCandidate)
mux.HandleFunc("/api/v1/releasestream/{release}/release/{tag}", c.apiReleaseInfo)
mux.HandleFunc("/api/v1/releasestream/{release}/release/{tag}/nodeimageinfo", c.apiNodeImageInfo)
mux.HandleFunc("/api/v1/releasestream/{release}/config", c.apiReleaseConfig)
mux.HandleFunc("/api/v1/releasestreams/accepted", c.apiAcceptedStreams)
mux.HandleFunc("/api/v1/releasestreams/rejected", c.apiRejectedStreams)
Expand Down Expand Up @@ -727,6 +728,246 @@ func (c *Controller) apiReleaseInfo(w http.ResponseWriter, req *http.Request) {
fmt.Fprintln(w)
}

func writeAPIError(w http.ResponseWriter, code int, message string) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(code)
response := releasecontroller.APIError{
Code: code,
Message: message,
}
data, err := json.MarshalIndent(&response, "", " ")
if err != nil {
klog.Errorf("Failed to marshal API error response: %v", err)
return
}
w.Write(data)
fmt.Fprintln(w)
}
Comment on lines +731 to +745
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

Unchecked w.Write error on line 743.

For consistency with writeAPIResponse (line 753-754), the write error should be logged even though it cannot be recovered from in an error path.

🔧 Proposed fix
-	w.Write(data)
+	if _, err := w.Write(data); err != nil {
+		klog.Errorf("Failed to write API error response: %v", err)
+	}
📝 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
func writeAPIError(w http.ResponseWriter, code int, message string) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(code)
response := releasecontroller.APIError{
Code: code,
Message: message,
}
data, err := json.MarshalIndent(&response, "", " ")
if err != nil {
klog.Errorf("Failed to marshal API error response: %v", err)
return
}
w.Write(data)
fmt.Fprintln(w)
}
func writeAPIError(w http.ResponseWriter, code int, message string) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(code)
response := releasecontroller.APIError{
Code: code,
Message: message,
}
data, err := json.MarshalIndent(&response, "", " ")
if err != nil {
klog.Errorf("Failed to marshal API error response: %v", err)
return
}
if _, err := w.Write(data); err != nil {
klog.Errorf("Failed to write API error response: %v", err)
}
fmt.Fprintln(w)
}
🧰 Tools
🪛 golangci-lint (2.11.4)

[error] 743-743: Error return value of w.Write is not checked

(errcheck)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/release-controller-api/http.go` around lines 731 - 745, In writeAPIError,
the result of w.Write(data) is not checked; update writeAPIError to capture and
log the error from w.Write (and also from fmt.Fprintln) using klog.Errorf just
like writeAPIResponse does: after marshalling, assign err = w.Write(data) and if
err != nil log it with a clear message referencing writeAPIError, and likewise
check the error returned by fmt.Fprintln(w) and log it if non-nil so both write
calls are consistently logged on failure.


func writeAPIResponse(w http.ResponseWriter, response interface{}) error {
data, err := json.MarshalIndent(response, "", " ")
if err != nil {
return err
}
w.Header().Set("Content-Type", "application/json")
if _, err := w.Write(data); err != nil {
klog.Errorf("Failed to write API response: %v", err)
}
fmt.Fprintln(w)
return nil
}

func (c *Controller) apiNodeImageInfo(w http.ResponseWriter, req *http.Request) {
start := time.Now()
defer func() { klog.V(4).Infof("rendered in %s", time.Since(start)) }()

vars := mux.Vars(req)
tag := vars["tag"]
from := req.URL.Query().Get("from")

// Parse target tag version and check if 4.19+
version, err := releasecontroller.SemverParseTolerant(tag)
if err != nil {
writeAPIError(w, http.StatusBadRequest,
fmt.Sprintf("Unable to parse version from tag '%s': %v", tag, err))
return
}

// Node Image Info is only available for OCP 4.19+
if version.Major == 4 && version.Minor < 19 {
writeAPIError(w, http.StatusBadRequest,
fmt.Sprintf("Node Image Info is only available for OCP 4.19 and later. Tag '%s' is version %d.%d.",
tag, version.Major, version.Minor))
return
}

// If from is provided, validate it is also 4.19+ and same minor version
if from != "" {
fromVersion, err := releasecontroller.SemverParseTolerant(from)
if err != nil {
writeAPIError(w, http.StatusBadRequest,
fmt.Sprintf("Unable to parse version from 'from' tag '%s': %v", from, err))
return
}
if fromVersion.Major == 4 && fromVersion.Minor < 19 {
writeAPIError(w, http.StatusBadRequest,
fmt.Sprintf("Node Image Info is only available for OCP 4.19 and later. 'from' tag '%s' is version %d.%d.",
from, fromVersion.Major, fromVersion.Minor))
return
}
// Require same major.minor version for comparisons
if version.Major != fromVersion.Major || version.Minor != fromVersion.Minor {
writeAPIError(w, http.StatusBadRequest,
fmt.Sprintf("Cannot compare releases from different minor versions. Tag '%s' is version %d.%d, but 'from' tag '%s' is version %d.%d.",
tag, version.Major, version.Minor, from, fromVersion.Major, fromVersion.Minor))
return
}
}

// Look up tags
release := vars["release"]
tagsToFind := []string{tag}
if from != "" {
tagsToFind = append(tagsToFind, from)
}
tags, ok := c.findReleaseStreamTags(false, tagsToFind...)
if !ok {
for k, v := range tags {
if v == nil {
writeAPIError(w, http.StatusNotFound,
fmt.Sprintf("Release tag not found: %s", k))
return
}
}
}

// Validate tag belongs to the specified release stream
if len(release) > 0 && tags[tag].Release.Config.Name != release {
writeAPIError(w, http.StatusNotFound,
fmt.Sprintf("Release tag %s does not belong to release %s", tag, release))
return
}

// Get pullspec for target tag
toPullSpec := releasecontroller.FindPublicImagePullSpec(tags[tag].Release.Target, tag)
if toPullSpec == "" {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Release target %s does not have a configured registry", tags[tag].Release.Target.Name))
return
}

// Get image info for target to get digest-based pullspec
toImageInfo, err := releasecontroller.GetImageInfo(c.releaseInfo, c.architecture, toPullSpec)
if err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to get image info for tag '%s': %v", tag, err))
return
}
toDigestPullSpec := toImageInfo.GenerateDigestPullSpec()

// Discover available CoreOS streams in the target release
streams, err := c.releaseInfo.ListMachineOSStreams(toDigestPullSpec)
if err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to list machine-os streams: %v", err))
return
}

// If no ?from= parameter, return full package list
if from == "" {
c.apiNodeImageInfoList(w, tag, toDigestPullSpec, streams)
return
}

// Get pullspec for from tag
fromPullSpec := releasecontroller.FindPublicImagePullSpec(tags[from].Release.Target, from)
if fromPullSpec == "" {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Release target %s does not have a configured registry", tags[from].Release.Target.Name))
return
}

// Get image info for from tag to get digest-based pullspec
fromImageInfo, err := releasecontroller.GetImageInfo(c.releaseInfo, c.architecture, fromPullSpec)
if err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to get image info for 'from' tag '%s': %v", from, err))
return
}
fromDigestPullSpec := fromImageInfo.GenerateDigestPullSpec()

// Return diff between from and to
c.apiNodeImageInfoDiff(w, from, tag, fromDigestPullSpec, toDigestPullSpec, streams)
}

// apiNodeImageInfoList returns the full RPM package list for each stream (no diff)
func (c *Controller) apiNodeImageInfoList(w http.ResponseWriter, tag, pullspec string, streams []releasecontroller.MachineOSStreamInfo) {
var response releasecontroller.APINodeImageInfo

if len(streams) == 0 {
// Fallback for older releases without discoverable streams
rpmList, err := c.releaseInfo.RpmList(pullspec)
if err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to get RPM list: %v", err))
return
}
response.Streams = []releasecontroller.APINodeImageStream{{
Name: "rhel-coreos",
Packages: rpmList.Packages,
Extensions: rpmList.Extensions,
}}
} else {
// Get RPM list for each stream
for _, stream := range streams {
extTag := stream.Tag + "-extensions"
rpmList, err := c.releaseInfo.RpmListForStream(pullspec, stream.Tag, extTag)
if err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to get RPM list for stream %s: %v", stream.Tag, err))
return
}
response.Streams = append(response.Streams, releasecontroller.APINodeImageStream{
Name: stream.Tag,
DisplayName: stream.DisplayName,
Packages: rpmList.Packages,
Extensions: rpmList.Extensions,
})
}
}

if err := writeAPIResponse(w, &response); err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to marshal response: %v", err))
}
}

// apiNodeImageInfoDiff returns the RPM package diff between two releases for each stream
func (c *Controller) apiNodeImageInfoDiff(w http.ResponseWriter, from, to, fromPullSpec, toPullSpec string, streams []releasecontroller.MachineOSStreamInfo) {
response := releasecontroller.APINodeImageDiff{
From: from,
To: to,
}

if len(streams) == 0 {
// Fallback for older releases without discoverable streams
rpmDiff, err := c.releaseInfo.RpmDiff(fromPullSpec, toPullSpec)
if err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to get RPM diff: %v", err))
return
}
response.Streams = []releasecontroller.APINodeImageStreamDiff{{
Name: "rhel-coreos",
Changed: rpmDiff.Changed,
Added: rpmDiff.Added,
Removed: rpmDiff.Removed,
}}
} else {
// Get RPM diff for each stream
for _, stream := range streams {
rpmDiff, err := c.releaseInfo.RpmDiffForStream(fromPullSpec, toPullSpec, stream.Tag)
if err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to get RPM diff for stream %s: %v", stream.Tag, err))
return
}
response.Streams = append(response.Streams, releasecontroller.APINodeImageStreamDiff{
Name: stream.Tag,
DisplayName: stream.DisplayName,
Changed: rpmDiff.Changed,
Added: rpmDiff.Added,
Removed: rpmDiff.Removed,
})
}
}

if err := writeAPIResponse(w, &response); err != nil {
writeAPIError(w, http.StatusInternalServerError,
fmt.Sprintf("Unable to marshal response: %v", err))
}
}

func (c *Controller) changeLogWorker(result *renderResult, tagInfo *releaseTagInfo, format string) {
ch := make(chan renderResult)

Expand Down
63 changes: 58 additions & 5 deletions pkg/release-controller/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,59 @@ type APIReleaseInfo struct {
ChangeLogJson ChangeLog `json:"changeLogJson,omitempty"`
}

// APINodeImageInfo contains RPM package information for the node image.
// For releases with multiple CoreOS streams (4.21+), Streams contains info for each stream.
// For older releases with a single stream, Streams contains one entry.
type APINodeImageInfo struct {
// Streams contains RPM package information for each CoreOS stream in the release
Streams []APINodeImageStream `json:"streams"`
}

// APINodeImageStream contains RPM package information for a single CoreOS stream.
type APINodeImageStream struct {
// Name is the component tag name (e.g., "rhel-coreos", "rhel-coreos-10")
Name string `json:"name"`
// DisplayName is the human-readable name (e.g., "Red Hat Enterprise Linux CoreOS 9.8")
DisplayName string `json:"displayName,omitempty"`
// Packages is a map of package name to version for all packages in this stream
Packages map[string]string `json:"packages"`
// Extensions is a map of extension package name to version for this stream
Extensions map[string]string `json:"extensions"`
}

// APINodeImageDiff contains the RPM package diff between two releases.
// This is returned when the ?from= query parameter is provided.
type APINodeImageDiff struct {
// From is the tag name of the base release being compared from
From string `json:"from"`
// To is the tag name of the target release being compared to
To string `json:"to"`
// Streams contains the package diff for each CoreOS stream
Streams []APINodeImageStreamDiff `json:"streams"`
}

// APINodeImageStreamDiff contains the RPM package diff for a single CoreOS stream.
type APINodeImageStreamDiff struct {
// Name is the component tag name (e.g., "rhel-coreos", "rhel-coreos-10")
Name string `json:"name"`
// DisplayName is the human-readable name (e.g., "Red Hat Enterprise Linux CoreOS 9.8")
DisplayName string `json:"displayName,omitempty"`
// Changed contains packages that have different versions between releases
Changed map[string]RpmChangedDiff `json:"changed,omitempty"`
// Added contains packages that exist in the target but not in the base release
Added map[string]string `json:"added,omitempty"`
// Removed contains packages that exist in the base but not in the target release
Removed map[string]string `json:"removed,omitempty"`
}

// APIError represents an error response from the API.
type APIError struct {
// Code is the HTTP status code
Code int `json:"code"`
// Message describes the error
Message string `json:"message"`
}

// Release holds information about the release used during processing.
type Release struct {
// Source is the image stream that the Config was loaded from and holds all
Expand Down Expand Up @@ -391,11 +444,11 @@ type ProwJobVerification struct {
}

type VerificationStatus struct {
State string `json:"state"`
URL string `json:"url"`
Retries int `json:"retries,omitempty"`
PreviousAttemptURLs []string `json:"previousAttemptURLs,omitempty"`
TransitionTime *metav1.Time `json:"transitionTime,omitempty"`
State string `json:"state"`
URL string `json:"url"`
Retries int `json:"retries,omitempty"`
PreviousAttemptURLs []string `json:"previousAttemptURLs,omitempty"`
TransitionTime *metav1.Time `json:"transitionTime,omitempty"`
}

type VerificationStatusMap map[string]*VerificationStatus
Expand Down