[v14] Prevent double registration of Kubernetes GVK for older Kube cl…

…usters (#33402) * Prevent double registration of Kubernetes GVK for older Kube clusters Older Kubernetes clusters might have types that live outside of default Kubernetes types but are registered using different APIs. When Teleport starts, it tries to pull the resources from the cluster and panics because a certain type is already registered. This PR adds a check that prevents double registration. Signed-off-by: Tiago Silva <tiago.silva@goteleport.com> * add unit test to check for panic --------- Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
gravitational · Oct 13, 2023 · addd901 · addd901
1 parent 8334494
commit addd901
Show file tree

Hide file tree

Showing 6 changed files with 194 additions and 5 deletions.
diff --git a/lib/kube/proxy/resource_deletecollection.go b/lib/kube/proxy/resource_deletecollection.go
@@ -29,6 +29,7 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	certificatesv1 "k8s.io/api/certificates/v1"
 	corev1 "k8s.io/api/core/v1"
+	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	networkingv1 "k8s.io/api/networking/v1"
 	authv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -415,6 +416,58 @@ func (f *Forwarder) handleDeleteCollectionReq(req *http.Request, sess *clusterSe
 			return internalErrStatus, trace.Wrap(err)
 		}
 		o.Items = pointerArrayToArray(items)
+	case *extensionsv1beta1.IngressList:
+		items, err := deleteResources(
+			params,
+			types.KindKubeIngress,
+			arrayToPointerArray(o.Items),
+			func(ctx context.Context, client kubernetes.Interface, name, namespace string) error {
+				return trace.Wrap(client.ExtensionsV1beta1().Ingresses(namespace).Delete(ctx, name, deleteOptions))
+			},
+		)
+		if err != nil {
+			return internalErrStatus, trace.Wrap(err)
+		}
+		o.Items = pointerArrayToArray(items)
+	case *extensionsv1beta1.DaemonSetList:
+		items, err := deleteResources(
+			params,
+			types.KindKubeDaemonSet,
+			arrayToPointerArray(o.Items),
+			func(ctx context.Context, client kubernetes.Interface, name, namespace string) error {
+				return trace.Wrap(client.ExtensionsV1beta1().DaemonSets(namespace).Delete(ctx, name, deleteOptions))
+			},
+		)
+		if err != nil {
+			return internalErrStatus, trace.Wrap(err)
+		}
+		o.Items = pointerArrayToArray(items)
+	case *extensionsv1beta1.DeploymentList:
+		items, err := deleteResources(
+			params,
+			types.KindKubeDeployment,
+			arrayToPointerArray(o.Items),
+			func(ctx context.Context, client kubernetes.Interface, name, namespace string) error {
+				return trace.Wrap(client.ExtensionsV1beta1().Deployments(namespace).Delete(ctx, name, deleteOptions))
+			},
+		)
+		if err != nil {
+			return internalErrStatus, trace.Wrap(err)
+		}
+		o.Items = pointerArrayToArray(items)
+	case *extensionsv1beta1.ReplicaSetList:
+		items, err := deleteResources(
+			params,
+			types.KindKubeReplicaSet,
+			arrayToPointerArray(o.Items),
+			func(ctx context.Context, client kubernetes.Interface, name, namespace string) error {
+				return trace.Wrap(client.ExtensionsV1beta1().ReplicaSets(namespace).Delete(ctx, name, deleteOptions))
+			},
+		)
+		if err != nil {
+			return internalErrStatus, trace.Wrap(err)
+		}
+		o.Items = pointerArrayToArray(items)
 	default:
 		return internalErrStatus, trace.BadParameter("unexpected type %T", obj)
 	}

diff --git a/lib/kube/proxy/resource_filters.go b/lib/kube/proxy/resource_filters.go
@@ -26,6 +26,7 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	certificatesv1 "k8s.io/api/certificates/v1"
 	corev1 "k8s.io/api/core/v1"
+	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	networkingv1 "k8s.io/api/networking/v1"
 	authv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -491,6 +492,66 @@ func (d *resourceFilterer) FilterObj(obj runtime.Object) (isAllowed bool, isList
 				arrayToPointerArray(o.Items), d.allowedResources, d.deniedResources, d.log),
 		)
 		return len(o.Items) > 0, true, nil
+	case *extensionsv1beta1.Ingress:
+		result, err := filterResource(d.kind, d.verb, o, d.allowedResources, d.deniedResources)
+		if err != nil {
+			d.log.WithError(err).Warn("Unable to compile regex expressions within kubernetes_resources.")
+		}
+		// if err is not nil or result is false, we should not include it.
+		return result, false, nil
+	case *extensionsv1beta1.IngressList:
+		o.Items = pointerArrayToArray(
+			filterResourceList(
+				d.kind, d.verb,
+				arrayToPointerArray(o.Items), d.allowedResources, d.deniedResources, d.log),
+		)
+		return len(o.Items) > 0, true, nil
+
+	case *extensionsv1beta1.DaemonSet:
+		result, err := filterResource(d.kind, d.verb, o, d.allowedResources, d.deniedResources)
+		if err != nil {
+			d.log.WithError(err).Warn("Unable to compile regex expressions within kubernetes_resources.")
+		}
+		// if err is not nil or result is false, we should not include it.
+		return result, false, nil
+	case *extensionsv1beta1.DaemonSetList:
+		o.Items = pointerArrayToArray(
+			filterResourceList(
+				d.kind, d.verb,
+				arrayToPointerArray(o.Items), d.allowedResources, d.deniedResources, d.log),
+		)
+		return len(o.Items) > 0, true, nil
+
+	case *extensionsv1beta1.Deployment:
+		result, err := filterResource(d.kind, d.verb, o, d.allowedResources, d.deniedResources)
+		if err != nil {
+			d.log.WithError(err).Warn("Unable to compile regex expressions within kubernetes_resources.")
+		}
+		// if err is not nil or result is false, we should not include it.
+		return result, false, nil
+	case *extensionsv1beta1.DeploymentList:
+		o.Items = pointerArrayToArray(
+			filterResourceList(
+				d.kind, d.verb,
+				arrayToPointerArray(o.Items), d.allowedResources, d.deniedResources, d.log),
+		)
+		return len(o.Items) > 0, true, nil
+
+	case *extensionsv1beta1.ReplicaSet:
+		result, err := filterResource(d.kind, d.verb, o, d.allowedResources, d.deniedResources)
+		if err != nil {
+			d.log.WithError(err).Warn("Unable to compile regex expressions within kubernetes_resources.")
+		}
+		// if err is not nil or result is false, we should not include it.
+		return result, false, nil
+	case *extensionsv1beta1.ReplicaSetList:
+		o.Items = pointerArrayToArray(
+			filterResourceList(
+				d.kind, d.verb,
+				arrayToPointerArray(o.Items), d.allowedResources, d.deniedResources, d.log),
+		)
+		return len(o.Items) > 0, true, nil
+
 	case *unstructured.Unstructured:
 		if o.IsList() {
 			hasElemts := filterUnstructuredList(d.verb, o, d.allowedResources, d.deniedResources, d.log)
@@ -514,7 +575,6 @@ func (d *resourceFilterer) FilterObj(obj runtime.Object) (isAllowed bool, isList
 			return false, false, trace.Wrap(err)
 		}
 		return len(o.Rows) > 0, true, nil
-
 	default:
 		// It's important default types are never blindly forwarded or protocol
 		// extensions could result in information disclosures.

diff --git a/lib/kube/proxy/resource_filters_test.go b/lib/kube/proxy/resource_filters_test.go
@@ -31,6 +31,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
+	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	networkingv1 "k8s.io/api/networking/v1"
 	authv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -192,6 +193,14 @@ func Test_filterBuffer(t *testing.T) {
 					resources = collectResourcesFromResponse(arrayToPointerArray(o.Items))
 				case *networkingv1.IngressList:
 					resources = collectResourcesFromResponse(arrayToPointerArray(o.Items))
+				case *extensionsv1beta1.IngressList:
+					resources = collectResourcesFromResponse(arrayToPointerArray(o.Items))
+				case *extensionsv1beta1.DaemonSetList:
+					resources = collectResourcesFromResponse(arrayToPointerArray(o.Items))
+				case *extensionsv1beta1.ReplicaSetList:
+					resources = collectResourcesFromResponse(arrayToPointerArray(o.Items))
+				case *extensionsv1beta1.DeploymentList:
+					resources = collectResourcesFromResponse(arrayToPointerArray(o.Items))
 				case *metav1.Table:
 					for i := range o.Rows {
 						row := &(o.Rows[i])

diff --git a/lib/kube/proxy/scheme.go b/lib/kube/proxy/scheme.go
@@ -99,7 +99,7 @@ func newClientNegotiator(codecFactory *serializer.CodecFactory) runtime.ClientNe
 // This schema includes all well-known Kubernetes types and all namespaced
 // custom resources.
 // It also returns a map of resources that we support RBAC restrictions for.
-func newClusterSchemaBuilder(client *kubernetes.Clientset) (serializer.CodecFactory, rbacSupportedResources, error) {
+func newClusterSchemaBuilder(client kubernetes.Interface) (serializer.CodecFactory, rbacSupportedResources, error) {
 	kubeScheme := runtime.NewScheme()
 	kubeCodecs := serializer.NewCodecFactory(kubeScheme)
 	supportedResources := maps.Clone(defaultRBACResources)
@@ -110,7 +110,7 @@ func newClusterSchemaBuilder(client *kubernetes.Clientset) (serializer.CodecFact
 	// discoveryErr is returned when the discovery of one or more API groups fails.
 	var discoveryErr *discovery.ErrGroupDiscoveryFailed
 	// register all namespaced custom resources
-	_, apiGroups, err := client.DiscoveryClient.ServerGroupsAndResources()
+	_, apiGroups, err := client.Discovery().ServerGroupsAndResources()
 	switch {
 	case errors.As(err, &discoveryErr) && len(discoveryErr.Groups) == 1:
 		// If the discovery error is of type `ErrGroupDiscoveryFailed` and it
@@ -154,11 +154,17 @@ func newClusterSchemaBuilder(client *kubernetes.Clientset) (serializer.CodecFact
 			// the namespace where the resource is located.
 			// This means that we need to map the resource to the namespace kind.
 			supportedResources[resourceKey] = utils.KubeCustomResource
-
+			// create the group version kind for the resource
+			gvk := groupVersion.WithKind(apiResource.Kind)
+			// check if the resource is already registered in the scheme
+			// if it is, we don't need to register it again.
+			if _, err := kubeScheme.New(gvk); err == nil {
+				continue
+			}
 			// register the resource with the scheme to be able to decode it
 			// into an unstructured object
 			kubeScheme.AddKnownTypeWithName(
-				groupVersion.WithKind(apiResource.Kind),
+				gvk,
 				&unstructured.Unstructured{},
 			)
 			// register the resource list with the scheme to be able to decode it

diff --git a/lib/kube/proxy/scheme_test.go b/lib/kube/proxy/scheme_test.go
@@ -0,0 +1,57 @@
+/*
+Copyright 2022 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package proxy
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/kubernetes"
+)
+
+// TestNewClusterSchemaBuilder tests that newClusterSchemaBuilder doesn't panic
+// when it's given types already registered in the global scheme.
+func Test_newClusterSchemaBuilder(t *testing.T) {
+	_, _, err := newClusterSchemaBuilder(&clientSet{})
+	require.NoError(t, err)
+}
+
+type clientSet struct {
+	kubernetes.Interface
+	discovery.DiscoveryInterface
+}
+
+func (c *clientSet) Discovery() discovery.DiscoveryInterface {
+	return c
+}
+
+func (c *clientSet) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) {
+	return nil, []*metav1.APIResourceList{
+		{
+			GroupVersion: "extensions/v1beta1",
+			APIResources: []metav1.APIResource{
+				{
+					Name:       "ingresses",
+					Kind:       "Ingress",
+					Namespaced: true,
+				},
+			},
+		},
+	}, nil
+}
diff --git a/lib/kube/proxy/url.go b/lib/kube/proxy/url.go
@@ -203,6 +203,10 @@ var defaultRBACResources = rbacSupportedResources{
 	{apiGroup: "batch", resourceKind: "jobs"}:                                     types.KindKubeJob,
 	{apiGroup: "certificates.k8s.io", resourceKind: "certificatesigningrequests"}: types.KindKubeCertificateSigningRequest,
 	{apiGroup: "networking.k8s.io", resourceKind: "ingresses"}:                    types.KindKubeIngress,
+	{apiGroup: "extensions", resourceKind: "deployments"}:                         types.KindKubeDeployment,
+	{apiGroup: "extensions", resourceKind: "replicasets"}:                         types.KindKubeReplicaSet,
+	{apiGroup: "extensions", resourceKind: "daemonsets"}:                          types.KindKubeDaemonSet,
+	{apiGroup: "extensions", resourceKind: "ingresses"}:                           types.KindKubeIngress,
 }
 
 // getResourceFromRequest returns a KubernetesResource if the user tried to access