fix: Store custom resources in JSON & YAML format (#1360)

fix: Store custom resources as JSON and YAML files

Makefile

@@ -85,6 +85,9 @@ build:
 	@echo "Build cli binaries"
 	$(MAKE) -j support-bundle preflight analyze collect
 
+mod-tidy:
+	go mod tidy
+
 .PHONY: support-bundle
 support-bundle:
 	go build ${BUILDFLAGS} ${LDFLAGS} -o bin/support-bundle github.com/replicatedhq/troubleshoot/cmd/troubleshoot
@@ -151,7 +154,7 @@ CONTROLLER_GEN=$(shell which controller-gen)
 .PHONY: client-gen
 client-gen:
 ifeq (, $(shell which client-gen 2>/dev/null))
-	go install k8s.io/code-generator/cmd/client-gen@v0.26.1
+	go install k8s.io/code-generator/cmd/client-gen@v0.28.2
 CLIENT_GEN=$(shell go env GOPATH)/bin/client-gen
 else
 CLIENT_GEN=$(shell which client-gen)

cmd/collect/cli/run.go

@@ -50,7 +50,7 @@ func runCollect(v *viper.Viper, arg string) error {
 
 		collectorContent = spec
 	} else if _, err = os.Stat(arg); err == nil {
-		b, err := ioutil.ReadFile(arg)
+		b, err := os.ReadFile(arg)
 		if err != nil {
 			return err
 		}

pkg/collect/cluster_resources.go

@@ -1235,15 +1235,26 @@ func crdsV1beta(ctx context.Context, config *rest.Config) ([]byte, []string) {
 }
 
 func crs(ctx context.Context, dyn dynamic.Interface, client *kubernetes.Clientset, config *rest.Config, namespaces []string) (map[string][]byte, map[string]string) {
+	errorList := make(map[string]string)
 	ok, err := discovery.HasResource(client, "apiextensions.k8s.io/v1", "CustomResourceDefinition")
 	if err != nil {
 		return nil, map[string]string{"discover apiextensions.k8s.io/v1": err.Error()}
 	}
 	if ok {
-		return crsV1(ctx, dyn, config, namespaces)
+		crdClient, err := apiextensionsv1clientset.NewForConfig(config)
+		if err != nil {
+			errorList["crdClient"] = err.Error()
+			return map[string][]byte{}, errorList
+		}
+		return crsV1(ctx, dyn, crdClient, namespaces)
 	}
 
-	return crsV1beta(ctx, dyn, config, namespaces)
+	crdClient, err := apiextensionsv1beta1clientset.NewForConfig(config)
+	if err != nil {
+		errorList["crdClient"] = err.Error()
+		return map[string][]byte{}, errorList
+	}
+	return crsV1beta(ctx, dyn, crdClient, namespaces)
 }
 
 // Selects the newest version by kube-aware priority.
@@ -1258,16 +1269,15 @@ func selectCRDVersionByPriority(versions []string) string {
 	return versions[len(versions)-1]
 }
 
-func crsV1(ctx context.Context, client dynamic.Interface, config *rest.Config, namespaces []string) (map[string][]byte, map[string]string) {
+func crsV1(
+	ctx context.Context,
+	client dynamic.Interface,
+	crdClient apiextensionsv1clientset.ApiextensionsV1Interface,
+	namespaces []string,
+) (map[string][]byte, map[string]string) {
 	customResources := make(map[string][]byte)
 	errorList := make(map[string]string)
 
-	crdClient, err := apiextensionsv1clientset.NewForConfig(config)
-	if err != nil {
-		errorList["crdClient"] = err.Error()
-		return customResources, errorList
-	}
-
 	crds, err := crdClient.CustomResourceDefinitions().List(ctx, metav1.ListOptions{})
 	if err != nil {
 		errorList["crdList"] = err.Error()
@@ -1319,12 +1329,11 @@ func crsV1(ctx context.Context, client dynamic.Interface, config *rest.Config, n
 			for _, item := range customResourceList.Items {
 				objects = append(objects, item.Object)
 			}
-			b, err := yaml.Marshal(objects)
+			err := storeCustomResource(crd.Name, objects, customResources)
 			if err != nil {
 				errorList[crd.Name] = err.Error()
 				continue
 			}
-			customResources[fmt.Sprintf("%s.yaml", crd.Name)] = b
 		} else {
 			// Group fetched resources by the namespace
 			perNamespace := map[string][]map[string]interface{}{}
@@ -1353,30 +1362,27 @@ func crsV1(ctx context.Context, client dynamic.Interface, config *rest.Config, n
 				}
 
 				namespacedName := fmt.Sprintf("%s/%s", crd.Name, ns)
-				b, err := yaml.Marshal(perNamespace[ns])
+				err := storeCustomResource(namespacedName, perNamespace[ns], customResources)
 				if err != nil {
 					errorList[namespacedName] = err.Error()
 					continue
 				}
-
-				customResources[fmt.Sprintf("%s.yaml", namespacedName)] = b
 			}
 		}
 	}
 
 	return customResources, errorList
 }
 
-func crsV1beta(ctx context.Context, client dynamic.Interface, config *rest.Config, namespaces []string) (map[string][]byte, map[string]string) {
+func crsV1beta(
+	ctx context.Context,
+	client dynamic.Interface,
+	crdClient apiextensionsv1beta1clientset.ApiextensionsV1beta1Interface,
+	namespaces []string,
+) (map[string][]byte, map[string]string) {
 	customResources := make(map[string][]byte)
 	errorList := make(map[string]string)
 
-	crdClient, err := apiextensionsv1beta1clientset.NewForConfig(config)
-	if err != nil {
-		errorList["crdClient"] = err.Error()
-		return customResources, errorList
-	}
-
 	crds, err := crdClient.CustomResourceDefinitions().List(ctx, metav1.ListOptions{})
 	if err != nil {
 		errorList["crdList"] = err.Error()
@@ -1430,12 +1436,13 @@ func crsV1beta(ctx context.Context, client dynamic.Interface, config *rest.Confi
 			for _, item := range customResourceList.Items {
 				objects = append(objects, item.Object)
 			}
-			b, err := yaml.Marshal(customResourceList.Items)
+
+			err = storeCustomResource(crd.Name, objects, customResources)
 			if err != nil {
 				errorList[crd.Name] = err.Error()
 				continue
 			}
-			customResources[fmt.Sprintf("%s.yaml", crd.Name)] = b
+
 		} else {
 			// Group fetched resources by the namespace
 			perNamespace := map[string][]map[string]interface{}{}
@@ -1464,13 +1471,11 @@ func crsV1beta(ctx context.Context, client dynamic.Interface, config *rest.Confi
 				}
 
 				namespacedName := fmt.Sprintf("%s/%s", crd.Name, ns)
-				b, err := yaml.Marshal(perNamespace[ns])
+				err := storeCustomResource(namespacedName, perNamespace[ns], customResources)
 				if err != nil {
 					errorList[namespacedName] = err.Error()
 					continue
 				}
-
-				customResources[fmt.Sprintf("%s.yaml", namespacedName)] = b
 			}
 		}
 	}
@@ -1630,7 +1635,7 @@ func getSelfSubjectRulesReviews(ctx context.Context, client *kubernetes.Clientse
 			continue
 		}
 
-		if response.Status.Incomplete == true {
+		if response.Status.Incomplete {
 			errorsByNamespace[namespace] = response.Status.EvaluationError
 		}
 
@@ -2106,3 +2111,23 @@ func configMaps(ctx context.Context, client kubernetes.Interface, namespaces []s
 
 	return configmapByNamespace, errorsByNamespace
 }
+
+// storeCustomResource stores a custom resource as JSON and YAML
+// We use both formats for backwards compatibility. This way we
+// avoid breaking existing tools and analysers that already rely on
+// the YAML format.
+func storeCustomResource(name string, objects any, m map[string][]byte) error {
+	j, err := json.MarshalIndent(objects, "", "  ")
+	if err != nil {
+		return err
+	}
+
+	y, err := yaml.Marshal(objects)
+	if err != nil {
+		return err
+	}
+
+	m[fmt.Sprintf("%s.json", name)] = j
+	m[fmt.Sprintf("%s.yaml", name)] = y
+	return nil
+}

pkg/collect/cluster_resources_test.go

@@ -3,20 +3,29 @@ package collect
 import (
 	"context"
 	"encoding/json"
+	"os"
 	"reflect"
 	"testing"
 
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/client/troubleshootclientset/scheme"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
+	apixfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	testdynamicclient "k8s.io/client-go/dynamic/fake"
 	"k8s.io/client-go/kubernetes"
 	testclient "k8s.io/client-go/kubernetes/fake"
+	"sigs.k8s.io/yaml"
 )
 
+func init() {
+	apixfake.AddToScheme(scheme.Scheme)
+}
+
 func Test_ConfigMaps(t *testing.T) {
 	tests := []struct {
 		name           string
@@ -458,3 +467,59 @@ func TestClusterResources_Merge(t *testing.T) {
 		})
 	}
 }
+
+func TestCollectClusterResources_CustomResource(t *testing.T) {
+	ctx := context.Background()
+
+	// Register supportbundle troubleshoot CRD
+	dat, err := os.ReadFile("../../config/crds/troubleshoot.sh_supportbundles.yaml")
+	require.NoError(t, err)
+
+	obj, _, err := scheme.Codecs.UniversalDeserializer().Decode(dat, nil, nil)
+	require.NoError(t, err)
+	apixClient := apixfake.NewSimpleClientset(obj)
+
+	// Create a CR
+	sbObject := troubleshootv1beta2.SupportBundle{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "supportbundle",
+			Namespace: "default",
+		},
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "SupportBundle",
+			APIVersion: "troubleshoot.sh/v1beta2",
+		},
+		Spec: troubleshootv1beta2.SupportBundleSpec{
+			Collectors: []*troubleshootv1beta2.Collect{
+				{
+					ClusterResources: &troubleshootv1beta2.ClusterResources{},
+				},
+			},
+		},
+	}
+
+	dynamicClient := testdynamicclient.NewSimpleDynamicClient(scheme.Scheme, &sbObject)
+
+	// Fetch the CR from cluster
+	res, errs := crsV1(ctx, dynamicClient, apixClient.ApiextensionsV1(), []string{"default"})
+	assert.Empty(t, errs)
+	require.Equal(t, 2, len(res))
+	assert.Equal(t, fromJSON(t, res["supportbundles.troubleshoot.sh/default.json"]), sbObject)
+	assert.Equal(t, fromYAML(t, res["supportbundles.troubleshoot.sh/default.yaml"]), sbObject)
+}
+
+func fromYAML(t *testing.T, dat []byte) troubleshootv1beta2.SupportBundle {
+	sb := []troubleshootv1beta2.SupportBundle{}
+	err := yaml.Unmarshal(dat, &sb)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(sb))
+	return sb[0]
+}
+
+func fromJSON(t *testing.T, dat []byte) troubleshootv1beta2.SupportBundle {
+	sb := []troubleshootv1beta2.SupportBundle{}
+	err := json.Unmarshal(dat, &sb)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(sb))
+	return sb[0]
+}