Skip to content

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

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jhadvig:CONSOLE-2892
Nov 5, 2021
Merged

CONSOLE-2892: Allow dynamic plugins to proxy to services on the cluster#10215
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jhadvig:CONSOLE-2892

Conversation

@jhadvig
Copy link
Copy Markdown
Member

@jhadvig jhadvig commented Oct 12, 2021

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

Adding --plugin-proxy flag which will contain all the various service types (but for now just just k8s Services) that console backend should proxy to.
PR is working in addition to console-operator changes in openshift/console-operator#603

/assign @spadgett @TheRealJon

@openshift-ci openshift-ci Bot added 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
@openshift-ci openshift-ci Bot requested review from florkbr and spadgett October 12, 2021 11:54
@openshift-ci openshift-ci Bot added component/backend Related to backend kind/dependency-change Categorizes issue or PR as related to changing dependencies approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 12, 2021
@jhadvig jhadvig force-pushed the CONSOLE-2892 branch 2 times, most recently from 3879cbe to 607d2e7 Compare October 12, 2021 16:26
@jhadvig 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 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
Comment thread pkg/serverconfig/types.go Outdated
Copy link
Copy Markdown
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jhadvig

Comment thread pkg/server/server.go Outdated
Comment thread pkg/plugins/handlers.go Outdated
Comment thread pkg/plugins/handlers.go Outdated
Comment thread pkg/serverconfig/types.go Outdated
@spadgett
Copy link
Copy Markdown
Member

We should add some details to the dynamic plugins README. We need to let developers know how to set up a proxy when running bridge locally.

@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Oct 14, 2021

@spadgett comments addressed. PTAL

Comment thread frontend/dynamic-demo-plugin/README.md Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Borrowed this part from the enhancement doc since I think it would be useful for the dev/user as well

@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Oct 25, 2021

@spadgett I've added proxy service to the console-demo-plugin which is pointing to the thanos-querier service in the openshift-monitoring. Also I've extended the dynamic-demo-plugin's console-extensions.json with /test-proxy-service route that will use the proxy service to query Thanos for list of Rules.

Screen:
Screenshot 2021-10-25 at 12 09 44

Please let me know if anything else is needed.

Comment thread frontend/dynamic-demo-plugin/README.md Outdated
Comment thread frontend/dynamic-demo-plugin/README.md Outdated
Comment thread frontend/dynamic-demo-plugin/src/components/ProxyService.tsx Outdated
Comment thread frontend/dynamic-demo-plugin/yarn.lock Outdated
Comment thread pkg/serverconfig/types.go Outdated
@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Oct 26, 2021

@spadgett thanks for review, I've addressed your comments. PTAL

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 28, 2021
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 1, 2021
@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Nov 1, 2021

PR is ready for final review.

QE Approver:
/assign @yapei

Docs Approver:
/assign @opayne1

PX Approver:
/assign @RickJWagner

Console Approver:
/assign @spadgett

@RickJWagner
Copy link
Copy Markdown

/label px-approved

@openshift-ci openshift-ci Bot added the px-approved Signifies that Product Support has signed off on this PR label Nov 2, 2021
@opayne1
Copy link
Copy Markdown
Contributor

opayne1 commented Nov 2, 2021

/label docs-approved

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

@spadgett spadgett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, thanks @jhadvig

Comment thread dynamic-demo-plugin/README.md Outdated
Comment thread dynamic-demo-plugin/src/components/ProxyService.tsx Outdated
Comment thread dynamic-demo-plugin/src/components/ProxyService.tsx Outdated
@yapei
Copy link
Copy Markdown
Contributor

yapei commented Nov 4, 2021

verified the changes locally and works

./bin/bridge --plugin-proxy='{"services":[{"endpoint":"http://localhost:8080","consoleAPIPath":"/api/proxy/namespace/openshift-monitoring/service/thanos-querier:9091/"}]}' -plugins console-demo-plugin=http://localhost:9001

Screen Shot 2021-11-04 at 6 36 29 PM

/label qe-approved

@openshift-ci openshift-ci Bot added the qe-approved Signifies that QE has signed off on this PR label Nov 4, 2021
@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Nov 4, 2021

@spadgett I've addressed your comments in a additional commit. PTAL

Copy link
Copy Markdown
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2021
@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Nov 4, 2021

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

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 4, 2021
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2021
@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Nov 4, 2021

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

/hold cancel

@openshift-ci 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
Copy link
Copy Markdown
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

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

openshift-ci Bot commented Nov 4, 2021

[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:

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

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

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

@openshift-merge-robot openshift-merge-robot merged commit 63281ba into openshift:master Nov 5, 2021
@spadgett spadgett added this to the v4.10 milestone Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/backend Related to backend docs-approved Signifies that Docs has signed off on this PR kind/dependency-change Categorizes issue or PR as related to changing dependencies lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants