CONSOLE-2892: Allow dynamic plugins to proxy to services on the cluster by jhadvig · Pull Request #603 · openshift/console-operator

jhadvig · 2021-10-12T07:18:49Z

Console changes for story https://issues.redhat.com/browse/CONSOLE-2892

Console-operator will now check for spec.proxy.services of the enabled ConsolePlugins and manifest those services to the console-config.yaml ConfigMap, which will trigger redeploy the console pod to pick up the updates

eg:

apiVersion: console.openshift.io/v1alpha1
kind: ConsolePlugin
metadata:
  name: console-demo-plugin
spec:
  displayName: 'OpenShift Console Demo Plugin'
  service:
    name: console-demo-plugin
    namespace: console-demo-plugin
    port: 9001
    basePath: '/'
  proxy:
    services:
    - name: thanos-querier
      namespace: openshift-monitoring
      port: 9091
      authorize: true

in the console browser window following query should be able to proxy the request to thanos and return 200:

await fetch('/api/proxy/namespace/openshift-monitoring/service/thanos-querier:9091/api/v1/rules')

/assign @spadgett @TheRealJon @florkbr

jhadvig · 2021-10-12T16:40:26Z

+				Namespace:     service.Namespace,
+				Endpoint:      getProxyServiceHostname(&service),
+				CACertificate: service.CACertificate,
+				Authorize:     service.Authorize,


Wondering if this is necessary and we shouldn't always proxy the Authorization header

We don't want to pass the user's token around unless the service really needs it. Even if we trust the service, it's an exposure. The token could get accidentally logged, etc. It's much better to only add the Authorization header when we know we have to.

jhadvig · 2021-10-26T14:13:27Z

/retest

jhadvig · 2021-11-01T19:25:16Z

PR is ready for final review.

QE Approver:
/assign @yapei

Docs Approver:
/assign @opayne1

PX Approver:
/assign @RickJWagner

Console Approver:
/assign @spadgett

jhadvig · 2021-11-02T10:00:11Z

/retest

RickJWagner · 2021-11-02T13:53:31Z

/label px-approved

opayne1 · 2021-11-02T16:15:46Z

/label docs-approved

spadgett

Looks good to me, just one nit on a function name

spadgett · 2021-11-03T19:30:39Z

+	return fmt.Sprintf("%snamespace/%s/service/%s/", pluginProxyEndpoint, service.Namespace, fmt.Sprintf("%s:%d", service.Name, service.Port))
+}
+
+func getProxyServiceHostname(service *v1alpha1.ConsolePluginProxyService) string {


This is returning a URL, not a hostname.

yapei · 2021-11-04T10:55:14Z

verified the changes locally and works
/label qe-approved

jhadvig · 2021-11-04T12:33:49Z

@spadgett comments addressed in a new commit. PTAL

jhadvig · 2021-11-04T14:07:57Z

/retest

spadgett

/lgtm

openshift-bot · 2021-11-04T17:27:22Z

/retest-required

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

openshift-bot · 2021-11-04T17:40:23Z

/retest-required

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

jhadvig · 2021-11-04T17:50:30Z

We need to get openshift/client-go#199 in and update the go deps. accordingly.
/hold

jhadvig · 2021-11-04T18:23:22Z

GO deps have been updated. @spadgett please could you retag the PR. Thank you

/hold cancel

spadgett

/lgtm

openshift-ci · 2021-11-04T18:31:49Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [jhadvig,spadgett]

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

spadgett · 2021-11-04T19:47:19Z

/retest

openshift-bot · 2021-11-04T20:03:24Z

/retest-required

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

openshift-bot · 2021-11-04T20:29:21Z

/retest-required

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

openshift-bot · 2021-11-04T20:42:22Z

/retest-required

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

openshift-bot · 2021-11-04T20:55:23Z

/retest-required

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

openshift-bot · 2021-11-04T21:08:23Z

/retest-required

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

openshift-bot · 2021-11-04T21:21:20Z

/retest-required

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

openshift-bot · 2021-11-04T21:34:21Z

/retest-required

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

openshift-bot · 2021-11-04T22:00:30Z

/retest-required

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

openshift-bot · 2021-11-04T22:13:21Z

/retest-required

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

openshift-bot · 2021-11-04T22:26:22Z

/retest-required

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

openshift-bot · 2021-11-04T22:39:22Z

/retest-required

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

openshift-bot · 2021-11-04T22:52:22Z

/retest-required

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

openshift-bot · 2021-11-04T23:05:23Z

/retest-required

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

openshift-bot · 2021-11-04T23:18:22Z

/retest-required

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

openshift-bot · 2021-11-04T23:31:22Z

/retest-required

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

openshift-ci · 2021-11-04T23:40:26Z

@jhadvig: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-single-node	`fc1df9b`	link	false	`/test e2e-aws-single-node`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2021-11-04T23:44:25Z

/retest-required

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

openshift-bot · 2021-11-04T23:57:21Z

/retest-required

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

openshift-bot · 2021-11-05T00:10:21Z

/retest-required

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

openshift-bot · 2021-11-05T00:23:24Z

/retest-required

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

openshift-bot · 2021-11-05T00:36:22Z

/retest-required

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

openshift-bot · 2021-11-05T00:49:23Z

/retest-required

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

openshift-ci Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 12, 2021

openshift-ci Bot requested review from bparees and spadgett October 12, 2021 07:21

jhadvig mentioned this pull request Oct 12, 2021

CONSOLE-2892: Allow dynamic plugins to proxy to services on the cluster openshift/console#10215

Merged

jhadvig changed the title ~~[WIP] CONSOLE-2892: Allow dynamic plugins to proxy to services on the cluster~~ CONSOLE-2892: Allow dynamic plugins to proxy to services on the cluster Oct 12, 2021

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 12, 2021

jhadvig commented Oct 12, 2021

View reviewed changes

jhadvig force-pushed the CONSOLE-2892 branch from e975f8d to 951abd1 Compare October 14, 2021 05:14

openshift-ci Bot assigned opayne1, RickJWagner, spadgett and yapei Nov 1, 2021

openshift-ci Bot added the px-approved Signifies that Product Support has signed off on this PR label Nov 2, 2021

openshift-ci Bot added the docs-approved Signifies that Docs has signed off on this PR label Nov 2, 2021

spadgett reviewed Nov 3, 2021

View reviewed changes

openshift-ci Bot added the qe-approved Signifies that QE has signed off on this PR label Nov 4, 2021

spadgett approved these changes Nov 4, 2021

View reviewed changes

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2021

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2021

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 4, 2021

spadgett approved these changes Nov 4, 2021

View reviewed changes

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2021

openshift-merge-robot merged commit 4583855 into openshift:master Nov 5, 2021

Conversation

jhadvig commented Oct 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jhadvig Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

spadgett Oct 13, 2021

Choose a reason for hiding this comment

Uh oh!

jhadvig commented Oct 26, 2021

Uh oh!

jhadvig commented Nov 1, 2021

Uh oh!

jhadvig commented Nov 2, 2021

Uh oh!

RickJWagner commented Nov 2, 2021

Uh oh!

opayne1 commented Nov 2, 2021

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

spadgett Nov 3, 2021

Choose a reason for hiding this comment

Uh oh!

yapei commented Nov 4, 2021

Uh oh!

jhadvig commented Nov 4, 2021

Uh oh!

jhadvig commented Nov 4, 2021

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

jhadvig commented Nov 4, 2021

Uh oh!

jhadvig commented Nov 4, 2021

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Nov 4, 2021

Uh oh!

spadgett commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

openshift-bot commented Nov 4, 2021

Uh oh!

jhadvig commented Oct 12, 2021 •

edited

Loading

openshift-ci Bot commented Nov 4, 2021 •

edited

Loading