chore: pr review feedback

Signed-off-by: Andy Stoneberg <astonebe@redhat.com>

Author: andyatmiami

pkg/lint/check/result/result.go

@@ -22,6 +22,13 @@ const (
 	// 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 (
@@ -41,10 +48,10 @@ type Impact string
 
 // Impact levels for diagnostic conditions, ordered by severity (highest first).
 const (
-	ImpactAborting Impact = "aborting" // Upgrade MUST NOT proceed
-	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.
@@ -78,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 (Aborting, 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 != ImpactAborting && 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, %q, or %q",
-				c.Impact, ImpactAborting, ImpactBlocking, ImpactAdvisory,
+				c.Impact, ImpactProhibited, ImpactBlocking, ImpactAdvisory,
 			)
 		}
 
@@ -261,8 +268,8 @@ func (r *DiagnosticResult) GetImpact() Impact {
 
 	for _, cond := range r.Status.Conditions {
 		switch cond.Impact {
-		case ImpactAborting:
-			return ImpactAborting
+		case ImpactProhibited:
+			return ImpactProhibited
 		case ImpactBlocking:
 			maxImpact = ImpactBlocking
 		case ImpactAdvisory:

pkg/lint/check/result/result_test.go

@@ -307,7 +307,7 @@ func TestDiagnosticResult_Validate_EmptyConditionReason(t *testing.T) {
 	g.Expect(err.Error()).To(ContainSubstring("has empty reason"))
 }
 
-func TestConditionValidate_AbortingWithStatusFalse(t *testing.T) {
+func TestConditionValidate_ProhibitedWithStatusFalse(t *testing.T) {
 	g := NewWithT(t)
 
 	cond := result.Condition{
@@ -316,13 +316,13 @@ func TestConditionValidate_AbortingWithStatusFalse(t *testing.T) {
 			Status: metav1.ConditionFalse,
 			Reason: "Inconsistent",
 		},
-		Impact: result.ImpactAborting,
+		Impact: result.ImpactProhibited,
 	}
 
 	g.Expect(cond.Validate()).ToNot(HaveOccurred())
 }
 
-func TestConditionValidate_AbortingWithStatusUnknown(t *testing.T) {
+func TestConditionValidate_ProhibitedWithStatusUnknown(t *testing.T) {
 	g := NewWithT(t)
 
 	cond := result.Condition{
@@ -331,13 +331,13 @@ func TestConditionValidate_AbortingWithStatusUnknown(t *testing.T) {
 			Status: metav1.ConditionUnknown,
 			Reason: "CannotDetermine",
 		},
-		Impact: result.ImpactAborting,
+		Impact: result.ImpactProhibited,
 	}
 
 	g.Expect(cond.Validate()).ToNot(HaveOccurred())
 }
 
-func TestConditionValidate_AbortingWithStatusTrue_Invalid(t *testing.T) {
+func TestConditionValidate_ProhibitedWithStatusTrue_Invalid(t *testing.T) {
 	g := NewWithT(t)
 
 	cond := result.Condition{
@@ -346,7 +346,7 @@ func TestConditionValidate_AbortingWithStatusTrue_Invalid(t *testing.T) {
 			Status: metav1.ConditionTrue,
 			Reason: "Consistent",
 		},
-		Impact: result.ImpactAborting,
+		Impact: result.ImpactProhibited,
 	}
 
 	err := cond.Validate()
@@ -839,7 +839,7 @@ func TestSetCondition_MultipleConditionTypes(t *testing.T) {
 
 // GetImpact tests
 
-func TestGetImpact_ReturnsAbortingAsHighest(t *testing.T) {
+func TestGetImpact_ReturnsProhibitedAsHighest(t *testing.T) {
 	g := NewWithT(t)
 
 	dr := result.New("workload", "kueue", "data-integrity", "test")
@@ -858,27 +858,27 @@ func TestGetImpact_ReturnsAbortingAsHighest(t *testing.T) {
 		check.WithImpact(result.ImpactBlocking),
 	))
 	dr.SetCondition(check.NewCondition(
-		"Aborting",
+		"Prohibited",
 		metav1.ConditionFalse,
 		check.WithReason("Reason"),
-		check.WithMessage("aborting finding"),
-		check.WithImpact(result.ImpactAborting),
+		check.WithMessage("prohibited finding"),
+		check.WithImpact(result.ImpactProhibited),
 	))
 
-	g.Expect(dr.GetImpact()).To(Equal(result.ImpactAborting))
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactProhibited))
 }
 
-func TestGetImpact_AbortingShortCircuits(t *testing.T) {
+func TestGetImpact_ProhibitedShortCircuits(t *testing.T) {
 	g := NewWithT(t)
 
-	// Aborting condition first — should return immediately without checking others.
+	// Prohibited condition first — should return immediately without checking others.
 	dr := result.New("workload", "kueue", "data-integrity", "test")
 	dr.SetCondition(check.NewCondition(
-		"Aborting",
+		"Prohibited",
 		metav1.ConditionFalse,
 		check.WithReason("Reason"),
-		check.WithMessage("aborting finding"),
-		check.WithImpact(result.ImpactAborting),
+		check.WithMessage("prohibited finding"),
+		check.WithImpact(result.ImpactProhibited),
 	))
 	dr.SetCondition(check.NewCondition(
 		"Passing",
@@ -887,10 +887,10 @@ func TestGetImpact_AbortingShortCircuits(t *testing.T) {
 		check.WithMessage("all good"),
 	))
 
-	g.Expect(dr.GetImpact()).To(Equal(result.ImpactAborting))
+	g.Expect(dr.GetImpact()).To(Equal(result.ImpactProhibited))
 }
 
-func TestGetImpact_BlockingWithoutAborting(t *testing.T) {
+func TestGetImpact_BlockingWithoutProhibited(t *testing.T) {
 	g := NewWithT(t)
 
 	dr := result.New("component", "kserve", "removal", "test")
@@ -912,6 +912,21 @@ func TestGetImpact_BlockingWithoutAborting(t *testing.T) {
 	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)
 

pkg/lint/check/verbose_formatter.go

@@ -217,9 +217,10 @@ func writeQualifiedObjects(out io.Writer, objects []qualifiedObject, indent stri
 }
 
 // buildCRDFQNByKind returns a map from Kind to CRD FQN for the impacted objects.
-// When all objects share the same kind and the AnnotationResourceCRDName annotation
-// is set, the annotation value is used for the most accurate plural form.
-// Otherwise, the CRD FQN is derived from each object's TypeMeta.
+// Resolution priority per kind:
+//  1. Per-object AnnotationObjectCRDName (authoritative, set from ResourceType.CRDFQN())
+//  2. Result-level AnnotationResourceCRDName (single-kind results only)
+//  3. Naive derivation from TypeMeta (fallback)
 func buildCRDFQNByKind(dr *result.DiagnosticResult) map[string]string {
 	fqnByKind := make(map[string]string)
 
@@ -228,11 +229,19 @@ func buildCRDFQNByKind(dr *result.DiagnosticResult) map[string]string {
 			continue
 		}
 
+		// Prefer per-object annotation set from ResourceType.CRDFQN().
+		if name, ok := obj.Annotations[result.AnnotationObjectCRDName]; ok && name != "" {
+			fqnByKind[obj.Kind] = name
+
+			continue
+		}
+
 		fqnByKind[obj.Kind] = DeriveCRDFQNFromTypeMeta([]metav1.PartialObjectMetadata{obj})
 	}
 
-	// When there is exactly one kind and the annotation is set, prefer the annotation
-	// for the most accurate plural form (e.g. "inferenceservices" not "inferenceservicss").
+	// When there is exactly one kind and the result-level annotation is set,
+	// prefer it over naive derivation (backward compatibility for callers
+	// that use SetImpactedObjects/AddImpactedObjects).
 	if len(fqnByKind) == 1 {
 		if name, ok := dr.Annotations[result.AnnotationResourceCRDName]; ok && name != "" {
 			for kind := range fqnByKind {
@@ -275,8 +284,8 @@ func DeriveCRDFQNFromTypeMeta(objects []metav1.PartialObjectMetadata) string {
 		return ""
 	}
 
-	// Kubernetes CRD plural convention: lowercase kind + "s"
-	// (e.g. Notebook → notebooks, InferenceService → inferenceservices, RayCluster → rayclusters).
+	// Naive plural as fallback — callers should set AnnotationResourceCRDName
+	// via ResourceType.CRDFQN() for authoritative plural forms.
 	plural := strings.ToLower(kind) + "s"
 
 	// Extract API group from APIVersion (e.g. "kubeflow.org/v1" → "kubeflow.org").

pkg/lint/check/verbose_formatter_test.go

@@ -292,6 +292,36 @@ func TestEnhancedVerboseFormatter_MixedKinds(t *testing.T) {
 	g.Expect(buf.String()).To(Equal(expected))
 }
 
+func TestEnhancedVerboseFormatter_MixedKindsIgnoresAnnotation(t *testing.T) {
+	g := NewWithT(t)
+
+	// When multiple kinds are present, the AnnotationResourceCRDName annotation
+	// should be ignored — it only applies to single-kind results.
+	dr := result.New("workload", "kueue", "data-integrity", "test description")
+	dr.Annotations[result.AnnotationResourceCRDName] = "notebooks.kubeflow.org"
+	dr.ImpactedObjects = []metav1.PartialObjectMetadata{
+		{
+			TypeMeta:   metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"},
+			ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "my-nb"},
+		},
+		{
+			TypeMeta:   metav1.TypeMeta{Kind: "RayCluster", APIVersion: "ray.io/v1"},
+			ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "my-ray"},
+		},
+	}
+
+	formatter := &check.EnhancedVerboseFormatter{}
+
+	var buf bytes.Buffer
+	formatter.FormatVerboseOutput(&buf, dr)
+
+	output := buf.String()
+
+	// RayCluster should use derived FQN, not the annotation value.
+	g.Expect(output).To(ContainSubstring("rayclusters.ray.io/my-ray"))
+	g.Expect(output).To(ContainSubstring("notebooks.kubeflow.org/my-nb"))
+}
+
 func TestEnhancedVerboseFormatter_MixedKindsSortedByCRDThenName(t *testing.T) {
 	g := NewWithT(t)
 

pkg/lint/checks/components/kueue/kueue_operator_installed.go

@@ -24,7 +24,7 @@ const (
 
 // OperatorInstalledCheck validates the Red Hat build of Kueue operator installation status against the Kueue
 // component management state:
-//   - Managed: aborting — no supported upgrade path from embedded Kueue, must migrate to RHBoK first
+//   - Managed: prohibited — no supported upgrade path from embedded Kueue, must migrate to RHBoK first
 //   - Unmanaged + operator absent: blocking — Unmanaged requires the Red Hat build of Kueue operator
 //   - Unmanaged + operator present: pass
 type OperatorInstalledCheck struct {
@@ -81,7 +81,7 @@ func (c *OperatorInstalledCheck) Validate(ctx context.Context, target check.Targ
 		})
 }
 
-// validateManaged flags Kueue Managed state as a fatal condition.
+// validateManaged flags Kueue Managed state as a prohibited condition.
 // There is no supported upgrade path from embedded Kueue — migration to the
 // Red Hat build of Kueue operator is required before upgrading.
 func (c *OperatorInstalledCheck) validateManaged(
@@ -93,7 +93,7 @@ func (c *OperatorInstalledCheck) validateManaged(
 		metav1.ConditionFalse,
 		check.WithReason(check.ReasonVersionIncompatible),
 		check.WithMessage(msgManagedNotSupported),
-		check.WithImpact(result.ImpactAborting),
+		check.WithImpact(result.ImpactProhibited),
 		check.WithRemediation(managementStateRemediation),
 	))
 }

pkg/lint/checks/components/kueue/kueue_operator_installed_test.go

@@ -82,7 +82,7 @@ func TestOperatorInstalledCheck_UnmanagedNotInstalled(t *testing.T) {
 	g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking))
 }
 
-// Managed + operator NOT installed = aborting.
+// Managed + operator NOT installed = prohibited.
 func TestOperatorInstalledCheck_ManagedNotInstalled(t *testing.T) {
 	g := NewWithT(t)
 	ctx := t.Context()
@@ -105,10 +105,10 @@ func TestOperatorInstalledCheck_ManagedNotInstalled(t *testing.T) {
 		"Reason":  Equal(check.ReasonVersionIncompatible),
 		"Message": ContainSubstring("migration to the Red Hat build of Kueue operator is required"),
 	}))
-	g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactAborting))
+	g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactProhibited))
 }
 
-// Managed + operator installed = aborting.
+// Managed + operator installed = prohibited.
 func TestOperatorInstalledCheck_ManagedInstalled(t *testing.T) {
 	g := NewWithT(t)
 	ctx := t.Context()
@@ -131,7 +131,7 @@ func TestOperatorInstalledCheck_ManagedInstalled(t *testing.T) {
 		"Reason":  Equal(check.ReasonVersionIncompatible),
 		"Message": ContainSubstring("migration to the Red Hat build of Kueue operator is required"),
 	}))
-	g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactAborting))
+	g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactProhibited))
 	g.Expect(result.Annotations).To(HaveKeyWithValue("operator.opendatahub.io/installed-version", "kueue-operator.v0.6.0"))
 }