Add NamespaceDashboard option to ConsoleLink #2698

rhamilto · 2019-09-12T14:02:06Z

Replaces #2579 as this PR does not include Cluster dashboard per @beanh66.

Note: the Launcher sections only appear if ConsoleLinks with location NamespaceDashboard exist (and optionally match the current namespace).

Updated ConsoleLink CRD:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: consolelinks.console.openshift.io
  annotations:
    displayName: ConsoleLinks
    description: Extension for customizing OpenShift web console links
spec:
  scope: Cluster
  group: console.openshift.io
  versions:
  - name: v1
    served: true
    storage: true
  names:
    plural: consolelinks
    singular: consolelink
    kind: ConsoleLink
    listKind: ConsoleLinkList
  additionalPrinterColumns:
  - name: Text
    type: string
    JSONPath: .spec.text
  - name: URL
    type: string
    JSONPath: .spec.href
  - name: Menu
    type: string
    JSONPath: .spec.menu
  - name: Age
    type: date
    JSONPath: .metadata.creationTimestamp
  subresources:
    status: {}
  validation:
    openAPIV3Schema:
      properties:
        spec:
          type: object
          description: Represents console link customizations spec
          required:
          - text
          - href
          - location
          properties:
            text:
              type: string
              description: Text of the link
            href:
              type: string
              description: Absolute secure URL for the link (must use https)
              pattern: '^https://([\w-]+.)+[\w-]+(/[\w- ./?%&=])?$'
            location:
              type: string
              description: Determines where the link is added (ApplicationMenu, HelpMenu, UserMenu, NamespaceDashboard)
              pattern: ^(ApplicationMenu|HelpMenu|UserMenu|NamespaceDashboard)$
            applicationMenu:
              type: object
              description: Object that holds application menu properties if the location is set to ApplicationMenu
              required:
                - section
              properties:
                section:
                  type: string
                  description: The section of the application menu in which the link should appear
                imageURL:
                  type: string
                  description: The URL for the icon used in front of the link in the application menu.  The URL must be an HTTPS URL or a Data URI. The image should be square and will be shown at 24x24 pixels.
            namespaceDashboard:
              type: object
              description: Object that holds namespace dashboard link properties if the location is set to NamespaceDashboard
              properties:
                namespaces:
                  type: array
                  description: The namespaces in which the dashboard link should appear.  If not specified, the link will appear in all namespaces.

rhamilto · 2019-09-12T14:40:21Z

cc: @jhadvig

serenamarie125 · 2019-09-13T03:47:28Z

FYI @sspeiche

jhadvig · 2019-09-13T15:15:43Z

frontend/public/components/overview/namespace-overview.tsx

+    if (link.spec.location !== 'NamespaceDashboard') {
+      return false;
+    }
+    const namespaces = _.get(link, ['spec', 'namespaceDashboard', 'namespaces']);


Do we also wanna cover case where the array is empty ?

spadgett · 2019-09-17T14:29:01Z

/kind feature

spadgett

Thanks, @rhamilto. A few comments

spadgett · 2019-09-24T13:30:22Z

frontend/public/components/overview/namespace-overview.tsx

+      return false;
+    }
+    const namespaces = _.get(link, ['spec', 'namespaceDashboard', 'namespaces']);
+    return !namespaces || _.includes(namespaces, ns.metadata.name);


You want to handle both empty arrays and null/undefined. _.isEmpty works for both.

Suggested change

return !namespaces || _.includes(namespaces, ns.metadata.name);

return _.isEmpty(namsepaces) || _.includes(namespaces, ns.metadata.name);

spadgett · 2019-09-24T13:31:04Z

frontend/public/components/overview/namespace-overview.tsx

+    if (link.spec.location !== 'NamespaceDashboard') {
+      return false;
+    }
+    const namespaces = _.get(link, ['spec', 'namespaceDashboard', 'namespaces']);


Suggested change

const namespaces = _.get(link, ['spec', 'namespaceDashboard', 'namespaces']);

const namespaces: string[] = _.get(link, ['spec', 'namespaceDashboard', 'namespaces']);

spadgett · 2019-09-24T13:33:48Z

frontend/public/components/overview/namespace-overview.tsx

+
+export const ConsoleLinks: React.FC<ConsoleLinksProps> = ({consoleLinks}) => {
+  return <ul className="list-unstyled">
+    {_.map(_.sortBy(consoleLinks, 'spec.text'), link => {


Suggested change

{_.map(_.sortBy(consoleLinks, 'spec.text'), link => {

{_.map(_.sortBy(consoleLinks, 'spec.text'), (link: K8sResourceKind) => {

spadgett · 2019-09-24T13:34:53Z

frontend/public/components/overview/namespace-overview.tsx

@@ -44,6 +45,38 @@ const OverviewNamespaceSummary = ({ns}) => <div className="group">
  </div>
 </div>;

+export const getNamespaceDashboardConsoleLinks = (ns, consoleLinks) => {


Suggested change

export const getNamespaceDashboardConsoleLinks = (ns, consoleLinks) => {

export const getNamespaceDashboardConsoleLinks = (ns: K8sResourceKind, consoleLinks: K8sResourceKind[]): K8sResourceKind[] => {

spadgett · 2019-09-24T13:36:23Z

frontend/public/components/overview/namespace-overview.tsx

  <OverviewHealth ns={ns} />
  <OverviewResourceQuotas ns={ns} />
  <OverviewResourceUsage ns={ns} />
+  <OverviewLinks ns={ns} consoleLinks={consoleLinks} />


It would be better to connect OverviewLinks instead of the entire OverviewNamespaceDashboard. You generally want to connect only the child component that needs the data when possible.

rhamilto · 2019-09-24T14:20:59Z

Thanks, @spadgett. Updated.

spadgett

/lgtm
/hold

openshift-ci-robot · 2019-09-24T16:06:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [rhamilto,spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

spadgett · 2019-09-25T18:40:08Z

/hold cancel

openshift-bot · 2019-09-25T22:38:30Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-26T01:14:30Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-26T03:37:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

abkieling · 2019-11-25T17:52:59Z

@rhamilto Is this feature included in OpenShfit 4.2?

rhamilto · 2019-11-25T19:12:00Z

@rhamilto Is this feature included in OpenShfit 4.2?

No. This is a 4.3 feature.

rhamilto mentioned this pull request Sep 12, 2019

[WIP] Add new ConsoleLink locations (namespace dashboard, cluster dashboard) #2579

Closed

openshift-ci-robot requested review from benjaminapetersen and jhadvig September 12, 2019 14:02

rhamilto force-pushed the namespace-links branch from 8266620 to f2b7fb6 Compare September 12, 2019 14:07

spadgett added this to the v4.3 milestone Sep 12, 2019

jhadvig reviewed Sep 13, 2019

View reviewed changes

openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 17, 2019

rhamilto force-pushed the namespace-links branch from f2b7fb6 to 798e4f3 Compare September 23, 2019 13:09

openshift-ci-robot added component/ceph Related to ceph-storage-plugin component/dashboard Related to dashboard labels Sep 23, 2019

rhamilto changed the base branch from master to master-4.3 September 23, 2019 13:09

rhamilto changed the title ~~[WIP] Add NamespaceLink option to ConsoleLink~~ [WIP] Add NamespaceDashboard option to ConsoleLink Sep 23, 2019

rhamilto changed the title ~~[WIP] Add NamespaceDashboard option to ConsoleLink~~ Add NamespaceDashboard option to ConsoleLink Sep 23, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2019

rhamilto force-pushed the namespace-links branch from 798e4f3 to 60dbd44 Compare September 24, 2019 13:18

spadgett requested changes Sep 24, 2019

View reviewed changes

Add NamespaceDashboard option to ConsoleLink

21c5707

rhamilto force-pushed the namespace-links branch from 60dbd44 to 21c5707 Compare September 24, 2019 14:19

spadgett approved these changes Sep 24, 2019

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 24, 2019

openshift-ci-robot assigned spadgett Sep 24, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 24, 2019

spadgett changed the base branch from master-4.3 to master September 25, 2019 18:40

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2019

openshift-merge-robot merged commit 5a79bc5 into openshift:master Sep 26, 2019

rhamilto deleted the namespace-links branch September 26, 2019 12:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NamespaceDashboard option to ConsoleLink #2698

Add NamespaceDashboard option to ConsoleLink #2698

rhamilto commented Sep 12, 2019 •

edited

Loading

rhamilto commented Sep 12, 2019

serenamarie125 commented Sep 13, 2019

jhadvig Sep 13, 2019

spadgett commented Sep 17, 2019

spadgett left a comment

spadgett Sep 24, 2019

spadgett Sep 24, 2019

spadgett Sep 24, 2019

spadgett Sep 24, 2019

spadgett Sep 24, 2019

rhamilto commented Sep 24, 2019

spadgett left a comment

openshift-ci-robot commented Sep 24, 2019

spadgett commented Sep 25, 2019

openshift-bot commented Sep 25, 2019

openshift-bot commented Sep 26, 2019

openshift-bot commented Sep 26, 2019

abkieling commented Nov 25, 2019

rhamilto commented Nov 25, 2019

	return !namespaces \|\| _.includes(namespaces, ns.metadata.name);
	return _.isEmpty(namsepaces) \|\| _.includes(namespaces, ns.metadata.name);

	const namespaces = _.get(link, ['spec', 'namespaceDashboard', 'namespaces']);
	const namespaces: string[] = _.get(link, ['spec', 'namespaceDashboard', 'namespaces']);

	{_.map(_.sortBy(consoleLinks, 'spec.text'), link => {
	{_.map(_.sortBy(consoleLinks, 'spec.text'), (link: K8sResourceKind) => {

	export const getNamespaceDashboardConsoleLinks = (ns, consoleLinks) => {
	export const getNamespaceDashboardConsoleLinks = (ns: K8sResourceKind, consoleLinks: K8sResourceKind[]): K8sResourceKind[] => {

Add NamespaceDashboard option to ConsoleLink #2698

Add NamespaceDashboard option to ConsoleLink #2698

Conversation

rhamilto commented Sep 12, 2019 • edited Loading

rhamilto commented Sep 12, 2019

serenamarie125 commented Sep 13, 2019

jhadvig Sep 13, 2019

Choose a reason for hiding this comment

spadgett commented Sep 17, 2019

spadgett left a comment

Choose a reason for hiding this comment

spadgett Sep 24, 2019

Choose a reason for hiding this comment

spadgett Sep 24, 2019

Choose a reason for hiding this comment

spadgett Sep 24, 2019

Choose a reason for hiding this comment

spadgett Sep 24, 2019

Choose a reason for hiding this comment

spadgett Sep 24, 2019

Choose a reason for hiding this comment

rhamilto commented Sep 24, 2019

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 24, 2019

spadgett commented Sep 25, 2019

openshift-bot commented Sep 25, 2019

openshift-bot commented Sep 26, 2019

openshift-bot commented Sep 26, 2019

abkieling commented Nov 25, 2019

rhamilto commented Nov 25, 2019

rhamilto commented Sep 12, 2019 •

edited

Loading