kyma-project / control-plane

A flexible and easy way to manage Kyma Runtimes
Apache License 2.0
17 stars 112 forks source link

Remove gardener-extension-provider-* dependencies from KMC component #1635

Closed nachtmaar closed 2 years ago

nachtmaar commented 2 years ago

Description

Gardener apis are used inside the KMC component to count the number of virtual networks. This results in several repeated security findings. To make our life easier, we should get rid of the gardener dependency.

Reasoning

The code using gardener-extension-provider-* libs ```go // source: https://github.com/kyma-project/control-plane/blob/d1a89b5c1326ec574146b4a65f17b021455d049e/components/kyma-metrics-collector/pkg/process/event_stream.go#L113 case Azure: // same pattern as for GCP case AWS: // same pattern as for GCP case GCP: decoder := serializer.NewCodecFactory(scheme.Scheme).UniversalDecoder() infraConfig := &gardenergcpv1alpha1.InfrastructureConfig{} if err := runtime.DecodeInto(decoder, rawExtension.Raw, infraConfig); err != nil { return nil, err } if infraConfig.Networks.VPC != nil && infraConfig.Networks.VPC.CloudRouter != nil { vnets += 1 } ```

Solution

Let's use a dynamic k8s client (k8s.io/client-go/dynamic) together with a unstructured.Unstructured struct to access the virtual network. There is an example PR for removing gardener from KEB component.

friedrichwilken commented 2 years ago

I briefly looked into this and here are my conclusions.

What's wrong?

KMC uses gardener (and gardener-extensions for AWS, GCP and Azure). There are some concerns about security since gardener uses a lot of dependencies that in theory also increases the used dependencies of KMC itself. At least code scanning could get influenced by the amount potential dependencies.

The interesting part is, that we hardly use any functionality of gardener: It looks like all the interactions in KMC take place at control-plane/components/kyma-metrics-collector/pkg/process/event_stream.go

For the extensions KMC uses the struct InfrastructureConfig that allows us to communicate with gardener clusters here (azure), here (aws) and here (gcp). In the line underneath this struct gets send to DecodeInto(). For this the struct needs to implement the Object interface which means it needs the DeepCopy and DeepCopyInto methods which usually gets autogenerated. If one just copy-pastes the struct into a new module all the autogeneration code breaks. So, this autogeneration instructions would need some adjustments.

For plain gardener KMC also uses a struct dubbed shoot in the the aforementioned event_stream.go file. At different places in this file different fields of shoot get read. In control-plane/components/kyma-metrics-collector/pkg/process/process.go the value for shoot gets written. For now to me it's unclear, if there are similar limitations as for the extension's structs.

What might be the solution?

Copy the code into a new repo, delete as much unneeded code (and thus dependencies) as possible, make the autogeneration of deepcopy work and uses this in KMC instead.

ghost commented 2 years ago

This issue has been automatically marked as stale due to the lack of recent activity. It will soon be closed if no further activity occurs. Thank you for your contributions.

ghost commented 2 years ago

This issue has been automatically marked as stale due to the lack of recent activity. It will soon be closed if no further activity occurs. Thank you for your contributions.

ghost commented 2 years ago

This issue has been automatically closed due to the lack of recent activity. /lifecycle rotten