Bug 1976072: Ignore deprecated descriptor x-capabilities

Previously, we were allowing deprecated x-descriptors to be used, which caused some unexpected behavior when both deprecated and non-deprecated descriptors were defined on the same property. Updated descriptor logic to ignore deprecated descriptors and log a warning.

frontend/packages/operator-lifecycle-manager/src/components/descriptors/const.ts

@@ -30,51 +30,93 @@ export const REGEXP_NESTED_ARRAY_PATH = /^.*\[\d+\]\.?.*\[\d+\]\.?.*$/;
 //    'this'  -> ['this']
 export const REGEXP_CAPTURE_GROUP_SUBGROUP = /^([^.]*)\.?(.*)$/;
 
-export const DEPRECATED_CAPABILITIES: (SpecCapability | StatusCapability)[] = [
+export const DEPRECATED_CAPABILITIES: SpecCapability[] = [
   SpecCapability.arrayFieldGroup,
   SpecCapability.fieldGroup,
-  SpecCapability.label,
   SpecCapability.namespaceSelector,
+  SpecCapability.label,
 ];
 
-export const OBJECT_COMPATIBLE_CAPABILITIES: (SpecCapability | StatusCapability)[] = [
+export const COMMON_COMPATIBLE_CAPABILITIES: SpecCapability[] = [
   SpecCapability.advanced,
   SpecCapability.fieldDependency,
   SpecCapability.hidden,
-  SpecCapability.namespaceSelector,
+  // TODO remove when deprecated descriptors are no longer supported
+  SpecCapability.arrayFieldGroup,
+  SpecCapability.fieldGroup,
+  // END TODO
+];
+
+export const OBJECT_COMPATIBLE_CAPABILITIES: (SpecCapability | StatusCapability)[] = [
+  StatusCapability.podStatuses,
+  SpecCapability.updateStrategy,
   SpecCapability.nodeAffinity,
   SpecCapability.podAffinity,
   SpecCapability.podAntiAffinity,
   SpecCapability.resourceRequirements,
   SpecCapability.selector,
-  SpecCapability.updateStrategy,
-  StatusCapability.podStatuses,
+  // TODO remove when deprecated descriptors are no longer supported
+  SpecCapability.label,
+  SpecCapability.namespaceSelector,
+  // END TODO
 ];
 
 export const ARRAY_COMPATIBLE_CAPABILITIES: (SpecCapability | StatusCapability)[] = [
-  SpecCapability.advanced,
   SpecCapability.endpointList,
-  SpecCapability.fieldDependency,
-  SpecCapability.hidden,
   StatusCapability.conditions,
 ];
 
 export const PRIMITIVE_COMPATIBLE_CAPABILITIES: (SpecCapability | StatusCapability)[] = [
-  SpecCapability.advanced,
+  StatusCapability.k8sPhase,
+  StatusCapability.k8sPhaseReason,
+  SpecCapability.k8sResourcePrefix,
+  SpecCapability.imagePullPolicy,
+  SpecCapability.podCount,
+  SpecCapability.select,
+  StatusCapability.w3Link,
   SpecCapability.booleanSwitch,
   SpecCapability.checkbox,
-  SpecCapability.fieldDependency,
+  SpecCapability.password,
+  SpecCapability.text,
+  StatusCapability.text,
+  SpecCapability.number,
+  // TODO remove when deprecated descriptors are no longer supported
+  SpecCapability.label,
+  // END TODO
+];
+
+export const CAPABILITY_SORT_ORDER: (SpecCapability | StatusCapability)[] = [
+  // Supported in details view and has a widget
   SpecCapability.hidden,
-  SpecCapability.imagePullPolicy,
+  SpecCapability.endpointList,
+  StatusCapability.conditions,
+  SpecCapability.resourceRequirements,
+  SpecCapability.updateStrategy,
+  StatusCapability.podStatuses,
+  SpecCapability.selector,
   SpecCapability.k8sResourcePrefix,
-  SpecCapability.number,
-  SpecCapability.password,
   SpecCapability.podCount,
+  SpecCapability.password,
+  StatusCapability.k8sPhaseReason,
+  SpecCapability.booleanSwitch,
+  SpecCapability.checkbox,
+  StatusCapability.w3Link,
+
+  // Supported in details view with no widget
   SpecCapability.select,
+  SpecCapability.imagePullPolicy,
+  StatusCapability.k8sPhase,
   SpecCapability.text,
-  StatusCapability.w3Link,
   StatusCapability.text,
-  StatusCapability.k8sPhase,
-  StatusCapability.k8sPhaseReason,
-  StatusCapability.k8sResourcePrefix,
+  SpecCapability.number,
+
+  // Unsupported on details view
+  SpecCapability.podAntiAffinity,
+  SpecCapability.podAffinity,
+  SpecCapability.nodeAffinity,
+
+  // Always last
+  SpecCapability.advanced,
+  SpecCapability.fieldDependency,
+  ...DEPRECATED_CAPABILITIES,
 ];

frontend/packages/operator-lifecycle-manager/src/components/descriptors/spec/index.tsx

@@ -44,7 +44,7 @@ const PodCount: React.FC<SpecCapabilityProps> = ({
       })
     }
   >
-    {value} pods
+    {_.isNil(value) ? '-' : `${value} pods`}
   </DetailsItem>
 );
 
@@ -66,7 +66,7 @@ const Label: React.FC<SpecCapabilityProps> = ({
     {_.isObject(value) ? (
       <LabelList kind={model.kind} labels={value} />
     ) : (
-      <span>{value || '--'}</span>
+      <span>{value || '-'}</span>
     )}
   </DetailsItem>
 );
@@ -317,7 +317,7 @@ const UpdateStrategy: React.FC<SpecCapabilityProps> = ({
 
 export const SpecDescriptorDetailsItem: React.FC<SpecCapabilityProps> = (props) => {
   const [capability] =
-    getValidCapabilitiesForValue<SpecCapability>(props.descriptor, props.value, true) ?? [];
+    getValidCapabilitiesForValue<SpecCapability>(props.descriptor, props.value) ?? [];
 
   if (capability?.startsWith(SpecCapability.k8sResourcePrefix)) {
     return <K8sResourceLinkCapability capability={capability} {...props} />;

frontend/packages/operator-lifecycle-manager/src/components/descriptors/status/index.tsx

@@ -129,7 +129,7 @@ const MainStatus: React.FC<StatusCapabilityProps> = ({
 
 export const StatusDescriptorDetailsItem: React.FC<StatusCapabilityProps> = (props) => {
   const [capability] =
-    getValidCapabilitiesForValue<StatusCapability>(props.descriptor, props.value, true) ?? [];
+    getValidCapabilitiesForValue<StatusCapability>(props.descriptor, props.value) ?? [];
 
   if (capability?.startsWith(StatusCapability.k8sResourcePrefix)) {
     return <K8sResourceLinkCapability capability={capability} {...props} />;

frontend/packages/operator-lifecycle-manager/src/components/descriptors/utils.tsx

@@ -10,6 +10,8 @@ import {
   REGEXP_ARRAY_PATH,
   REGEXP_CAPTURE_GROUP_SUBGROUP,
   REGEXP_NESTED_ARRAY_PATH,
+  COMMON_COMPATIBLE_CAPABILITIES,
+  CAPABILITY_SORT_ORDER,
 } from './const';
 import { Descriptor, SpecCapability, StatusCapability, CommonCapability } from './types';
 
@@ -120,11 +122,13 @@ export const getPatchPathFromDescriptor = (descriptor: Descriptor): string =>
 const getCompatibleCapabilities = (type: string): (StatusCapability | SpecCapability)[] => {
   switch (type) {
     case 'object':
-      return OBJECT_COMPATIBLE_CAPABILITIES;
+      return [...COMMON_COMPATIBLE_CAPABILITIES, ...OBJECT_COMPATIBLE_CAPABILITIES];
     case 'array':
-      return ARRAY_COMPATIBLE_CAPABILITIES;
+      return [...COMMON_COMPATIBLE_CAPABILITIES, ...ARRAY_COMPATIBLE_CAPABILITIES];
+    case 'primitive':
+      return [...COMMON_COMPATIBLE_CAPABILITIES, ...PRIMITIVE_COMPATIBLE_CAPABILITIES];
     default:
-      return PRIMITIVE_COMPATIBLE_CAPABILITIES;
+      return [];
   }
 };
 
@@ -134,54 +138,47 @@ const getCompatibleCapabilities = (type: string): (StatusCapability | SpecCapabi
 export function getValidCapabilitiesForDataType<CapabilityType extends string = SpecCapability>(
   descriptor: Descriptor<CapabilityType>,
   type: string,
-  allowDeprecated?: boolean,
 ): CapabilityType[] {
   const compatibleCapabilities = getCompatibleCapabilities(type);
-  const [valid, invalid, deprecated] = _.reduce(
-    descriptor?.['x-descriptors'] ?? [],
-    ([validAccumulator, invalidAccumulator, deprecatedAccumulator], capability) => {
-      const isDeprecated = DEPRECATED_CAPABILITIES.some((deprecatedCapability) =>
-        capability.startsWith(deprecatedCapability),
-      );
-
+  return (descriptor?.['x-descriptors'] ?? [])
+    .filter((capability) => {
       const isCompatible =
         type === 'any' ||
-        compatibleCapabilities.some((compatibleCapability) =>
+        compatibleCapabilities.findIndex((compatibleCapability) =>
           capability.startsWith(compatibleCapability),
         );
-
-      const isValid = (!isDeprecated || allowDeprecated) && isCompatible;
-      return [
-        [...(validAccumulator ?? []), ...(isValid ? [capability] : [])],
-        [...(invalidAccumulator ?? []), ...(!isValid ? [capability] : [])],
-        [...(deprecatedAccumulator ?? []), ...(isDeprecated ? [capability] : [])],
-      ];
-    },
-    [[], [], []],
-  );
-
-  if (invalid?.length) {
-    invalid.forEach((invalidCapability) => {
-      // eslint-disable-next-line no-console
-      console.warn(
-        `[Invalid x-descriptor] "${invalidCapability}" is incompatible with ${type} values and will have no effect`,
-        descriptor,
+      const isDeprecated = DEPRECATED_CAPABILITIES.some((deprecatedCapability) =>
+        capability.startsWith(deprecatedCapability),
       );
-    });
-  }
 
-  if (deprecated?.length) {
-    deprecated.forEach((deprecatedCapability) => {
-      // eslint-disable-next-line no-console
-      console.warn(
-        `[Deprecated x-descriptor] "${deprecatedCapability}" is no longer supported ${!allowDeprecated &&
-          'and will have no effect'}`,
-        descriptor,
-      );
-    });
-  }
+      if (isDeprecated) {
+        // eslint-disable-next-line no-console
+        console.warn(
+          `[Deprecated x-descriptor] "${capability}" is deprecated and support will be removed in a future release.`,
+          descriptor,
+        );
+      }
 
-  return valid ?? [];
+      if (!isCompatible) {
+        // eslint-disable-next-line no-console
+        console.warn(
+          `[Invalid x-descriptor] "${capability}" is incompatible with ${type} values and will have no effect`,
+          descriptor,
+        );
+        return false;
+      }
+
+      return true;
+    })
+    .sort((a, b) => {
+      const aIndex = CAPABILITY_SORT_ORDER.findIndex((capability) => a.startsWith(capability));
+      const bIndex = CAPABILITY_SORT_ORDER.findIndex((capability) => b.startsWith(capability));
+      // If either a or b don't exist in the sorting array, sort the missing item last
+      if (aIndex < 0 || bIndex < 0) {
+        return bIndex - aIndex;
+      }
+      return aIndex - bIndex;
+    });
 }
 
 const getValueType = (value: any): string => {
@@ -197,19 +194,17 @@ const getValueType = (value: any): string => {
 export function getValidCapabilitiesForValue<CapabilityType extends string = SpecCapability>(
   descriptor: Descriptor<CapabilityType>,
   value: any,
-  allowDeprecated?: boolean,
 ): CapabilityType[] {
   const type = getValueType(value);
-  return getValidCapabilitiesForDataType<CapabilityType>(descriptor, type, allowDeprecated);
+  return getValidCapabilitiesForDataType<CapabilityType>(descriptor, type);
 }
 
 export function getValidCapabilitiesForSchema<CapabilityType extends string = SpecCapability>(
   descriptor: Descriptor<CapabilityType>,
   schema: JSONSchema6,
-  allowDeprecated?: boolean,
 ): CapabilityType[] {
   const type = getSchemaType(schema);
-  return getValidCapabilitiesForDataType<CapabilityType>(descriptor, type, allowDeprecated);
+  return getValidCapabilitiesForDataType<CapabilityType>(descriptor, type);
 }
 
 export const isMainStatusDescriptor = (descriptor: Descriptor): boolean =>