Skip to content

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

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

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

Conversation

@jhadvig
Copy link
Copy Markdown
Member

@jhadvig jhadvig commented Oct 1, 2021

First draft of the proposed feature for allowing dynamic plugins to proxy to services on the cluster.

Story: https://issues.redhat.com/browse/CONSOLE-2892

/assign @spadgett @vojtechszocs @christianvogt

@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 1, 2021
@openshift-ci openshift-ci Bot requested review from sjenning and spadgett October 1, 2021 10:54
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
@spadgett
Copy link
Copy Markdown
Member

spadgett commented Oct 1, 2021

cc @jamestalton

Comment thread enhancements/console/dynamic-plugins.md Outdated
After that plugin can make request to the console endpoint, which will
proxy the request to the dependent service. Plugin's request shall use
following console's endpoint:
`/api/plugin-proxy/<dependent-service-name>/<request-path>`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This path isn't guaranteed to be unique. We should include the service namespace and port in the path.

Copy link
Copy Markdown
Member

@spadgett spadgett Oct 6, 2021

Choose a reason for hiding this comment

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

Maybe

Suggested change
`/api/plugin-proxy/<dependent-service-name>/<request-path>`
`/api/proxy/namespace/<service-namespace>/service/<service-name>:<port-number>/<request-path>?<optional-query-parameters>`

I'd suggest providing a concrete example URL and what service URL that it maps to.

Copy link
Copy Markdown
Contributor

@vojtechszocs vojtechszocs Oct 6, 2021

Choose a reason for hiding this comment

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

Another option is to use e.g. /api/proxy/<plugin-name>/<service-name>/<request-path> since there can be only one proxy service with the given name for the given ConsolePlugin resource manifest.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Service name is not unique... A plugin can proxy to two services with the same name in different namespaces, or the same service, but different ports.

@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 6, 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 6, 2021
@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Oct 6, 2021

@spadgett @vojtechszocs PTAL

@jhadvig jhadvig closed this Oct 6, 2021
@jhadvig jhadvig reopened this Oct 6, 2021
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
@jhadvig
Copy link
Copy Markdown
Member Author

jhadvig commented Oct 6, 2021

Thanks for additional comments! I've addressed them 👍

Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md Outdated
Comment thread enhancements/console/dynamic-plugins.md 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.

/lgtm

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

openshift-ci Bot commented Oct 6, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 6, 2021
@openshift-merge-robot openshift-merge-robot merged commit 06e6aeb into openshift:master Oct 6, 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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants