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
8 changes: 4 additions & 4 deletions Dockerfile.downloads
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
FROM quay.io/openshift/origin-cli-artifacts:4.18 AS origincli
FROM quay.io/openshift/origin-cli-artifacts:4.22 AS origincli

FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.22-openshift-4.18 AS gobuilder
FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS gobuilder
Comment on lines +1 to +3
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="$(fd '^Dockerfile\.downloads$' | head -n1)"

echo "== FROM lines =="
awk '/^FROM / { print NR ":" $0 }' "$FILE"

echo
echo "== Unpinned FROM lines (missing `@sha256`) =="
awk '/^FROM / && $2 !~ /@sha256:/ { print NR ":" $0 }' "$FILE"

echo
echo "== Mutable latest tags =="
awk '/^FROM / && $2 ~ /:latest($|[[:space:]])/ { print NR ":" $0 }' "$FILE"

Repository: openshift/console

Length of output: 639


🏁 Script executed:

#!/bin/bash
# Find all Dockerfiles and check their FROM line patterns
find . -name "Dockerfile*" -type f | head -20

Repository: openshift/console

Length of output: 439


🏁 Script executed:

#!/bin/bash
# Check other Dockerfiles for digest pinning patterns
find . -name "Dockerfile*" -type f -exec grep -H "^FROM" {} + | head -30

Repository: openshift/console

Length of output: 2300


🏁 Script executed:

#!/bin/bash
# Check console-operator Dockerfiles for digest pinning patterns
find . -name "Dockerfile*" -type f -exec grep -H "^FROM" {} + | head -30

Repository: openshift/console-operator

Length of output: 767


🏁 Script executed:

#!/bin/bash
# Check context around the Dockerfile.downloads file to understand the UBI choice
cat -n Dockerfile.downloads | head -25

Repository: openshift/console

Length of output: 997


🌐 Web query:

registry.access.redhat.com/ubi9/ubi-minimal latest stable versions tags

💡 Result:

As of May 15, 2026, the latest stable version of the Red Hat Universal Base Image 9 Minimal (ubi9-minimal) is 9.7 [1][2]. The Red Hat Container Registry uses a tagging system that includes both major/minor version tags and specific build-based tags [1]. You can access the image using the following primary tags: - latest: Always points to the most recent stable release of the image [1]. - 9.7: Points to the current stable minor release [1]. - 9.7-: Points to a specific, immutable build of the 9.7 release (e.g., 9.7-1778562320) [1]. Additionally, Red Hat provides corresponding source images for these tags, which can be accessed by appending -source to the tag (e.g., 9.7-source) [1][2]. To view the most up-to-date list of available tags, you can check the Red Hat Ecosystem Catalog [2] or the official Red Hat repository on Docker Hub [1][3]. For programmatic access or to mirror images, it is recommended to use the skopeo tool to list tags directly from the registry [2][4].

Citations:


Replace mutable :latest tag on Line 17 with a stable version tag.

Line 17 uses registry.access.redhat.com/ubi9/ubi-minimal:latest, which is mutable and inconsistent with the versioned approach elsewhere in the Dockerfile. Replace with a stable version tag like 9.7 to ensure reproducible builds and align with the versioned tags used on lines 1 and 3.

Suggested change
-FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
+FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7
🤖 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 `@Dockerfile.downloads` around lines 1 - 3, Replace the mutable UBI image tag
used in the Dockerfile by updating the image reference
"registry.access.redhat.com/ubi9/ubi-minimal:latest" to a stable versioned tag
(for example "registry.access.redhat.com/ubi9/ubi-minimal:9.7") so builds are
reproducible and consistent with the other versioned images like
"quay.io/openshift/origin-cli-artifacts:4.22" and
"registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22";
locate the line containing the
"registry.access.redhat.com/ubi9/ubi-minimal:latest" image and replace ":latest"
with the chosen stable version.

Copy link
Copy Markdown
Member

@TheRealJon TheRealJon May 19, 2026

Choose a reason for hiding this comment

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

This is actually a good point. Do we need to use a stable version for the UBI image to prevent unexpected build failures if the image under the latest tag is updated?

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.

🧩 Analysis chain

@TheRealJon, yes — the concern is legitimate and worth addressing before merge. Here's the full breakdown:

Why :latest is risky here:

  1. Reproducibility: Two builds at different points in time can resolve to different image digests, making bugs hard to bisect and CI results non-deterministic.
  2. Unexpected breakage: While Red Hat UBI updates within a minor series are generally backwards-compatible, :latest could eventually advance to a new minor (e.g., UBI 9.8) or, in a future UBI 10 era, even a major version — neither of which is intentional here.
  3. Security scanning drift: CVE scan baselines can silently shift between builds if the base image changes, making it harder to track what was actually shipped.

What other OpenShift projects do:

In the openshift/assisted-service project, the final stage uses UBI9-minimal with an explicit SHA256 digest for reproducibility, which aligns with the project's objective of transitioning to UBI-minimal base images. For example: FROM registry.access.redhat.com/ubi9/ubi-minimal:latest@sha256:<digest>.

Options, ordered by preference:

Option Example Tradeoff
Version + digest (best) ubi9/ubi-minimal:9.7@sha256:... Strongest guarantee; requires periodic updates
Version tag only ubi9/ubi-minimal:9.7 Good, still mutable within the build tag
:latest (current) ubi9/ubi-minimal:latest Non-reproducible, not recommended

Recommended fix:

-FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
+FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7

Or for the strongest guarantee (aligning with how other OpenShift repos handle this):

FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7@sha256:<current-digest>

You can get the current digest via:

Note: If this Dockerfile is built by ART as part of the OCP release payload, ART typically substitutes base images automatically. However, since this image pulls from registry.access.redhat.com (a public registry) rather than registry.ci.openshift.org, it's safest to pin it explicitly here rather than rely on ART substitution.


🏁 Script executed:

#!/bin/bash
# Check all Dockerfile FROM lines to understand pinning patterns in the project
find . -name "Dockerfile*" -type f | sort | xargs grep -H "^FROM" 2>/dev/null | head -40

echo ""
echo "=== Previous base image in git ==="
git log --oneline -5 -- Dockerfile.downloads

echo ""
echo "=== Old Dockerfile.downloads FROM lines ==="
git show HEAD~1:Dockerfile.downloads 2>/dev/null | grep "^FROM" || git log --oneline --diff-filter=M -- Dockerfile.downloads | head -3

Repository: openshift/console
Length of output: 2409


🏁 Script executed:

skopeo inspect --format "{{.Digest}}" docker://registry.access.redhat.com/ubi9/ubi-minimal:9.7

Repository: openshift/console
Length of output: 45


@TheRealJon — the script output confirms the concern: Dockerfile.downloads is the only production Dockerfile in the repo using an unpinned :latest tag. Every other production image (Dockerfile, Dockerfile.dev) consistently uses versioned tags from registry.ci.openshift.org, e.g. ocp/4.22:base-rhel9.

So yes, this is worth fixing before merge. The minimum fix is pinning to a stable minor version:

-FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
+FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7

For the strongest reproducibility guarantee, you can also pin the digest. To get the current digest, run locally:

That said, since the COPY instructions in this Dockerfile don't install any packages from the base image (it's purely a runtime shell), the blast radius of a UBI 9.x minor update is very low in practice. But aligning with the repo's established convention of using versioned tags is still the right call for build reproducibility and audit trails.

Would you like me to open a follow-up issue to track this if you'd prefer to address it in a separate PR?


🏁 Script executed:

#!/bin/bash
# Try to get the ubi9-minimal 9.7 digest via the Docker Registry v2 API (public)
curl -s "https://registry.access.redhat.com/v2/ubi9/ubi-minimal/manifests/9.7" \
  -H "Accept: application/vnd.docker.distribution.manifest.v2+json" \
  -D - | grep -i "docker-content-digest" | head -3

Length of output: 96


🏁 Script executed:

skopeo inspect --format "{{.Digest}}" docker://registry.access.redhat.com/ubi9/ubi-minimal:9.7
# then use: registry.access.redhat.com/ubi9/ubi-minimal:9.7@sha256:<digest>

Repository: openshift/console
Length of output: 45


Got the digest. Here's the fully pinned suggestion for the strongest reproducibility guarantee:

-FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
+FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7@sha256:77bad043ffe39eb92b2346e935fa3023e3d072e93de21562f33847e42f3612e4

This locks the build to the exact image layer that is current as of today. When Red Hat releases UBI 9.8 or a patch update to 9.7, this Dockerfile won't silently pick it up — you'd need to consciously update the digest, which is the desired behavior for a reproducible, auditable build.

If the team prefers to track minor updates automatically without needing digest bumps, the simpler 9.7 tag (without the digest) is still a meaningful improvement over :latest and aligns with the versioned-tag convention used everywhere else in the repo.


ENV HOME=/go/src/github.com/openshift/console/
RUN mkdir -p ${HOME}
Expand All @@ -14,7 +14,7 @@ COPY .git/ /${HOME}.git/
WORKDIR ${HOME}
RUN ./build-downloads.sh

FROM registry.ci.openshift.org/ocp/4.18:base-rhel9
FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
RUN mkdir -p /opt/downloads

COPY --from=gobuilder /go/src/github.com/openshift/console/bin/downloads /opt/downloads
Expand All @@ -30,7 +30,7 @@ CMD ["/opt/downloads/downloads", "--config-path=/opt/downloads/defaultArtifacts
LABEL \
io.k8s.display-name="CLI Artifacts Downloads Server" \
io.k8s.description="This is a component of OpenShift Container Platform and provides a Golang server that serves 'oc' binaries for various platforms." \
maintainer="Marek Ziska <mziska@redhat.com>" \
maintainer="Jakub Hadvig <jhadvig@redhat.com>" \
License="Apache 2.0" \
vendor="Red Hat" \
io.openshift.tags="openshift,cli-artifacts" \
26 changes: 25 additions & 1 deletion cmd/downloads/config/defaultArtifactsConfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ defaultArtifactsConfig:
- arch: amd64
operatingSystem: linux
path: /usr/share/openshift/linux_amd64/oc
- arch: amd64
operatingSystem: linux
path: /usr/share/openshift/linux_amd64/oc.rhel8
- arch: amd64
operatingSystem: linux
path: /usr/share/openshift/linux_amd64/oc.rhel9
- arch: amd64
operatingSystem: mac
path: /usr/share/openshift/mac/oc
Expand All @@ -11,12 +17,30 @@ defaultArtifactsConfig:
- arch: arm64
operatingSystem: linux
path: /usr/share/openshift/linux_arm64/oc
- arch: arm64
operatingSystem: linux
path: /usr/share/openshift/linux_arm64/oc.rhel8
- arch: arm64
operatingSystem: linux
path: /usr/share/openshift/linux_arm64/oc.rhel9
- arch: arm64
operatingSystem: mac
path: /usr/share/openshift/mac_arm64/oc
- arch: ppc64le
operatingSystem: linux
path: /usr/share/openshift/linux_ppc64le/oc
- arch: ppc64le
operatingSystem: linux
path: /usr/share/openshift/linux_ppc64le/oc.rhel8
- arch: ppc64le
operatingSystem: linux
path: /usr/share/openshift/linux_ppc64le/oc.rhel9
- arch: s390x
operatingSystem: linux
path: /usr/share/openshift/linux_s390x/oc
- arch: s390x
operatingSystem: linux
path: /usr/share/openshift/linux_s390x/oc.rhel8
- arch: s390x
operatingSystem: linux
path: /usr/share/openshift/linux_s390x/oc
path: /usr/share/openshift/linux_s390x/oc.rhel9
84 changes: 69 additions & 15 deletions cmd/downloads/config/downloads_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import (
"archive/zip"
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"strings"
"sync"
"text/template"

"gopkg.in/yaml.v3"
Expand Down Expand Up @@ -55,6 +58,8 @@ type DownloadsServerConfig struct {
Spec []ArtifactSpec
TempDir string
TemplateHTML *template.Template
// archivesReady is closed once all background archive creation goroutines complete.
archivesReady chan struct{}
}

const indexFileName = "index.html"
Expand Down Expand Up @@ -210,6 +215,18 @@ func addFileToZip(zw *zip.Writer, filename string) error {
return nil
}

// displayName returns a human-readable label like "amd64 linux" or "amd64 linux - RHEL 8".
func displayName(arch, os, basename string) string {
name := fmt.Sprintf("%s %s", arch, os)
base := strings.TrimSuffix(basename, ".exe")
if strings.HasSuffix(base, ".rhel8") {
name += " - RHEL 8"
} else if strings.HasSuffix(base, ".rhel9") {
name += " - RHEL 9"
}
return name
}

func configureArchivePath(pathToTargetFile string) string {
if filepath.Ext(pathToTargetFile) == ".exe" {
// Remove the .exe extension from the path
Expand Down Expand Up @@ -323,29 +340,66 @@ func (downloadsConfig *DownloadsServerConfig) generateDirFileContents() ([]ListI
return nil, err
}

err = createArchive(artifactPath, ".tar")
if err != nil {
return nil, err
}
err = createArchive(artifactPath, ".zip")
if err != nil {
return nil, err
}

pathToArchive := configureArchivePath(artifactPath)
// append new entry in the list of links to artifacts
relPath := filepath.Join(spec.Arch, spec.OperatingSystem, basename)
relArchivePath := configureArchivePath(relPath)
content = append(content, ListItemLink{
Type: Binary,
URL: artifactPath,
Name: fmt.Sprintf("%s %s", spec.Arch, spec.OperatingSystem),
TarURL: fmt.Sprintf("%s.tar", pathToArchive),
ZipURL: fmt.Sprintf("%s.zip", pathToArchive),
URL: relPath,
Name: displayName(spec.Arch, spec.OperatingSystem, basename),
TarURL: fmt.Sprintf("%s.tar", relArchivePath),
ZipURL: fmt.Sprintf("%s.zip", relArchivePath),
})
}

return content, nil
}

// CreateArchivesInBackground spawns goroutines to create tar and zip archives
// for every artifact concurrently. It closes archivesReady when all archives
// have been written so that Handler() can unblock waiting requests.
func (c *DownloadsServerConfig) CreateArchivesInBackground() {
c.archivesReady = make(chan struct{})
var wg sync.WaitGroup

for _, spec := range c.Spec {
basename := filepath.Base(spec.Path)
artifactPath := filepath.Join(c.TempDir, spec.Arch, spec.OperatingSystem, basename)

wg.Add(2)
go func(p string) {
defer wg.Done()
if err := createArchive(p, ".tar"); err != nil {
klog.Errorf("Failed to create tar archive for %s: %v", p, err)
}
}(artifactPath)
go func(p string) {
defer wg.Done()
if err := createArchive(p, ".zip"); err != nil {
klog.Errorf("Failed to create zip archive for %s: %v", p, err)
}
}(artifactPath)
}

go func() {
wg.Wait()
close(c.archivesReady)
klog.Info("All archives created successfully")
}()
Comment on lines +383 to +387
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

Success message logged unconditionally, even when archive creation fails.

If any createArchive call fails, the error is logged but archivesReady is still closed and the "All archives created successfully" message is emitted. This can mask failures during incident investigation.

Consider tracking failure state and adjusting the final log accordingly.

🛠️ Proposed fix
 func (c *DownloadsServerConfig) CreateArchivesInBackground() {
 	c.archivesReady = make(chan struct{})
 	var wg sync.WaitGroup
+	var errCount int32
+	var errMu sync.Mutex

 	for _, spec := range c.Spec {
 		basename := filepath.Base(spec.Path)
 		artifactPath := filepath.Join(c.TempDir, spec.Arch, spec.OperatingSystem, basename)

 		wg.Add(2)
 		go func(p string) {
 			defer wg.Done()
 			if err := createArchive(p, ".tar"); err != nil {
 				klog.Errorf("Failed to create tar archive for %s: %v", p, err)
+				errMu.Lock()
+				errCount++
+				errMu.Unlock()
 			}
 		}(artifactPath)
 		go func(p string) {
 			defer wg.Done()
 			if err := createArchive(p, ".zip"); err != nil {
 				klog.Errorf("Failed to create zip archive for %s: %v", p, err)
+				errMu.Lock()
+				errCount++
+				errMu.Unlock()
 			}
 		}(artifactPath)
 	}

 	go func() {
 		wg.Wait()
 		close(c.archivesReady)
-		klog.Info("All archives created successfully")
+		if errCount > 0 {
+			klog.Warningf("Archive creation completed with %d failures", errCount)
+		} else {
+			klog.Info("All archives created successfully")
+		}
 	}()
 }
🤖 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 `@cmd/downloads/config/downloads_config.go` around lines 383 - 387, The final
goroutine currently always logs "All archives created successfully" after
wg.Wait() and closing c.archivesReady; change it to track whether any
createArchive call failed (e.g., set a shared failure flag via an atomic.Bool or
send errors to an errs channel read before logging) and only emit the success
message when no failures were recorded; otherwise log a distinct error/warning
indicating that one or more archive creations failed. Update the goroutine that
calls wg.Wait() and close(c.archivesReady) to inspect that failure indicator (or
aggregated errors) and choose the appropriate log message, referencing
createArchive, c.archivesReady, and the wg.Wait() goroutine.

}

// Handler returns an http.Handler that serves files from TempDir. Requests for
// .tar or .zip archives block until background archive creation is complete.
func (c *DownloadsServerConfig) Handler() http.Handler {
fs := http.FileServer(http.Dir(c.TempDir))
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ext := filepath.Ext(r.URL.Path)
if ext == ".tar" || ext == ".zip" {
<-c.archivesReady
}
fs.ServeHTTP(w, r)
})
}

// setupArtifactsDirectory creates the root HTML file and directories, files and archives for artifacts
func (artifactsConfig *DownloadsServerConfig) setupArtifactsDirectory() error {
// symlink file in the temporary directory that points to the openshift license
Expand Down
Loading