Merge pull request #184 from andyatmiami/feat/cherry-pick-kueue-checks

feat: add aggressive kueue gating and enhanced verbose output

Author: lburgazzoli

pkg/lint/check/result/result.go

@@ -16,6 +16,19 @@ const (
 	// (e.g., "notebooks.kubeflow.org"). Automatically set by SetImpactedObjects and
 	// AddImpactedObjects from the ResourceType.
 	AnnotationResourceCRDName = "resource.opendatahub.io/crd-name"
+
+	// AnnotationObjectContext is an optional per-object annotation key that provides
+	// brief clarifying context about why a specific object is impacted. When set on
+	// an ImpactedObject's ObjectMeta.Annotations, the EnhancedVerboseFormatter renders
+	// it as a sub-bullet beneath the object reference.
+	AnnotationObjectContext = "result.opendatahub.io/context"
+
+	// AnnotationObjectCRDName is an optional per-object annotation key for the CRD
+	// fully-qualified name (e.g., "notebooks.kubeflow.org"). Set from ResourceType.CRDFQN()
+	// when building impacted objects. Used by buildCRDFQNByKind for authoritative plural
+	// forms, avoiding naive derivation from Kind. Especially useful for multi-kind results
+	// where the result-level AnnotationResourceCRDName cannot represent all types.
+	AnnotationObjectCRDName = "result.opendatahub.io/crd-name"
 )
 
 const (
@@ -33,11 +46,12 @@ const (
 // Impact represents the upgrade impact level of a diagnostic condition.
 type Impact string
 
-// Impact levels for diagnostic conditions.
+// Impact levels for diagnostic conditions, ordered by severity (highest first).
 const (
-	ImpactBlocking Impact = "blocking" // Upgrade CANNOT proceed
-	ImpactAdvisory Impact = "advisory" // Upgrade CAN proceed with warning
-	ImpactNone     Impact = ""         // No impact (omitted from JSON/YAML)
+	ImpactProhibited Impact = "prohibited" // Upgrade is NOT POSSIBLE
+	ImpactBlocking   Impact = "blocking"   // Upgrade CANNOT proceed
+	ImpactAdvisory   Impact = "advisory"   // Upgrade CAN proceed with warning
+	ImpactNone       Impact = ""           // No impact (omitted from JSON/YAML)
 )
 
 // Condition represents a diagnostic condition with severity level.
@@ -71,16 +85,16 @@ func (c Condition) Validate() error {
 		// False/Unknown status must have impact specified.
 		if c.Impact == ImpactNone || c.Impact == "" {
 			return fmt.Errorf(
-				"condition with Status=%q must have Impact specified (Blocking or Advisory), got Impact=%q",
+				"condition with Status=%q must have Impact specified (Prohibited, Blocking, or Advisory), got Impact=%q",
 				c.Status, c.Impact,
 			)
 		}
 
 		// Validate impact values.
-		if c.Impact != ImpactBlocking && c.Impact != ImpactAdvisory {
+		if c.Impact != ImpactProhibited && c.Impact != ImpactBlocking && c.Impact != ImpactAdvisory {
 			return fmt.Errorf(
-				"invalid Impact=%q, must be %q or %q",
-				c.Impact, ImpactBlocking, ImpactAdvisory,
+				"invalid Impact=%q, must be %q, %q, or %q",
+				c.Impact, ImpactProhibited, ImpactBlocking, ImpactAdvisory,
 			)
 		}
 
@@ -254,10 +268,14 @@ func (r *DiagnosticResult) GetImpact() Impact {
 
 	for _, cond := range r.Status.Conditions {
 		switch cond.Impact {
+		case ImpactProhibited:
+			return ImpactProhibited
 		case ImpactBlocking:
-			return ImpactBlocking
+			maxImpact = ImpactBlocking
 		case ImpactAdvisory:
-			maxImpact = ImpactAdvisory
+			if maxImpact == ImpactNone {
+				maxImpact = ImpactAdvisory
+			}
 		case ImpactNone:
 			// No impact - continue checking other conditions
 		}

pkg/lint/check/result/result_test.go

@@ -307,6 +307,53 @@ func TestDiagnosticResult_Validate_EmptyConditionReason(t *testing.T) {
 	g.Expect(err.Error()).To(ContainSubstring("has empty reason"))
 }
 
+func TestConditionValidate_ProhibitedWithStatusFalse(t *testing.T) {
+	g := NewWithT(t)
+
+	cond := result.Condition{
+		Condition: metav1.Condition{
+			Type:   "Consistency",
+			Status: metav1.ConditionFalse,
+			Reason: "Inconsistent",
+		},
+		Impact: result.ImpactProhibited,
+	}
+
+	g.Expect(cond.Validate()).ToNot(HaveOccurred())
+}
+
+func TestConditionValidate_ProhibitedWithStatusUnknown(t *testing.T) {
+	g := NewWithT(t)
+
+	cond := result.Condition{
+		Condition: metav1.Condition{
+			Type:   "Consistency",
+			Status: metav1.ConditionUnknown,
+			Reason: "CannotDetermine",
+		},
+		Impact: result.ImpactProhibited,
+	}
+
+	g.Expect(cond.Validate()).ToNot(HaveOccurred())
+}
+
+func TestConditionValidate_ProhibitedWithStatusTrue_Invalid(t *testing.T) {
+	g := NewWithT(t)
+
+	cond := result.Condition{
+		Condition: metav1.Condition{
+			Type:   "Consistency",
+			Status: metav1.ConditionTrue,
+			Reason: "Consistent",
+		},
+		Impact: result.ImpactProhibited,
+	}
+
+	err := cond.Validate()
+	g.Expect(err).To(HaveOccurred())
+	g.Expect(err.Error()).To(ContainSubstring("Status=True must have Impact=None"))
+}
+
 // T023: Multiple conditions array tests
 
 func TestMultipleConditions_OrderedByExecutionSequence(t *testing.T) {
@@ -790,6 +837,118 @@ func TestSetCondition_MultipleConditionTypes(t *testing.T) {
 	}))
 }
 
+// GetImpact tests
+
+func TestGetImpact_ReturnsProhibitedAsHighest(t *testing.T) {
+	g := NewWithT(t)
+
+	dr := result.New("workload", "kueue", "data-integrity", "test")
+	dr.SetCondition(check.NewCondition(
+		"Advisory",
+		metav1.ConditionFalse,
+		check.WithReason("Reason"),
+		check.WithMessage("advisory finding"),
+		check.WithImpact(result.ImpactAdvisory),
+	))
+	dr.SetCondition(check.NewCondition(
+		"Blocking",
+		metav1.ConditionFalse,
+		check.WithReason("Reason"),
+		check.WithMessage("blocking finding"),
+		check.WithImpact(result.ImpactBlocking),
+	))
+	dr.SetCondition(check.NewCondition(
+		"Prohibited",
+		metav1.ConditionFalse,
+		check.WithReason("Reason"),
+		check.WithMessage("prohibited finding"),
+		check.WithImpact(result.ImpactProhibited),
+	))
+
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactProhibited))
+}
+
+func TestGetImpact_ProhibitedShortCircuits(t *testing.T) {
+	g := NewWithT(t)
+
+	// Prohibited condition first — should return immediately without checking others.
+	dr := result.New("workload", "kueue", "data-integrity", "test")
+	dr.SetCondition(check.NewCondition(
+		"Prohibited",
+		metav1.ConditionFalse,
+		check.WithReason("Reason"),
+		check.WithMessage("prohibited finding"),
+		check.WithImpact(result.ImpactProhibited),
+	))
+	dr.SetCondition(check.NewCondition(
+		"Passing",
+		metav1.ConditionTrue,
+		check.WithReason("Reason"),
+		check.WithMessage("all good"),
+	))
+
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactProhibited))
+}
+
+func TestGetImpact_BlockingWithoutProhibited(t *testing.T) {
+	g := NewWithT(t)
+
+	dr := result.New("component", "kserve", "removal", "test")
+	dr.SetCondition(check.NewCondition(
+		"Blocking",
+		metav1.ConditionFalse,
+		check.WithReason("Reason"),
+		check.WithMessage("blocking finding"),
+		check.WithImpact(result.ImpactBlocking),
+	))
+	dr.SetCondition(check.NewCondition(
+		"Advisory",
+		metav1.ConditionFalse,
+		check.WithReason("Reason"),
+		check.WithMessage("advisory finding"),
+		check.WithImpact(result.ImpactAdvisory),
+	))
+
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactBlocking))
+}
+
+func TestGetImpact_AdvisoryWithoutBlockingOrProhibited(t *testing.T) {
+	g := NewWithT(t)
+
+	dr := result.New("component", "dashboard", "migration", "test")
+	dr.SetCondition(check.NewCondition(
+		"Advisory",
+		metav1.ConditionFalse,
+		check.WithReason("Reason"),
+		check.WithMessage("advisory finding"),
+		check.WithImpact(result.ImpactAdvisory),
+	))
+
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactAdvisory))
+}
+
+func TestGetImpact_NoneWhenAllPassing(t *testing.T) {
+	g := NewWithT(t)
+
+	dr := result.New("component", "dashboard", "status", "test")
+	dr.SetCondition(check.NewCondition(
+		"Available",
+		metav1.ConditionTrue,
+		check.WithReason("Ready"),
+		check.WithMessage("all good"),
+	))
+
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactNone))
+}
+
+func TestGetImpact_NoneWhenNoConditions(t *testing.T) {
+	g := NewWithT(t)
+
+	dr := result.New("component", "test", "check", "test")
+
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactNone))
+}
+
 // SetImpactedObjects and AddImpactedObjects tests
 
 func TestSetImpactedObjects(t *testing.T) {