fixup! feat: Preflight check opt-out

Use 'skip' instead of 'opt-out' to be consistent with similar usage in core Cluster API

Author: dlipovetsky

pkg/webhook/preflight/preflight.go

@@ -16,7 +16,7 @@ import (
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
-	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/optout"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/skip"
 )
 
 type (
@@ -95,16 +95,16 @@ func (h *WebhookHandler) Handle(ctx context.Context, req admission.Request) admi
 		return admission.Allowed("")
 	}
 
-	optoutEvaluator := optout.New(cluster)
+	skipEvaluator := skip.New(cluster)
 
-	if optoutEvaluator.ForAll() {
-		// If the cluster has opted out of all checks, return allowed.
+	if skipEvaluator.ForAll() {
+		// If the cluster has skipped all checks, return allowed.
 		return admission.Allowed("").WithWarnings(
-			"Cluster has opted out of all preflight checks",
+			"Cluster has skipped all preflight checks",
 		)
 	}
 
-	resultsOrderedByCheckerAndCheck := run(ctx, h.client, cluster, optoutEvaluator, h.checkers)
+	resultsOrderedByCheckerAndCheck := run(ctx, h.client, cluster, skipEvaluator, h.checkers)
 
 	// Summarize the results.
 	resp := admission.Response{
@@ -156,7 +156,7 @@ func (h *WebhookHandler) Handle(ctx context.Context, req admission.Request) admi
 func run(ctx context.Context,
 	client ctrlclient.Client,
 	cluster *clusterv1.Cluster,
-	optoutEvaluator *optout.Evaluator,
+	skipEvaluator *skip.Evaluator,
 	checkers []Checker,
 ) [][]namedResult {
 	resultsOrderedByCheckerAndCheck := make([][]namedResult, len(checkers))
@@ -168,7 +168,7 @@ func run(ctx context.Context,
 			ctx context.Context,
 			client ctrlclient.Client,
 			cluster *clusterv1.Cluster,
-			optoutEvaluator *optout.Evaluator,
+			skipEvaluator *skip.Evaluator,
 			checker Checker,
 			i int,
 		) {
@@ -179,16 +179,15 @@ func run(ctx context.Context,
 
 			checksWG := sync.WaitGroup{}
 			for j, check := range checks {
-				if optoutEvaluator.For(check.Name()) {
-					// If the cluster has opted out of this check, skip it.
+				if skipEvaluator.For(check.Name()) {
 					resultsOrderedByCheck[j] = namedResult{
 						Name: check.Name(),
 						CheckResult: CheckResult{
 							Allowed: true,
 							Error:   false,
 							Causes:  nil,
 							Warnings: []string{
-								fmt.Sprintf("Cluster has opted out of preflight check %q", check.Name()),
+								fmt.Sprintf("Cluster has skipped preflight check %q", check.Name()),
 							},
 						},
 					}
@@ -238,7 +237,7 @@ func run(ctx context.Context,
 			ctx,
 			client,
 			cluster,
-			optoutEvaluator,
+			skipEvaluator,
 			checker,
 			i,
 		)

pkg/webhook/preflight/preflight_test.go

@@ -24,7 +24,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
-	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/optout"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/skip"
 )
 
 type mockChecker struct {
@@ -63,12 +63,12 @@ func (m *mockDecoder) DecodeRaw(_ runtime.RawExtension, _ runtime.Object) error
 	return m.err
 }
 
-func topologyCluster(optoutCheckNames ...string) *clusterv1.Cluster {
+func topologyCluster(skippedCheckNames ...string) *clusterv1.Cluster {
 	return &clusterv1.Cluster{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-cluster",
 			Annotations: map[string]string{
-				optout.AnnotationKey: strings.Join(optoutCheckNames, ","),
+				skip.AnnotationKey: strings.Join(skippedCheckNames, ","),
 			},
 		},
 		Spec: clusterv1.ClusterSpec{
@@ -146,8 +146,8 @@ func TestHandle(t *testing.T) {
 			},
 		},
 		{
-			name:    "if cluster opts out of all checks, then allowed, with a warning",
-			cluster: topologyCluster(optout.OptOutAllChecksAnnotationValue),
+			name:    "if cluster skips all checks, then allowed, with a warning",
+			cluster: topologyCluster(skip.SkipAllChecksAnnotationValue),
 			checkers: []Checker{
 				&mockChecker{
 					checks: []Check{},
@@ -157,19 +157,19 @@ func TestHandle(t *testing.T) {
 				AdmissionResponse: admissionv1.AdmissionResponse{
 					Allowed: true,
 					Warnings: []string{
-						"Cluster has opted out of all preflight checks",
+						"Cluster has skipped all preflight checks",
 					},
 				},
 			},
 		},
 		{
-			name:    "if cluster opts out of a check, then that check is not run",
-			cluster: topologyCluster("OptOutCheck"),
+			name:    "if cluster skips a check, then that check is not run",
+			cluster: topologyCluster("SkippedCheck"),
 			checkers: []Checker{
 				&mockChecker{
 					checks: []Check{
 						&mockCheck{
-							name:   "OptOutCheck",
+							name:   "SkippedCheck",
 							result: CheckResult{},
 						},
 						&mockCheck{
@@ -185,7 +185,7 @@ func TestHandle(t *testing.T) {
 				AdmissionResponse: admissionv1.AdmissionResponse{
 					Allowed: true,
 					Warnings: []string{
-						"Cluster has opted out of preflight check \"OptOutCheck\"",
+						"Cluster has skipped preflight check \"SkippedCheck\"",
 					},
 				},
 			},
@@ -550,7 +550,7 @@ func TestRun_SingleCheckerSingleCheck(t *testing.T) {
 			},
 		},
 	}
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker})
 	if len(resultsOrderedByCheckerAndCheck) != 1 {
 		t.Fatalf("expected results for 1 checker, got %d", len(resultsOrderedByCheckerAndCheck))
 	}
@@ -590,7 +590,7 @@ func TestRun_MultipleCheckersMultipleChecks(t *testing.T) {
 		},
 	}
 
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker1, checker2})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker1, checker2})
 	if len(resultsOrderedByCheckerAndCheck) != 2 {
 		t.Fatalf("expected results for 2 checkers, got %d", len(resultsOrderedByCheckerAndCheck))
 	}
@@ -654,7 +654,7 @@ func TestRun_ChecksRunInParallel(t *testing.T) {
 			},
 		},
 	}
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker})
 
 	results := resultsOrderedByCheckerAndCheck[0]
 	if len(results) != 2 {
@@ -693,7 +693,7 @@ func TestRun_CheckersRunInParallel(t *testing.T) {
 		},
 	}
 
-	results := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker1, checker2})
+	results := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker1, checker2})
 	if len(results) != 2 {
 		t.Fatalf("expected 2 results, got %d", len(results))
 	}
@@ -750,7 +750,7 @@ func TestRun_ContextCancellation(t *testing.T) {
 		cancel()
 	}()
 
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker})
 
 	select {
 	case <-completed:
@@ -798,7 +798,7 @@ func TestRun_OrderOfResults(t *testing.T) {
 		},
 	}
 
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker1, checker2})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker1, checker2})
 	if len(resultsOrderedByCheckerAndCheck) != 2 {
 		t.Fatalf("expected results for 2 checkers, got %d", len(resultsOrderedByCheckerAndCheck))
 	}
@@ -843,7 +843,7 @@ func TestRun_LargeNumberOfCheckersAndChecks(t *testing.T) {
 	}
 
 	start := time.Now()
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), checkers)
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), checkers)
 	duration := time.Since(start)
 
 	resultTotal := 0
@@ -878,7 +878,7 @@ func TestRun_ErrorHandlingInChecks(t *testing.T) {
 	}
 
 	// Run the checks
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker})
 	assert.Len(t, resultsOrderedByCheckerAndCheck, 1, "expected results for 1 checker")
 	assert.Len(t, resultsOrderedByCheckerAndCheck[0], 1, "expected 1 result from the checker")
 
@@ -933,7 +933,7 @@ func TestRun_PanicHandlingInChecks(t *testing.T) {
 	}
 
 	// Run the checks
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{checker})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{checker})
 	assert.Len(t, resultsOrderedByCheckerAndCheck, 1, "expected results for 1 checker")
 	assert.Len(t, resultsOrderedByCheckerAndCheck[0], 2, "expected 2 results from the checker")
 
@@ -980,7 +980,7 @@ func TestRun_ZeroChecksFromChecker(t *testing.T) {
 		},
 	}
 
-	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, optout.New(cluster), []Checker{emptyChecker, normalChecker})
+	resultsOrderedByCheckerAndCheck := run(ctx, nil, cluster, skip.New(cluster), []Checker{emptyChecker, normalChecker})
 
 	if len(resultsOrderedByCheckerAndCheck) != 2 {
 		t.Fatalf("expected results for 2 checkers, got %d", len(resultsOrderedByCheckerAndCheck))

pkg/webhook/preflight/optout/optout.go renamed to pkg/webhook/preflight/skip/skip.go

@@ -1,6 +1,6 @@
 // Copyright 2025 Nutanix. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
-package optout
+package skip
 
 import (
 	"strings"
@@ -9,14 +9,15 @@ import (
 )
 
 const (
-	// AnnotationKey is the key of the annotation on the Cluster used to opt out preflight checks.
-	AnnotationKey = "preflight.cluster.caren.nutanix.com/opt-out"
+	// AnnotationKey is the key of the annotation on the Cluster used to skip preflight checks.
+	AnnotationKey = "preflight.cluster.caren.nutanix.com/skip"
 
-	// OptOutAllChecksAnnotationValue is the value used in the cluster's annotations to indicate
-	// that all checks are opted out.
-	OptOutAllChecksAnnotationValue = "all"
+	// SkipAllChecksAnnotationValue is the value used in the cluster's annotations to indicate
+	// that all checks are skipped.
+	SkipAllChecksAnnotationValue = "all"
 )
 
+// Evaluator is used to determine which checks should be skipped, based on the cluster's annotations.
 type Evaluator struct {
 	normalizedCheckNames map[string]struct{}
 	all                  bool
@@ -48,32 +49,32 @@ func New(cluster *clusterv1.Cluster) *Evaluator {
 		normalizedCheckName := strings.TrimSpace(strings.ToLower(checkName))
 		o.normalizedCheckNames[normalizedCheckName] = struct{}{}
 	}
-	if _, exists := o.normalizedCheckNames[OptOutAllChecksAnnotationValue]; exists && len(o.normalizedCheckNames) == 1 {
+	if _, exists := o.normalizedCheckNames[SkipAllChecksAnnotationValue]; exists && len(o.normalizedCheckNames) == 1 {
 		o.all = true
 	}
 	return o
 }
 
-// For checks if the cluster has opted out of a specific check.
-// It returns true if the cluster has opted out of the check with the given name.
+// For checks if the specific check should be skipped.
+// It returns true if the cluster skip annotation contains the check name.
 // The check name is case-insensitive, so "CheckName1" and "checkname1" will both match.
-// If the cluster has opted out of all checks, For will return true for any check name.
+// If the cluster has skipped all checks, For will return true for any check name.
 //
-// For example, if the cluster has opted out of "CheckName1", then calling
+// For example, if the cluster has skipped "CheckName1", then calling
 // For("CheckName1") or For("checkname1") will return true, but For("CheckName2") will
 // return false.
 func (o *Evaluator) For(checkName string) bool {
 	if o.all {
-		// If the cluster has opted out of all checks, return true for any check name.
+		// If the cluster has skipped all checks, return true for any check name.
 		return true
 	}
 	normalizedCheckName := strings.TrimSpace(strings.ToLower(checkName))
 	_, exists := o.normalizedCheckNames[normalizedCheckName]
 	return exists
 }
 
-// ForAll checks if the cluster has opted out of all checks.
-// It returns true if the cluster has a single prefix "all" in its opt-out annotations.
+// ForAll checks if all checks should be skipped.
+// It returns true if the cluster skip annotation contains "all".
 // The check is case-insensitive, so "all", "ALL", and "All" will all match.
 func (o *Evaluator) ForAll() bool {
 	return o.all