Skip to content

Kubernetes License check for Kubernetes Proxies#24543

Merged
tigrato merged 4 commits intomasterfrom
tigrato/ensure-license-kube-proxies
Apr 18, 2023
Merged

Kubernetes License check for Kubernetes Proxies#24543
tigrato merged 4 commits intomasterfrom
tigrato/ensure-license-kube-proxies

Conversation

@tigrato
Copy link
Copy Markdown
Contributor

@tigrato tigrato commented Apr 13, 2023

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

Copy link
Copy Markdown
Contributor

@espadolini espadolini left a comment

Choose a reason for hiding this comment

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

You can just use (*TeleportProcess).getClusterFeatures instead of running your own periodic check; it gets updated whenever any service in the process (re)connects to the auth. Up to you if you think you should check it every time (by passing some closure) or only whenever we initialize the forwarder for the first time.

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
@tigrato tigrato force-pushed the tigrato/ensure-license-kube-proxies branch from b0f1cd7 to a69a09d Compare April 15, 2023 17:31
@tigrato tigrato requested a review from espadolini April 15, 2023 17:32
@tigrato tigrato force-pushed the tigrato/ensure-license-kube-proxies branch from a69a09d to f913660 Compare April 15, 2023 17:34
@tigrato tigrato force-pushed the tigrato/ensure-license-kube-proxies branch from f913660 to 90321e7 Compare April 15, 2023 21:09
Comment thread lib/kube/proxy/forwarder.go Outdated
// 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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this isn't too much hassle, it might be worth testing that the error is truly translated to a 500 error code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in 9a24956

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be a 500 or a 403 (Forbidden)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We seem to return trace.AccessDenied (which becomes a 403 in http) whenever some license feature check fails, which seems appropriate here. Is there a kube-specific reason why it should not be returned as an access denied?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I preferred 500 to distinguish from cases where the user does not have access but reverted to Forbidden

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from probakowski April 17, 2023 06:51
@tigrato tigrato added this pull request to the merge queue Apr 18, 2023
Merged via the queue into master with commit 3ad31fb Apr 18, 2023
@tigrato tigrato deleted the tigrato/ensure-license-kube-proxies branch April 18, 2023 09:53
@public-teleport-github-review-bot
Copy link
Copy Markdown

@tigrato See the table below for backport results.

Branch Result
branch/v13 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants