Fix 1970011: Fix edge case for "managed by" links

In the 'managed by' link components and helpers, account for the edge case where a resource has a CSV as an ownerReference.
openshift · Jun 23, 2021 · 07f7e4a · 07f7e4a
1 parent e8fb60e
commit 07f7e4a
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 40 deletions.
diff --git a/frontend/public/components/utils/managed-by.tsx b/frontend/public/components/utils/managed-by.tsx
@@ -5,9 +5,7 @@ import { useTranslation } from 'react-i18next';
 
 import { ResourceIcon } from './resource-icon';
 import {
-  groupVersionFor,
   K8sResourceCommon,
-  referenceForGroupVersionKind,
   referenceForModel,
   referenceForOwnerRef,
   OwnerReference,
@@ -19,38 +17,39 @@ import { ClusterServiceVersionModel } from '@console/operator-lifecycle-manager/
 import { ClusterServiceVersionKind } from '@console/operator-lifecycle-manager';
 import { ResourceLink } from '.';
 
-export const ManagedByOperatorResourceLink: React.SFC<ManagerLinkProps> = (props) => {
-  const { csvName, namespace, owner, className } = props;
-  const name = owner.name;
-  const { group, version } = groupVersionFor(owner.apiVersion);
-  const kind = owner.kind;
-
-  const reference = referenceForGroupVersionKind(group)(version)(kind);
-  const { namespaced } = modelFor(reference);
-
-  const link = `/k8s/ns/${namespace}/${referenceForModel(
-    ClusterServiceVersionModel,
-  )}/${csvName}/${reference}/${name}`;
-
+export const ManagedByOperatorResourceLink: React.SFC<ManagerLinkProps> = ({
+  csvName,
+  namespace,
+  owner,
+  className,
+}) => {
+  const csvGroupVersionKind = referenceForModel(ClusterServiceVersionModel);
+  const ownerGroupVersionKind = referenceForOwnerRef(owner);
+  const ownerIsCSV = ownerGroupVersionKind === csvGroupVersionKind;
+  const { namespaced } = modelFor(ownerGroupVersionKind);
+  const link = `/k8s/ns/${namespace}/${csvGroupVersionKind}/${csvName}`;
   return (
     <span className={className}>
       {namespaced ? (
         <>
-          <ResourceIcon kind={referenceForOwnerRef(owner)} />
-          <Link to={link} className="co-resource-item__resource-name" data-test-operand-link={name}>
-            {name}
+          <ResourceIcon kind={ownerGroupVersionKind} />
+          <Link
+            to={ownerIsCSV ? link : `${link}/${ownerGroupVersionKind}/${owner.name}`}
+            className="co-resource-item__resource-name"
+            data-test-operand-link={owner.name}
+          >
+            {owner.name}
           </Link>
         </>
       ) : (
-        <ResourceLink kind={reference} name={name} />
+        <ResourceLink kind={ownerGroupVersionKind} name={owner.name} />
       )}
     </span>
   );
 };
 
-export const ManagedByOperatorLink: React.SFC<ManagedByLinkProps> = (props) => {
+export const ManagedByOperatorLink: React.SFC<ManagedByLinkProps> = ({ obj, className }) => {
   const { t } = useTranslation();
-  const { obj, className } = props;
   const [data, setData] = React.useState<ClusterServiceVersionKind[] | undefined>();
   const namespace = obj.metadata.namespace;
   React.useEffect(() => {
@@ -67,20 +66,17 @@ export const ManagedByOperatorLink: React.SFC<ManagedByLinkProps> = (props) => {
   const owner = findOwner(obj, data);
   const csv = data && owner ? matchOwnerAndCSV(owner, data) : undefined;
 
-  if (owner && csv) {
-    return (
-      <div className={classNames('co-m-pane__heading-owner', className)}>
-        {t('public~Managed by')}{' '}
-        <ManagedByOperatorResourceLink
-          className="co-resource-item"
-          namespace={namespace}
-          csvName={csv.metadata.name}
-          owner={owner}
-        />
-      </div>
-    );
-  }
-  return null;
+  return owner && csv ? (
+    <div className={classNames('co-m-pane__heading-owner', className)}>
+      {t('public~Managed by')}{' '}
+      <ManagedByOperatorResourceLink
+        className="co-resource-item"
+        namespace={namespace}
+        csvName={csv.metadata.name}
+        owner={owner}
+      />
+    </div>
+  ) : null;
 };
 
 type ManagedByLinkProps = {

diff --git a/frontend/public/module/k8s/managed-by.ts b/frontend/public/module/k8s/managed-by.ts
@@ -1,6 +1,13 @@
-import { groupVersionFor, K8sResourceCommon, OwnerReference } from './';
+import {
+  referenceForOwnerRef,
+  K8sResourceCommon,
+  OwnerReference,
+  referenceFor,
+  groupVersionFor,
+} from './';
 import { ClusterServiceVersionKind } from '@console/operator-lifecycle-manager';
 
+// Check if owner is an Operand under csv
 const isOwnedByOperator = (csv: ClusterServiceVersionKind, owner: OwnerReference) => {
   const { group } = groupVersionFor(owner.apiVersion);
   return csv.spec?.customresourcedefinitions?.owned?.some((owned) => {
@@ -9,15 +16,22 @@ const isOwnedByOperator = (csv: ClusterServiceVersionKind, owner: OwnerReference
   });
 };
 
-const isOwnedByCSV = (csv: ClusterServiceVersionKind, owner: OwnerReference) =>
-  csv.kind === owner.kind && csv.apiVersion === owner.apiVersion;
+// Check if csv === owner (there is no need to check namespace because ownerReferences must
+// be in the same namespace as the owned resource, or be cluster-scoped).
+const isOwnedByCSV = (csv: ClusterServiceVersionKind, owner: OwnerReference) => {
+  const csvGroupVersionKind = referenceFor(csv);
+  const ownerGroupVersionKind = referenceForOwnerRef(owner);
+  return csvGroupVersionKind === ownerGroupVersionKind && csv.metadata.name === owner.name;
+};
 
+// Find an Operator CSV that either is the owner or owns the owner
 export const matchOwnerAndCSV = (owner: OwnerReference, csvs: ClusterServiceVersionKind[] = []) => {
   return csvs.find((csv) => isOwnedByOperator(csv, owner) || isOwnedByCSV(csv, owner));
 };
 
-export const findOwner = (obj: K8sResourceCommon, data: ClusterServiceVersionKind[]) => {
+// Find onwerReference that is either a CSV or is an Operand managed by a CSV.
+export const findOwner = (obj: K8sResourceCommon, csvs: ClusterServiceVersionKind[]) => {
   return obj?.metadata?.ownerReferences?.find((o) =>
-    data?.some((csv) => isOwnedByOperator(csv, o) || isOwnedByCSV(csv, o)),
+    csvs?.some((csv) => isOwnedByOperator(csv, o) || isOwnedByCSV(csv, o)),
   );
 };