Skip to content

CONSOLE-5293: Use console-downloads image instead of cli-artifacts#1158

Open
jhadvig wants to merge 1 commit into
openshift:mainfrom
jhadvig:CONSOLE-5293
Open

CONSOLE-5293: Use console-downloads image instead of cli-artifacts#1158
jhadvig wants to merge 1 commit into
openshift:mainfrom
jhadvig:CONSOLE-5293

Conversation

@jhadvig

@jhadvig jhadvig commented May 15, 2026

Copy link
Copy Markdown
Member

Analysis / Root cause:

The downloads deployment currently uses the cli-artifacts image and runs an inline Python HTTP server script to serve CLI binaries. This approach is being replaced by a dedicated console-downloads image
(openshift/console Dockerfile.downloads) that contains a purpose-built Golang HTTP server with its own entrypoint, making the inline Python
script unnecessary.

Jira: https://redhat.atlassian.net/browse/CONSOLE-5293

Solution description:

  1. manifests/07-operator.yaml and manifests/07-operator-ibm-cloud-managed.yaml — Updated the DOWNLOADS_IMAGE env var from registry.svc.ci.openshift.org/openshift:cli-artifacts to
    registry.svc.ci.openshift.org/openshift:console-downloads.
  2. bindata/assets/deployments/downloads-deployment.yaml — Removed the inline Python HTTP server script (command, args), the /tmp volumeMount, and the tmp emptyDir volume. The new
    console-downloads image has its own entrypoint (/opt/downloads/downloads --config-path=/opt/downloads/defaultArtifactsConfig.yaml), so no custom command/args are needed.

No Go code changes were required — the operator reads the deployment YAML from bindata and sets the image from the DOWNLOADS_IMAGE env var, which continues to work as before.

Test setup:

The console-downloads image must be present in the release payload. Ensure the image is built and available via the CI config in openshift/release.

Test cases:

  • Verify the downloads deployment starts successfully with the new console-downloads image
  • Verify the downloads page at https://downloads-openshift-console.apps.<cluster>/ serves CLI binaries correctly
  • Verify oc download links for all architectures (amd64, arm64, ppc64le, s390x) and platforms (linux, mac, windows) are functional
  • Verify the liveness and readiness probes pass on port 8080
  • Verify the operator reconciles correctly after a fresh install
  • Verify the operator reconciles correctly on upgrade from a version using cli-artifacts

Needs to be tested with openshift/console#16455

Depends on: openshift/release#79355

Browser conformance:

  • Chrome
  • Firefox
  • Safari (or Epiphany on Linux)

N/A — this is a backend/operator change with no UI modifications. Downloads page was verified accessible via Chrome.

Additional info:

This is a net deletion of ~200 lines of inline Python. The new Golang-based downloads server in the console-downloads image provides the same functionality with better performance and maintainability.

Summary by CodeRabbit

  • Chores
    • Simplified the downloads deployment startup to run the download-server binary directly with the default artifacts configuration.
    • Updated the downloads-related image reference used by the operator to the console-downloads image for more consistent runtime assets.
  • Tests
    • Adjusted downloads deployment test expectations to match the new command and arguments.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 15, 2026
@openshift-ci-robot

openshift-ci-robot commented May 15, 2026

Copy link
Copy Markdown
Contributor

@jhadvig: This pull request references CONSOLE-5293 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Analysis / Root cause:

The downloads deployment currently uses the cli-artifacts image and runs an inline Python HTTP server script to serve CLI binaries. This approach is being replaced by a dedicated console-downloads image
(openshift/console Dockerfile.downloads) that contains a purpose-built Golang HTTP server with its own entrypoint, making the inline Python
script unnecessary.

Jira: https://redhat.atlassian.net/browse/CONSOLE-5293

Solution description:

  1. manifests/07-operator.yaml and manifests/07-operator-ibm-cloud-managed.yaml — Updated the DOWNLOADS_IMAGE env var from registry.svc.ci.openshift.org/openshift:cli-artifacts to
    registry.svc.ci.openshift.org/openshift:console-downloads.
  2. bindata/assets/deployments/downloads-deployment.yaml — Removed the inline Python HTTP server script (command, args), the /tmp volumeMount, and the tmp emptyDir volume. The new
    console-downloads image has its own entrypoint (/opt/downloads/downloads --config-path=/opt/downloads/defaultArtifactsConfig.yaml), so no custom command/args are needed.

No Go code changes were required — the operator reads the deployment YAML from bindata and sets the image from the DOWNLOADS_IMAGE env var, which continues to work as before.

Test setup:

The console-downloads image must be present in the release payload. Ensure the image is built and available via the CI config in openshift/release.

Test cases:

  • Verify the downloads deployment starts successfully with the new console-downloads image
  • Verify the downloads page at https://downloads-openshift-console.apps.<cluster>/ serves CLI binaries correctly
  • Verify oc download links for all architectures (amd64, arm64, ppc64le, s390x) and platforms (linux, mac, windows) are functional
  • Verify the liveness and readiness probes pass on port 8080
  • Verify the operator reconciles correctly after a fresh install
  • Verify the operator reconciles correctly on upgrade from a version using cli-artifacts

Needs to be tested with openshift/console#16455

Browser conformance:

  • Chrome
  • Firefox
  • Safari (or Epiphany on Linux)

N/A — this is a backend/operator change with no UI modifications. Downloads page was verified accessible via Chrome.

Additional info:

This is a net deletion of ~200 lines of inline Python. The new Golang-based downloads server in the console-downloads image provides the same functionality with better performance and maintainability.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 36ffd62d-9425-4501-b870-5ec5a378786e

📥 Commits

Reviewing files that changed from the base of the PR and between 97735a7 and 8fc3133.

📒 Files selected for processing (4)
  • bindata/assets/deployments/downloads-deployment.yaml
  • manifests/07-operator-ibm-cloud-managed.yaml
  • manifests/07-operator.yaml
  • pkg/console/subresource/deployment/deployment_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift/console (manual)
🚧 Files skipped from review as they are similar to previous changes (2)
  • manifests/07-operator.yaml
  • manifests/07-operator-ibm-cloud-managed.yaml
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Follow Go coding standards and patterns documented in CONVENTIONS.md
Organize imports according to conventions documented in CONVENTIONS.md
Use gofmt to format Go code with standard formatting
Run go vet checks on all Go packages

Follow Go coding standards and patterns as documented in CONVENTIONS.md, including proper import organization

Organize Go code following the repository structure: main entry point in cmd/console/main.go, API constants in pkg/api/, operator command setup in pkg/cmd/operator/, and version command in pkg/cmd/version/

**/*.go: Use gofmt for formatting Go code
Follow standard Go naming conventions
Group imports in order: standard lib, 3rd party, kube/openshift, internal (marked with comments)
Use meaningful error messages with context in Go code
Set status conditions using status.Handle* functions with type prefixes (*Degraded, *Progressing, *Available, *Upgradeable)
Use typed errors and wrap errors to preserve stack context

**/*.go: Imports must be grouped with comments in the order: standard lib, 3rd party, kube, openshift, and operator (internal)
Wrap errors with context using fmt.Errorf("failed to X: %w", err) pattern to provide meaningful error messages

**/*.go: Do NOT use deprecated ioutil package functions. Use os.ReadFile() instead of ioutil.ReadFile(), os.WriteFile() instead of ioutil.WriteFile(), and io.ReadAll() instead of ioutil.ReadAll() (Go 1.16+)
Do NOT use net.Dial() directly. Use (&net.Dialer{}).DialContext() to support context cancellation
Use %w verb in fmt.Errorf() to wrap errors and maintain error chains, not %v which loses the error chain
Add descriptive context to errors when wrapping them, including relevant identifiers and state information
Use specific error type checks (e.g., apierrors.IsNotFound()) instead of string matching with strings.Contains() on error.Error()
Always propagate the parent context in function calls instead of using context.Background(), to respect parent cont...

Files:

  • pkg/console/subresource/deployment/deployment_test.go

⚙️ CodeRabbit configuration file

**/*.go: Review Go code following OpenShift operator patterns.
See CONVENTIONS.md for coding standards and patterns.

Refer to the following skills based on CODE PATTERNS, not just file paths:

Refer to /controller-review when code contains:

  • Controller struct types (e.g., type *Controller struct)
  • func New*Controller( factory functions
  • factory.New().WithFilteredEventsInformers( pattern
  • .ToController( method calls
  • Sync(ctx context.Context, controllerContext factory.SyncContext) methods
  • operatorConfig.Spec.ManagementState checks
  • status.NewStatusHandler or status.Handle* functions

Refer to /sync-handler-review when code contains:

  • Main operator sync functions (e.g., sync_v400.go content)
  • Sequential resource syncing with early returns
  • Incremental reconciliation loops
  • Multiple resourceapply.Apply*() calls in sequence
  • Dependency ordering of ConfigMaps → Secrets → Service Accounts → RBAC → Services → Deployments → Routes
  • Feature gate conditional logic

Refer to /go-quality-review for all Go code to check:

  • Deprecated imports: ioutil.ReadFile, ioutil.WriteFile, ioutil.ReadAll
  • Deprecated patterns: Dial without DialContext
  • Error handling: missing %w in fmt.Errorf
  • Code smells: deep nesting (4+ levels), functions >100 lines
  • Magic values: unexplained numbers/strings
  • Context propagation: context.Background() instead of passed ctx
  • Missing godoc on exported functions

Files:

  • pkg/console/subresource/deployment/deployment_test.go
**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

Follow testing patterns and commands documented in TESTING.md

Follow testing patterns and commands as documented in TESTING.md, including running unit tests with 'make test-unit' and checks with 'make check'

**/*_test.go: Use table-driven tests for comprehensive coverage
Use httptest for HTTP handler testing in Go
Include proper cleanup functions in tests
Test both success and failure paths

Check and handle all errors in tests using t.Fatalf() or t.Errorf(); do not ignore errors with blank identifiers

Files:

  • pkg/console/subresource/deployment/deployment_test.go

⚙️ CodeRabbit configuration file

**/*_test.go: Review test code for quality and patterns.

Refer to /unit-test-review when test is in pkg//*_test.go:**

  • Table-driven test structure with test cases
  • Use of go-test/deep for struct comparisons
  • Test naming conventions (TestFunctionName)
  • Error handling with wantErr pattern
  • Edge case coverage (nil, empty, boundary values)
  • Proper assertions with helpful error messages
  • Test isolation (no shared mutable state)

Refer to /e2e-test-review when test contains:

  • framework.MustNewClientset(t, nil) or similar e2e framework usage
  • wait.Poll or wait.PollImmediate patterns
  • retry.RetryOnConflict for updates
  • Cleanup via defer functions
  • Console/operator CR manipulations
  • Test assertions on cluster state

Suggest to use /e2e-test-review when:

  • PR adds new feature requiring e2e coverage
  • Test file is empty or skeleton
  • Comments indicate "TODO: add test"

Review for common issues:

  • Missing cleanup (defer statements)
  • Using time.Sleep instead of wait.Poll
  • Missing context timeouts
  • Vague error messages in assertions
  • Tests without table-driven structure when testing multiple cases
  • Ignoring errors with _
  • Tests without assertions

Files:

  • pkg/console/subresource/deployment/deployment_test.go
{pkg,cmd}/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Use gofmt for code formatting on pkg and cmd directories

{pkg,cmd}/**/*.go: Format code using gofmt -w ./pkg ./cmd
Run go vet checks on all Go packages in ./pkg and ./cmd

Files:

  • pkg/console/subresource/deployment/deployment_test.go
pkg/console/subresource/**/*.go

📄 CodeRabbit inference engine (ARCHITECTURE.md)

Use pkg/console/subresource/ packages for resource builders, with separate packages for each resource type (authentication, configmap, deployment, oauthclient, route, secret, etc.)

Files:

  • pkg/console/subresource/deployment/deployment_test.go
pkg/**/*_test.go

📄 CodeRabbit inference engine (.claude/skills/unit-test-review.md)

pkg/**/*_test.go: Go unit tests in pkg/**/*_test.go files should use table-driven test pattern for clarity and completeness with struct containing test cases
Test function names should be descriptive and clearly indicate what is being tested (e.g., TestGetNodeComputeEnvironments, TestNewRouteConfig)
Test case names within table-driven tests should explain the scenario being tested (e.g., 'Custom hostname and TLS secret set'), not be vague (e.g., 'test1')
Use github.com/go-test/deep package for struct comparisons to show exact differences instead of simple equality checks or manual field-by-field comparisons
Test both success and failure paths, including edge cases such as empty inputs (nil, "", empty slices/maps), boundary values (0, -1, max int), missing labels/fields, duplicate values, and large inputs
Organize test code using Arrange-Act-Assert pattern: setup test data in Arrange phase, call the function in Act phase, verify results in Assert phase
Check error presence using pattern (err != nil) != tt.wantErr instead of ignoring errors with underscore or missing error checks
When checking specific error messages, use strings.Contains to verify error content instead of just checking error presence
Extract common test setup into helper functions rather than duplicating setup code across multiple tests
Write specific assertion error messages with context (e.g., 'expected %d nodes, got %d') rather than vague messages (e.g., 'failed')
Inline simple test data in test structs; extract complex fixtures into testdata files or helper functions
Avoid red flags in tests: no table-driven tests for multi-scenario functions, tests without subtests, tests depending on execution order, global mutable state, hardcoded sleeps, tests without assertions, ignoring errors with underscore, testing implementation details instead of behavior, or overly complex test setup

Files:

  • pkg/console/subresource/deployment/deployment_test.go
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}

⚙️ CodeRabbit configuration file

**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}: Injection prevention (prodsec-skills):

  • SQL: parameterized queries only; no string concatenation
  • Command: no shell=True, os.system, or backtick exec with user input
  • LDAP/XPath: escape special characters in filters
  • Path traversal: canonicalize paths, reject ../
  • Deserialization: no pickle/yaml.load()/eval on untrusted data
  • Prototype pollution: no recursive merge of untrusted objects
  • Validate at trust boundaries with allow-lists, not deny-lists
  • Normalize Unicode and anchor regexes (^$); watch for ReDoS

Files:

  • pkg/console/subresource/deployment/deployment_test.go
**

⚙️ CodeRabbit configuration file

**: # OpenShift Console Operator - AI Context Hub

This file serves as the central AI documentation hub for the OpenShift Console Operator project. AI assistants (Claude, Cursor, Copilot, CodeRabbit, etc.) use this and the linked documents to understand project context.

Go Version and Dependencies

Go Version and Dependencies

  • Go version: 1.24.0 (toolchain: go1.24.4)
  • Dependency management: Uses go.mod with vendoring
  • Build flags: Use GOFLAGS="-mod=vendor" for builds and tests to ensure vendored dependencies are used
  • Key dependencies: openshift/api, openshift/library-go, k8s.io client libraries
  • Go version: 1.24.0 (toolchain: go1.24.4)
  • Dependency management: Uses go.mod with vendoring
  • Build flags: Use GOFLAGS="-mod=vendor" for builds and tests to ensure vendored dependencies are used
  • Key dependencies: openshift/api, openshift/library-go, k8s.io client libraries

Quick Reference

This Repository

Document Purpose
ARCHITECTURE.md System architecture, components, repository structure
CONVENTIONS.md Go coding standards, patterns, import organization
TESTING.md Testing patterns, commands, debugging
README.md Project README with setup instructions

Console Repository (openshift/console)

For frontend-related guidelines, see the openshift/console repository:

Document Purpose
STYLEGUIDE.md Frontend code style guidelines
INTERNATIONALIZATION.md i18n patterns and translation guidelines
CONTRIBUTING.md Contribution guidelines for the console project

Project Summary

The **console-operator...

Files:

  • pkg/console/subresource/deployment/deployment_test.go
  • bindata/assets/deployments/downloads-deployment.yaml
bindata/assets/**/*.yaml

📄 CodeRabbit inference engine (ARCHITECTURE.md)

Store static YAML assets in bindata/assets/ organized by resource type (configmaps/, deployments/, pdb/, routes/, services/)

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
{manifests,bindata/assets,quickstarts,examples}/**/*.yaml

📄 CodeRabbit inference engine (.claude/skills/manifest-review.md)

{manifests,bindata/assets,quickstarts,examples}/**/*.yaml: CVO manifests MUST include cluster profile annotations: include.release.openshift.io/hypershift, include.release.openshift.io/ibm-cloud-managed, include.release.openshift.io/self-managed-high-availability, include.release.openshift.io/single-node-developer, and capability.openshift.io/name: Console
Console resources should include capability.openshift.io/name: Console annotation
Kubernetes apiGroups in RBAC manifests must be correct (empty string for core API, specific groups for CRDs)
Resources in manifests must maintain namespace consistency: console workload resources in openshift-console namespace, operator resources in openshift-console-operator namespace
YAML manifests must use proper indentation (2 spaces), consistent field ordering, and include --- separators between multiple resources
When binding roles to service accounts, use console-operator SA for cross-namespace permissions and console SA for same-namespace console workload bindings
New manifests must not have missing cluster profile annotations to ensure resources are deployed to appropriate clusters

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
**/*.{yaml,yml,json}

📄 CodeRabbit inference engine (Custom checks)

**/*.{yaml,yml,json}: Flag privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN capability, running as root without justification, and allowPrivilegeEscalation: true in container/Kubernetes manifests
When deployment manifests, operator code, or controllers are added/modified, ensure they do not introduce scheduling constraints assuming standard HA topology (3+ control-plane nodes, dedicated workers). Flag: required pod anti-affinity with maxUnavailable: 0 (deadlocks on SNO/TNF/TNA), pod topology spread with DoNotSchedule and hostname key (breaks on SNO), replica counts derived from node count without topology awareness, nodeSelector/node affinity targeting control-plane nodes (fails on HyperShift), scheduling to all control-plane nodes equally without excluding arbiter nodes (TNA), assuming dedicated worker nodes exist (SNO/TNF), or PodDisruptionBudgets designed for 3+ nodes (TNF/TNA). Do not flag if change checks ControlPlaneTopology, node counts, or topology labels before applying constraints.

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
**/*.yaml

⚙️ CodeRabbit configuration file

**/*.yaml: Review YAML manifests based on content and kind.

Refer to /manifest-review when YAML contains:

  • kind: Role or kind: ClusterRole (RBAC review)
  • kind: RoleBinding or kind: ClusterRoleBinding
  • annotations: section (check for cluster profiles)
  • verbs: ["*"] or wildcard permissions
  • apiGroups: ["*"] or overly broad permissions
  • ServiceAccount references in subjects

Check for required annotations in manifests/:

  • include.release.openshift.io/hypershift
  • include.release.openshift.io/ibm-cloud-managed
  • include.release.openshift.io/self-managed-high-availability
  • include.release.openshift.io/single-node-developer
  • capability.openshift.io/name: Console

For quickstarts/, additionally check:

  • QuickStart spec structure
  • Task descriptions and prerequisites
  • See quickstarts/README.md for guidelines

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
**/*.{yaml,yml}

⚙️ CodeRabbit configuration file

**/*.{yaml,yml}: If this is a Kubernetes/OpenShift manifest or Helm template:

  • securityContext: runAsNonRoot, readOnlyRootFilesystem,
    allowPrivilegeEscalation: false
  • Drop ALL capabilities, add only what is required
  • Resource limits (cpu, memory) on every container
  • No hostPID, hostNetwork, hostIPC, privileged: true
  • NetworkPolicy defined for the namespace
  • OpenShift: SCC must be restricted or custom-scoped
  • Liveness + readiness probes defined
  • automountServiceAccountToken: false unless needed
  • RBAC: least privilege; no cluster-admin for workloads
  • Helm: no .Values interpolation in shell commands

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
🔀 Multi-repo context openshift/console

Based on my exploration of the openshift/console repository, I have gathered sufficient cross-repository context to inform the review. Here are my findings:

Linked repositories findings

openshift/console

Downloads Server Implementation (Golang-based):

  • The repository already contains a fully implemented Golang downloads server in cmd/downloads/main.go [::openshift/console::cmd/downloads/main.go:1-47], which:
    • Listens on port 8080 by default [::openshift/console::cmd/downloads/main.go:16]
    • Accepts --config-path flag with default value /opt/downloads/defaultArtifactsConfig.yaml [::openshift/console::cmd/downloads/main.go:19]
    • Uses http.ListenAndServe() for serving [::openshift/console::cmd/downloads/main.go:44]

Dockerfile.downloads Already Implements the Target Setup:

  • The existing Dockerfile.downloads [::openshift/console::Dockerfile.downloads:1-36] already:
    • Builds the Go downloads binary from source [::openshift/console::Dockerfile.downloads:15]
    • Copies binary to /opt/downloads/downloads [::openshift/console::Dockerfile.downloads:20]
    • Copies defaultArtifactsConfig.yaml to /opt/downloads [::openshift/console::Dockerfile.downloads:21]
    • Runs the exact command from the operator PR: ["/opt/downloads/downloads", "--config-path=/opt/downloads/defaultArtifactsConfig.yaml"] [::openshift/console::Dockerfile.downloads:28]
    • Uses restrictive USER 1001 and ubi-minimal base image

Supported Platforms in Configuration:

  • The defaultArtifactsConfig.yaml [::openshift/console::cmd/downloads/config/defaultArtifactsConfig.yaml:1-46] defines artifacts for all required architectures and platforms:
    • amd64 (linux, mac, windows) + RHEL 8/9 variants
    • arm64 (linux, mac) + RHEL 8/9 variants
    • ppc64le (linux) + RHEL 8/9 variants
    • s390x (linux) + RHEL 8/9 variants
    • These match exactly what the PR testing requirements specify

Tests and Archive Generation:

  • cmd/downloads/config/downloads_config_test.go [::openshift/console::cmd/downloads/config/downloads_config_test.go:1-372] provides comprehensive test coverage for:
    • Archive creation (tar/zip) for all architectures [::openshift/console::cmd/downloads/config/downloads_config_test.go:129-183]
    • Configuration loading and validation [::openshift/console::cmd/downloads/config/downloads_config_test.go:251-275]
    • HTTP handler that blocks archive requests until creation completes [::openshift/console::cmd/downloads/config/downloads_config_test.go:185-238]
    • URL generation with relative paths [::openshift/console::cmd/downloads/config/downloads_config_test.go:82-127]

Documentation Gaps (Secondary Finding):

  • README.md still references the old quay.io/repository/openshift/origin-cli-artifacts image [::openshift/console::README.md:443] and doesn't mention the new console-downloads image tag
  • The Dockerfile.downloads image label still uses io.openshift.tags="openshift,cli-artifacts" [::openshift/console::Dockerfile.downloads:36], not "console-downloads"
  • README example shows port 8081 [::openshift/console::README.md:468] but the server runs on port 8080 [::openshift/console::cmd/downloads/main.go:16]

Cross-Repo Synchronization Point:

  • The .coderabbit.yaml [::openshift/console::.coderabbit.yaml] indicates that pkg/serverconfig/types.go must be kept in sync with the console-operator's equivalent, flagging this as a critical cross-boundary concern

No Code Breaking Changes Detected:

  • The operator PR's deployment YAML changes (replacing inline shell/Python with the downloads binary command) are already fully supported by the existing Dockerfile.downloads
  • The configuration path, binary location, and command syntax all match exactly
  • All required platforms and architectures are already defined in the config
🔇 Additional comments (3)
bindata/assets/deployments/downloads-deployment.yaml (1)

51-82: LGTM!

pkg/console/subresource/deployment/deployment_test.go (2)

678-753: LGTM!


1668-1926: LGTM!


Walkthrough

The pull request migrates the OpenShift console downloads service from a Python-based HTTP server (cli-artifacts image) to a compiled downloads binary (console-downloads image). The downloads Deployment is updated to directly invoke the binary with a config path, both operator manifests are updated to reference the new image, and test expectations are updated to match the new startup structure.

Changes

Downloads Service Migration

Layer / File(s) Summary
Downloads Deployment binary startup
bindata/assets/deployments/downloads-deployment.yaml
The downloads container startup is refactored to run /opt/downloads/downloads --config-path=/opt/downloads/defaultArtifactsConfig.yaml directly, removing the embedded shell script that previously generated /tmp/serve.py and ran a Python HTTP server. The /tmp emptyDir volume mount and security context remain unchanged.
Operator console-downloads image configuration
manifests/07-operator.yaml, manifests/07-operator-ibm-cloud-managed.yaml
Both operator manifests update the DOWNLOADS_IMAGE environment variable from openshift:cli-artifacts to openshift:console-downloads, aligning with the new binary-based service image.
Test expectations for deployment startup
pkg/console/subresource/deployment/deployment_test.go
TestDefaultDownloadsDeployment expectations are updated to match the new container Command and Args for the downloads binary invocation, with resource requests remaining unchanged.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: replacing the cli-artifacts image with console-downloads image, which is the primary objective of this pull request.
Description check ✅ Passed The description provides comprehensive coverage including analysis, solution details, test setup, test cases, browser conformance, and additional context. All required sections are completed with relevant information.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed This repository uses standard Go testing (func Test* with t.Run), not Ginkgo. The PR modifies test expectations in deployment_test.go with stable subtest names like "Test Downloads Deployment for S...
Test Structure And Quality ✅ Passed PR contains no Ginkgo tests; all tests use standard Go testing (func TestXXX(t *testing.T) pattern). Custom check is specifically for Ginkgo tests and is not applicable.
Microshift Test Compatibility ✅ Passed This PR does not add new Ginkgo e2e tests. All test files use standard Go testing package, not Ginkgo framework. The custom check for MicroShift compatibility is designed for Ginkgo e2e tests and i...
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests added. The new test/e2e/downloads_test.go is a standard Go test (func TestDownloadsEndpoint) that only verifies HTTP endpoints are accessible via routes/services; it makes n...
Topology-Aware Scheduling Compatibility ✅ Passed The PR only updates the downloads image and removes inline Python scripts; it does not introduce any new scheduling constraints. The existing topology-aware scheduling logic (replicas, anti-affinit...
Ote Binary Stdout Contract ✅ Passed PR contains only YAML configuration changes and test expectation updates; no process-level code modifications that could violate the OTE Binary Stdout Contract.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added in this PR. Changes are limited to Kubernetes manifests and updating unit test expectations in a standard Go test file (using testing.T, not Ginkgo).
No-Weak-Crypto ✅ Passed PR contains only YAML configuration and test updates; no weak crypto (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto, or insecure secret comparisons are present.
Container-Privileges ✅ Passed All Kubernetes manifests maintain restrictive security contexts: allowPrivilegeEscalation is false, readOnlyRootFilesystem is true, capabilities only drop (no adds), runAsNonRoot is enabled, and no...
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data (passwords, tokens, API keys, PII, session IDs, or credentials) is exposed in logs or commands. The new args field contains only a configuration file path, and environment variabl...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from TheRealJon and spadgett May 15, 2026 17:38
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@bindata/assets/deployments/Untitled`:
- Line 1: Delete the stray asset file named "Untitled" that contains only the
token "readOnlyRootFilesystem" from the assets/deployments bundle; remove it
from the repository and any asset-generation lists so it won't be packaged into
bindata, and if the readOnlyRootFilesystem setting was intended for a real
deployment manifest, move that key into the proper deployment YAML (not the
"Untitled" stub).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 07e1b599-077b-438b-adb5-324ed8e45c96

📥 Commits

Reviewing files that changed from the base of the PR and between ee0e804 and 2e562b9.

📒 Files selected for processing (4)
  • bindata/assets/deployments/Untitled
  • bindata/assets/deployments/downloads-deployment.yaml
  • manifests/07-operator-ibm-cloud-managed.yaml
  • manifests/07-operator.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
manifests/**/*.yaml

⚙️ CodeRabbit configuration file

manifests/**/*.yaml: Review CVO manifests carefully.
These are deployed to clusters and changes have wide impact.

Files:

  • manifests/07-operator-ibm-cloud-managed.yaml
  • manifests/07-operator.yaml
bindata/assets/**/*.yaml

⚙️ CodeRabbit configuration file

bindata/assets/**/*.yaml: Review YAML assets for Kubernetes resource correctness.
Ensure proper annotations for cluster profiles.

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
🔀 Multi-repo context openshift/console

[::openshift/console::] README.md — references the origin-cli-artifacts image and oc binaries from quay.io/repository/openshift/origin-cli-artifacts (match at README.md:443 and README.md:463).

[::openshift/console::] Dockerfile.downloads — builds FROM quay.io/openshift/origin-cli-artifacts:4.18 and labels image as "openshift,cli-artifacts" (matches at Dockerfile.downloads:1 and Dockerfile.downloads:36).

Note: manifests/ and bindata/ were not present in this checkout (shell reported "No such file or directory"), so I could not locate the exact deployment YAML files changed in the PR within this repository.

🔇 Additional comments (1)
bindata/assets/deployments/downloads-deployment.yaml (1)

51-54: LGTM!

Comment thread bindata/assets/deployments/Untitled Outdated
@@ -0,0 +1 @@
readOnlyRootFilesystem No newline at end of file

@coderabbitai coderabbitai Bot May 15, 2026

Copy link
Copy Markdown

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

Remove this stray file.

bindata/assets/deployments/Untitled is not valid deployment YAML and looks like an accidental editor artifact. Leaving it under the assets tree risks confusing future maintainers or being swept into asset generation by mistake.

🤖 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 `@bindata/assets/deployments/Untitled` at line 1, Delete the stray asset file
named "Untitled" that contains only the token "readOnlyRootFilesystem" from the
assets/deployments bundle; remove it from the repository and any
asset-generation lists so it won't be packaged into bindata, and if the
readOnlyRootFilesystem setting was intended for a real deployment manifest, move
that key into the proper deployment YAML (not the "Untitled" stub).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@TheRealJon, glad we're on the same page! Once the bindata/assets/deployments/Untitled file is removed from the PR, feel free to let me know and I'll mark this resolved.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@bindata/assets/deployments/downloads-deployment.yaml`:
- Around line 51-54: The deployment currently runs the downloads binary
(command: /opt/downloads/downloads) with a readOnlyRootFilesystem but lacks a
writable /tmp, causing the Go server's os.MkdirTemp("", "artifacts") to fail;
update the Downloads Deployment to add a volumeMount for /tmp (mountPath: /tmp)
in the container spec and add a corresponding emptyDir volume (name like tmp or
downloads-tmp) in the pod spec so the container has a writable /tmp when
readOnlyRootFilesystem is true.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 023cf83e-cb7f-47c2-b8e2-1271d4209157

📥 Commits

Reviewing files that changed from the base of the PR and between 2e562b9 and b427ee2.

⛔ Files ignored due to path filters (1)
  • pkg/console/subresource/deployment/deployment_test.go is excluded by !**/*_test.go
📒 Files selected for processing (4)
  • bindata/assets/deployments/Untitled
  • bindata/assets/deployments/downloads-deployment.yaml
  • manifests/07-operator-ibm-cloud-managed.yaml
  • manifests/07-operator.yaml
✅ Files skipped from review due to trivial changes (1)
  • manifests/07-operator-ibm-cloud-managed.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • bindata/assets/deployments/Untitled
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
manifests/**/*.yaml

⚙️ CodeRabbit configuration file

manifests/**/*.yaml: Review CVO manifests carefully.
These are deployed to clusters and changes have wide impact.

Files:

  • manifests/07-operator.yaml
bindata/assets/**/*.yaml

⚙️ CodeRabbit configuration file

bindata/assets/**/*.yaml: Review YAML assets for Kubernetes resource correctness.
Ensure proper annotations for cluster profiles.

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
🔀 Multi-repo context openshift/console

[::openshift/console::] Dockerfile.downloads:1 — FROM quay.io/openshift/origin-cli-artifacts:4.18 (file shows image built-from still references origin-cli-artifacts).
[::openshift/console::] Dockerfile.downloads:36 — image label io.openshift.tags="openshift,cli-artifacts".
[::openshift/console::] README.md:443,463 — README documents that the server provides oc binaries from quay.io/repository/openshift/origin-cli-artifacts and notes origin-cli-artifacts usage for local builds.
[::openshift/console::] Dockerfile.downloads:20-21,28 — the repo already builds and bundles the Go downloads server binary (/opt/downloads/downloads) and defaultArtifactsConfig.yaml; Dockerfile.downloads CMD is "/opt/downloads/downloads --config-path=/opt/downloads/defaultArtifactsConfig.yaml".
[::openshift/console::] cmd/downloads/main.go & cmd/downloads/config/* — implementation of the Go downloads server and config generation present in-tree (server listens on configured port and serves the generated TempDir).

Summary: repository contains the Go downloads server and Dockerfile that builds an image from origin-cli-artifacts base; README also references origin-cli-artifacts. The PR switching operator manifests to use registry.svc.ci.openshift.org/openshift:console-downloads will require the console-downloads image to be produced/pushed (openshift/release) and reconciled with the existing Dockerfile.downloads / README references that still mention origin-cli-artifacts.

🔇 Additional comments (1)
manifests/07-operator.yaml (1)

72-73: LGTM!

Comment thread bindata/assets/deployments/downloads-deployment.yaml
@jhadvig

jhadvig commented May 18, 2026

Copy link
Copy Markdown
Member Author

/retest

@TheRealJon TheRealJon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just the stray file to remove, other than that it looks good!

Comment thread bindata/assets/deployments/Untitled Outdated
@@ -0,0 +1 @@
readOnlyRootFilesystem No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@TheRealJon TheRealJon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2026
@openshift-ci

openshift-ci Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhadvig, TheRealJon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jhadvig

jhadvig commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

/retest

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 18, 2026
@openshift-ci

openshift-ci Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

Replace the cli-artifacts image with the new console-downloads image
for the downloads deployment. The new image contains a Golang HTTP
server with its own entrypoint, so the inline Python server script
and associated command/args/volumes are removed from the deployment
template.
The Go downloads server writes to /tmp/artifacts at startup
(readOnlyRootFilesystem is true), so a writable tmpfs is required.
@openshift-ci

openshift-ci Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@jhadvig: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gcp-ovn 8fc3133 link true /test e2e-gcp-ovn
ci/prow/e2e-azure-ovn-upgrade 8fc3133 link true /test e2e-azure-ovn-upgrade
ci/prow/e2e-aws-console 8fc3133 link true /test e2e-aws-console

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants