Skip to content

Conversation

@leoryu
Copy link
Contributor

@leoryu leoryu commented Mar 18, 2021

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

fix #1134

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@leoryu leoryu requested a review from choujimmy as a code owner March 18, 2021 11:56
client, err := proxy.ClientSet(req.Request.Context(), platformClient)
if err != nil {
resp.WriteHeaderAndEntity(http.StatusBadRequest, "err1"+err.Error())
return
Copy link
Contributor

Choose a reason for hiding this comment

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

why err1 rather than "err.Error()"

Copy link
Contributor

Choose a reason for hiding this comment

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

to keep compatibility, please add logic to check if token or client crt/key pass to back-end, then proxy the request to remote k8s server, otherwise get version info from local


if err != nil {
resp.WriteHeaderAndEntity(http.StatusInternalServerError, "err2"+err.Error())
return
Copy link
Contributor

Choose a reason for hiding this comment

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

why "err2"?

@@ -0,0 +1,51 @@
package apiserver

Copy link
Contributor

@huxiaoliang huxiaoliang Mar 19, 2021

Choose a reason for hiding this comment

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

  1. please place and rename this file to tke/pkg/platform/proxy/version/rest/storage.go
  2. add some comments to highlight this new feature: how to add new rest api which doesn't belong to native k8s resouce to tke-platform-api
  1. xx
  2. yy

@huxiaoliang
Copy link
Contributor

please fix CI build issue

@leoryu leoryu force-pushed the proxy_version_api branch from b412bb1 to 5fc64a1 Compare March 19, 2021 06:15
@huxiaoliang huxiaoliang merged commit ea282b5 into tkestack:master Mar 19, 2021
@leoryu leoryu deleted the proxy_version_api branch May 8, 2021 06:23
pavlelee pushed a commit to GaoXiaodong/tke that referenced this pull request Sep 6, 2021
leoryu added a commit that referenced this pull request Sep 8, 2021
leoryu added a commit that referenced this pull request Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tke platform api support proxy /version api to remote cluster

2 participants