-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support routing to Kubernetes clusters by request path #50567
Open
timothyb89
wants to merge
10
commits into
master
Choose a base branch
from
timothyb89/kubernetes-path-routing
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+385
−7
Open
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
f7883aa
Support routing to Kubernetes clusters by request path
timothyb89 f291749
Remove routeSourcer in favor of identity values
timothyb89 52e3570
Revert more dead code
timothyb89 d299b18
Revert more unnecessary changes; validate utf8 parameters
timothyb89 dccfa8f
Merge remote-tracking branch 'origin/master' into timothyb89/kubernet…
timothyb89 3bb59e0
Build fixes after merge with slog transition
timothyb89 74fd48d
Add basic tests for Kubernetes path routing
timothyb89 48d7b71
Fix imports
timothyb89 5f5760a
Check identity parameters when per session MFA is enabled
timothyb89 2d7b958
Fix failing TestSingleCertRouting due to improper reuse of rest config
timothyb89 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
/* | ||
* Teleport | ||
* Copyright (C) 2025 Gravitational, Inc. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
package proxy | ||
|
||
import ( | ||
"encoding/base64" | ||
"net/http" | ||
"strings" | ||
"unicode/utf8" | ||
|
||
"github.com/gravitational/trace" | ||
"github.com/julienschmidt/httprouter" | ||
|
||
"github.com/gravitational/teleport/lib/authz" | ||
"github.com/gravitational/teleport/lib/httplib" | ||
logutils "github.com/gravitational/teleport/lib/utils/log" | ||
) | ||
|
||
const ( | ||
// paramTeleportCluster is the path parameter key containing a base64 | ||
// encoded Teleport cluster name for path-routed forwarding. | ||
paramTeleportCluster = "encodedTeleportCluster" | ||
|
||
// paramKubernetesCluster is the path parameter key containing a base64 | ||
// encoded Teleport cluster name for path-routed forwarding. | ||
paramKubernetesCluster = "encodedKubernetesCluster" | ||
) | ||
|
||
// parseRouteFromPath extracts route information from the given path parameters | ||
// using constant-defined parameter keys. | ||
func parseRouteFromPath(p httprouter.Params) (string, string, error) { | ||
encodedTeleportCluster := p.ByName(paramTeleportCluster) | ||
if encodedTeleportCluster == "" { | ||
return "", "", trace.BadParameter("no Teleport cluster name found in path") | ||
} | ||
|
||
decodedTeleportCluster, err := base64.RawURLEncoding.DecodeString(encodedTeleportCluster) | ||
if err != nil { | ||
return "", "", trace.Wrap(err) | ||
} | ||
|
||
encodedKubernetesCluster := p.ByName(paramKubernetesCluster) | ||
if encodedKubernetesCluster == "" { | ||
return "", "", trace.BadParameter("no Kubernetes cluster name found in path") | ||
} | ||
|
||
decodedKubernetesCluster, err := base64.RawURLEncoding.DecodeString(encodedKubernetesCluster) | ||
if err != nil { | ||
return "", "", trace.Wrap(err) | ||
} | ||
|
||
if !utf8.Valid(decodedTeleportCluster) { | ||
return "", "", trace.BadParameter("invalid Teleport cluster name") | ||
} | ||
|
||
if !utf8.Valid(decodedKubernetesCluster) { | ||
return "", "", trace.BadParameter("invalid Kubernetes cluster name") | ||
} | ||
|
||
return string(decodedTeleportCluster), string(decodedKubernetesCluster), nil | ||
} | ||
|
||
// singleCertHandler extracts routing information from base64-encoded URL | ||
// parameters into the current auth user context and forwards the request back | ||
// to the main router with the path prefix (and its embedded routing parameters) | ||
// stripped. | ||
func (f *Forwarder) singleCertHandler() httprouter.Handle { | ||
return httplib.MakeHandlerWithErrorWriter(func(w http.ResponseWriter, req *http.Request, p httprouter.Params) (any, error) { | ||
teleportCluster, kubeCluster, err := parseRouteFromPath(p) | ||
if err != nil { | ||
return nil, trace.Wrap(err) | ||
} | ||
|
||
userTypeI, err := authz.UserFromContext(req.Context()) | ||
if err != nil { | ||
f.log.WarnContext(req.Context(), "error getting user from context", "error", err) | ||
return nil, trace.AccessDenied(accessDeniedMsg) | ||
} | ||
|
||
// Insert the extracted routing information from the path into the | ||
// identity. Some implementation notes: | ||
// - This still relies on RouteToCluster and KubernetesCluster identity | ||
// fields, even though these fields are not part of the TLS identity | ||
// when using path-based routing. | ||
// - If the Teleport+Kube cluster names resolve to the local node, these | ||
// values will be used directly in their proper handlers once the | ||
// request is rewritten. | ||
// - If the route resolves to a remote node, the identity is encoded (in | ||
// JSON form) into forwarding headers using | ||
// `auth.IdentityForwardingHeaders`. The destination node's auth | ||
// middleware is configured to extract this identity (due to | ||
// EnableCredentialsForwarding) and implicitly trusts this routing | ||
// data, assuming the request originated from a proxy. | ||
// - In either case, the destination node is ultimately responsible for | ||
// authorizing the request, and routing information set in the | ||
// identity should not be implicitly trusted. (This was ideally never | ||
// the case, given access to resources could be revoked via roles | ||
// before certs expired.) | ||
|
||
var userType authz.IdentityGetter | ||
switch o := userTypeI.(type) { | ||
case authz.LocalUser: | ||
o.Identity.RouteToCluster = teleportCluster | ||
o.Identity.KubernetesCluster = kubeCluster | ||
userType = o | ||
case authz.RemoteUser: | ||
o.Identity.RouteToCluster = teleportCluster | ||
o.Identity.KubernetesCluster = kubeCluster | ||
userType = o | ||
default: | ||
f.log.WarnContext(req.Context(), "Denying proxy access to unsupported user type", "user_type", logutils.TypeAttr(userTypeI)) | ||
return nil, trace.AccessDenied(accessDeniedMsg) | ||
} | ||
|
||
ctx := authz.ContextWithUser(req.Context(), userType) | ||
req = req.WithContext(ctx) | ||
|
||
path := p.ByName("path") | ||
if !strings.HasPrefix(path, "/") { | ||
path = "/" + path | ||
} | ||
|
||
req.URL.Path = path | ||
req.RequestURI = req.URL.RequestURI() | ||
|
||
f.router.ServeHTTP(w, req) | ||
return nil, nil | ||
}, f.formatStatusResponseError) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
/* | ||
* Teleport | ||
* Copyright (C) 2025 Gravitational, Inc. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
package proxy | ||
|
||
import ( | ||
"context" | ||
"encoding/base64" | ||
"fmt" | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/gravitational/teleport/api/types" | ||
testingkubemock "github.com/gravitational/teleport/lib/kube/proxy/testing/kube_server" | ||
"github.com/stretchr/testify/require" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/client-go/kubernetes" | ||
"k8s.io/client-go/rest" | ||
) | ||
|
||
// pathRoutedKubeClient uses the given rest.Config to build a Kubernetes client | ||
// using path-based routing derived from the provided Teleport and Kubernetes | ||
// cluster names. | ||
func pathRoutedKubeClient(t *testing.T, restConfig *rest.Config, teleportCluster, kubeCluster string) *kubernetes.Clientset { | ||
t.Helper() | ||
|
||
encTeleportCluster := base64.RawURLEncoding.EncodeToString([]byte(teleportCluster)) | ||
encKubeCluster := base64.RawURLEncoding.EncodeToString([]byte(kubeCluster)) | ||
restConfig.Host += fmt.Sprintf("/v1/teleport/%s/%s", encTeleportCluster, encKubeCluster) | ||
|
||
client, err := kubernetes.NewForConfig(restConfig) | ||
require.NoError(t, err) | ||
|
||
return client | ||
} | ||
|
||
func TestSingleCertRouting(t *testing.T) { | ||
kubeMockA, err := testingkubemock.NewKubeAPIMock() | ||
require.NoError(t, err) | ||
t.Cleanup(func() { kubeMockA.Close() }) | ||
|
||
kubeMockB, err := testingkubemock.NewKubeAPIMock( | ||
// This endpoint returns a known mock error so we can determine from the | ||
// response which cluster the request was routed to. | ||
testingkubemock.WithGetPodError( | ||
metav1.Status{ | ||
Status: metav1.StatusFailure, | ||
Message: "cluster b error", | ||
Reason: metav1.StatusReasonInternalError, | ||
Code: http.StatusInternalServerError, | ||
}, | ||
), | ||
) | ||
require.NoError(t, err) | ||
t.Cleanup(func() { kubeMockB.Close() }) | ||
|
||
defaultRoleSpec := RoleSpec{ | ||
Name: roleName, | ||
KubeUsers: roleKubeUsers, | ||
KubeGroups: roleKubeGroups, | ||
} | ||
|
||
const clusterName = "root.example.com" | ||
|
||
tests := []struct { | ||
name string | ||
|
||
kubeClusterOverride string | ||
roleSpec RoleSpec | ||
assert func(t *testing.T, restConfig *rest.Config) | ||
}{ | ||
{ | ||
name: "successful path routing to multiple clusters", | ||
roleSpec: defaultRoleSpec, | ||
assert: func(t *testing.T, restConfig *rest.Config) { | ||
clientA := pathRoutedKubeClient(t, restConfig, clusterName, "a") | ||
_, err := clientA.CoreV1().Pods(metav1.NamespaceDefault).List(context.Background(), metav1.ListOptions{}) | ||
require.NoError(t, err) | ||
|
||
clientB := pathRoutedKubeClient(t, restConfig, clusterName, "b") | ||
_, err = clientB.CoreV1().Pods(metav1.NamespaceDefault).List(context.Background(), metav1.ListOptions{}) | ||
require.NoError(t, err) | ||
}, | ||
}, | ||
{ | ||
name: "cannot access nonexistent cluster", | ||
roleSpec: defaultRoleSpec, | ||
assert: func(t *testing.T, restConfig *rest.Config) { | ||
client := pathRoutedKubeClient(t, restConfig, clusterName, "c") | ||
_, err = client.CoreV1().Pods(metav1.NamespaceDefault).List(context.Background(), metav1.ListOptions{}) | ||
require.ErrorContains(t, err, "not found") | ||
}, | ||
}, | ||
{ | ||
name: "cannot access cluster denied by roles", | ||
roleSpec: RoleSpec{ | ||
Name: roleName, | ||
KubeUsers: roleKubeUsers, | ||
KubeGroups: roleKubeGroups, | ||
SetupRoleFunc: func(r types.Role) { | ||
r.SetKubeResources(types.Deny, []types.KubernetesResource{{Kind: types.KindKubePod, Name: types.Wildcard, Namespace: types.Wildcard, Verbs: []string{types.Wildcard}}}) | ||
}, | ||
}, | ||
assert: func(t *testing.T, restConfig *rest.Config) { | ||
client := pathRoutedKubeClient(t, restConfig, clusterName, "a") | ||
_, err = client.CoreV1().Pods(metav1.NamespaceDefault).List(context.Background(), metav1.ListOptions{}) | ||
require.ErrorContains(t, err, "cannot list resource") | ||
}, | ||
}, | ||
{ | ||
name: "path route overrides identity", | ||
roleSpec: defaultRoleSpec, | ||
kubeClusterOverride: "a", | ||
assert: func(t *testing.T, restConfig *rest.Config) { | ||
client := pathRoutedKubeClient(t, restConfig, clusterName, "b") | ||
_, err = client.CoreV1().Pods(metav1.NamespaceDefault).Get(context.Background(), "foo", metav1.GetOptions{}) | ||
require.ErrorContains(t, err, "cluster b error") | ||
}, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
testCtx := SetupTestContext( | ||
context.Background(), | ||
t, | ||
TestConfig{ | ||
Clusters: []KubeClusterConfig{ | ||
{Name: "a", APIEndpoint: kubeMockA.URL}, | ||
{Name: "b", APIEndpoint: kubeMockB.URL}, | ||
}, | ||
}, | ||
) | ||
t.Cleanup(func() { require.NoError(t, testCtx.Close()) }) | ||
|
||
_, _ = testCtx.CreateUserAndRole( | ||
testCtx.Context, | ||
t, | ||
username, | ||
tt.roleSpec) | ||
|
||
_, restConfig := testCtx.GenTestKubeClientTLSCert( | ||
t, | ||
username, | ||
tt.kubeClusterOverride, // normally empty for path routing | ||
) | ||
|
||
tt.assert(t, restConfig) | ||
}) | ||
} | ||
|
||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed a mild behavior change here, I think since this just wasn't possible before. Attempting to e.g. list resources on a cluster denied by roles now yields (
kubectl get pods -v8
):...however, attempting to list a nonexistent cluster returns:
(and a
Kubernetes cluster "$name" not found"
is logged on the server)Previously, you could only ever get certs for a cluster if the name was valid and the user is authorized to see it, and returns a consistent error otherwise before the cert is ever issued. Would we consider this a resource name oracle?
It seems like the two responses probably ought to be identical but changing them might be a mild breaking change to existing behavior (403 vs 404).