Repository: vmware-tanzu/octant
Pull request #548 (2020-01-17 20:14:44)

Handle multiple CRD versions more gracefully
* When viewing a CRD, show all versions and the custom resources if they exist
* Update URL to custom resources to include the version
* Show custom resource version in titles

Signed-off-by: bryanl <bryanliles@gmail.com>


Patch:
diff --git a/changelogs/unreleased/547-bryanl b/changelogs/unreleased/547-bryanl
new file mode 100644
index 00000000..a14b0e0a
--- /dev/null
+++ b/changelogs/unreleased/547-bryanl
@@ -0,0 +1,1 @@
+Support multiple CRD versions
diff --git a/internal/describer/crd.go b/internal/describer/crd.go
index fb152105..26317e40 100644
--- a/internal/describer/crd.go
+++ b/internal/describer/crd.go
@@ -108,6 +108,7 @@ func (c *crd) Describe(ctx context.Context, namespace string, options Options) (
 	title := component.Title(
 		component.NewText("Custom Resources"),
 		component.NewText(crd.GetName()),
+		component.NewText(object.GroupVersionKind().Version),
 		component.NewText(object.GetName()))
 
 	iconName, iconSource := loadIcon(icon.CustomResourceDefinition)
diff --git a/internal/describer/customresource.go b/internal/describer/customresource.go
index 937c6793..1b51c30e 100644
--- a/internal/describer/customresource.go
+++ b/internal/describer/customresource.go
@@ -45,9 +45,19 @@ func AddCRD(ctx context.Context, crd *unstructured.Unstructured, pm *PathMatcher
 		pm.Register(ctx, pf)
 	}
 
-	cd := newCRD(name, crdObjectPath(name))
-	for _, pf := range cd.PathFilters() {
-		pm.Register(ctx, pf)
+	// TODO: there could be multiple paths here, so iterate through crd.spec.versions['name']
+
+	versions, err := crdVersions(crd)
+	if err != nil {
+		logger.WithErr(err).Errorf("get crd versions: %w", err)
+		return
+	}
+
+	for _, version := range versions {
+		cd := newCRD(name, crdObjectPath(crd, version))
+		for _, pf := range cd.PathFilters() {
+			pm.Register(ctx, pf)
+		}
 	}
 
 	if err := m.AddCRD(ctx, crd); err != nil {
@@ -55,6 +65,32 @@ func AddCRD(ctx context.Context, crd *unstructured.Unstructured, pm *PathMatcher
 	}
 }
 
+func crdVersions(crd *unstructured.Unstructured) ([]string, error) {
+	rawVersions, _, err := unstructured.NestedSlice(crd.Object, "spec", "versions")
+	if err != nil {
+		return nil, fmt.Errorf("unable to extract versions from crd: %w", err)
+	}
+
+	var list []string
+
+	for _, rawVersion := range rawVersions {
+		versionDesc, ok := rawVersion.(map[string]interface{})
+		if !ok {
+			return nil, fmt.Errorf("crd version descriptor was not an object (it was %T)", rawVersion)
+		}
+		versionName, found, err := unstructured.NestedString(versionDesc, "name")
+		if err != nil {
+			return nil, fmt.Errorf("unable to extract version name from version descriptor: %w", err)
+		}
+
+		if found {
+			list = append(list, versionName)
+		}
+	}
+
+	return list, nil
+}
+
 func DeleteCRD(ctx context.Context, crd *unstructured.Unstructured, pm *PathMatcher, crdSection *CRDSection, m module.Module) {
 	name := crd.GetName()
 
@@ -62,7 +98,15 @@ func DeleteCRD(ctx context.Context, crd *unstructured.Unstructured, pm *PathMatc
 	logger.Debugf("deleting CRD")
 
 	pm.Deregister(ctx, crdListPath(name))
-	pm.Deregister(ctx, crdObjectPath(name))
+
+	versions, err := crdVersions(crd)
+	if err != nil {
+		logger.WithErr(err).Errorf("get crd versions: %w", err)
+		return
+	}
+	for _, version := range versions {
+		pm.Deregister(ctx, crdObjectPath(crd, version))
+	}
 
 	crdSection.Remove(name)
 
@@ -76,6 +120,6 @@ func crdListPath(name string) string {
 	return path.Join("/custom-resources", name)
 }
 
-func crdObjectPath(name string) string {
-	return path.Join(crdListPath(name), ResourceNameRegex)
+func crdObjectPath(object *unstructured.Unstructured, version string) string {
+	return path.Join(crdListPath(object.GetName()), version, ResourceNameRegex)
 }
diff --git a/internal/modules/clusteroverview/path.go b/internal/modules/clusteroverview/path.go
index cc9e5321..8287a22c 100644
--- a/internal/modules/clusteroverview/path.go
+++ b/internal/modules/clusteroverview/path.go
@@ -24,8 +24,8 @@ var (
 
 const rbacAPIVersion = "rbac.authorization.k8s.io/v1"
 
-func crdPath(namespace, crdName, name string) (string, error) {
-	return path.Join("/cluster-overview/custom-resources", crdName, name), nil
+func crdPath(namespace, crdName, version, name string) (string, error) {
+	return path.Join("/cluster-overview/custom-resources", crdName, version, name), nil
 }
 
 func gvkPath(namespace, apiVersion, kind, name string) (string, error) {
diff --git a/internal/modules/overview/path.go b/internal/modules/overview/path.go
index 6817b314..54185686 100644
--- a/internal/modules/overview/path.go
+++ b/internal/modules/overview/path.go
@@ -38,12 +38,12 @@ var (
 	}
 )
 
-func crdPath(namespace, crdName, name string) (string, error) {
+func crdPath(namespace, crdName, version, name string) (string, error) {
 	if namespace == "" {
 		return "", errors.Errorf("unable to create CRD path for %s due to missing namespace", crdName)
 	}
 
-	return path.Join("/overview/namespace", namespace, "custom-resources", crdName, name), nil
+	return path.Join("/overview/namespace", namespace, "custom-resources", crdName, version, name), nil
 }
 
 func gvkPath(namespace, apiVersion, kind, name string) (string, error) {
diff --git a/internal/octant/object_path.go b/internal/octant/object_path.go
index 743dde0e..dc749d64 100644
--- a/internal/octant/object_path.go
+++ b/internal/octant/object_path.go
@@ -21,7 +21,7 @@ import (
 )
 
 // CRDPathGenFunc is a function that generates a custom resource path.
-type CRDPathGenFunc func(namespace, crdName, name string) (string, error)
+type CRDPathGenFunc func(namespace, crdName, version, name string) (string, error)
 
 // PathLookupFunc looks up paths for an object.
 type PathLookupFunc func(namespace, apiVersion, kind, name string) (string, error)
@@ -186,8 +186,9 @@ func (op *ObjectPath) GroupVersionKindPath(namespace, apiVersion, kind, name str
 		}
 
 		if dashStrings.Contains(apiVersion, apiVersions) {
-			return op.crdPathGenFunc(namespace, crd.GetName(), name)
+			return op.crdPathGenFunc(namespace, crd.GetName(), g.Version, name)
 		}
+
 	}
 
 	return op.lookupFunc(namespace, apiVersion, kind, name)
diff --git a/internal/printer/customresource.go b/internal/printer/customresource.go
index 21f1c3cc..a8580126 100644
--- a/internal/printer/customresource.go
+++ b/internal/printer/customresource.go
@@ -28,7 +28,8 @@ func CustomResourceListHandler(crdObject *unstructured.Unstructured, resources *
 	}
 
 	tableName := fmt.Sprintf("%s/%s", crdObject.GetName(), version)
-	table := component.NewTable(tableName, "We couldn't find any custom resources!", component.NewTableCols("Name", "Labels"))
+	placeholder := fmt.Sprintf("We could not find any %s!", tableName)
+	table := component.NewTable(tableName, placeholder, component.NewTableCols("Name", "Labels"))
 
 	crd, err := octant.NewCustomResourceDefinition(crdObject)
 	if err != nil {
@@ -54,6 +55,10 @@ func CustomResourceListHandler(crdObject *unstructured.Unstructured, resources *
 
 	for i := range resources.Items {
 		versionName := resources.Items[i].GroupVersionKind().Version
+		if version != versionName {
+			continue
+		}
+
 		version, err := crd.Version(versionName)
 		if err != nil {
 			return nil, fmt.Errorf("get version '%s' from crd '%s': %w", versionName, crdObject.GetName(), err)
diff --git a/internal/printer/customresourcedefinition.go b/internal/printer/customresourcedefinition.go
index 0b18b731..cb0034de 100644
--- a/internal/printer/customresourcedefinition.go
+++ b/internal/printer/customresourcedefinition.go
@@ -40,9 +40,10 @@ func CustomResourceDefinitionHandler(ctx context.Context, crd *unstructured.Unst
 	}
 
 	for i := range versions {
+		version := versions[i]
+
 		object.RegisterItems(ItemDescriptor{
 			Func: func() (c component.Component, err error) {
-				version := versions[i]
 				crGVK, err := gvk.CustomResource(crd, version)
 				if err != nil {
 					return nil, err


Test patch:
diff --git a/internal/modules/clusteroverview/path_test.go b/internal/modules/clusteroverview/path_test.go
index 1af8d5a2..19d1cee3 100644
--- a/internal/modules/clusteroverview/path_test.go
+++ b/internal/modules/clusteroverview/path_test.go
@@ -14,10 +14,10 @@ import (
 )
 
 func Test_crdPath(t *testing.T) {
-	got, err := crdPath("namespace", "crdName", "name")
+	got, err := crdPath("namespace", "crdName", "version", "name")
 	require.NoError(t, err)
 
-	expected := path.Join("/cluster-overview", "custom-resources", "crdName", "name")
+	expected := path.Join("/cluster-overview", "custom-resources", "crdName", "version", "name")
 	assert.Equal(t, expected, got)
 }
 
diff --git a/internal/modules/overview/path_test.go b/internal/modules/overview/path_test.go
index ec81f818..15ef44b1 100644
--- a/internal/modules/overview/path_test.go
+++ b/internal/modules/overview/path_test.go
@@ -14,10 +14,10 @@ import (
 )
 
 func Test_crdPath(t *testing.T) {
-	got, err := crdPath("default", "crdName", "name")
+	got, err := crdPath("default", "crdName", "version", "name")
 	require.NoError(t, err)
 
-	expected := path.Join("/overview", "namespace", "default", "custom-resources", "crdName", "name")
+	expected := path.Join("/overview", "namespace", "default", "custom-resources", "crdName", "version", "name")
 	assert.Equal(t, expected, got)
 }
 
diff --git a/internal/octant/object_path_test.go b/internal/octant/object_path_test.go
index 28f784d6..acf51082 100644
--- a/internal/octant/object_path_test.go
+++ b/internal/octant/object_path_test.go
@@ -30,7 +30,7 @@ func TestObjectPathConfig_Validate(t *testing.T) {
 			pathLookupFunc: func(string, string, string, string) (string, error) {
 				return "/path", nil
 			},
-			crdPathGenFunc: func(string, string, string) (string, error) {
+			crdPathGenFunc: func(string, string, string, string) (string, error) {
 				return "/path", nil
 			},
 		},
@@ -62,7 +62,7 @@ func TestObjectPath(t *testing.T) {
 		PathLookupFunc: func(string, string, string, string) (string, error) {
 			return "/path", nil
 		},
-		CRDPathGenFunc: func(string, string, string) (string, error) {
+		CRDPathGenFunc: func(string, string, string, string) (string, error) {
 			return "/crd-path", nil
 		},
 	}
diff --git a/internal/printer/customresource_test.go b/internal/printer/customresource_test.go
index 85c4f7f0..d47bdaa8 100644
--- a/internal/printer/customresource_test.go
+++ b/internal/printer/customresource_test.go
@@ -41,7 +41,7 @@ func Test_CustomResourceListHandler(t *testing.T) {
 	require.NoError(t, err)
 
 	expected := component.NewTableWithRows(
-		"crontabs.stable.example.com/v1", "We couldn't find any custom resources!",
+		"crontabs.stable.example.com/v1", "We could not find any crontabs.stable.example.com/v1!",
 		component.NewTableCols("Name", "Labels", "Age"),
 		[]component.TableRow{
 			{
@@ -77,7 +77,7 @@ func Test_CustomResourceListHandler_custom_columns(t *testing.T) {
 	require.NoError(t, err)
 
 	expected := component.NewTableWithRows(
-		"crontabs.stable.example.com/v1", "We couldn't find any custom resources!",
+		"crontabs.stable.example.com/v1", "We could not find any crontabs.stable.example.com/v1!",
 		component.NewTableCols("Name", "Labels", "Spec", "Replicas", "Errors", "Resource Age", "Age"),
 		[]component.TableRow{
 			{