Skip to content

fix: add cluster wait actions to RequiresCluster - #5225

Open
mjnagel wants to merge 2 commits into
zarf-dev:mainfrom
mjnagel:fix-cluster-wait-requirement
Open

fix: add cluster wait actions to RequiresCluster#5225
mjnagel wants to merge 2 commits into
zarf-dev:mainfrom
mjnagel:fix-cluster-wait-requirement

Conversation

@mjnagel

@mjnagel mjnagel commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description

Cluster wait actions also require a cluster but were missing from the RequiresCluster checks in both api versions.

Checklist before merging

@mjnagel
mjnagel requested review from a team as code owners August 14, 2026 15:39
@netlify

netlify Bot commented Aug 14, 2026

Copy link
Copy Markdown

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 062de26
🔍 Latest deploy log https://app.netlify.com/projects/zarf-docs/deploys/6a7f36c78167ec0008ab8260

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.57143% with 15 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/api/v1beta1/component.go 0.00% 10 Missing ⚠️
src/api/v1alpha1/component.go 54.54% 3 Missing and 2 partials ⚠️
Files with missing lines Coverage Δ
src/api/v1alpha1/component.go 65.21% <54.54%> (-0.89%) ⬇️
src/api/v1beta1/component.go 45.00% <0.00%> (-36.82%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

hasRepos := len(c.Repos) > 0
hasDataInjections := len(c.DataInjections) > 0
hasHealthChecks := len(c.HealthChecks) > 0
hasClusterWait := c.Actions.hasClusterWait()

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.

Agree on the correctness here - a component that has a cluster wait DOES require cluster access.

I am stewing on how to capture the implications of requiring state now. As this change now requires setting up state when the wait action doesn't strictly require as much...

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.

I think this may require broader changes as the use of component.RequiresCluster is used during deploy/remove to manage state - whereas I don't think that a component solely comprised of a cluster wait should need state to be initialized (or the requirements such as RBAC) to do so.

I think we may want to distinguish as much between "needs cluster" and "needs state" but that is broader to your changes.

I believe currently a cluster wait can run without requiring zarf initialization and this would break that as-is.

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants