Skip to content

Commit

Permalink
Address crs
Browse files Browse the repository at this point in the history
  • Loading branch information
kimlisa committed Sep 20, 2024
1 parent ca6a136 commit cde17d4
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 37 deletions.
25 changes: 20 additions & 5 deletions lib/teleterm/api/uri/uri.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ var pathDbs = urlpath.New("/clusters/:cluster/dbs/:dbName")
var pathLeafDbs = urlpath.New("/clusters/:cluster/leaves/:leaf/dbs/:dbName")
var pathKubes = urlpath.New("/clusters/:cluster/kubes/:kubeName")
var pathLeafKubes = urlpath.New("/clusters/:cluster/leaves/:leaf/kubes/:kubeName")
var pathKubeResourceNamespace = urlpath.New("/clusters/:cluster/kube-resources/namespace/:kubeName/:namespaceName")
var pathLeafKubeResourceNamespace = urlpath.New("/clusters/:cluster/leaves/:leaf/kube-resources/namespace/:kubeName/:namespaceName")
var pathApps = urlpath.New("/clusters/:cluster/apps/:appName")
var pathLeafApps = urlpath.New("/clusters/:cluster/leaves/:leaf/apps/:appName")

Expand Down Expand Up @@ -122,6 +124,21 @@ func (r ResourceURI) GetKubeName() string {
return ""
}

// GetKubeResourceNamespace extracts the kube resource namespacefrom r. Returns an empty string if path is not a kube resource URI.
func (r ResourceURI) GetKubeResourceNamespace() string {
result, ok := pathKubeResourceNamespace.Match(r.path)
if ok {
return result.Params["namespaceName"]
}

result, ok = pathLeafKubeResourceNamespace.Match(r.path)
if ok {
return result.Params["namespaceName"]
}

return ""
}

// GetAppName extracts the app name from r. Returns an empty string if the path is not an app URI.
func (r ResourceURI) GetAppName() string {
result, ok := pathApps.Match(r.path)
Expand Down Expand Up @@ -189,11 +206,9 @@ func (r ResourceURI) AppendKube(name string) ResourceURI {
return r
}

// AppendKubeResource appends kube resource segment to the URI.
// Modeled after how access request constructs the resource ID:
// <teleport-cluster-name>/<subresource kind>/<kube_cluster name>/<subresource name>
func (r ResourceURI) AppendKubeResource(kubeResourceKind string, kubeClusterName string, kubeResourceName string) ResourceURI {
r.path = fmt.Sprintf("%v/%v/%v/%v", r.path, kubeResourceKind, kubeClusterName, kubeResourceName)
// AppendKubeResourceNamespace appends kube resource namespace segment to the URI.
func (r ResourceURI) AppendKubeResourceNamespace(kubeClusterName string, namespaceName string) ResourceURI {
r.path = fmt.Sprintf("%v/kube-resources/namespace/%v/%v", r.path, kubeClusterName, namespaceName)
return r
}

Expand Down
51 changes: 48 additions & 3 deletions lib/teleterm/api/uri/uri_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/teleterm/api/uri"
)

Expand All @@ -46,8 +45,8 @@ func TestString(t *testing.T) {
"/clusters/teleport.sh/dbs/dbhost1",
},
{
uri.NewClusterURI("teleport.sh").AppendKubeResource(types.KindKubeNamespace, "kube-cluster-name", "namespace-name"),
"/clusters/teleport.sh/namespace/kube-cluster-name/namespace-name",
uri.NewClusterURI("teleport.sh").AppendKubeResourceNamespace("kube-cluster-name", "namespace-name"),
"/clusters/teleport.sh/kube-resources/namespace/kube-cluster-name/namespace-name",
},
}

Expand Down Expand Up @@ -181,6 +180,52 @@ func TestGetKubeName(t *testing.T) {
}
}

func TestGetKubeResourceNamespace(t *testing.T) {
tests := []struct {
name string
in uri.ResourceURI
out string
}{
{
name: "returns root cluster namespace name",
in: uri.NewClusterURI("foo").AppendKubeResourceNamespace("k8s", "default"),
out: "default",
},
{
name: "returns leaf cluster namespace name",
in: uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendKubeResourceNamespace("default", "default"),
out: "default",
},
{
name: "returns empty string when given root cluster URI",
in: uri.NewClusterURI("foo"),
out: "",
},
{
name: "returns empty string when given leaf cluster URI",
in: uri.NewClusterURI("foo").AppendLeafCluster("bar"),
out: "",
},
{
name: "returns empty string when given root cluster non-kube resource namespace URI",
in: uri.NewClusterURI("foo").AppendDB("postgres"),
out: "",
},
{
name: "returns empty string when given leaf cluster non-kube resource namespace URI",
in: uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendDB("postgres"),
out: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
out := tt.in.GetKubeResourceNamespace()
require.Equal(t, tt.out, out)
})
}
}

func TestGetServerUUID(t *testing.T) {
tests := []struct {
name string
Expand Down
13 changes: 7 additions & 6 deletions lib/teleterm/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,12 @@ func (s *Service) ListKubernetesResources(ctx context.Context, clusterURI uri.Re
return nil, trace.Wrap(err)
}

proxyGRPCClient, err := tc.NewKubernetesServiceClient(ctx, cluster.Name)
var proxyGRPCClient kubeproto.KubeServiceClient

err = clusters.AddMetadataToRetryableError(ctx, func() error {
proxyGRPCClient, err = tc.NewKubernetesServiceClient(ctx, cluster.Name)
return trace.Wrap(err)
})
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -841,11 +846,7 @@ func (s *Service) ListKubernetesResources(ctx context.Context, clusterURI uri.Re
KubernetesNamespace: req.GetKubernetesNamespace(),
TeleportCluster: cluster.Name,
})
if err != nil {
return trace.Wrap(err)
}

return nil
return trace.Wrap(err)
})

return resources, trace.Wrap(err)
Expand Down
19 changes: 9 additions & 10 deletions web/packages/teleterm/src/ui/uri.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { KubeResourceParam, Params, routing } from './uri';
import { KubeResourceNamespaceParam, Params, routing } from './uri';

describe('getServerUri', () => {
const tests: Array<
Expand Down Expand Up @@ -67,29 +67,28 @@ describe('getServerUri', () => {

describe('getKubeResourceUri', () => {
const tests: Array<
{ name: string; input: KubeResourceParam } & { output: string }
{ name: string; input: KubeResourceNamespaceParam } & { output: string }
> = [
{
name: 'returns a kube resource URI for a root cluster',
name: 'returns a kube resource namespace URI for a root cluster',
input: {
rootClusterId: 'foo',
kubeId: 'kubeClusterName',
kubeResourceId: 'kubeResourceName',
kubeResourceKind: 'namespace',
namespaceId: 'namespace',
},
output: '/clusters/foo/namespace/kubeClusterName/kubeResourceName',
output:
'/clusters/foo/kube-resources/namespace/kubeClusterName/namespace',
},
{
name: 'returns a kube resource URI for a leaf cluster',
name: 'returns a kube resource namespace URI for a leaf cluster',
input: {
rootClusterId: 'foo',
leafClusterId: 'bar',
kubeId: 'kubeClusterName',
kubeResourceId: 'kubeResourceName',
kubeResourceKind: 'namespace',
namespaceId: 'namespace',
},
output:
'/clusters/foo/leaves/bar/namespace/kubeClusterName/kubeResourceName',
'/clusters/foo/leaves/bar/kube-resources/namespace/kubeClusterName/namespace',
},
];

Expand Down
28 changes: 15 additions & 13 deletions web/packages/teleterm/src/ui/uri.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,10 @@ export const paths = {
'/clusters/:rootClusterId/leaves/:leafClusterId/servers/:serverId',
kube: '/clusters/:rootClusterId/(leaves)?/:leafClusterId?/kubes/:kubeId',
kubeLeaf: '/clusters/:rootClusterId/leaves/:leafClusterId/kubes/:kubeId',
kubeResource:
'/clusters/:rootClusterId/(leaves)?/:leafClusterId?/:kubeResourceKind/:kubeId/:kubeResourceId',
kubeResourceLeaf:
'/clusters/:rootClusterId/leaves/:leafClusterId/:kubeResourceKind/:kubeId/:kubeResourceId',
kubeResourceNamespace:
'/clusters/:rootClusterId/(leaves)?/:leafClusterId?/kube-resources/namespace/:kubeId/:namespaceId',
kubeResourceNamespaceLeaf:
'/clusters/:rootClusterId/leaves/:leafClusterId/kube-resources/namespace/:kubeId/:namespaceId',
db: '/clusters/:rootClusterId/(leaves)?/:leafClusterId?/dbs/:dbId',
dbLeaf: '/clusters/:rootClusterId/leaves/:leafClusterId/dbs/:dbId',
app: '/clusters/:rootClusterId/(leaves)?/:leafClusterId?/apps/:appId',
Expand Down Expand Up @@ -135,8 +135,8 @@ export const routing = {
return routing.parseUri(uri, paths.kube);
},

parseKubeResourceUri(uri: string) {
return routing.parseUri(uri, paths.kubeResource);
parseKubeResourceNamespaceUri(uri: string) {
return routing.parseUri(uri, paths.kubeResourceNamespace);
},

parseAppUri(uri: string) {
Expand Down Expand Up @@ -245,15 +245,18 @@ export const routing = {
}
},

getKubeResourceUri(params: KubeResourceParam) {
getKubeResourceUri(params: KubeResourceNamespaceParam) {
if (params.leafClusterId) {
// paths.kubeResourceLeaf is needed as path-to-regexp used by react-router doesn't support
// optional groups with params. https://github.com/pillarjs/path-to-regexp/issues/142
//
// If we used paths.kubeResource instead, then the /leaves/ part of the URI would be missing.
return generatePath(paths.kubeResourceLeaf, params as any) as string;
return generatePath(
paths.kubeResourceNamespaceLeaf,
params as any
) as string;
} else {
return generatePath(paths.kubeResource, params as any) as string;
return generatePath(paths.kubeResourceNamespace, params as any) as string;
}
},

Expand Down Expand Up @@ -314,8 +317,7 @@ export type Params = {
leafClusterId?: string;
serverId?: string;
kubeId?: string;
kubeResourceId?: string;
kubeResourceKind?: string;
namespaceId?: string;
dbId?: string;
gatewayId?: string;
tabId?: string;
Expand All @@ -324,5 +326,5 @@ export type Params = {
appId?: string;
};

export type KubeResourceParam = Partial<Params> &
Required<Pick<Params, 'kubeId' | 'kubeResourceId' | 'kubeResourceKind'>>;
export type KubeResourceNamespaceParam = Partial<Params> &
Required<Pick<Params, 'kubeId' | 'namespaceId'>>;

0 comments on commit cde17d4

Please sign in to comment.