feat(controller): add enabled/disabled to NodeScanConfiguration#1238
Open
fabriziosestito wants to merge 3 commits into
Open
feat(controller): add enabled/disabled to NodeScanConfiguration#1238fabriziosestito wants to merge 3 commits into
fabriziosestito wants to merge 3 commits into
Conversation
…leteAllOf Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
fabriziosestito
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an enable/disable switch for node scanning via NodeScanConfiguration.spec.enabled, and adds controller logic to clean up node-scan resources when scanning is disabled or the configuration is removed. It also changes node-deletion cleanup to use cached index-based lookups instead of DeleteAllOf.
Changes:
- Add
spec.enabledtoNodeScanConfigurationand skip scan cycles when disabled. - Add a
NodeScanConfigurationcontroller that deletes allNodeScanJobandNodeSBOMresources when disabled/missing. - Fix node-deletion cleanup by switching from
DeleteAllOfto List+Delete using field indexes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/nodescan_test.go | Adds an E2E step to disable the configuration and assert cleanup. |
| internal/controller/nodescanconfiguration_controller.go | New reconciler that performs cluster-wide cleanup when node scanning is disabled or config is missing. |
| internal/controller/nodescanconfiguration_controller_test.go | Unit tests for the new configuration reconciler behavior (enabled/disabled/missing). |
| internal/controller/nodescan_runner.go | Skips node scan cycles when the configuration is disabled. |
| internal/controller/nodescan_runner_test.go | Updates runner tests to explicitly set Enabled: true in created configs. |
| internal/controller/nodescan_controller.go | Reworks per-node cleanup to List+Delete via cached field indexes. |
| cmd/controller/main.go | Registers the new NodeScanConfigurationReconciler with the manager. |
| charts/sbomscanner/templates/crd/sbomscanner.kubewarden.io_nodescanconfigurations.yaml | Adds enabled to the CRD schema (with default) and marks it required. |
| charts/sbomscanner/templates/controller/role.yaml | Expands RBAC to include deletecollection for node scan resources and broader verbs for nodescanjobs. |
| api/v1alpha1/nodescanconfiguration_types.go | Adds the Enabled field to the API type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
16
to
+20
| // NodeScanConfigurationSpec defines the desired configuration for node scanning. | ||
| type NodeScanConfigurationSpec struct { | ||
| // Enabled controls whether node scanning is active. | ||
| // +kubebuilder:default=true | ||
| Enabled bool `json:"enabled"` |
Comment on lines
+137
to
139
| required: | ||
| - enabled | ||
| type: object |
Comment on lines
+72
to
+75
| if !config.Spec.Enabled { | ||
| log.V(1).Info("NodeScanConfiguration disabled, skipping node scan cycle") | ||
| return nil | ||
| } |
Comment on lines
+42
to
+48
| if !config.Spec.Enabled { | ||
| logger.Info("NodeScanConfiguration disabled, cleaning up all node scan resources") | ||
| if err := r.cleanupAllNodeResources(ctx); err != nil { | ||
| return ctrl.Result{}, fmt.Errorf("failed to cleanup all node scan resources: %w", err) | ||
| } | ||
| return ctrl.Result{}, nil | ||
| } |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1238 +/- ##
==========================================
- Coverage 53.44% 49.74% -3.70%
==========================================
Files 61 78 +17
Lines 5340 6527 +1187
==========================================
+ Hits 2854 3247 +393
- Misses 2088 2816 +728
- Partials 398 464 +66
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
Description
DeleteAllOfnot working with cache indexes in NodeScan controllerFixes #1179