Update Helm linting and testing workflow #86
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: empovit 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 |
WalkthroughAdds a workflow_dispatch input and a conditional "should-install" gating to the Helm lint/install GitHub Actions workflow; upgrades several action versions. Extends the ConsolePlugin CRD: adds v1 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as Workflow trigger
participant GH as GitHub Actions
participant Decide as should-install step
participant CT as chart-testing (ct)
participant Kind as Kind action
participant Kubectl as kubectl/setup
participant OpenShift as OpenShift deps
User->>GH: trigger (workflow_dispatch) + force_install_unchanged
GH->>Decide: run should-install (input + chart diff)
alt should-install = true
Note right of Decide `#DDEEFF`: Lint & Install path
Decide->>CT: ct lint (append --all if forced)
Decide->>Kind: create Kind cluster (v1.13.0, version/node_image)
Kind-->>Kubectl: provide cluster context
Decide->>Kubectl: setup kubectl (v4)
Decide->>OpenShift: setup OpenShift deps (conditional)
Decide->>CT: ct install (append --all if forced)
else should-install = false
Note right of Decide `#F8EBDD`: Skip cluster/setup/install
Decide-->>GH: skip cluster/setup/install
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
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 |
empovit
force-pushed
the
update-chart-test
branch
from
157b636 to
b84e816
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/helm-lint-test.yamldeployment/hack/crds/consoleplugin.customresourcedefinition.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/helm-lint-test.yaml
🔇 Additional comments (3)
deployment/hack/crds/consoleplugin.customresourcedefinition.yaml (3)
241-250: LGTM: Empty string added as valid loadType value.The addition of the empty string ("") to the i18n.loadType enum is consistent with the updated description stating that omitting the value or setting it to empty string is equivalent to Lazy type behavior. This provides flexibility for plugin developers.
346-346: LGTM: Atomic list type for proxy array.The addition of
x-kubernetes-list-type: atomicensures that the entire proxy array is replaced as a unit during updates, which is appropriate for this configuration structure and prevents partial updates that could lead to inconsistent states.
107-229: CEL validation rules for contentSecurityPolicy are correctly implemented.All four requested verifications pass:
Uniqueness validation (
self.all(x, self.exists_one(y, x == y))) correctly ensures each value appears exactly once in the array using the standard CEL pattern.Character restriction validations properly reject quotes, whitespace, commas, semicolons, and wildcards through individual CEL rules using
contains()andmatches()operators.Total size constraint (
self.map(x, x.values.map(y, y.size()).sum()).sum() < 8192) accurately sums character lengths across all directives and enforces the 8KB limit through nested mapping.x-kubernetes-list-type: map with
directiveas the list-map-key correctly prevents duplicate directive types.The implementation is comprehensive, well-structured, and follows Kubernetes CEL validation best practices.
empovit
force-pushed
the
update-chart-test
branch
from
b84e816 to
1f732a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/helm-lint-test.yamldeployment/hack/crds/consoleplugin.customresourcedefinition.yaml
🔇 Additional comments (9)
.github/workflows/helm-lint-test.yaml (4)
7-16: LGTM! Manual workflow dispatch is a useful addition.The
workflow_dispatchtrigger with theforce_install_unchangedinput is well-structured and provides flexibility for manual testing runs.
39-50: Excellent conditional logic for gating installations.The
should-installstep correctly determines whether to run the subsequent steps based on:
- The
force_install_unchangedinput flag, OR- Detection of changed charts via
ct list-changedThe conditional execution and
--allflag usage in the lint step are implemented correctly. When triggered by pull requests (where the input is undefined), the logic properly falls back to checking for chart changes.
52-80: LGTM! Consistent conditional gating across all installation steps.All subsequent steps (kind cluster creation, kubectl setup, OpenShift dependencies, and chart installation) are properly gated on the
should-installoutput. This prevents unnecessary resource allocation when charts are unchanged and the force flag is not set. The--allflag usage in the install step is consistent with the lint step.
23-23: All action versions are confirmed to exist and are stable.Verification confirms that all updated GitHub Actions versions are published and actively used:
actions/checkout@v4(v4.0.0+, requires Actions Runner >= v2.308.0)azure/setup-helm@v4(tagged releases available)helm/chart-testing-action@v2.7.0(released Jan 20, includes updates to ct v3.11.0, yamllint 1.33.0, yamale 4.0.4)helm/kind-action@v1.10.0(stable tag in use across workflows)azure/setup-kubectl@v4(latest v4.0.1 on GitHub Marketplace)deployment/hack/crds/consoleplugin.customresourcedefinition.yaml (5)
4-12: LGTM! Standard OpenShift CRD annotations.The metadata annotations follow OpenShift conventions and indicate proper API approval via the referenced GitHub PR.
107-229: Excellent CSP implementation with comprehensive validations.The new
contentSecurityPolicyfield is well-designed with:
- Proper list-type annotations (map for directives, atomic for values)
- Robust CEL validations preventing injection attacks (no quotes, whitespace, commas, semicolons, wildcards)
- Uniqueness enforcement and size limits (8kb total)
- Clear documentation with examples and MDN references
The implementation correctly ensures each directive appears once via the list-map-keys annotation.
346-346: Appropriate list-type annotation for proxy array.The
atomiclist type is a safe choice for the proxy array, requiring full array replacement during updates. This prevents patch conflicts and is reasonable given proxy configurations are typically small.
28-48: Excellent documentation improvements throughout v1 schema.The description enhancements provide:
- Better formatting with multi-line YAML strings for readability
- More detailed explanations and practical examples
- Improved API documentation clarity
- Proper handling of empty string in
loadTypeenum for backward compatibilityAlso applies to: 52-70, 99-100, 231-236, 241-251, 256-274, 289-294, 300-305, 320-322, 333-334
279-287: No action needed — authorization field change is properly aligned with upstream OpenShift.The field evolution from
authorize(boolean) toauthorization(enum string) between v1alpha1 and v1 is documented in OpenShift's API. The enum supportsUserToken(include auth token) andNone(no token), with defaultNone, maintaining backward compatibility semantics. The api-approved annotation references the proper upstream PR.
empovit
force-pushed
the
update-chart-test
branch
from
1f732a5 to
509826a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/helm-lint-test.yaml (1)
52-57: Consider updating node_image to match Kind v0.31.0's provided images.Kind v0.31.0 provides pre-built images including "v1.32.11: kindest/node:v1.32.11@sha256:5fc52d52a7b9574015299724bd68f183702956aa4a2116ae75a63cb574b35af8". The workflow specifies
kindest/node:v1.32.0, which may not be built for Kind v0.31.0.For improved reliability and reproducibility, consider either:
- Updating to
kindest/node:v1.32.11(latest 1.32.x for v0.31.0)- Pinning the image with its sha256 digest for guaranteed compatibility
🔎 Suggested fix
uses: helm/kind-action@v1.13.0 with: version: v0.31.0 - node_image: kindest/node:v1.32.0 + node_image: kindest/node:v1.32.11@sha256:5fc52d52a7b9574015299724bd68f183702956aa4a2116ae75a63cb574b35af8
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/helm-lint-test.yamldeployment/hack/crds/consoleplugin.customresourcedefinition.yaml
🔇 Additional comments (10)
.github/workflows/helm-lint-test.yaml (6)
7-16: LGTM!The
workflow_dispatchtrigger with theforce_install_unchangedinput is well-structured, allowing manual triggering with an optional override to force install even when charts are unchanged.
39-46: LGTM!The logic correctly determines whether to proceed with installation based on either the manual force flag or detected chart changes. The script handles the
pull_requesttrigger case whereforce_install_unchangedwill be empty.
48-50: LGTM!The conditional execution and the dynamic
--allflag based on the force input are correctly implemented.
59-76: LGTM!The kubectl setup and OpenShift dependencies configuration are properly gated and correctly configure the test environment with CRDs and TLS certificates.
78-80: LGTM!The chart-testing install step is correctly gated and uses the same dynamic
--allflag pattern as the lint step.
22-37: All action versions verified and appropriate.All specified action versions exist and are valid:
actions/checkout@v6(v6.0.1, released Dec 2, 2025)azure/setup-helm@v4(v4.3.1, latest stable)actions/setup-python@v6(v6.1.0, released Nov 25)helm/chart-testing-action@v2.8.0(confirmed in official examples)Helm v3.19.0 is a feature release that addresses previous compatibility concerns. The version bumps are appropriate and ready to merge.
deployment/hack/crds/consoleplugin.customresourcedefinition.yaml (4)
4-12: LGTM!The metadata annotations follow standard OpenShift CRD conventions for API approval, capability declaration, and release inclusion flags.
28-106: LGTM!The enhanced descriptions provide clearer documentation for the CRD fields, following Kubernetes API conventions with proper multi-line formatting.
107-229: Comprehensive CSP configuration with correct validation rules.The contentSecurityPolicy field provides a well-structured CSP model with appropriate CEL validations for security constraints. The validation expressions correctly enforce:
- No single quotes, whitespace, commas, or semicolons in values
- No wildcard (
*) values- Uniqueness of values within each directive
- Total combined size limit of 8KB
Note: The schema defines 6 directive types (DefaultSrc, ScriptSrc, StyleSrc, ImgSrc, FontSrc, ConnectSrc) but
maxItems: 5at line 221 means only 5 can be specified simultaneously. This appears to be intentional from the upstream OpenShift API.
237-346: LGTM!The i18n and proxy field updates are well-structured:
- The
x-kubernetes-list-type: atomicat line 346 ensures atomic updates to the proxy list, which is appropriate for this use case- The
authorizationfield withUserToken/Noneenum provides clear configuration options- Enhanced descriptions improve API documentation
* Bump action versions * Bump and pin Kind and K8s versions * Update the ConsolePlugin CRD * Allow manual dispatching * Allow forcing lint and test even if no changes were made
empovit
force-pushed
the
update-chart-test
branch
from
509826a to
544d952
Compare
empovit
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @.github/workflows/helm-lint-test.yaml:
- Line 23: The workflow updates introduce action version compatibility risks;
verify your runners meet the minimum GitHub Actions Runner versions (>=
v2.327.1, preferably v2.329.0+) and either pin or adjust action versions
accordingly: confirm actions/checkout@v6 is supported by your runners or
revert/pin to a compatible release (e.g., v5) if using older runners, pin
azure/setup-helm to a stable patch (e.g., v4.3.1) rather than a moving v4 tag,
and ensure actions/setup-python and helm/chart-testing-action versions match
your runner/node requirements before merging.
In @deployment/hack/crds/consoleplugin.customresourcedefinition.yaml:
- Around line 221-225: The contentSecurityPolicy array currently sets maxItems:
5 but the enum/doc list six directive types (DefaultSrc, ScriptSrc, StyleSrc,
ImgSrc, FontSrc, ConnectSrc); update the schema by changing maxItems from 5 to 6
for the contentSecurityPolicy array (and ensure x-kubernetes-list-type/map keys
remain unchanged) so all six directives can be defined once each.
♻️ Duplicate comments (1)
deployment/hack/crds/consoleplugin.customresourcedefinition.yaml (1)
231-233: Typo acknowledged from upstream.The typo "dispalyName" was previously flagged and the author confirmed it matches upstream for easier comparison. No further action needed.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/helm-lint-test.yamldeployment/hack/crds/consoleplugin.customresourcedefinition.yaml
🔇 Additional comments (17)
deployment/hack/crds/consoleplugin.customresourcedefinition.yaml (8)
4-12: LGTM!The metadata annotations follow standard OpenShift CRD conventions and provide appropriate capability and release inclusion markers.
28-47: LGTM!The descriptions follow Kubernetes API conventions and provide appropriate documentation links.
199-201: Verify the quote validation CEL rule.The validation rule
!self.contains("''")appears to check for two consecutive single quotes ('') rather than a single quote character. Based on the message "CSP directive value cannot contain a quote", the intent seems to be checking for a single quote.If this is from upstream, you may want to verify the behavior matches the documented intent.
241-251: LGTM!The description clearly documents the empty string option and its equivalence to Lazy loading behavior.
279-287: LGTM!The
authorizationfield change from boolean to string enum provides better extensibility while maintaining backward compatibility with the default value ofNone.
345-346: LGTM!Adding
x-kubernetes-list-type: atomicto the proxy array ensures proper replace-on-update semantics, which is appropriate for this type of configuration.
182-216: LGTM!The CSP values array has appropriate constraints with proper uniqueness validation using CEL's
exists_onepattern.
107-148: Well-documented CSP configuration.The contentSecurityPolicy field has comprehensive documentation with clear examples showing how directives are aggregated across plugins. The MDN links provide helpful reference material for users.
.github/workflows/helm-lint-test.yaml (9)
7-16: LGTM!The workflow_dispatch trigger with the force_install_unchanged input is well-structured. Using
type: choiceprovides good UX and prevents invalid values.
32-34: LGTM!Python 3.13 is the latest stable Python version and is appropriate for CI workflows.
39-46: LGTM!The conditional gating logic is well-implemented. It correctly sets the
should-installoutput based on either theforce_install_unchangedinput or detected chart changes viact list-changed.
49-50: LGTM!The conditional execution and dynamic
--allflag addition are correctly implemented. Whenforce_install_unchangedis true, all charts will be linted regardless of changes.
63-63: LGTM!The conditional gating for OpenShift dependencies and chart installation is consistently applied across all relevant steps. The dynamic
--allflag handling in the install step correctly mirrors the lint step.Also applies to: 79-80
59-60: azure/setup-kubectl@v4 is available.The action is released with versions v4.0.0 and v4.0.1, so the reference in the workflow is valid.
56-57: Consider updating to Kubernetes v1.33 or later; Kind v0.31.0 exists but defaults to v1.35.0.Kind v0.31.0 was released December 18, 2025 and defaults to Kubernetes v1.35.0, suggesting much newer versions are readily available. Kubernetes 1.32 reaches end-of-life February 28, 2026—approximately 2 months away. Update to v1.33.x or higher for extended support and consistency with Kind's upstream defaults.
Likely an incorrect or invalid review comment.
30-30: Helm v3.19.0 is compatible with Kubernetes v1.32.0. The upgrade from v3.16.3 resolves the previously identified incompatibility, as Helm v3.19.0 officially supports Kubernetes 1.31.x through 1.34.x.
54-54: helm/kind-action v1.13.0 is a valid release. It exists and has been publicly released with documented changes including kubectl sha256 verification and kind bumping to v0.29.0.
1 participant
Summary by CodeRabbit
New Features
Documentation
Breaking Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.