Skip to content

Commit

Permalink
Experiment with CRD support in k8s resource mapping
Browse files Browse the repository at this point in the history
  • Loading branch information
creack committed Feb 9, 2025
1 parent 0c4a8b5 commit bc9d365
Show file tree
Hide file tree
Showing 16 changed files with 826 additions and 287 deletions.
38 changes: 19 additions & 19 deletions api/types/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,81 +95,81 @@ func TestAppPortsValidation(t *testing.T) {
{
name: "valid ranges and single ports",
tcpPorts: []*PortRange{
&PortRange{Port: 22, EndPort: 25},
&PortRange{Port: 26},
&PortRange{Port: 65535},
{Port: 22, EndPort: 25},
{Port: 26},
{Port: 65535},
},
check: hasNoErr,
},
{
name: "valid overlapping ranges",
tcpPorts: []*PortRange{
&PortRange{Port: 100, EndPort: 200},
&PortRange{Port: 150, EndPort: 175},
&PortRange{Port: 111},
&PortRange{Port: 150, EndPort: 210},
&PortRange{Port: 1, EndPort: 65535},
{Port: 100, EndPort: 200},
{Port: 150, EndPort: 175},
{Port: 111},
{Port: 150, EndPort: 210},
{Port: 1, EndPort: 65535},
},
check: hasNoErr,
},
{
name: "valid non-TCP app with ports ignored",
uri: "http://localhost:8000",
tcpPorts: []*PortRange{
&PortRange{Port: 123456789},
&PortRange{Port: 10, EndPort: 2},
{Port: 123456789},
{Port: 10, EndPort: 2},
},
check: hasNoErr,
},
// Test cases for invalid ports.
{
name: "port smaller than 1",
tcpPorts: []*PortRange{
&PortRange{Port: 0},
{Port: 0},
},
check: hasErrTypeBadParameter,
},
{
name: "port bigger than 65535",
tcpPorts: []*PortRange{
&PortRange{Port: 78787},
{Port: 78787},
},
check: hasErrTypeBadParameter,
},
{
name: "end port smaller than 2",
tcpPorts: []*PortRange{
&PortRange{Port: 5, EndPort: 1},
{Port: 5, EndPort: 1},
},
check: hasErrTypeBadParameterAndContains("end port must be between 6 and 65535"),
},
{
name: "end port bigger than 65535",
tcpPorts: []*PortRange{
&PortRange{Port: 1, EndPort: 78787},
{Port: 1, EndPort: 78787},
},
check: hasErrTypeBadParameter,
},
{
name: "end port smaller than port",
tcpPorts: []*PortRange{
&PortRange{Port: 10, EndPort: 5},
{Port: 10, EndPort: 5},
},
check: hasErrTypeBadParameterAndContains("end port must be between 11 and 65535"),
},
{
name: "uri specifies port",
uri: "tcp://localhost:1234",
tcpPorts: []*PortRange{
&PortRange{Port: 1000, EndPort: 1500},
{Port: 1000, EndPort: 1500},
},
check: hasErrTypeBadParameterAndContains("must not include a port number"),
},
{
name: "invalid uri",
uri: "%",
tcpPorts: []*PortRange{
&PortRange{Port: 1000, EndPort: 1500},
{Port: 1000, EndPort: 1500},
},
check: hasErrAndContains("invalid URL escape"),
},
Expand Down Expand Up @@ -566,8 +566,8 @@ func TestNewAppV3(t *testing.T) {

func TestPortRangesContains(t *testing.T) {
portRanges := PortRanges([]*PortRange{
&PortRange{Port: 10, EndPort: 20},
&PortRange{Port: 42},
{Port: 10, EndPort: 20},
{Port: 42},
})

tests := []struct {
Expand Down
7 changes: 4 additions & 3 deletions api/types/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -1866,9 +1866,10 @@ func setDefaultKubernetesVerbs(spec *RoleSpecV6) {
// - Namespace is not empty
func validateKubeResources(roleVersion string, kubeResources []KubernetesResource) error {
for _, kubeResource := range kubeResources {
if !slices.Contains(KubernetesResourcesKinds, kubeResource.Kind) && kubeResource.Kind != Wildcard {
return trace.BadParameter("KubernetesResource kind %q is invalid or unsupported; Supported: %v", kubeResource.Kind, append([]string{Wildcard}, KubernetesResourcesKinds...))
}
// TODO(@creack): Move the validation to the server side so we can lookup the list of valid CRDs.
// if !slices.Contains(KubernetesResourcesKinds, kubeResource.Kind) && kubeResource.Kind != Wildcard {
// return trace.BadParameter("KubernetesResource kind %q is invalid or unsupported; Supported: %v", kubeResource.Kind, append([]string{Wildcard}, KubernetesResourcesKinds...))
// }

for _, verb := range kubeResource.Verbs {
if !slices.Contains(KubernetesVerbs, verb) && verb != Wildcard && !strings.Contains(verb, "{{") {
Expand Down
13 changes: 11 additions & 2 deletions api/types/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,16 @@ func TestRole_GetKubeResources(t *testing.T) {
},
},
},
assertErrorCreation: require.Error,
want: []KubernetesResource{
{
Kind: "invalid resource",
Namespace: "test",
Name: "test",
},
},
// TODO(@creack): Find a way to validate the resource kind. For now as we support
// arbitrary CRDs, we can't validate it.
assertErrorCreation: require.NoError,
},
{
name: "v7",
Expand Down Expand Up @@ -692,7 +701,7 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
require.ErrorContains(t, err, contains)
}
}
newRole := func(t *testing.T, spec RoleSpecV6) *RoleV6 {
newRole := func(_ *testing.T, spec RoleSpecV6) *RoleV6 {
return &RoleV6{
Metadata: Metadata{
Name: "test",
Expand Down
3 changes: 1 addition & 2 deletions lib/kube/proxy/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func failsForCluster(clusterName string) servicecfg.ImpersonationPermissionsChec
func TestGetKubeCreds(t *testing.T) {
t.Parallel()
// kubeMock is a Kubernetes API mock for the session tests.
kubeMock, err := testingkubemock.NewKubeAPIMock()
kubeMock, err := testingkubemock.NewKubeAPIMock(testingkubemock.WithTeleportRoleCRD)
require.NoError(t, err)
t.Cleanup(func() { kubeMock.Close() })
targetAddr := kubeMock.Address
Expand Down Expand Up @@ -338,7 +338,6 @@ current-context: foo
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.desc, func(t *testing.T) {
t.Parallel()
fwd := &Forwarder{
Expand Down
1 change: 1 addition & 0 deletions lib/kube/proxy/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ func TestExecWebsocketEndToEndErrReturn(t *testing.T) {
Minor: "31",
GitVersion: "v1.31.0",
}),
testingkubemock.WithTeleportRoleCRD,
)
require.NoError(t, err)
t.Cleanup(func() {
Expand Down
4 changes: 4 additions & 0 deletions lib/kube/proxy/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"net"
"net/http"
"net/url"
"path"
"slices"
"strconv"
"strings"
Expand Down Expand Up @@ -810,6 +811,9 @@ func (f *Forwarder) setupContext(
return nil, trace.Wrap(err)
}
}
if kubeResource != nil && kubeResource.Kind == utils.KubeCustomResource {
kubeResource.Kind = path.Join(apiResource.apiGroup, apiResource.apiGroupVersion, apiResource.resourceKind)
}

netConfig, err := f.cfg.CachingAuthClient.GetClusterNetworkingConfig(f.ctx)
if err != nil {
Expand Down
8 changes: 5 additions & 3 deletions lib/kube/proxy/resource_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"log/slog"
"mime"
"net/http"
"path"

"github.com/gravitational/trace"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -617,7 +618,7 @@ func (d *resourceFilterer) encode(obj runtime.Object, w io.Writer) error {
}

// filterResourceList excludes resources the user should not have access to.
func filterResourceList[T kubeObjectInterface](kind, verb string, originalList []T, allowed, denied []types.KubernetesResource, log *slog.Logger) []T {
func filterResourceList[T kubeObjectInterface](kind, verb string, originalList []T, allowed, denied []types.KubernetesResource, _ *slog.Logger) []T {
filteredList := make([]T, 0, len(originalList))
for _, resource := range originalList {
if result, err := filterResource(kind, verb, resource, allowed, denied); err == nil && result {
Expand Down Expand Up @@ -784,7 +785,7 @@ func filterBuffer(filterWrapper responsewriters.FilterWrapper, src *responsewrit
// filterUnstructuredList filters the unstructured list object to exclude resources
// that the user must not have access to.
// The filtered list is re-assigned to `obj.Object["items"]`.
func filterUnstructuredList(verb string, obj *unstructured.Unstructured, allowed, denied []types.KubernetesResource, log *slog.Logger) (hasElems bool) {
func filterUnstructuredList(verb string, obj *unstructured.Unstructured, allowed, denied []types.KubernetesResource, _ *slog.Logger) (hasElems bool) {
const (
itemsKey = "items"
)
Expand All @@ -800,7 +801,8 @@ func filterUnstructuredList(verb string, obj *unstructured.Unstructured, allowed

filteredList := make([]any, 0, len(objList.Items))
for _, resource := range objList.Items {
r := getKubeResource(utils.KubeCustomResource, verb, &resource)
kind := path.Join(resource.GetAPIVersion(), resource.GetKind())
r := getKubeResource(kind, verb, &resource)
if result, err := matchKubernetesResource(
r,
allowed, denied,
Expand Down
50 changes: 50 additions & 0 deletions lib/kube/proxy/resource_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"bytes"
"io"
"net/http"
"path"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -58,6 +59,9 @@ func (f *Forwarder) listResources(sess *clusterSession, w http.ResponseWriter, r
resourceKind := ""
if isLocalKubeCluster {
resourceKind, supportsType = sess.rbacSupportedResources.getTeleportResourceKindFromAPIResource(sess.apiResource)
if resourceKind == utils.KubeCustomResource {
resourceKind = path.Join(sess.apiResource.apiGroup, sess.apiResource.apiGroupVersion, sess.apiResource.resourceKind)
}
}

// status holds the returned response code.
Expand All @@ -71,6 +75,49 @@ func (f *Forwarder) listResources(sess *clusterSession, w http.ResponseWriter, r
status = rw.Status()
} else {
allowedResources, deniedResources := sess.Checker.GetKubeResources(sess.kubeCluster)

details, err := f.findKubeDetailsByClusterName(sess.kubeClusterName)
if err != nil {
return nil, trace.Wrap(err, "failed to find kube details by cluster name %q", sess.kubeClusterName)
}
makeKind := func(resources []types.KubernetesResource) []types.KubernetesResource {
var out []types.KubernetesResource // Not pre-allocating as we don't know the final size. Likely 0 if no CRD resource defined.
loop:
for _, r := range resources {
tmp := strings.Split(r.Kind, "/")
if len(tmp) < 3 { // Expect <group>/<version>/<kind>. If not, skip.
continue
}
group, version, kind := tmp[0], tmp[1], strings.Join(tmp[2:], "/")

// Look for the resource in the supported list from it's fqdn.
if r2 := details.gvkSupportedResources[gvkSupportedResourcesKey{
name: kind, apiGroup: group, version: version,
}]; r2 != nil {
// If found, append the resource with it's Kind.
r3 := r
r3.Kind = path.Join(group, version, r2.Kind)
out = append(out, r3)
continue loop
}
// If not found, look up the resource from it's Kind.
for k, v := range details.gvkSupportedResources {
if k.apiGroup == group && k.version == version && v.Kind == kind && !strings.Contains(k.name, "/") {
// If found, append the resource with it's plural.
r3 := r
r3.Kind = path.Join(group, version, k.name)
out = append(out, r3)
continue loop
}
}
// If we reach this point, there is no match for the resource. Probably a typo in the resource Kind.
// Not erroring out as a typo would break access for everyone.
}
return out
}
allowedResources = append(allowedResources, makeKind(allowedResources)...)
deniedResources = append(deniedResources, makeKind(deniedResources)...)

shouldBeAllowed, err := matchListRequestShouldBeAllowed(sess, resourceKind, allowedResources, deniedResources)
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -119,6 +166,9 @@ func (f *Forwarder) listResourcesList(req *http.Request, w http.ResponseWriter,
if !ok {
return http.StatusBadRequest, trace.BadParameter("unknown resource kind %q", sess.apiResource.resourceKind)
}
if resourceKind == utils.KubeCustomResource {
resourceKind = path.Join(sess.apiResource.apiGroup, sess.apiResource.apiGroupVersion, sess.apiResource.resourceKind)
}
verb := sess.requestVerb
// filterBuffer filters the response to exclude resources the user doesn't have access to.
// The filtered payload will be written into memBuffer again.
Expand Down
Loading

0 comments on commit bc9d365

Please sign in to comment.