Remove all pending endpoints from Kubeops #5249

castelblanque · 2022-08-31T14:22:01Z

Description of the change

This PR completes the removal of UI calls to Kubeops. It leaves the backend Kubeops http-handler.go empty, without any route to be served:

func SetupDefaultRoutes(_ *mux.Router, _ int, _ float32, _ kube.ClustersConfig) error {
	return nil
}

Two routes have been moved into valid Kubeapps sections:

can-i has been added as an endpoint into the resources plugin, being served through GRPC/GRPC-web/REST now. This could be something temporary, creating a new "rbac" plugin into which to copy the endpoint.
operators logo icon endpoint. More information in Additional information section below.

All the above means that Kubeops can be now removed without users noticing.

Benefits

Kubeops is not needed anymore! Kubeapps will be lighter.

Possible drawbacks

Anyone using kubeops directly for whatever reason might be affected.

Applicable issues

fixes Remove/migrate UI calls to Kubeops #4785

Additional information

It turns out that moving the operators logo away from Kubeops it has been the hardest task.

Initial situation

For retrieving an operator logo, Kubeops only acts as a proxy, relaying the request to K8s API with the kubeops service account.
The endpoint returns an operator logo bytes by name, and then it is used as "src" attribute in HTML images.

First try: Switching the call from UI directly to K8s APIs

Direct communication to K8s APIs is accessible when operators are enabled.
It didn't work, as the request needs authorization. Authorization headers can't be provided to the src attribute URL OOB. It needed work in TS/JS to retrieve the data with Axios + authorization, and set the data as base64 in the src attribute. Using the current user token meant also that the user should have get RBAC on packages.operators.coreos.com/packagemanifests/icon, something that the Kubeops serviceaccount has defined in our chart. If the user didn't have that RBAC, he/she would see the list of operators without logos.
Main problem with this approach is that all requests for the operators summary are triggered at the same time. Hanlding so many base64 data at once made the UI terribly sluggish. I even ended up implementing a lazy loading for the icons, which improved the experience, but adding testing to that was challenging and time consuming.

Second try: Moving the endpoint from Kubeops to Kubeappsapis

Defined the endpoint as something temporary in the Resources plugin (until the Operators plugin is built).
That meant having it defined in protobuf, which doesn't allow to return directly bytes (or Any) to serve for a src attribute. A message format must be returned, with at least one field.
Lesson learned: we can't serve an image's bytes with GRPC, a protocol that is not resource oriented.

Third try: Leaving the endpoint in kubeapps-apis as REST only

Finally, the approach that worked was having the endpoint as a special case in kubeapps-apis, see here.
This is something temporary, a new plugin for operators is scheduled and this endpoint should be served from there in the future.

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

netlify · 2022-08-31T14:22:23Z

✅ Deploy Preview for kubeapps-dev canceled.

Name	Link
🔨 Latest commit	`2e3294a`
🔍 Latest deploy log	https://app.netlify.com/sites/kubeapps-dev/deploys/630f6e8c9ffa800009a30b0f

castelblanque · 2022-08-31T14:22:42Z

chart/kubeapps/templates/kubeappsapis/rbac.yaml

+  - apiGroups:
+      - packages.operators.coreos.com
+    resources:
+      - packagemanifests/icon
+    verbs:
+      - get


Needed for retrieving operators logos

castelblanque · 2022-08-31T14:23:48Z

cmd/kubeapps-apis/server/server.go

+
+	// TODO(rcastelblanq) Move this endpoint to the Operators plugin when implementing #4920
+	// Proxies the operator icon request to K8s
+	err = gwmux.HandlePath(http.MethodGet, "/operators/namespaces/{namespace}/operator/{name}/logo", func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) {


Operators logos are served now from here. Not the most elegant solution, but it should be something temporary.

castelblanque · 2022-08-31T14:24:42Z

cmd/kubeapps-apis/server/server.go

@@ -243,5 +245,41 @@ func gatewayMux() (*runtime.ServeMux, error) {
 		return nil, fmt.Errorf("failed to serve: %v", err)
 	}

+	svcRestConfig, err := rest.InClusterConfig()


Using pod's SA to avoid forcing the user's token to have RBAC on operators icons.

I'm not 100% sure, but using such a powerful SA for proxying a k8s API when we are directly injecting unsanitized input from the URL (below), seems like a great attack vector for malicious malformed URLs escalating privileges 😅 .

That said, it is something strictly temporary we should fix near soon, and we clearly discourage enabling operators in production envs, so I'm fine with the change given what you told in the PR comments.

fmt.Sprintf("/apis/packages.operators.coreos.com/v1/namespaces/%s/packagemanifests/%s/icon", namespace, name)

but using such a powerful SA for proxying a k8s API when we are directly injecting unsanitized input from the URL

Totally agree on this. It seems that net/http's ServeMux does some sanitizing of the input (see cleanPath here), but we are using GRPC's ServeMux which apparently doesn't.

We could mimic the net/http behavior using path.Clean or use an external dependency.

antgamdia

+1 as we need to get rid of Kubeops as soon as possible. There are some security concerns as I said, but it's fair enough given the current status of the operators support. Thanks for the changes!

antgamdia · 2022-08-31T15:25:08Z

cmd/kubeapps-apis/docs/kubeapps-apis.swagger.json

@@ -3565,6 +3565,69 @@
        ]
      }
    },
+    "/plugins/resources/v1alpha1/c/{context.cluster}/can-i": {


This API docs should also be (manually) copied to the dashboard/src/public/openapi.yaml file, but we can do it once we remove the kubeapps deployment entirely.

antgamdia · 2022-08-31T15:31:38Z

cmd/kubeapps-apis/server/server.go

@@ -243,5 +245,41 @@ func gatewayMux() (*runtime.ServeMux, error) {
 		return nil, fmt.Errorf("failed to serve: %v", err)
 	}

+	svcRestConfig, err := rest.InClusterConfig()


I'm not 100% sure, but using such a powerful SA for proxying a k8s API when we are directly injecting unsanitized input from the URL (below), seems like a great attack vector for malicious malformed URLs escalating privileges 😅 .

That said, it is something strictly temporary we should fix near soon, and we clearly discourage enabling operators in production envs, so I'm fine with the change given what you told in the PR comments.

fmt.Sprintf("/apis/packages.operators.coreos.com/v1/namespaces/%s/packagemanifests/%s/icon", namespace, name)

antgamdia · 2022-08-31T15:32:47Z

dashboard/src/shared/url.ts

@@ -160,8 +153,8 @@ export const api = {
  },

  operators: {
-    operatorIcon: (cluster: string, namespace: string, name: string) =>
-      `api/v1/clusters/${cluster}/namespaces/${namespace}/operator/${name}/logo`,
+    operatorIcon: (_cluster: string, namespace: string, name: string) =>


Not sure about the current multicluster support for operators, but given the upcoming changes, doesn't really matter I guess.

Yes, in the multicluster scenario we are assuming now that operators logos live in the main (Kubeapps) cluster.

Rafa Castelblanque and others added 10 commits August 26, 2022 17:51

Migrate namespaces retrieval from Kubeops to Kubeapps APIs

d0fd648

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Added missing copyright headers

ad13c98

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Merge branch 'main' into 4785-remove-kubeops

df390ae

Removed all calls to Kubeops

f35c0df

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Fixes and improvements

b56b1ac

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Added tests for CanI

17f5bc8

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Added test for frontend CanI

24da58a

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Run prettier

432e8fa

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Fixed linter issue

54cca89

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Merge branch 'main' into 4785-remove-kubeops-2

2e3294a

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

vmwclabot added the cla-not-required label Aug 31, 2022

castelblanque commented Aug 31, 2022

View reviewed changes

castelblanque marked this pull request as ready for review August 31, 2022 14:25

This was referenced Aug 31, 2022

Remove Kubeops #5253

Closed

Disable Kubeops by default #5254

Merged

Remove/migrate UI calls to Kubeops #4785

Closed

antgamdia approved these changes Aug 31, 2022

View reviewed changes

castelblanque merged commit 5ab31f5 into main Sep 1, 2022

castelblanque deleted the 4785-remove-kubeops-2 branch September 1, 2022 09:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove all pending endpoints from Kubeops #5249

Remove all pending endpoints from Kubeops #5249

castelblanque commented Aug 31, 2022

netlify bot commented Aug 31, 2022 •

edited

Loading

castelblanque Aug 31, 2022

castelblanque Aug 31, 2022

castelblanque Aug 31, 2022

antgamdia Aug 31, 2022

castelblanque Sep 1, 2022

antgamdia left a comment

antgamdia Aug 31, 2022

antgamdia Aug 31, 2022

antgamdia Aug 31, 2022

castelblanque Sep 1, 2022

Remove all pending endpoints from Kubeops #5249

Remove all pending endpoints from Kubeops #5249

Conversation

castelblanque commented Aug 31, 2022

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

Initial situation

First try: Switching the call from UI directly to K8s APIs

Second try: Moving the endpoint from Kubeops to Kubeappsapis

Third try: Leaving the endpoint in kubeapps-apis as REST only

netlify bot commented Aug 31, 2022 • edited Loading

✅ Deploy Preview for kubeapps-dev canceled.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antgamdia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Aug 31, 2022 •

edited

Loading