Update Helm download manifests for Helm 3 and Helm 4.#1175
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
WalkthroughThe ChangesHelm CLI Download Manifests
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sowmya-sl The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@manifests/07-downloads-helm.yaml`:
- Around line 19-20: The YAML document separator uses four dashes (----) instead
of the correct three dashes (---), which causes invalid YAML parsing. Locate the
line with the four-dash separator in the manifests file and replace it with the
proper three-dash document separator (---) to comply with YAML specification and
ensure proper multi-document parsing.
- Line 4: The resource has been renamed from `helm-download-links` to
`helm-download-links-v3` but no automatic cleanup mechanism removes the old
resource when clusters upgrade. Update any test fixtures and references in
related codebases (particularly in openshift/console) that still reference the
old `helm-download-links` name to use the new `helm-download-links-v3` name to
prevent confusion and test failures. Additionally, ensure documentation or
migration notes are added to communicate that manual deletion of the
pre-existing `helm-download-links` ConsoleCLIDownload resource is required
during or after cluster upgrades to prevent duplicate Helm entries in the
console UI.
🪄 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: 83aa783e-ce8d-4ad2-99ae-d0b103ed5457
📒 Files selected for processing (1)
manifests/07-downloads-helm.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/console(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
{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:
manifests/07-downloads-helm.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:
manifests/07-downloads-helm.yaml
**
⚙️ CodeRabbit configuration file
**: # OpenShift Console Operator - AI Context HubThis 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.modwith 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.modwith 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:
manifests/07-downloads-helm.yaml
**/*.yaml
⚙️ CodeRabbit configuration file
**/*.yaml: Review YAML manifests based on content and kind.Refer to /manifest-review when YAML contains:
kind: Roleorkind: ClusterRole(RBAC review)kind: RoleBindingorkind: ClusterRoleBindingannotations:section (check for cluster profiles)verbs: ["*"]or wildcard permissionsapiGroups: ["*"]or overly broad permissions- ServiceAccount references in subjects
Check for required annotations in manifests/:
include.release.openshift.io/hypershiftinclude.release.openshift.io/ibm-cloud-managedinclude.release.openshift.io/self-managed-high-availabilityinclude.release.openshift.io/single-node-developercapability.openshift.io/name: ConsoleFor quickstarts/, additionally check:
- QuickStart spec structure
- Task descriptions and prerequisites
- See quickstarts/README.md for guidelines
Files:
manifests/07-downloads-helm.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:
manifests/07-downloads-helm.yaml
🪛 YAMLlint (1.37.1)
manifests/07-downloads-helm.yaml
[error] 20-20: syntax error: could not find expected ':'
(syntax)
🔀 Multi-repo context openshift/console
Based on my comprehensive exploration of the openshift/console repository, I have gathered the relevant cross-repository context for this PR.
Linked repositories findings
openshift/console
Test fixture referencing the old Helm download entry name:
[::openshift/console::]
- File:
frontend/public/components/__tests__/command-line-tools.spec.tsx(lines 8-14) - The test contains a mock ConsoleCLIDownload object with
metadata.name: 'helm-download-links' - This is a test data fixture used to validate the CommandLineTools component's rendering and ordering behavior
Component referencing ConsoleCLIDownload resources:
[::openshift/console::]
- File:
frontend/public/components/command-line-tools.tsx(lines 22-56) - The
CommandLineToolscomponent dynamically renders all ConsoleCLIDownload entries passed to it - It does NOT hardcode specific download entry names - it uses
tool.metadata.uidas the React key and dynamically readstool.spec.displayName - The component is flexible and will work with any ConsoleCLIDownload entry name (e.g.,
helm-download-links-v3,helm-download-links-v4)
Impact Assessment:
The test mock currently references the old name helm-download-links. However, this test is not broken by the PR because:
- The test validates ordering behavior (that 'oc' CLI appears first), not the specific name of the download entries
- The component itself is agnostic to download entry names
- However, if deployed clusters have ConsoleCLIDownload resources with the name
helm-download-links, they would need to be updated when the manifest source changes
Note: The manifest file manifests/07-downloads-helm.yaml mentioned in the PR does not exist in the openshift/console repository. This PR may be against a separate manifests or Helm charts repository, or these manifests are deployed through a different means. The test fixture in console will accept whatever ConsoleCLIDownload entries are deployed with any name.
| kind: ConsoleCLIDownload | ||
| metadata: | ||
| name: helm-download-links | ||
| name: helm-download-links-v3 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where ConsoleCLIDownload desired objects are defined/applied
fd -i 'controller.go' pkg/console/controllers/clidownloads --exec sed -n '1,280p' {}
# 2) Check for delete/prune logic for ConsoleCLIDownload resources
rg -n -C3 --type=go '\bDelete\s*\(|prun|reconcil|ApplyCLIDownloads|ConsoleCLIDownload' pkg/console/controllers
# 3) Confirm old/new names present in manifests/assets
rg -n -C2 'helm-download-links(-v3|-v4)?' manifests bindata/assets
# Expected:
# - Either explicit cleanup/deletion of old name exists, or
# - A deliberate compatibility decision is documented/tested.Repository: openshift/console-operator
Length of output: 30974
🏁 Script executed:
#!/bin/bash
# Find the api constants to see what CLI downloads are managed
grep -r "OCCLIDownloadsCustomResourceName\|helm-download" pkg/api --include="*.go"
# Check if there are any other constant definitions for CLI downloads
grep -r "CliDownload\|CLIDownload" pkg/api/constants.go 2>/dev/null || grep -r "const.*Download" pkg/api --include="*.go"Repository: openshift/console-operator
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Check for migration or cleanup logic related to ConsoleCLIDownload
rg -A5 -B5 "helm-download|v3|v4" pkg/console/controllers/clidownloads/ --type=go
# Check if the main sync loop or operator handles cleanup of renamed resources
rg "ConsoleCLIDownload" pkg/console/operator/sync_v400.go --type=go -A3 -B3Repository: openshift/console-operator
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Verify what the actual managed resource names are
grep -n "OCCLIDownloadsCustomResourceName\|helm" pkg/api/constants.goRepository: openshift/console-operator
Length of output: 218
🏁 Script executed:
#!/bin/bash
# List all ConsoleCLIDownload resources in manifests
find manifests bindata/assets -name "*.yaml" -type f -exec grep -l "ConsoleCLIDownload" {} \;
# Show what helm resources are in manifests
rg "ConsoleCLIDownload" manifests bindata/assets --type=yaml -B2 -A5Repository: openshift/console-operator
Length of output: 1678
🏁 Script executed:
#!/bin/bash
# Check if old "helm-download-links" (without version suffix) exists anywhere
rg "helm-download-links['\"]?\s*$|helm-download-links\s*$" --type=yaml manifests bindata/assets
# Look for deletion/cleanup manifests
find manifests -name "*.yaml" -exec grep -l "kind.*Delete\|DeleteOptions\|metadata.name.*helm" {} \;Repository: openshift/console-operator
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Check if there's any migration guidance or comments about this change
git log --all --oneline -30 2>/dev/null | head -20 || echo "Git not available"
rg "helm-download-links" . --type=yaml -B3 -A3Repository: openshift/console-operator
Length of output: 2693
🏁 Script executed:
#!/bin/bash
# Check git history for when "helm-download-links" (old name) was introduced
git log --all --full-history -S "helm-download-links" --oneline -- manifests/ 2>/dev/null | head -20
# Also check if old name appears in any recent tags or branches
git show HEAD~10:manifests/07-downloads-helm.yaml 2>/dev/null | grep "name:" | head -5 || echo "Cannot access 10 commits back"Repository: openshift/console-operator
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Check for any existing CVO cleanup/prune mechanisms for ConsoleCLIDownload
rg "OwnerReference|foregroundDeletion|metadata.uid" manifests/ bindata/assets/ --type=yaml
# See if manifests have ownership or deletion cascade setup
grep -r "owner\|delete" manifests/07-downloads-helm.yamlRepository: openshift/console-operator
Length of output: 164
No automatic cleanup exists for the renamed resource. Manually delete the old helm-download-links ConsoleCLIDownload from clusters during or after the upgrade to avoid duplicate Helm entries.
CVO applies static manifests without removing previous versions of renamed resources. If clusters have a pre-existing helm-download-links resource deployed from an earlier manifest, it will coexist with the new helm-download-links-v3 and helm-download-links-v4 resources after upgrade, creating duplicate Helm download entries in the console UI.
The operator's CLIDownloadsController does not manage helm CLI downloads—it only manages the operator-deployed oc-cli-downloads resource. The helm downloads are static CVO resources with no built-in migration path. Additionally, the test fixture in openshift/console still references the old name helm-download-links, which should be updated separately.
🤖 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 `@manifests/07-downloads-helm.yaml` at line 4, The resource has been renamed
from `helm-download-links` to `helm-download-links-v3` but no automatic cleanup
mechanism removes the old resource when clusters upgrade. Update any test
fixtures and references in related codebases (particularly in openshift/console)
that still reference the old `helm-download-links` name to use the new
`helm-download-links-v3` name to prevent confusion and test failures.
Additionally, ensure documentation or migration notes are added to communicate
that manual deletion of the pre-existing `helm-download-links`
ConsoleCLIDownload resource is required during or after cluster upgrades to
prevent duplicate Helm entries in the console UI.
Source: Linked repositories
There was a problem hiding this comment.
@sowmya-sl, this PR is going to be merged into the main branch which will target OCP 5.0 (and 4.23). In that version (and probably in 4.23), we're only going to provide Helm v4. So, you should leave the name helm-download-links unchanged and update the content with stand-alone text for Helm v4 (similar to the current text for Helm v3, without the comparison to the previous version). This will produce the correct results going forward, and it will get us around Coderabbitai's concern, here.
Then, when you backport this change to OCP 4.22, you should add a second CRD for Helm v3. If you like, in that manifest, you can restore the previous description and make the new description for Helm v4 more introductory (like you have here, currently). But, you should keep the helm-download-links for one of the two CRDs and use a new name for the other.
Also, in the backport, the text of the hyperlinks should be different -- e.g., "Download Helm v3" vs. "Download Helm v4".
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Split the CLI download entries and clarify Helm 4 description relative to Helm 3 so users can distinguish versions. Co-authored-by: Cursor <cursoragent@cursor.com>
sowmya-sl
force-pushed
the
helm-release-docs
branch
from
e8918a5 to
4b4e2cb
Compare
webbnh
left a comment
webbnh
left a comment
There was a problem hiding this comment.
There's a context problem with this PR: this PR is targeting OCP 5.0, but it's carrying a change which is targeting OCP 4.22.
I think you're on the right path -- we should first change the main branch (which this PR is targeting) and then backport the change to OCP 4.22 -- but the changes needed for 5.0 and 4.22 are different, so you should proceed with the 5.0 change but then do a manual backport to 4.22 with different content.
| kind: ConsoleCLIDownload | ||
| metadata: | ||
| name: helm-download-links | ||
| name: helm-download-links-v3 |
There was a problem hiding this comment.
@sowmya-sl, this PR is going to be merged into the main branch which will target OCP 5.0 (and 4.23). In that version (and probably in 4.23), we're only going to provide Helm v4. So, you should leave the name helm-download-links unchanged and update the content with stand-alone text for Helm v4 (similar to the current text for Helm v3, without the comparison to the previous version). This will produce the correct results going forward, and it will get us around Coderabbitai's concern, here.
Then, when you backport this change to OCP 4.22, you should add a second CRD for Helm v3. If you like, in that manifest, you can restore the previous description and make the new description for Helm v4 more introductory (like you have here, currently). But, you should keep the helm-download-links for one of the two CRDs and use a new name for the other.
Also, in the backport, the text of the hyperlinks should be different -- e.g., "Download Helm v3" vs. "Download Helm v4".
| Helm 4 is the next major release of Helm and, compared to Helm 3, includes | ||
| updated workflows for defining, installing, and upgrading Helm Charts. |
There was a problem hiding this comment.
I think that the v3 description of Helm is better -- it's a tool for defining, installing, and upgrading applications. (I don't think Helm v4 provides much in the way of new capabilities for manipulating charts -- it provides new ways to use charts to define and maintain applications.)
This comment was marked as outdated.
This comment was marked as outdated.
|
/test all |
|
Also, the PR title is going to need the Jira prefix. |
|
@sowmya-sl: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
Split the CLI download entries and clarify Helm 4 description relative to Helm 3 so users can distinguish versions.
Analysis / Root cause:
Helm 4 has been released recently. Update Command Line Tools to reflect that.
Additional info:
Summary by CodeRabbit
Release Notes