Fix wrong link generation when the ingress URL contains a regex (#1120)…

… (#2131)

* Fix wrong link generation when the ingress URL contains a regex  (#1120)

* Consistent semver usage of @clr/ui dependency

Dockerfile and package.json now use the same major Clarity version.

* Revert "Consistent semver usage of @clr/ui dependency"

This reverts commit e7b2214.
(commit to wrong branch, not corresponding to this PR)

* Improve e2e test case

Removed not useful snapshot and improve the logic of the test case for a better detection of the span content and the absence of anchors.

* Improve test case with PR comments

Now each div (getUnknown and getSpan) has a unique 'key' property

dashboard/src/components/AppView/AccessURLTable/AccessURLItem/AccessURLIngressHelper.test.ts

@@ -1,5 +1,5 @@
 import { IHTTPIngressPath, IIngressRule, IResource } from "shared/types";
-import { GetURLItemFromIngress } from "./AccessURLIngressHelper";
+import { GetURLItemFromIngress, IsURL } from "./AccessURLIngressHelper";
 
 describe("GetURLItemFromIngress", () => {
   interface ITest {
@@ -111,3 +111,89 @@ describe("GetURLItemFromIngress", () => {
     });
   });
 });
+
+describe("IsURL", () => {
+  interface ITest {
+    description: string;
+    fullURL: string;
+    expected: boolean;
+  }
+  const tests: ITest[] = [
+    {
+      description: "it should return true to a simple url",
+      fullURL: "http://wordpress.local/example-test",
+      expected: true,
+    },
+    {
+      description: "it should return false to a 'rfc-ilegal' url with a regex",
+      fullURL: "http://wordpress.local/example-bad(/|$)(.*)",
+      expected: false,
+    },
+    {
+      description: "it should return false to a 'rfc-legal' url with a regex",
+      fullURL: "http://wordpress.local/example-bad/[a-z]+",
+      expected: false,
+    },
+    {
+      description: "it should return true to a simple url (https)",
+      fullURL: "https://wordpress.local/example-test",
+      expected: true,
+    },
+    {
+      description: "it should return false to a 'rfc-ilegal' url with a regex (https)",
+      fullURL: "https://wordpress.local/example-bad(/|$)(.*)",
+      expected: false,
+    },
+    {
+      description: "it should return false to a 'rfc-legal' url with a regex (https)",
+      fullURL: "https://wordpress.local/example-bad/[a-z]+",
+      expected: false,
+    },
+    {
+      description: "it should return true to a simple url (host is ip)",
+      fullURL: "http://1.1.1.1/example-test",
+      expected: true,
+    },
+    {
+      description: "it should return false to a 'rfc-ilegal' url with a regex (host is ip)",
+      fullURL: "http://1.1.1.1/example-bad(/|$)(.*)",
+      expected: false,
+    },
+    {
+      description: "it should return false to a 'rfc-legal' url with a regex (host is ip)",
+      fullURL: "http://1.1.1.1/example-bad/[a-z]+",
+      expected: false,
+    },
+    {
+      description: "it should return false to a 'rfc-legal' url with a regex",
+      fullURL: "http://wordpress.local/example-bad/*",
+      expected: false,
+    },
+    {
+      description: "it should return true to a translated idn url",
+      fullURL: "http://wordpress.local/example-good/xn--kgbechtv",
+      expected: true,
+    },
+    {
+      description: "it should return true to a translated idn url (path)",
+      fullURL: "http://wordpress.local/example-good/xn--kgbechtv",
+      expected: true,
+    },
+    {
+      description: "it should return true to a translated idn url (host)",
+      fullURL: "http://wordpress.xn--kgbechtv/example-good",
+      expected: true,
+    },
+    {
+      description: "it should return false to a non-translated idn url",
+      fullURL: "http://wordpress.local/♨️/shouldBeIDNTranslated",
+      expected: false,
+    },
+  ];
+  tests.forEach(test => {
+    it(test.fullURL, () => {
+      const isURL = IsURL(test.fullURL);
+      expect(isURL).toBe(test.expected);
+    });
+  });
+});

dashboard/src/components/AppView/AccessURLTable/AccessURLItem/AccessURLIngressHelper.tsx

@@ -1,6 +1,8 @@
 import { IIngressSpec, IIngressTLS, IResource } from "shared/types";
 import { IURLItem } from "./IURLItem";
 
+const isURLRegex = /^(https?:\/\/)?((([a-z\d]([a-z\d-]*[a-z\d])*)\.)+[a-z-]{2,}|((\d{1,3}\.){3}\d{1,3}))(:\d+)?(\/[-a-z\d%_.~+]*)*(\?[;&a-z\d%_.~+=-]*)?(#[-a-z\d_]*)?$/i;
+
 // URLs returns the list of URLs obtained from the service status
 function URLs(ingress: IResource): string[] {
   const spec = ingress.spec as IIngressSpec;
@@ -44,3 +46,7 @@ export function GetURLItemFromIngress(ingress: IResource) {
     URLs: URLs(ingress),
   } as IURLItem;
 }
+
+export function IsURL(url: string): boolean {
+  return isURLRegex.test(url);
+}

dashboard/src/components/AppView/AccessURLTable/AccessURLTable.test.tsx

@@ -178,6 +178,42 @@ context("when the app contains ingresses", () => {
     expect(wrapper.find("a").findWhere(a => a.prop("href") === "http://foo.bar/ready")).toExist();
     expect(wrapper).toMatchSnapshot();
   });
+
+  it("should show the table with available ingresses without anchors if a regex is present in the path", () => {
+    const ingress = {
+      kind: "Ingress",
+      metadata: {
+        name: "foo",
+        selfLink: "/ingresses/foo",
+      },
+      spec: {
+        rules: [
+          {
+            host: "foo.bar",
+            http: {
+              paths: [{ path: "/ready(/|$)(.*)" }],
+            },
+          },
+        ],
+      } as IIngressSpec,
+    } as IResource;
+    const ingressItem = { isFetching: false, item: ingress };
+    const url = ingress.metadata.selfLink;
+    const ingressRefs = [{ name: "svc", getResourceURL: jest.fn(() => url) } as any];
+    const state = { kube: { items: { [url]: ingressItem } } };
+    const store = getStore(state);
+    const wrapper = mountWrapper(
+      store,
+      <AccessURLTable {...defaultProps} ingressRefs={ingressRefs} />,
+    );
+    expect(wrapper.find("Table")).toExist();
+    expect(wrapper.find("a")).not.toExist();
+    const matchingSpans = wrapper.find("span").findWhere(s => s.text().includes("foo.bar/ready"));
+    expect(matchingSpans).not.toHaveLength(0);
+    matchingSpans.forEach(element => {
+      expect(element.text()).toEqual("http://foo.bar/ready(/|$)(.*)");
+    });
+  });
 });
 
 context("when the app contains services and ingresses", () => {

dashboard/src/components/AppView/AccessURLTable/AccessURLTable.tsx

@@ -10,7 +10,7 @@ import { flattenResources } from "shared/utils";
 import LoadingWrapper from "../../../components/LoadingWrapper/LoadingWrapper";
 import { IK8sList, IKubeItem, IResource, IServiceSpec, IStoreState } from "../../../shared/types";
 import isSomeResourceLoading from "../helpers";
-import { GetURLItemFromIngress } from "./AccessURLItem/AccessURLIngressHelper";
+import { GetURLItemFromIngress, IsURL } from "./AccessURLItem/AccessURLIngressHelper";
 import { GetURLItemFromService } from "./AccessURLItem/AccessURLServiceHelper";
 
 interface IAccessURLTableProps {
@@ -77,18 +77,38 @@ function flattenIngresses(ingresses: Array<IKubeItem<IResource | IK8sList<IResou
 }
 
 function getAnchors(URLs: string[]) {
-  return URLs.map(URL => (
-    <div className="margin-b-sm">
-      <a href={URL} target="_blank" rel="noopener noreferrer" key={URL}>
+  return URLs.map(URL => getAnchor(URL));
+}
+
+function getAnchor(URL: string) {
+  return (
+    <div className="margin-b-sm" key={URL}>
+      <a href={URL} target="_blank" rel="noopener noreferrer">
         {URL}
       </a>
     </div>
-  ));
+  );
+}
+
+function getSpan(URL: string) {
+  return (
+    <div className="margin-b-sm" key={URL}>
+      <span>{URL}</span>
+    </div>
+  );
+}
+
+function getUnknown(key: string) {
+  return (
+    <div className="margin-b-sm" key={key}>
+      <span>Unknown</span>
+    </div>
+  );
 }
 
 function getNotes(resource?: IResource) {
   if (!resource) {
-    return <span>Unknown</span>;
+    return getUnknown("unknown-notes");
   }
   const ips: Array<{ ip: string }> = get(resource, "status.loadBalancer.ingress", []);
   if (ips.length) {
@@ -164,9 +184,15 @@ export default function AccessURLTable({ ingressRefs, serviceRefs }: IAccessURLT
         };
       })
       .concat(
-        allIngresses.map(ingress => {
+        allIngresses.map((ingress, index) => {
           return {
-            url: ingress.item ? getAnchors(GetURLItemFromIngress(ingress.item).URLs) : "Unknown",
+            url: ingress.item
+              ? GetURLItemFromIngress(ingress.item).URLs.map(
+                  // check whether each URL is, indeed, a valid URL.
+                  // If so, render the <a>, othersiwe, render a simple <span>
+                  url => (IsURL(url) ? getAnchor(url) : getSpan(url)),
+                )
+              : [getUnknown(index.toString())], // render a simple span with "unknown"
             type: "Ingress",
             notes: ingress.error ? (
               <span>Error: {ingress.error.message}</span>

dashboard/src/components/AppView/AccessURLTable/__snapshots__/AccessURLTable.test.tsx.snap

@@ -240,10 +240,10 @@ exports[`when the app contains ingresses should show the table with available in
                     >
                       <div
                         className="margin-b-sm"
+                        key="http://foo.bar/ready"
                       >
                         <a
                           href="http://foo.bar/ready"
-                          key="http://foo.bar/ready"
                           rel="noopener noreferrer"
                           target="_blank"
                         >
@@ -457,7 +457,15 @@ exports[`when the app contains resources with errors displays the error 1`] = `
                     could not find Ingress
                   </span>,
                   "type": "Ingress",
-                  "url": "Unknown",
+                  "url": Array [
+                    <div
+                      className="margin-b-sm"
+                    >
+                      <span>
+                        Unknown
+                      </span>
+                    </div>,
+                  ],
                 },
               ]
             }
@@ -517,7 +525,15 @@ exports[`when the app contains resources with errors displays the error 1`] = `
                         could not find Ingress
                       </span>,
                       "type": "Ingress",
-                      "url": "Unknown",
+                      "url": Array [
+                        <div
+                          className="margin-b-sm"
+                        >
+                          <span>
+                            Unknown
+                          </span>
+                        </div>,
+                      ],
                     }
                   }
                 >
@@ -526,7 +542,14 @@ exports[`when the app contains resources with errors displays the error 1`] = `
                       className=""
                       key="0-url"
                     >
-                      Unknown
+                      <div
+                        className="margin-b-sm"
+                        key="0"
+                      >
+                        <span>
+                          Unknown
+                        </span>
+                      </div>
                     </td>
                     <td
                       className=""
@@ -821,10 +844,10 @@ exports[`when the app contains services and ingresses should show the table with
                     >
                       <div
                         className="margin-b-sm"
+                        key="http://1.2.3.4:8080"
                       >
                         <a
                           href="http://1.2.3.4:8080"
-                          key="http://1.2.3.4:8080"
                           rel="noopener noreferrer"
                           target="_blank"
                         >
@@ -919,10 +942,10 @@ exports[`when the app contains services and ingresses should show the table with
                     >
                       <div
                         className="margin-b-sm"
+                        key="http://foo.bar/ready"
                       >
                         <a
                           href="http://foo.bar/ready"
-                          key="http://foo.bar/ready"
                           rel="noopener noreferrer"
                           target="_blank"
                         >