Redesign download server UI and add Konflux Containerfile#158
Open
Joeavaikath wants to merge 4 commits intomigtools:oadp-devfrom
Open
Redesign download server UI and add Konflux Containerfile#158Joeavaikath wants to merge 4 commits intomigtools:oadp-devfrom
Joeavaikath wants to merge 4 commits intomigtools:oadp-devfrom
Conversation
Replace inline HTML with Go html/template and separated CSS using embed.FS. Group downloads by OS (Linux/macOS/Windows) in table layout with architecture badges, SHA256 checksums (click to copy), and per-command copy buttons. Use official Red Hat brand colors. Also optimize Containerfile to clean build cache within RUN steps and include .sha256 checksum files in the image. Signed-off-by: Joseph <jvaikath@redhat.com>
Uses the OSBS golang builder with FIPS-compliant build flags (CGO_ENABLED=1, strictfipsruntime). Dependencies are prefetched by the Konflux pipeline via Hermeto, so no go mod download is needed. Includes Red Hat metadata labels and license copy. Signed-off-by: Joseph <jvaikath@redhat.com>
📝 WalkthroughWalkthroughAdds a containerized download server: a templated HTML UI with embedded static assets and CSS, platform-categorized archive listings with SHA256 checksums, environment-driven config, and new multi-stage Containerfile(s) to build release archives and a UBI9 runtime image. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Server
participant FS as Archive FileSystem
participant Template as Template Renderer
participant Static as Static Assets
Client->>Server: GET /
Server->>FS: List files in /archives
Server->>FS: Read corresponding `*.sha256` checksums
Server->>Server: Parse platforms, build archiveFile collections
Server->>Template: Render index.html with categorized files
Template-->>Server: HTML
Server-->>Client: 200 OK + HTML
Client->>Static: GET /static/style.css
Static-->>Client: CSS file
Client->>Server: GET /download/{archive}
Server->>FS: Stream archive file
FS-->>Client: Archive bytes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
/cherry-pick release-1.6 |
|
@Joeavaikath: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
konflux.Containerfile.download (1)
5-5: Pin immutable base images in the hermetic variant.Floating tags, especially
registry.redhat.io/ubi9/ubi:latest, make repeated builds resolve different base layers over time, which works against the stated hermetic goal. Please pin digests here, or document where Konflux resolves and locks them before build.Also applies to: 24-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux.Containerfile.download` at line 5, The FROM line uses a floating tag for the hermetic builder image ("FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_9_golang_1.24"); update this to pin the image to an immutable digest (sha256) or add documentation pointing to where Konflux resolves and locks the digest before builds. Locate the FROM statement in the Containerfile (the builder stage) and replace the tag with the corresponding image@sha256:<digest> OR add a short comment and/or README note explaining the digest-resolution/locking process and where artifacts storing the locked digests live so hermetic builds are reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/downloads/server.go`:
- Around line 80-92: The switch on osName in the loop that builds archiveFile
entries (using parsePlatform, readChecksum and archiveFile) silently defaults
unknown OS values into linuxFiles; change this so unknown/malformed results are
not misclassified: detect when parsePlatform returns an unexpected osName and
either skip adding the archiveFile and log a warning (including the filename and
osName) or append it to a new unknownFiles slice for later inspection instead of
falling through to the linuxFiles case; update the switch/default to perform the
skip/log or push to unknownFiles and ensure any callers that expected only
linux/darwin/windows handle the new behavior.
In `@cmd/downloads/templates/index.html`:
- Line 38: The template currently shows only a checksum prefix and attaches copy
behavior to a non-interactive span; update the markup to render the full SHA256
string in the DOM (use {{.Checksum}} instead of {{slice .Checksum 0 16}}...),
replace the clickable span with a real button (e.g. class "copy-checksum" or
similar) that uses navigator.clipboard.writeText(this.dataset.checksum) or reads
the full checksum from a data-checksum attribute, include an accessible
label/aria-label like "Copy full checksum" and keep the visible short display
handled via CSS if you want truncation, and apply this same change for the other
occurrences referenced (lines 66 and 94) so keyboard users and manual
verification can access the complete digest.
- Around line 103-133: Replace the hard-coded POSIX install block in the
template with OS- and client-specific blocks: detect the OS/template variable
and render a Linux/macOS section that keeps the tar/chmod/mv commands (replace
static strings like "tar -xzf kubectl-oadp_*.tar.gz", "chmod +x kubectl-oadp",
"sudo mv kubectl-oadp /usr/local/bin/") and a Windows section that uses
zip/unzip or .exe guidance and PATH instructions instead of POSIX commands;
update the final verification line so it is neutral or client-aware (use
"kubectl-oadp --help" by default and render "oc oadp --help" only when the oc
client is the advertised artifact) and ensure the copy buttons' data-cmd values
and onclick="copyCmd(this)" targets are set per-platform/command.
---
Nitpick comments:
In `@konflux.Containerfile.download`:
- Line 5: The FROM line uses a floating tag for the hermetic builder image
("FROM
brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_9_golang_1.24");
update this to pin the image to an immutable digest (sha256) or add
documentation pointing to where Konflux resolves and locks the digest before
builds. Locate the FROM statement in the Containerfile (the builder stage) and
replace the tag with the corresponding image@sha256:<digest> OR add a short
comment and/or README note explaining the digest-resolution/locking process and
where artifacts storing the locked digests live so hermetic builds are
reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25cfad9b-e61b-41b2-a54f-6b8bb4f632fa
📒 Files selected for processing (5)
Containerfile.downloadcmd/downloads/server.gocmd/downloads/static/style.csscmd/downloads/templates/index.htmlkonflux.Containerfile.download
mpryc
previously approved these changes
Mar 17, 2026
Contributor
|
/lgtm |
Add RHEL technology icon (Red RGB) and Windows logo SVGs to static assets. Update section headers to use img tags with proper sizing instead of Unicode emoji characters. Signed-off-by: Joseph <jvaikath@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/downloads/static/style.css (1)
48-58: Consider contrast for subtitle and binary-note text.The reduced-opacity white text (
rgba(255,255,255,0.8)andrgba(255,255,255,0.65)) on the red header may fall below WCAG AA contrast ratio (4.5:1), especially the.binary-noteat 0.8rem font size. If accessibility compliance is required, consider increasing opacity or using solid white.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/downloads/static/style.css` around lines 48 - 58, The translucent white text colors for .subtitle (rgba(255,255,255,0.8)) and .binary-note (rgba(255,255,255,0.65)) may not meet WCAG AA contrast on the red header; update the CSS rules for .subtitle and .binary-note to use higher-contrast color values (e.g., increase opacity to 1.0 or switch to solid `#ffffff`) or adjust font-size/weight for .binary-note so the computed contrast ratio meets at least 4.5:1 (verify with a contrast checker) while keeping the selectors .subtitle and .binary-note unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/downloads/static/style.css`:
- Around line 169-183: Add accessible keyboard focus styles for interactive
selectors by adding corresponding :focus and/or :focus-visible rules that mirror
the :hover visual feedback and provide a clear visible indicator (e.g., outline
or focus ring and/or matching background change). Update the CSS for
.download-btn, .copy-btn, .logo-link, and .footer a to include these focus
selectors (or a shared utility focus style) so keyboard users see focus; prefer
using :focus-visible to avoid showing the ring on mouse interactions and ensure
contrast and thickness are sufficient for visibility.
---
Nitpick comments:
In `@cmd/downloads/static/style.css`:
- Around line 48-58: The translucent white text colors for .subtitle
(rgba(255,255,255,0.8)) and .binary-note (rgba(255,255,255,0.65)) may not meet
WCAG AA contrast on the red header; update the CSS rules for .subtitle and
.binary-note to use higher-contrast color values (e.g., increase opacity to 1.0
or switch to solid `#ffffff`) or adjust font-size/weight for .binary-note so the
computed contrast ratio meets at least 4.5:1 (verify with a contrast checker)
while keeping the selectors .subtitle and .binary-note unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a36d214d-5baa-4326-b92a-5dd7da62976a
⛔ Files ignored due to path filters (2)
cmd/downloads/static/rhel-icon.svgis excluded by!**/*.svgcmd/downloads/static/windows-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
cmd/downloads/static/style.csscmd/downloads/templates/index.html
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/downloads/templates/index.html
Comment on lines
+169
to
+183
| .download-btn { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| gap: 0.35rem; | ||
| background: var(--rh-red); | ||
| color: var(--rh-white); | ||
| padding: 0.45rem 0.9rem; | ||
| border-radius: 0.4rem; | ||
| text-decoration: none; | ||
| font-size: 0.82rem; | ||
| font-weight: 500; | ||
| transition: background 0.15s; | ||
| } | ||
|
|
||
| .download-btn:hover { background: var(--rh-red-dark); } |
There was a problem hiding this comment.
Missing :focus styles for keyboard accessibility.
Interactive elements (.download-btn, .copy-btn, .logo-link, .footer a) have :hover states but lack :focus styles. Keyboard users won't see which element is currently focused, making navigation difficult.
♿ Proposed fix to add focus states
.logo-link:hover { text-decoration: underline; }
+.logo-link:focus { outline: 2px solid var(--rh-white); outline-offset: 2px; }
.checksum:hover { color: var(--rh-gray-60); }
+.checksum:focus { outline: 2px solid var(--rh-red); outline-offset: 2px; }
.download-btn:hover { background: var(--rh-red-dark); }
+.download-btn:focus { outline: 2px solid var(--rh-black); outline-offset: 2px; }
.copy-btn:hover {
border-color: var(--rh-gray-30);
color: var(--rh-gray-20);
background: rgba(255,255,255,0.08);
}
+.copy-btn:focus { outline: 2px solid var(--rh-white); outline-offset: 2px; }
.footer a:hover { text-decoration: underline; }
+.footer a:focus { outline: 2px solid var(--rh-red); outline-offset: 2px; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/downloads/static/style.css` around lines 169 - 183, Add accessible
keyboard focus styles for interactive selectors by adding corresponding :focus
and/or :focus-visible rules that mirror the :hover visual feedback and provide a
clear visible indicator (e.g., outline or focus ring and/or matching background
change). Update the CSS for .download-btn, .copy-btn, .logo-link, and .footer a
to include these focus selectors (or a shared utility focus style) so keyboard
users see focus; prefer using :focus-visible to avoid showing the ring on mouse
interactions and ensure contrast and thickness are sufficient for visibility.
shubham-pampattiwar
previously approved these changes
Mar 18, 2026
Member
|
/cherry-pick oadp-1.6 |
|
@kaovilai: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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. |
kaovilai
requested changes
Mar 18, 2026
Signed-off-by: Joseph <jvaikath@redhat.com>
kaovilai
approved these changes
Mar 18, 2026
Member
|
/lgtm |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
konflux.Containerfile.download (2)
24-24: Consider pinning base image for reproducible builds.Using
ubi9/ubi:latestmay produce different images over time as thelatesttag is updated. For a hermetic/reproducible Konflux build, consider pinning to a specific version tag (e.g.,ubi9/ubi:9.4) or using a digest reference.♻️ Suggested change
-FROM registry.redhat.io/ubi9/ubi:latest +FROM registry.redhat.io/ubi9/ubi:9.4Or use a digest for maximum reproducibility:
FROM registry.redhat.io/ubi9/ubi@sha256:<specific-digest>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux.Containerfile.download` at line 24, The Docker base image in the Containerfile currently uses an unpinned tag "FROM registry.redhat.io/ubi9/ubi:latest"; change this to a specific, immutable reference to ensure reproducible builds by replacing the ":latest" tag with a concrete version tag (e.g., ":9.4") or, even better, a digest reference ("@sha256:<digest>") so the FROM line in the Containerfile always resolves to the exact same image.
26-26: Consider simplifying package installation.The
reinstall tzdataapproach works but could be replaced with a simpler install command. If tzdata is already in the base image, installing it again is a no-op; if it's missing, install will add it.♻️ Optional simplification
-RUN dnf -y install openssl && dnf -y reinstall tzdata && dnf clean all +RUN dnf -y install openssl tzdata && dnf clean all🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux.Containerfile.download` at line 26, Replace the RUN line that currently uses "dnf -y reinstall tzdata" with a simpler install invocation: change "RUN dnf -y install openssl && dnf -y reinstall tzdata && dnf clean all" to use "dnf -y install tzdata" (or combine into a single install command such as "dnf -y install openssl tzdata && dnf clean all"); update the command containing "dnf -y reinstall tzdata" so tzdata is installed if missing and avoids an unnecessary reinstall when present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@konflux.Containerfile.download`:
- Line 24: The Docker base image in the Containerfile currently uses an unpinned
tag "FROM registry.redhat.io/ubi9/ubi:latest"; change this to a specific,
immutable reference to ensure reproducible builds by replacing the ":latest" tag
with a concrete version tag (e.g., ":9.4") or, even better, a digest reference
("@sha256:<digest>") so the FROM line in the Containerfile always resolves to
the exact same image.
- Line 26: Replace the RUN line that currently uses "dnf -y reinstall tzdata"
with a simpler install invocation: change "RUN dnf -y install openssl && dnf -y
reinstall tzdata && dnf clean all" to use "dnf -y install tzdata" (or combine
into a single install command such as "dnf -y install openssl tzdata && dnf
clean all"); update the command containing "dnf -y reinstall tzdata" so tzdata
is installed if missing and avoids an unnecessary reinstall when present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d0ac5eb-c69f-4068-bc9f-4df78802fa5b
📒 Files selected for processing (1)
konflux.Containerfile.download
kaovilai
approved these changes
Mar 18, 2026
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Joeavaikath, kaovilai The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
and official Red Hat brand colors
architecture badges, SHA256 checksums (click to copy), and per-command
copy buttons
html/templatewithembed.FSfor bothtemplates and static assets
Containerfile.downloadto clean build cache within RUN stepsand include
.sha256checksum fileskonflux.Containerfile.downloadfor hermetic downstream builds(FIPS-compliant, Hermeto-prefetched dependencies)
Test plan
podman build -f Containerfile.download -t oadp-cli-binaries .ARCHIVE_DIR=cmd/downloads/test-archives go run ./cmd/downloads/localhost:8080renders correctly with OS grouping🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores