From e66f3e4b2551918a962771a1c1a30eb2458fa9bd Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Thu, 13 Apr 2023 17:00:38 +0100 Subject: [PATCH 1/4] Kubernetes License check for root clusters This PR ensures that a given Teleport root cluster is licensed for Kubernetes access when forwarding credentials to leaf clusters. Previously, the root Kube proxy would call auth server to generate a new cert-key pair with the user identity and during the call Auth ensured that it was licensed for Kubernetes usage. Given the recent developments for #22533, the call mentioned was removed and it's now possible for a root enterprise cluster to forward requests to a OSS leaf cluster without validating its license. Auth server already enforces Kubernetes license for Kubernetes service when it tries to heartbeat the cluster. If the cluster is not properly licensed, it's not possible to register kubernetes clusters. Part of #22533 --- lib/kube/proxy/forwarder.go | 69 ++++++++++++++++++++++++++++++-- lib/kube/proxy/forwarder_test.go | 1 + 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index 11a2988833fdd..eef095b109ea0 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -35,6 +35,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "time" "github.com/google/uuid" @@ -274,7 +275,8 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) { } closeCtx, close := context.WithCancel(cfg.Context) - + var isKubeLicensed atomic.Bool + isKubeLicensed.Store(true) fwd := &Forwarder{ log: cfg.log, cfg: cfg, @@ -287,9 +289,11 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) { ReadBufferSize: 1024, WriteBufferSize: 1024, }, - clusterDetails: make(map[string]*kubeDetails), - cachedTransport: transportClients, + clusterDetails: make(map[string]*kubeDetails), + cachedTransport: transportClients, + isClusterKubernetesLicensed: isKubeLicensed, } + router := httprouter.New() router.UseRawPath = true @@ -339,9 +343,59 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) { return nil, trace.Wrap(err) } } + + // Start the proxy license watcher only if it not a kube service. + // Kube service validation is performed by the auth server when it tries + // to heartbeat the cluster. + if fwd.cfg.KubeServiceType != KubeService { + go fwd.proxylicenseWatcher() + } + return fwd, nil } +// proxylicenseWatcher periodically checks if the auth server is licensed for +// kubernetes. This check is required after the introduction of the credentials +// forwarding feature. When a root kube proxy is forwarding requests to a leaf +// kube proxy, the root kube proxy must ensure that it is licensed for +// kubernetes otherwise an enterprise root cluster will be able to forward +// requests to an OSS leaf cluster and bypass the license check. +// This check is only performed on Kubernetes proxies. +func (f *Forwarder) proxylicenseWatcher() { + t := time.NewTicker(30 * time.Minute) + defer t.Stop() + for { + licensed, err := f.checkIfAuthIsLicensed() + if err != nil { + f.log.WithError(err).Warn("Failed to check if auth server is licensed for Kubernetes usage.") + } else { + f.isClusterKubernetesLicensed.Store(licensed) + } + select { + case <-t.C: + case <-f.ctx.Done(): + return + } + } +} + +// checkIfAuthIsLicensed checks if the auth server is licensed for kubernetes. +// This check is required after the introduction of the credentials forwarding +// feature. When a root kube proxy is forwarding requests to a leaf kube proxy, +// the root kube proxy must ensure that it is licensed for kubernetes otherwise +// an enterprise root cluster will be able to forward requests to an OSS leaf +// cluster and bypass the license check. +func (f *Forwarder) checkIfAuthIsLicensed() (bool, error) { + ctx, cancel := context.WithTimeout(f.ctx, defaults.HTTPRequestTimeout) + defer cancel() + // If this is a kube_service, we can just return the local kube servers. + ping, err := f.cfg.AuthClient.Ping(ctx) + if err != nil { + return false, trace.Wrap(err) + } + return ping.ServerFeatures.Kubernetes, nil +} + // Forwarder intercepts kubernetes requests, acting as Kubernetes API proxy. // it blindly forwards most of the requests on HTTPS protocol layer, // however some requests like exec sessions it intercepts and records. @@ -384,6 +438,9 @@ type Forwarder struct { cachedTransport *ttlmap.TTLMap // cachedTransportMu is a mutex used to protect the cachedTransport. cachedTransportMu sync.Mutex + // isClusterKubernetesLicensed is a pointer to a boolean that indicates + // whether the cluster is licensed for Kubernetes. + isClusterKubernetesLicensed atomic.Bool } // getKubeServersByNameFunc is a function that returns a list of @@ -498,6 +555,12 @@ const accessDeniedMsg = "[00] access denied" // authenticate function authenticates request func (f *Forwarder) authenticate(req *http.Request) (*authContext, error) { + // If the cluster is not licensed for Kubernetes, return an error to the client. + if !f.isClusterKubernetesLicensed.Load() { + // If the cluster is not licensed for Kubernetes, return an error to the client + // but in a way that the code is translated to 500 error code. + return nil, trace.Errorf("Teleport cluster is not licensed for Kubernetes") + } ctx, span := f.cfg.tracer.Start( req.Context(), "kube.Forwarder/authenticate", diff --git a/lib/kube/proxy/forwarder_test.go b/lib/kube/proxy/forwarder_test.go index 5af24953e7af8..00337b5d6ca27 100644 --- a/lib/kube/proxy/forwarder_test.go +++ b/lib/kube/proxy/forwarder_test.go @@ -178,6 +178,7 @@ func TestAuthenticate(t *testing.T) { return filtered, nil }, } + f.isClusterKubernetesLicensed.Store(true) const remoteAddr = "user.example.com" activeAccessRequests := []string{uuid.NewString(), uuid.NewString()} From 90321e7beaaec53dafc6706b30e6a0a0c87ba93d Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Sat, 15 Apr 2023 18:28:11 +0100 Subject: [PATCH 2/4] remove watcher and use in process.getClusterFeatures callback --- lib/kube/proxy/forwarder.go | 72 ++++++-------------------------- lib/kube/proxy/forwarder_test.go | 10 ++++- lib/kube/proxy/utils_testing.go | 4 +- lib/service/kubernetes.go | 1 + lib/service/service.go | 3 +- lib/web/apiserver_test.go | 5 +++ 6 files changed, 33 insertions(+), 62 deletions(-) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index eef095b109ea0..6a6241cbf0367 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -35,7 +35,6 @@ import ( "strconv" "strings" "sync" - "sync/atomic" "time" "github.com/google/uuid" @@ -65,6 +64,7 @@ import ( kubeexec "k8s.io/client-go/util/exec" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/observability/tracing" @@ -172,8 +172,14 @@ type ForwarderConfig struct { // the upstream Teleport proxy or Kubernetes service when forwarding requests // using the forward identity (i.e. proxy impersonating a user) method. ConnTLSConfig *tls.Config + // ClusterFeaturesGetter is a function that returns the Teleport cluster licensed features. + // It is used to determine if the cluster is licensed for Kubernetes usage. + ClusterFeatures ClusterFeaturesGetter } +// ClusterFeaturesGetter is a function that returns the Teleport cluster licensed features. +type ClusterFeaturesGetter func() proto.Features + // CheckAndSetDefaults checks and sets default values func (f *ForwarderConfig) CheckAndSetDefaults() error { if f.AuthClient == nil { @@ -200,6 +206,9 @@ func (f *ForwarderConfig) CheckAndSetDefaults() error { if f.HostID == "" { return trace.BadParameter("missing parameter ServerID") } + if f.ClusterFeatures == nil { + return trace.BadParameter("missing parameter ClusterFeatures") + } if f.KubeServiceType != KubeService && f.PROXYSigner == nil { return trace.BadParameter("missing parameter PROXYSigner") } @@ -275,8 +284,6 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) { } closeCtx, close := context.WithCancel(cfg.Context) - var isKubeLicensed atomic.Bool - isKubeLicensed.Store(true) fwd := &Forwarder{ log: cfg.log, cfg: cfg, @@ -289,9 +296,8 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) { ReadBufferSize: 1024, WriteBufferSize: 1024, }, - clusterDetails: make(map[string]*kubeDetails), - cachedTransport: transportClients, - isClusterKubernetesLicensed: isKubeLicensed, + clusterDetails: make(map[string]*kubeDetails), + cachedTransport: transportClients, } router := httprouter.New() @@ -344,58 +350,9 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) { } } - // Start the proxy license watcher only if it not a kube service. - // Kube service validation is performed by the auth server when it tries - // to heartbeat the cluster. - if fwd.cfg.KubeServiceType != KubeService { - go fwd.proxylicenseWatcher() - } - return fwd, nil } -// proxylicenseWatcher periodically checks if the auth server is licensed for -// kubernetes. This check is required after the introduction of the credentials -// forwarding feature. When a root kube proxy is forwarding requests to a leaf -// kube proxy, the root kube proxy must ensure that it is licensed for -// kubernetes otherwise an enterprise root cluster will be able to forward -// requests to an OSS leaf cluster and bypass the license check. -// This check is only performed on Kubernetes proxies. -func (f *Forwarder) proxylicenseWatcher() { - t := time.NewTicker(30 * time.Minute) - defer t.Stop() - for { - licensed, err := f.checkIfAuthIsLicensed() - if err != nil { - f.log.WithError(err).Warn("Failed to check if auth server is licensed for Kubernetes usage.") - } else { - f.isClusterKubernetesLicensed.Store(licensed) - } - select { - case <-t.C: - case <-f.ctx.Done(): - return - } - } -} - -// checkIfAuthIsLicensed checks if the auth server is licensed for kubernetes. -// This check is required after the introduction of the credentials forwarding -// feature. When a root kube proxy is forwarding requests to a leaf kube proxy, -// the root kube proxy must ensure that it is licensed for kubernetes otherwise -// an enterprise root cluster will be able to forward requests to an OSS leaf -// cluster and bypass the license check. -func (f *Forwarder) checkIfAuthIsLicensed() (bool, error) { - ctx, cancel := context.WithTimeout(f.ctx, defaults.HTTPRequestTimeout) - defer cancel() - // If this is a kube_service, we can just return the local kube servers. - ping, err := f.cfg.AuthClient.Ping(ctx) - if err != nil { - return false, trace.Wrap(err) - } - return ping.ServerFeatures.Kubernetes, nil -} - // Forwarder intercepts kubernetes requests, acting as Kubernetes API proxy. // it blindly forwards most of the requests on HTTPS protocol layer, // however some requests like exec sessions it intercepts and records. @@ -438,9 +395,6 @@ type Forwarder struct { cachedTransport *ttlmap.TTLMap // cachedTransportMu is a mutex used to protect the cachedTransport. cachedTransportMu sync.Mutex - // isClusterKubernetesLicensed is a pointer to a boolean that indicates - // whether the cluster is licensed for Kubernetes. - isClusterKubernetesLicensed atomic.Bool } // getKubeServersByNameFunc is a function that returns a list of @@ -556,7 +510,7 @@ const accessDeniedMsg = "[00] access denied" // authenticate function authenticates request func (f *Forwarder) authenticate(req *http.Request) (*authContext, error) { // If the cluster is not licensed for Kubernetes, return an error to the client. - if !f.isClusterKubernetesLicensed.Load() { + if !f.cfg.ClusterFeatures().Kubernetes { // If the cluster is not licensed for Kubernetes, return an error to the client // but in a way that the code is translated to 500 error code. return nil, trace.Errorf("Teleport cluster is not licensed for Kubernetes") diff --git a/lib/kube/proxy/forwarder_test.go b/lib/kube/proxy/forwarder_test.go index 00337b5d6ca27..c6e6b52596da9 100644 --- a/lib/kube/proxy/forwarder_test.go +++ b/lib/kube/proxy/forwarder_test.go @@ -46,6 +46,7 @@ import ( "k8s.io/client-go/transport" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/testauthority" @@ -124,6 +125,12 @@ func TestRequestCertificate(t *testing.T) { require.Empty(t, cmp.Diff(*idFromCSR, ctx.UnmappedIdentity.GetIdentity())) } +func fakeClusterFeatures() proto.Features { + return proto.Features{ + Kubernetes: true, + } +} + func TestAuthenticate(t *testing.T) { t.Parallel() @@ -163,6 +170,7 @@ func TestAuthenticate(t *testing.T) { CachingAuthClient: ap, TracerProvider: otel.GetTracerProvider(), tracer: otel.Tracer(teleport.ComponentKube), + ClusterFeatures: fakeClusterFeatures, }, getKubernetesServersForKubeCluster: func(ctx context.Context, name string) ([]types.KubeServer, error) { servers, err := ap.GetKubernetesServers(ctx) @@ -178,7 +186,6 @@ func TestAuthenticate(t *testing.T) { return filtered, nil }, } - f.isClusterKubernetesLicensed.Store(true) const remoteAddr = "user.example.com" activeAccessRequests := []string{uuid.NewString(), uuid.NewString()} @@ -1267,6 +1274,7 @@ func newMockForwader(ctx context.Context, t *testing.T) *Forwarder { Context: ctx, TracerProvider: otel.GetTracerProvider(), tracer: otel.Tracer(teleport.ComponentKube), + ClusterFeatures: fakeClusterFeatures, }, clientCredentials: clientCreds, activeRequests: make(map[string]context.Context), diff --git a/lib/kube/proxy/utils_testing.go b/lib/kube/proxy/utils_testing.go index 9bd78c249a744..e93b960664426 100644 --- a/lib/kube/proxy/utils_testing.go +++ b/lib/kube/proxy/utils_testing.go @@ -38,6 +38,7 @@ import ( clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/client/proto" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" @@ -202,7 +203,8 @@ func SetupTestContext(ctx context.Context, t *testing.T, cfg TestConfig) *TestCo CheckImpersonationPermissions: func(ctx context.Context, clusterName string, sarClient authztypes.SelfSubjectAccessReviewInterface) error { return nil }, - Clock: clockwork.NewRealClock(), + Clock: clockwork.NewRealClock(), + ClusterFeatures: func() proto.Features { return proto.Features{Kubernetes: true} }, }, DynamicLabels: nil, TLS: tlsConfig, diff --git a/lib/service/kubernetes.go b/lib/service/kubernetes.go index a97d8d2b252d7..8b4087f4b5c7f 100644 --- a/lib/service/kubernetes.go +++ b/lib/service/kubernetes.go @@ -231,6 +231,7 @@ func (process *TeleportProcess) initKubernetesService(log *logrus.Entry, conn *C LockWatcher: lockWatcher, CheckImpersonationPermissions: cfg.Kube.CheckImpersonationPermissions, PublicAddr: publicAddr, + ClusterFeatures: process.getClusterFeatures, }, TLS: tlsConfig, AccessPoint: accessPoint, diff --git a/lib/service/service.go b/lib/service/service.go index 6dd3c285cfa8a..64a5113dfe779 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -4070,7 +4070,8 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { // using Impersonation headers. The upstream service will validate if // the provided connection certificate is from a proxy server and // will impersonate the identity of the user that is making the request. - ConnTLSConfig: tlsConfig.Clone(), + ConnTLSConfig: tlsConfig.Clone(), + ClusterFeatures: process.getClusterFeatures, }, TLS: tlsConfig.Clone(), LimiterConfig: cfg.Proxy.Limiter, diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index bc62a82f6baa7..aaeeb984f71cc 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -7943,6 +7943,11 @@ func startKubeWithoutCleanup(ctx context.Context, t *testing.T, cfg startKubeOpt }, ConnTLSConfig: tlsConfig, Clock: clockwork.NewRealClock(), + ClusterFeatures: func() clientproto.Features { + return clientproto.Features{ + Kubernetes: true, + } + }, }, TLS: tlsConfig, AccessPoint: client, From 9a249562a6963d76981bdeb3e9939f04378c308f Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Mon, 17 Apr 2023 09:34:40 +0100 Subject: [PATCH 3/4] add test to validate kube api license enforcement --- lib/kube/proxy/forwarder.go | 9 ++-- lib/kube/proxy/forwarder_test.go | 78 ++++++++++++++++++++++++++++++++ lib/kube/proxy/utils_testing.go | 8 +++- 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index 6a6241cbf0367..591ed379a71c0 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -728,14 +728,15 @@ func (f *Forwarder) writeResponseErrorToBody(rw http.ResponseWriter, respErr err // formatStatusResponseError formats the error response into a kube Status object. func (f *Forwarder) formatStatusResponseError(rw http.ResponseWriter, respErr error) { + code := trace.ErrorToCode(respErr) status := &metav1.Status{ Status: metav1.StatusFailure, // Don't trace.Unwrap the error, in case it was wrapped with a // user-friendly message. The underlying root error is likely too // low-level to be useful. Message: respErr.Error(), - Code: int32(trace.ErrorToCode(respErr)), - Reason: errorToKubeStatusReason(respErr), + Code: int32(code), + Reason: errorToKubeStatusReason(respErr, code), } data, err := runtime.Encode(kubeCodecs.LegacyCodec(), status) if err != nil { @@ -3008,7 +3009,7 @@ func getRequestVerb(method string) string { // errorToKubeStatusReason returns an appropriate StatusReason based on the // provided error type. -func errorToKubeStatusReason(err error) metav1.StatusReason { +func errorToKubeStatusReason(err error, code int) metav1.StatusReason { switch { case trace.IsAggregate(err): return metav1.StatusReasonTimeout @@ -3028,6 +3029,8 @@ func errorToKubeStatusReason(err error) metav1.StatusReason { return metav1.StatusReasonTooManyRequests case trace.IsConnectionProblem(err): return metav1.StatusReasonTimeout + case code == http.StatusInternalServerError: + return metav1.StatusReasonInternalError default: return metav1.StatusReasonUnknown } diff --git a/lib/kube/proxy/forwarder_test.go b/lib/kube/proxy/forwarder_test.go index c6e6b52596da9..81094f16901a0 100644 --- a/lib/kube/proxy/forwarder_test.go +++ b/lib/kube/proxy/forwarder_test.go @@ -43,6 +43,8 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel" + kubeerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/transport" "github.com/gravitational/teleport" @@ -54,6 +56,7 @@ import ( "github.com/gravitational/teleport/lib/backend/memory" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/fixtures" + testingkubemock "github.com/gravitational/teleport/lib/kube/proxy/testing/kube_server" "github.com/gravitational/teleport/lib/reversetunnel" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/services/local" @@ -1830,3 +1833,78 @@ func Test_copyImpersonationHeaders(t *testing.T) { }) } } + +func TestKubernetesLicenseEnforcement(t *testing.T) { + t.Parallel() + // kubeMock is a Kubernetes API mock for the session tests. + kubeMock, err := testingkubemock.NewKubeAPIMock() + require.NoError(t, err) + t.Cleanup(func() { kubeMock.Close() }) + + tests := []struct { + name string + features proto.Features + assertErrFunc require.ErrorAssertionFunc + }{ + { + name: "kubernetes agent is licensed", + features: proto.Features{ + Kubernetes: true, + }, + assertErrFunc: require.NoError, + }, + { + name: "kubernetes isn't licensed", + features: proto.Features{ + Kubernetes: false, + }, + assertErrFunc: func(tt require.TestingT, err error, i ...interface{}) { + require.Error(tt, err) + var kubeErr *kubeerrors.StatusError + require.ErrorAs(tt, err, &kubeErr) + require.Equal(tt, kubeErr.ErrStatus.Code, int32(500)) + require.Equal(tt, kubeErr.ErrStatus.Reason, metav1.StatusReasonInternalError) + require.Equal(tt, kubeErr.ErrStatus.Message, "Teleport cluster is not licensed for Kubernetes") + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + // creates a Kubernetes service with a configured cluster pointing to mock api server + testCtx := SetupTestContext( + context.Background(), + t, + TestConfig{ + Clusters: []KubeClusterConfig{{Name: kubeCluster, APIEndpoint: kubeMock.URL}}, + ClusterFeatures: func() proto.Features { + return tt.features + }, + }, + ) + // close tests + t.Cleanup(func() { require.NoError(t, testCtx.Close()) }) + + _, _ = testCtx.CreateUserAndRole( + testCtx.Context, + t, + username, + RoleSpec{ + Name: roleName, + KubeUsers: roleKubeUsers, + KubeGroups: roleKubeGroups, + }) + + // generate a kube client with user certs for auth + client, _ := testCtx.GenTestKubeClientTLSCert( + t, + username, + kubeCluster, + ) + + _, err = client.CoreV1().Pods(metav1.NamespaceDefault).List(context.Background(), metav1.ListOptions{}) + tt.assertErrFunc(t, err) + }) + } +} diff --git a/lib/kube/proxy/utils_testing.go b/lib/kube/proxy/utils_testing.go index e93b960664426..3a6a59170a12e 100644 --- a/lib/kube/proxy/utils_testing.go +++ b/lib/kube/proxy/utils_testing.go @@ -83,6 +83,7 @@ type TestConfig struct { ResourceMatchers []services.ResourceMatcher OnReconcile func(types.KubeClusters) OnEvent func(apievents.AuditEvent) + ClusterFeatures func() proto.Features } // SetupTestContext creates a kube service with clusters configured. @@ -174,6 +175,11 @@ func SetupTestContext(ctx context.Context, t *testing.T, cfg TestConfig) *TestCo // heartbeatsWaitChannel waits for clusters heartbeats to start. heartbeatsWaitChannel := make(chan struct{}, len(cfg.Clusters)+1) client := newAuthClientWithStreamer(testCtx) + + features := func() proto.Features { return proto.Features{Kubernetes: true} } + if cfg.ClusterFeatures != nil { + features = cfg.ClusterFeatures + } // Create kubernetes service server. testCtx.KubeServer, err = NewTLSServer(TLSServerConfig{ ForwarderConfig: ForwarderConfig{ @@ -204,7 +210,7 @@ func SetupTestContext(ctx context.Context, t *testing.T, cfg TestConfig) *TestCo return nil }, Clock: clockwork.NewRealClock(), - ClusterFeatures: func() proto.Features { return proto.Features{Kubernetes: true} }, + ClusterFeatures: features, }, DynamicLabels: nil, TLS: tlsConfig, From 60e9ff38fec3bfdbc614ea432aba5624eed20466 Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Mon, 17 Apr 2023 10:06:33 +0100 Subject: [PATCH 4/4] move err code to forbidden --- lib/kube/proxy/forwarder.go | 5 ++--- lib/kube/proxy/forwarder_test.go | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index 591ed379a71c0..9ab0612a55f39 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -511,9 +511,8 @@ const accessDeniedMsg = "[00] access denied" func (f *Forwarder) authenticate(req *http.Request) (*authContext, error) { // If the cluster is not licensed for Kubernetes, return an error to the client. if !f.cfg.ClusterFeatures().Kubernetes { - // If the cluster is not licensed for Kubernetes, return an error to the client - // but in a way that the code is translated to 500 error code. - return nil, trace.Errorf("Teleport cluster is not licensed for Kubernetes") + // If the cluster is not licensed for Kubernetes, return an error to the client. + return nil, trace.AccessDenied("Teleport cluster is not licensed for Kubernetes") } ctx, span := f.cfg.tracer.Start( req.Context(), diff --git a/lib/kube/proxy/forwarder_test.go b/lib/kube/proxy/forwarder_test.go index 81094f16901a0..4275081c6ab6e 100644 --- a/lib/kube/proxy/forwarder_test.go +++ b/lib/kube/proxy/forwarder_test.go @@ -1862,8 +1862,8 @@ func TestKubernetesLicenseEnforcement(t *testing.T) { require.Error(tt, err) var kubeErr *kubeerrors.StatusError require.ErrorAs(tt, err, &kubeErr) - require.Equal(tt, kubeErr.ErrStatus.Code, int32(500)) - require.Equal(tt, kubeErr.ErrStatus.Reason, metav1.StatusReasonInternalError) + require.Equal(tt, kubeErr.ErrStatus.Code, int32(http.StatusForbidden)) + require.Equal(tt, kubeErr.ErrStatus.Reason, metav1.StatusReasonForbidden) require.Equal(tt, kubeErr.ErrStatus.Message, "Teleport cluster is not licensed for Kubernetes") }, },