From fc90e2b4354f92b71a5c838b814af2b54e8a5a8b Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 17 Oct 2023 17:38:07 +0200 Subject: [PATCH 01/10] Refactor /inventory code to expose some helpers - That can be used by other similar inventory endpoints --- core/server/inventory.go | 185 ++++++++++++++++++++++++--------------- 1 file changed, 113 insertions(+), 72 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index cc89592217..3145ce27d6 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -14,8 +14,10 @@ import ( helmv2 "github.com/fluxcd/helm-controller/api/v2beta1" kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" "github.com/fluxcd/pkg/ssa" + "github.com/go-logr/logr" "github.com/weaveworks/weave-gitops/core/server/types" pb "github.com/weaveworks/weave-gitops/pkg/api/core" + "github.com/weaveworks/weave-gitops/pkg/health" "github.com/weaveworks/weave-gitops/pkg/server/auth" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -26,6 +28,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +// an object that can store unstructued and its children +type ObjectWithChildren struct { + Object *unstructured.Unstructured + Children []*ObjectWithChildren +} + func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequest) (*pb.GetInventoryResponse, error) { clustersClient, err := cs.clustersManager.GetImpersonatedClient(ctx, auth.Principal(ctx)) if err != nil { @@ -37,16 +45,16 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ return nil, fmt.Errorf("error getting scoped client for cluster=%s: %w", msg.ClusterName, err) } - var entries []*unstructured.Unstructured + var inventoryRefs []*unstructured.Unstructured switch msg.Kind { case kustomizev1.KustomizationKind: - entries, err = cs.getKustomizationInventory(ctx, client, msg.Name, msg.Namespace) + inventoryRefs, err = cs.getKustomizationInventory(ctx, client, msg.Name, msg.Namespace) if err != nil { return nil, fmt.Errorf("failed getting kustomization inventory: %w", err) } case helmv2.HelmReleaseKind: - entries, err = cs.getHelmReleaseInventory(ctx, client, msg.Name, msg.Namespace) + inventoryRefs, err = cs.getHelmReleaseInventory(ctx, client, msg.Name, msg.Namespace) if err != nil { return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) } @@ -61,7 +69,20 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ } } - resources := cs.getInventoryResources(ctx, msg.ClusterName, client, entries, msg.Namespace, msg.WithChildren) + objsWithChildren, err := GetObjectsWithChildren(ctx, inventoryRefs, client, msg.WithChildren, cs.logger) + if err != nil { + return nil, fmt.Errorf("failed getting objects with children: %w", err) + } + + entries := []*pb.InventoryEntry{} + clusterUserNamespaces := cs.clustersManager.GetUserNamespaces(auth.Principal(ctx)) + for _, oc := range objsWithChildren { + entry, err := unstructuredToInventoryEntry(msg.ClusterName, *oc, clusterUserNamespaces, cs.healthChecker) + if err != nil { + return nil, fmt.Errorf("failed converting inventory entry: %w", err) + } + entries = append(entries, entry) + } return &pb.GetInventoryResponse{ Entries: resources, @@ -69,30 +90,31 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ } func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { - kust := &kustomizev1.Kustomization{ + ks := &kustomizev1.Kustomization{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, } - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(kust), kust); err != nil { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(ks), ks); err != nil { return nil, fmt.Errorf("failed to get kustomization: %w", err) } - if kust.Status.Inventory == nil { + if ks.Status.Inventory == nil { return nil, nil } - if kust.Status.Inventory.Entries == nil { + if ks.Status.Inventory.Entries == nil { return nil, nil } objects := []*unstructured.Unstructured{} - for _, e := range kust.Status.Inventory.Entries { - obj, err := resourceRefToUnstructured(e) + for _, ref := range ks.Status.Inventory.Entries { + obj, err := ResourceRefToUnstructured(ref.ID, ref.Version) if err != nil { - return nil, fmt.Errorf("failed converting inventory entry: %w", err) + cs.logger.Error(err, "failed converting inventory entry", "entry", ref) + return nil, err } objects = append(objects, &obj) } @@ -117,41 +139,14 @@ func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, k8sClient cli return nil, fmt.Errorf("failed to get helm release objects: %w", err) } - return objects, nil -} - -func (cs *coreServer) getInventoryResources(ctx context.Context, clusterName string, k8sClient client.Client, objects []*unstructured.Unstructured, namespace string, withChildren bool) []*pb.InventoryEntry { - result := []*pb.InventoryEntry{} - resultMu := sync.Mutex{} - - wg := sync.WaitGroup{} - - for _, o := range objects { - wg.Add(1) - - go func(obj unstructured.Unstructured) { - defer wg.Done() - - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(&obj), &obj); err != nil { - cs.logger.Error(err, "failed to get object", "entry", obj) - return - } - - entry, err := cs.unstructuredToInventoryEntry(ctx, clusterName, k8sClient, obj, namespace, withChildren) - if err != nil { - cs.logger.Error(err, "failed converting inventory entry", "entry", obj) - return - } - - resultMu.Lock() - result = append(result, entry) - resultMu.Unlock() - }(*o) + // FIXME: do we need this? + for _, obj := range objects { + if obj.GetNamespace() == "" { + obj.SetNamespace(namespace) + } } - wg.Wait() - - return result + return objects, nil } // Returns the list of resources applied in the helm chart. @@ -222,36 +217,34 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel return objects, nil } -func (cs *coreServer) unstructuredToInventoryEntry(ctx context.Context, clusterName string, k8sClient client.Client, unstructuredObj unstructured.Unstructured, ns string, withChildren bool) (*pb.InventoryEntry, error) { - var err error - +func unstructuredToInventoryEntry(clusterName string, objWithChildren ObjectWithChildren, clusterUserNamespaces map[string][]v1.Namespace, healthChecker health.HealthChecker) (*pb.InventoryEntry, error) { + unstructuredObj := *objWithChildren.Object if unstructuredObj.GetKind() == "Secret" { - unstructuredObj, err = sanitizeUnstructuredSecret(unstructuredObj) + var err error + unstructuredObj, err = SanitizeUnstructuredSecret(unstructuredObj) if err != nil { return nil, fmt.Errorf("error sanitizing secrets: %w", err) } } - - children := []*pb.InventoryEntry{} - - if withChildren { - children, err = cs.getChildren(ctx, clusterName, k8sClient, unstructuredObj, ns) - if err != nil { - return nil, err - } - } - bytes, err := unstructuredObj.MarshalJSON() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) } - clusterUserNss := cs.clustersManager.GetUserNamespaces(auth.Principal(ctx)) - tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNss) + tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNamespaces) - health, err := cs.healthChecker.Check(unstructuredObj) + health, err := healthChecker.Check(unstructuredObj) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to check health: %w", err) + } + + children := []*pb.InventoryEntry{} + for _, c := range objWithChildren.Children { + child, err := unstructuredToInventoryEntry(clusterName, *c, clusterUserNamespaces, healthChecker) + if err != nil { + return nil, fmt.Errorf("failed converting child inventory entry: %w", err) + } + children = append(children, child) } entry := &pb.InventoryEntry{ @@ -268,7 +261,50 @@ func (cs *coreServer) unstructuredToInventoryEntry(ctx context.Context, clusterN return entry, nil } -func (cs *coreServer) getChildren(ctx context.Context, clusterName string, k8sClient client.Client, parentObj unstructured.Unstructured, ns string) ([]*pb.InventoryEntry, error) { +func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstructured, k8sClient client.Client, withChildren bool, logger logr.Logger) ([]*ObjectWithChildren, error) { + result := []*ObjectWithChildren{} + resultMu := sync.Mutex{} + + wg := sync.WaitGroup{} + + for _, o := range objects { + wg.Add(1) + + go func(obj unstructured.Unstructured) { + defer wg.Done() + + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(&obj), &obj); err != nil { + logger.Error(err, "failed to get object", "entry", obj) + return + } + + children := []*ObjectWithChildren{} + if withChildren { + var err error + children, err = GetChildren(ctx, k8sClient, obj) + if err != nil { + logger.Error(err, "failed getting children", "entry", obj) + return + } + } + + entry := &ObjectWithChildren{ + Object: &obj, + Children: children, + } + + resultMu.Lock() + result = append(result, entry) + resultMu.Unlock() + }(*o) + } + + wg.Wait() + + return result, nil +} + +func GetChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { listResult := unstructured.UnstructuredList{} switch parentObj.GetObjectKind().GroupVersionKind().Kind { @@ -285,10 +321,10 @@ func (cs *coreServer) getChildren(ctx context.Context, clusterName string, k8sCl Kind: "Pod", }) default: - return []*pb.InventoryEntry{}, nil + return []*ObjectWithChildren{}, nil } - if err := k8sClient.List(ctx, &listResult, client.InNamespace(ns)); err != nil { + if err := k8sClient.List(ctx, &listResult, client.InNamespace(parentObj.GetNamespace())); err != nil { return nil, fmt.Errorf("could not get unstructured object: %s", err) } @@ -308,24 +344,29 @@ func (cs *coreServer) getChildren(ctx context.Context, clusterName string, k8sCl } } - children := []*pb.InventoryEntry{} + children := []*ObjectWithChildren{} for _, c := range unstructuredChildren { - entry, err := cs.unstructuredToInventoryEntry(ctx, clusterName, k8sClient, c, ns, true) + var err error + children, err = GetChildren(ctx, k8sClient, c) if err != nil { return nil, err } + entry := &ObjectWithChildren{ + Object: &c, + Children: children, + } children = append(children, entry) } return children, nil } -func resourceRefToUnstructured(entry kustomizev1.ResourceRef) (unstructured.Unstructured, error) { +func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, error) { u := unstructured.Unstructured{} - objMetadata, err := object.ParseObjMetadata(entry.ID) + objMetadata, err := object.ParseObjMetadata(id) if err != nil { return u, err } @@ -333,7 +374,7 @@ func resourceRefToUnstructured(entry kustomizev1.ResourceRef) (unstructured.Unst u.SetGroupVersionKind(schema.GroupVersionKind{ Group: objMetadata.GroupKind.Group, Kind: objMetadata.GroupKind.Kind, - Version: entry.Version, + Version: version, }) u.SetName(objMetadata.Name) u.SetNamespace(objMetadata.Namespace) @@ -341,7 +382,7 @@ func resourceRefToUnstructured(entry kustomizev1.ResourceRef) (unstructured.Unst return u, nil } -func sanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { +func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { redactedUnstructured := unstructured.Unstructured{} s := &v1.Secret{} From 773663991ab3e0805f3c66a177eeb12fae59529c Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Wed, 18 Oct 2023 15:25:33 +0200 Subject: [PATCH 02/10] Move the loops, docs a bit --- core/server/inventory.go | 84 ++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 3145ce27d6..54eb7ded25 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -74,14 +74,10 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ return nil, fmt.Errorf("failed getting objects with children: %w", err) } - entries := []*pb.InventoryEntry{} clusterUserNamespaces := cs.clustersManager.GetUserNamespaces(auth.Principal(ctx)) - for _, oc := range objsWithChildren { - entry, err := unstructuredToInventoryEntry(msg.ClusterName, *oc, clusterUserNamespaces, cs.healthChecker) - if err != nil { - return nil, fmt.Errorf("failed converting inventory entry: %w", err) - } - entries = append(entries, entry) + entries, err := unstructuredToInventoryEntry(msg.ClusterName, objsWithChildren, clusterUserNamespaces, cs.healthChecker) + if err != nil { + return nil, fmt.Errorf("failed converting inventory entry: %w", err) } return &pb.GetInventoryResponse{ @@ -217,50 +213,54 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel return objects, nil } -func unstructuredToInventoryEntry(clusterName string, objWithChildren ObjectWithChildren, clusterUserNamespaces map[string][]v1.Namespace, healthChecker health.HealthChecker) (*pb.InventoryEntry, error) { - unstructuredObj := *objWithChildren.Object - if unstructuredObj.GetKind() == "Secret" { - var err error - unstructuredObj, err = SanitizeUnstructuredSecret(unstructuredObj) +func unstructuredToInventoryEntry(clusterName string, objWithChildren []*ObjectWithChildren, clusterUserNamespaces map[string][]v1.Namespace, healthChecker health.HealthChecker) ([]*pb.InventoryEntry, error) { + entries := []*pb.InventoryEntry{} + for _, c := range objWithChildren { + unstructuredObj := *c.Object + if unstructuredObj.GetKind() == "Secret" { + var err error + unstructuredObj, err = SanitizeUnstructuredSecret(unstructuredObj) + if err != nil { + return nil, fmt.Errorf("error sanitizing secrets: %w", err) + } + } + bytes, err := unstructuredObj.MarshalJSON() if err != nil { - return nil, fmt.Errorf("error sanitizing secrets: %w", err) + return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) } - } - bytes, err := unstructuredObj.MarshalJSON() - if err != nil { - return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) - } - tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNamespaces) + tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNamespaces) - health, err := healthChecker.Check(unstructuredObj) - if err != nil { - return nil, fmt.Errorf("failed to check health: %w", err) - } + health, err := healthChecker.Check(unstructuredObj) + if err != nil { + return nil, fmt.Errorf("failed to check health: %w", err) + } - children := []*pb.InventoryEntry{} - for _, c := range objWithChildren.Children { - child, err := unstructuredToInventoryEntry(clusterName, *c, clusterUserNamespaces, healthChecker) + children, err := unstructuredToInventoryEntry(clusterName, c.Children, clusterUserNamespaces, healthChecker) if err != nil { return nil, fmt.Errorf("failed converting child inventory entry: %w", err) } - children = append(children, child) - } - entry := &pb.InventoryEntry{ - Payload: string(bytes), - Tenant: tenant, - ClusterName: clusterName, - Children: children, - Health: &pb.HealthStatus{ - Status: string(health.Status), - Message: health.Message, - }, + entry := &pb.InventoryEntry{ + Payload: string(bytes), + Tenant: tenant, + ClusterName: clusterName, + Children: children, + Health: &pb.HealthStatus{ + Status: string(health.Status), + Message: health.Message, + }, + } + + entries = append(entries, entry) } - return entry, nil + return entries, nil } +// GetObjectsWithChildren returns objects with their children populated if withChildren is true. +// Objects are retrieved in parallel. +// Children are retrieved recusively, e.g. Deployment -> ReplicaSet -> Pod func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstructured, k8sClient client.Client, withChildren bool, logger logr.Logger) ([]*ObjectWithChildren, error) { result := []*ObjectWithChildren{} resultMu := sync.Mutex{} @@ -281,7 +281,7 @@ func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstruc children := []*ObjectWithChildren{} if withChildren { var err error - children, err = GetChildren(ctx, k8sClient, obj) + children, err = getChildren(ctx, k8sClient, obj) if err != nil { logger.Error(err, "failed getting children", "entry", obj) return @@ -304,7 +304,7 @@ func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstruc return result, nil } -func GetChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { +func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { listResult := unstructured.UnstructuredList{} switch parentObj.GetObjectKind().GroupVersionKind().Kind { @@ -348,7 +348,7 @@ func GetChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc for _, c := range unstructuredChildren { var err error - children, err = GetChildren(ctx, k8sClient, c) + children, err = getChildren(ctx, k8sClient, c) if err != nil { return nil, err } @@ -363,6 +363,7 @@ func GetChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc return children, nil } +// ResourceRefToUnstructured converts a flux like resource entry pair of (id, version) into a unstructured object func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, error) { u := unstructured.Unstructured{} @@ -382,6 +383,7 @@ func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, e return u, nil } +// SanitizeUnstructuredSecret redacts the data field of a Secret object func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { redactedUnstructured := unstructured.Unstructured{} s := &v1.Secret{} From 25e58e4c66d06520ba000064003eadb43ecd24c8 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Fri, 20 Oct 2023 14:17:30 +0200 Subject: [PATCH 03/10] Adds some tests, tidy up --- core/server/inventory.go | 20 ++++---- core/server/inventory_test.go | 87 +++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 54eb7ded25..9fd704c10d 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -112,7 +112,7 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient c cs.logger.Error(err, "failed converting inventory entry", "entry", ref) return nil, err } - objects = append(objects, &obj) + objects = append(objects, obj) } return objects, nil @@ -218,8 +218,8 @@ func unstructuredToInventoryEntry(clusterName string, objWithChildren []*ObjectW for _, c := range objWithChildren { unstructuredObj := *c.Object if unstructuredObj.GetKind() == "Secret" { - var err error - unstructuredObj, err = SanitizeUnstructuredSecret(unstructuredObj) + sanitizedUnstructuredObj, err := SanitizeUnstructuredSecret(unstructuredObj) + unstructuredObj = *sanitizedUnstructuredObj if err != nil { return nil, fmt.Errorf("error sanitizing secrets: %w", err) } @@ -364,12 +364,12 @@ func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc } // ResourceRefToUnstructured converts a flux like resource entry pair of (id, version) into a unstructured object -func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, error) { - u := unstructured.Unstructured{} +func ResourceRefToUnstructured(id, version string) (*unstructured.Unstructured, error) { + u := &unstructured.Unstructured{} objMetadata, err := object.ParseObjMetadata(id) if err != nil { - return u, err + return nil, fmt.Errorf("unable to parse inventory entry id: %w", err) } u.SetGroupVersionKind(schema.GroupVersionKind{ @@ -384,20 +384,20 @@ func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, e } // SanitizeUnstructuredSecret redacts the data field of a Secret object -func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { - redactedUnstructured := unstructured.Unstructured{} +func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (*unstructured.Unstructured, error) { + redactedUnstructured := &unstructured.Unstructured{} s := &v1.Secret{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), s) if err != nil { - return redactedUnstructured, fmt.Errorf("converting unstructured to helmrelease: %w", err) + return nil, fmt.Errorf("converting unstructured to secret: %w", err) } s.Data = map[string][]byte{"redacted": []byte(nil)} redactedObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(s) if err != nil { - return redactedUnstructured, fmt.Errorf("converting unstructured to helmrelease: %w", err) + return nil, fmt.Errorf("converting unstructured to secret: %w", err) } redactedUnstructured.SetUnstructuredContent(redactedObj) diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 16cc1138b6..3975a957bb 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -13,12 +13,14 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1" . "github.com/onsi/gomega" "github.com/weaveworks/weave-gitops/core/clustersmngr/cluster" + "github.com/weaveworks/weave-gitops/core/server" "github.com/weaveworks/weave-gitops/core/server/types" pb "github.com/weaveworks/weave-gitops/pkg/api/core" "github.com/weaveworks/weave-gitops/pkg/kube" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -317,3 +319,88 @@ func TestGetInventoryHelmReleaseWithKubeconfig(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(res.Entries).To(HaveLen(0)) } + +func TestResourceRefToUnstructured(t *testing.T) { + testCases := []struct { + name string + id string + version string + expected *unstructured.Unstructured + expectedErr string + }{ + { + name: "valid id", + id: "test-namespace_test-name_apps_Deployment", + version: "v1", + expected: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "test-name", + "namespace": "test-namespace", + }, + }, + }, + }, + { + name: "invalid id", + id: "foo", + version: "v1", + expectedErr: "unable to parse inventory entry id:", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + res, err := server.ResourceRefToUnstructured(tc.id, tc.version) + if tc.expectedErr != "" { + g.Expect(err).To(MatchError(MatchRegexp(tc.expectedErr))) + } else { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(tc.expected)) + } + }) + } +} + +func TestSanitizeUnstructuredSecret(t *testing.T) { + unstructuredSecret := unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "my-secret", + "namespace": "my-namespace", + }, + "type": "Opaque", + "data": map[string]interface{}{ + "key": "dGVzdA==", + }, + }, + } + + expected := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "my-secret", + "namespace": "my-namespace", + "creationTimestamp": nil, + }, + "type": "Opaque", + "data": map[string]interface{}{ + "redacted": nil, + }, + }, + } + + secret, err := server.SanitizeUnstructuredSecret(unstructuredSecret) + + g := NewGomegaWithT(t) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(secret).To(Equal(expected)) +} From bf10ddf151a46c390e0778fc56745278a7f08491 Mon Sep 17 00:00:00 2001 From: Rana Tarek Hassan Date: Tue, 31 Oct 2023 15:15:51 +0200 Subject: [PATCH 04/10] Rebase with main Fix tests --- core/server/inventory.go | 103 +++++++++++++++++----------------- core/server/inventory_test.go | 6 +- 2 files changed, 56 insertions(+), 53 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 9fd704c10d..e3f7a93117 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -63,7 +63,7 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ if err != nil { return nil, err } - entries, err = getFluxLikeInventory(ctx, client, msg.Name, msg.Namespace, *gvk) + inventoryRefs, err = getFluxLikeInventory(ctx, client, msg.Name, msg.Namespace, *gvk) if err != nil { return nil, fmt.Errorf("failed getting flux like inventory: %w", err) } @@ -74,14 +74,18 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ return nil, fmt.Errorf("failed getting objects with children: %w", err) } + entries := []*pb.InventoryEntry{} clusterUserNamespaces := cs.clustersManager.GetUserNamespaces(auth.Principal(ctx)) - entries, err := unstructuredToInventoryEntry(msg.ClusterName, objsWithChildren, clusterUserNamespaces, cs.healthChecker) - if err != nil { - return nil, fmt.Errorf("failed converting inventory entry: %w", err) + for _, oc := range objsWithChildren { + entry, err := unstructuredToInventoryEntry(msg.ClusterName, *oc, clusterUserNamespaces, cs.healthChecker) + if err != nil { + return nil, fmt.Errorf("failed converting inventory entry: %w", err) + } + entries = append(entries, entry) } return &pb.GetInventoryResponse{ - Entries: resources, + Entries: entries, }, nil } @@ -112,7 +116,7 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient c cs.logger.Error(err, "failed converting inventory entry", "entry", ref) return nil, err } - objects = append(objects, obj) + objects = append(objects, &obj) } return objects, nil @@ -213,49 +217,48 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel return objects, nil } -func unstructuredToInventoryEntry(clusterName string, objWithChildren []*ObjectWithChildren, clusterUserNamespaces map[string][]v1.Namespace, healthChecker health.HealthChecker) ([]*pb.InventoryEntry, error) { - entries := []*pb.InventoryEntry{} - for _, c := range objWithChildren { - unstructuredObj := *c.Object - if unstructuredObj.GetKind() == "Secret" { - sanitizedUnstructuredObj, err := SanitizeUnstructuredSecret(unstructuredObj) - unstructuredObj = *sanitizedUnstructuredObj - if err != nil { - return nil, fmt.Errorf("error sanitizing secrets: %w", err) - } - } - bytes, err := unstructuredObj.MarshalJSON() +func unstructuredToInventoryEntry(clusterName string, objWithChildren ObjectWithChildren, clusterUserNamespaces map[string][]v1.Namespace, healthChecker health.HealthChecker) (*pb.InventoryEntry, error) { + unstructuredObj := *objWithChildren.Object + if unstructuredObj.GetKind() == "Secret" { + var err error + unstructuredObj, err = SanitizeUnstructuredSecret(unstructuredObj) if err != nil { - return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) + return nil, fmt.Errorf("error sanitizing secrets: %w", err) } + } + bytes, err := unstructuredObj.MarshalJSON() + if err != nil { + return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) + } - tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNamespaces) + tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNamespaces) - health, err := healthChecker.Check(unstructuredObj) - if err != nil { - return nil, fmt.Errorf("failed to check health: %w", err) - } + health, err := healthChecker.Check(unstructuredObj) + if err != nil { + return nil, fmt.Errorf("failed to check health: %w", err) + } - children, err := unstructuredToInventoryEntry(clusterName, c.Children, clusterUserNamespaces, healthChecker) + children := []*pb.InventoryEntry{} + for _, c := range objWithChildren.Children { + child, err := unstructuredToInventoryEntry(clusterName, *c, clusterUserNamespaces, healthChecker) if err != nil { return nil, fmt.Errorf("failed converting child inventory entry: %w", err) } + children = append(children, child) + } - entry := &pb.InventoryEntry{ - Payload: string(bytes), - Tenant: tenant, - ClusterName: clusterName, - Children: children, - Health: &pb.HealthStatus{ - Status: string(health.Status), - Message: health.Message, - }, - } - - entries = append(entries, entry) + entry := &pb.InventoryEntry{ + Payload: string(bytes), + Tenant: tenant, + ClusterName: clusterName, + Children: children, + Health: &pb.HealthStatus{ + Status: string(health.Status), + Message: health.Message, + }, } - return entries, nil + return entry, nil } // GetObjectsWithChildren returns objects with their children populated if withChildren is true. @@ -281,7 +284,7 @@ func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstruc children := []*ObjectWithChildren{} if withChildren { var err error - children, err = getChildren(ctx, k8sClient, obj) + children, err = GetChildren(ctx, k8sClient, obj) if err != nil { logger.Error(err, "failed getting children", "entry", obj) return @@ -304,7 +307,7 @@ func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstruc return result, nil } -func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { +func GetChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { listResult := unstructured.UnstructuredList{} switch parentObj.GetObjectKind().GroupVersionKind().Kind { @@ -348,7 +351,7 @@ func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc for _, c := range unstructuredChildren { var err error - children, err = getChildren(ctx, k8sClient, c) + children, err = GetChildren(ctx, k8sClient, c) if err != nil { return nil, err } @@ -364,12 +367,12 @@ func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc } // ResourceRefToUnstructured converts a flux like resource entry pair of (id, version) into a unstructured object -func ResourceRefToUnstructured(id, version string) (*unstructured.Unstructured, error) { - u := &unstructured.Unstructured{} +func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, error) { + u := unstructured.Unstructured{} objMetadata, err := object.ParseObjMetadata(id) if err != nil { - return nil, fmt.Errorf("unable to parse inventory entry id: %w", err) + return u, err } u.SetGroupVersionKind(schema.GroupVersionKind{ @@ -384,20 +387,20 @@ func ResourceRefToUnstructured(id, version string) (*unstructured.Unstructured, } // SanitizeUnstructuredSecret redacts the data field of a Secret object -func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (*unstructured.Unstructured, error) { - redactedUnstructured := &unstructured.Unstructured{} +func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { + redactedUnstructured := unstructured.Unstructured{} s := &v1.Secret{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), s) if err != nil { - return nil, fmt.Errorf("converting unstructured to secret: %w", err) + return redactedUnstructured, fmt.Errorf("converting unstructured to helmrelease: %w", err) } s.Data = map[string][]byte{"redacted": []byte(nil)} redactedObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(s) if err != nil { - return nil, fmt.Errorf("converting unstructured to secret: %w", err) + return redactedUnstructured, fmt.Errorf("converting unstructured to helmrelease: %w", err) } redactedUnstructured.SetUnstructuredContent(redactedObj) @@ -439,8 +442,8 @@ func parseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstruct } objects := []*unstructured.Unstructured{} - for _, entry := range resourceInventory.Entries { - u, err := resourceRefToUnstructured(entry) + for _, ref := range resourceInventory.Entries { + u, err := ResourceRefToUnstructured(ref.ID, ref.Version) if err != nil { return nil, fmt.Errorf("error converting resource ref to unstructured: %w", err) } diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 3975a957bb..c8b06cb468 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -347,7 +347,7 @@ func TestResourceRefToUnstructured(t *testing.T) { name: "invalid id", id: "foo", version: "v1", - expectedErr: "unable to parse inventory entry id:", + expectedErr: "unable to parse stored object metadata:", }, } @@ -360,7 +360,7 @@ func TestResourceRefToUnstructured(t *testing.T) { g.Expect(err).To(MatchError(MatchRegexp(tc.expectedErr))) } else { g.Expect(err).NotTo(HaveOccurred()) - g.Expect(res).To(Equal(tc.expected)) + g.Expect(&res).To(Equal(tc.expected)) } }) } @@ -402,5 +402,5 @@ func TestSanitizeUnstructuredSecret(t *testing.T) { g := NewGomegaWithT(t) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(secret).To(Equal(expected)) + g.Expect(&secret).To(Equal(expected)) } From af298fdf9de8884fd750d28a78c0814ddcb1bf41 Mon Sep 17 00:00:00 2001 From: Rana Tarek Hassan Date: Mon, 6 Nov 2023 15:09:49 +0200 Subject: [PATCH 05/10] Remove namespace check in getHelmReleaseInventory --- core/server/inventory.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index e3f7a93117..d99381e890 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -139,13 +139,6 @@ func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, k8sClient cli return nil, fmt.Errorf("failed to get helm release objects: %w", err) } - // FIXME: do we need this? - for _, obj := range objects { - if obj.GetNamespace() == "" { - obj.SetNamespace(namespace) - } - } - return objects, nil } From 5c3183c6502e1ea1f2881d2f245926706ed543a8 Mon Sep 17 00:00:00 2001 From: Rana Tarek Hassan <17128393+ranatrk@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:31:01 +0200 Subject: [PATCH 06/10] Update ObjectWithChildren doc string to include unstructured type Co-authored-by: Kevin McDermott --- core/server/inventory.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index d99381e890..c23d5158a7 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -28,7 +28,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// an object that can store unstructued and its children +// ObjectWithChildren is a recursive data structure containing a tree of Unstructured +// values. type ObjectWithChildren struct { Object *unstructured.Unstructured Children []*ObjectWithChildren From 59a610122d01b296bb81f5bea3783e978ceb85d0 Mon Sep 17 00:00:00 2001 From: Rana Tarek Hassan Date: Mon, 6 Nov 2023 15:34:52 +0200 Subject: [PATCH 07/10] Change SanitizeUnstructuredSecret to private sanitizeUnstructuredSecret in inventory and tests --- core/server/inventory.go | 6 ++-- core/server/inventory_internal_test.go | 39 ++++++++++++++++++++++++++ core/server/inventory_test.go | 39 -------------------------- 3 files changed, 42 insertions(+), 42 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index c23d5158a7..32f37c0cbe 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -215,7 +215,7 @@ func unstructuredToInventoryEntry(clusterName string, objWithChildren ObjectWith unstructuredObj := *objWithChildren.Object if unstructuredObj.GetKind() == "Secret" { var err error - unstructuredObj, err = SanitizeUnstructuredSecret(unstructuredObj) + unstructuredObj, err = sanitizeUnstructuredSecret(unstructuredObj) if err != nil { return nil, fmt.Errorf("error sanitizing secrets: %w", err) } @@ -380,8 +380,8 @@ func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, e return u, nil } -// SanitizeUnstructuredSecret redacts the data field of a Secret object -func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { +// sanitizeUnstructuredSecret redacts the data field of a Secret object +func sanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { redactedUnstructured := unstructured.Unstructured{} s := &v1.Secret{} diff --git a/core/server/inventory_internal_test.go b/core/server/inventory_internal_test.go index 49446d18b7..a7ee0380db 100644 --- a/core/server/inventory_internal_test.go +++ b/core/server/inventory_internal_test.go @@ -218,3 +218,42 @@ func TestParseInventoryFromUnstructured(t *testing.T) { }) } } + +func TestSanitizeUnstructuredSecret(t *testing.T) { + unstructuredSecret := unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "my-secret", + "namespace": "my-namespace", + }, + "type": "Opaque", + "data": map[string]interface{}{ + "key": "dGVzdA==", + }, + }, + } + + expected := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "my-secret", + "namespace": "my-namespace", + "creationTimestamp": nil, + }, + "type": "Opaque", + "data": map[string]interface{}{ + "redacted": nil, + }, + }, + } + + secret, err := sanitizeUnstructuredSecret(unstructuredSecret) + + g := NewGomegaWithT(t) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(&secret).To(Equal(expected)) +} diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index c8b06cb468..45f536981f 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -365,42 +365,3 @@ func TestResourceRefToUnstructured(t *testing.T) { }) } } - -func TestSanitizeUnstructuredSecret(t *testing.T) { - unstructuredSecret := unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Secret", - "metadata": map[string]interface{}{ - "name": "my-secret", - "namespace": "my-namespace", - }, - "type": "Opaque", - "data": map[string]interface{}{ - "key": "dGVzdA==", - }, - }, - } - - expected := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Secret", - "metadata": map[string]interface{}{ - "name": "my-secret", - "namespace": "my-namespace", - "creationTimestamp": nil, - }, - "type": "Opaque", - "data": map[string]interface{}{ - "redacted": nil, - }, - }, - } - - secret, err := server.SanitizeUnstructuredSecret(unstructuredSecret) - - g := NewGomegaWithT(t) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(&secret).To(Equal(expected)) -} From 33061864da3c9e91cad60b65e580bf57c80c9010 Mon Sep 17 00:00:00 2001 From: Rana Tarek Hassan Date: Tue, 7 Nov 2023 13:51:46 +0200 Subject: [PATCH 08/10] Change node_modules installation in makefile to use lockfile instead f pure --- Makefile | 2 +- gitops-server.dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index acc55d63aa..34f997a012 100644 --- a/Makefile +++ b/Makefile @@ -164,7 +164,7 @@ ui: node_modules $(shell find ui -type f) ## Build the UI node_modules: ## Install node modules rm -rf .parcel-cache - yarn config set network-timeout 600000 && yarn --pure-lockfile + yarn config set network-timeout 600000 && yarn --frozen-lockfile ui-lint: ## Run linter against the UI yarn lint diff --git a/gitops-server.dockerfile b/gitops-server.dockerfile index 920d35c25f..54ab6f8945 100644 --- a/gitops-server.dockerfile +++ b/gitops-server.dockerfile @@ -5,7 +5,7 @@ RUN mkdir -p /home/app && chown -R node:node /home/app WORKDIR /home/app USER node COPY --chown=node:node package*.json /home/app/ -COPY --chown=node:node yarn.lock /home/app +COPY --chown=node:node yarn.lock /home/app/ COPY --chown=node:node Makefile /home/app/ COPY --chown=node:node tsconfig.json /home/app/ COPY --chown=node:node .parcelrc /home/app/ From ada2561a1d5ab74d7404c1656d754811ad900cb7 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 7 Nov 2023 14:14:40 +0100 Subject: [PATCH 09/10] Check that we can list children in another ns --- core/server/inventory_test.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 45f536981f..20b26190ac 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -40,10 +40,19 @@ func TestGetInventoryKustomization(t *testing.T) { }, } + anotherNs := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "another-namespace", + Labels: map[string]string{ + "toolkit.fluxcd.io/tenant": "tenant", + }, + }, + } + deployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "my-deployment", - Namespace: ns.Name, + Namespace: "another-namespace", UID: "this-is-not-an-uid", }, Spec: appsv1.DeploymentSpec{ @@ -69,7 +78,7 @@ func TestGetInventoryKustomization(t *testing.T) { rs := &appsv1.ReplicaSet{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-123abcd", automationName), - Namespace: ns.Name, + Namespace: "another-namespace", }, Spec: appsv1.ReplicaSetSpec{ Template: deployment.Spec.Template, @@ -101,7 +110,7 @@ func TestGetInventoryKustomization(t *testing.T) { Inventory: &kustomizev1.ResourceInventory{ Entries: []kustomizev1.ResourceRef{ { - ID: fmt.Sprintf("%s_%s_apps_Deployment", ns.Name, deployment.Name), + ID: fmt.Sprintf("%s_%s_apps_Deployment", "another-namespace", deployment.Name), Version: "v1", }, }, @@ -112,7 +121,7 @@ func TestGetInventoryKustomization(t *testing.T) { scheme, err := kube.CreateScheme() g.Expect(err).To(BeNil()) - client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(&ns, kust, deployment, rs).Build() + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(&ns, &anotherNs, kust, deployment, rs).Build() cfg := makeServerConfig(client, t, "") c := makeServer(cfg, t) From 71167b3dd0c7d3dbb661f617655eb417cc836064 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 7 Nov 2023 17:56:46 +0100 Subject: [PATCH 10/10] Don't expose GetChildren --- core/server/inventory.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 32f37c0cbe..4cc4bd4c43 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -278,7 +278,7 @@ func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstruc children := []*ObjectWithChildren{} if withChildren { var err error - children, err = GetChildren(ctx, k8sClient, obj) + children, err = getChildren(ctx, k8sClient, obj) if err != nil { logger.Error(err, "failed getting children", "entry", obj) return @@ -301,7 +301,7 @@ func GetObjectsWithChildren(ctx context.Context, objects []*unstructured.Unstruc return result, nil } -func GetChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { +func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { listResult := unstructured.UnstructuredList{} switch parentObj.GetObjectKind().GroupVersionKind().Kind { @@ -345,7 +345,7 @@ func GetChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc for _, c := range unstructuredChildren { var err error - children, err = GetChildren(ctx, k8sClient, c) + children, err = getChildren(ctx, k8sClient, c) if err != nil { return nil, err }