From e9e208aa9106ab535bf851f1b7c1f801f79f051b Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 17 Oct 2023 17:38:07 +0200 Subject: [PATCH 01/17] Refactor /inventory code to expose some helpers - That can be used by other similar inventory endpoints --- core/server/inventory.go | 230 ++++++++++++++++++++------------------- 1 file changed, 121 insertions(+), 109 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 62215a9e5b..9af3a198b9 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 []*pb.InventoryEntry + var inventoryRefs []*unstructured.Unstructured switch msg.Kind { case kustomizev1.KustomizationKind: - entries, err = cs.getKustomizationInventory(ctx, msg.ClusterName, client, msg.Name, msg.Namespace, msg.WithChildren) + 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, msg.ClusterName, client, msg.Name, msg.Namespace, msg.WithChildren) + inventoryRefs, err = cs.getHelmReleaseInventory(ctx, client, msg.Name, msg.Namespace) if err != nil { return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) } @@ -54,71 +62,60 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ return nil, fmt.Errorf("unknown kind: %s", msg.Kind) } + 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: entries, }, nil } -func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { - kust := &kustomizev1.Kustomization{ +func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { + 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 } - result := []*pb.InventoryEntry{} - resultMu := sync.Mutex{} - - wg := sync.WaitGroup{} - - for _, e := range kust.Status.Inventory.Entries { - wg.Add(1) - - go func(ref kustomizev1.ResourceRef) { - defer wg.Done() - - obj, err := resourceRefToUnstructured(ref) - if err != nil { - cs.logger.Error(err, "failed converting inventory entry", "entry", ref) - return - } - - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(&obj), &obj); err != nil { - cs.logger.Error(err, "failed to get object", "entry", ref) - return - } - - entry, err := cs.unstructuredToInventoryEntry(ctx, clusterName, k8sClient, obj, namespace, withChildren) - if err != nil { - cs.logger.Error(err, "failed converting inventory entry", "entry", ref) - return - } - - resultMu.Lock() - result = append(result, entry) - resultMu.Unlock() - }(e) + objects := []*unstructured.Unstructured{} + for _, ref := range ks.Status.Inventory.Entries { + obj, err := ResourceRefToUnstructured(ref.ID, ref.Version) + if err != nil { + cs.logger.Error(err, "failed converting inventory entry", "entry", ref) + return nil, err + } + objects = append(objects, &obj) } - wg.Wait() - - return result, nil + return objects, nil } -func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { +func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { release := &helmv2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -135,45 +132,14 @@ func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName s return nil, fmt.Errorf("failed to get helm release objects: %w", err) } - if len(objects) == 0 { - return []*pb.InventoryEntry{}, nil - } - - 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 obj.GetNamespace() == "" { - obj.SetNamespace(release.GetReleaseNamespace()) - } - - 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, nil + return objects, nil } // Returns the list of resources applied in the helm chart. @@ -244,36 +210,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{ @@ -290,7 +254,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 { @@ -307,10 +314,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) } @@ -330,24 +337,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 } @@ -355,7 +367,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) @@ -363,7 +375,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 13a96298f25bd7e0adc59b12d74e624219b016b9 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Wed, 18 Oct 2023 15:25:33 +0200 Subject: [PATCH 02/17] 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 9af3a198b9..6a33c8d0f8 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -67,14 +67,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{ @@ -210,50 +206,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{} @@ -274,7 +274,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 @@ -297,7 +297,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 { @@ -341,7 +341,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 } @@ -356,6 +356,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{} @@ -375,6 +376,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 10d0da20a3a582456016a62116c750abefcb9fb0 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Thu, 19 Oct 2023 15:18:01 +0200 Subject: [PATCH 03/17] Handle unstructured reconcilation of "ks-like" resoureces - Expose useInventory hook too --- core/server/inventory.go | 56 +++++++++++++++++++++++++++++++++++++++- ui/hooks/inventory.ts | 1 + ui/index.ts | 2 ++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 6a33c8d0f8..ba41b295aa 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -59,7 +59,14 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) } default: - return nil, fmt.Errorf("unknown kind: %s", msg.Kind) + gvk, err := cs.primaryKinds.Lookup(msg.Kind) + if err != nil { + return nil, err + } + inventoryRefs, err = cs.getUnknownInventory(ctx, client, msg.Name, msg.Namespace, *gvk) + if err != nil { + return nil, fmt.Errorf("failed getting %s inventory: %w", msg.Kind, err) + } } objsWithChildren, err := GetObjectsWithChildren(ctx, inventoryRefs, client, msg.WithChildren, cs.logger) @@ -111,6 +118,53 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient c return objects, nil } +func (cs *coreServer) getUnknownInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { + // Create an unstructured object with the desired GVK (GroupVersionKind) + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + obj.SetName(name) + obj.SetNamespace(namespace) + + // Get the object from the Kubernetes cluster + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { + return nil, fmt.Errorf("failed to get kustomization: %w", err) + } + + content := obj.UnstructuredContent() + + // Check if status.inventory is present + inventory, found, err := unstructured.NestedMap(content, "status", "inventory") + if err != nil || !found { + return nil, nil + } + + // Check if status.inventory.entries is present + entries, found, err := unstructured.NestedSlice(inventory, "entries") + if err != nil || !found { + return nil, nil + } + + objects := []*unstructured.Unstructured{} + for _, entryInterface := range entries { + entry, ok := entryInterface.(map[string]interface{}) + if !ok { + // Handle error, the type is not as expected + continue + } + + id, _, _ := unstructured.NestedString(entry, "ID") + version, _, _ := unstructured.NestedString(entry, "Version") + obj, err := ResourceRefToUnstructured(id, version) + if err != nil { + cs.logger.Error(err, "failed converting inventory entry", "entry", entry) + return nil, err + } + objects = append(objects, &obj) + } + + return objects, nil +} + func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { release := &helmv2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ diff --git a/ui/hooks/inventory.ts b/ui/hooks/inventory.ts index 4bad1edac4..96966c863d 100644 --- a/ui/hooks/inventory.ts +++ b/ui/hooks/inventory.ts @@ -40,6 +40,7 @@ export function useGetInventory( opts ); } + function convertEntries(entries: InventoryEntry[]) { return entries.map((obj) => { const parsedObj = new FluxObject(obj); diff --git a/ui/index.ts b/ui/index.ts index 28c0376f17..219368fd4e 100644 --- a/ui/index.ts +++ b/ui/index.ts @@ -98,6 +98,7 @@ import { useToggleSuspend, } from "./hooks/flux"; import { useCheckCRDInstalled } from "./hooks/imageautomation"; +import { useGetInventory } from "./hooks/inventory"; import useNavigation from "./hooks/navigation"; import { useListAlerts, useListProviders } from "./hooks/notifications"; import { useGetObject, useListObjects } from "./hooks/objects"; @@ -268,6 +269,7 @@ export { useDebounce, useFeatureFlags, useGetObject, + useGetInventory, useLinkResolver, useListAlerts, useListAutomations, From 6a3dc48a7a6ee8057129ab00d534589ca17b6a49 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Thu, 19 Oct 2023 16:42:27 +0200 Subject: [PATCH 04/17] Use correc json field names for inv structs --- core/server/inventory.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index ba41b295aa..67140eca39 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -48,22 +48,22 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ var inventoryRefs []*unstructured.Unstructured switch msg.Kind { - case kustomizev1.KustomizationKind: - 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: inventoryRefs, err = cs.getHelmReleaseInventory(ctx, client, msg.Name, msg.Namespace) if err != nil { return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) } + // case kustomizev1.KustomizationKind: + // inventoryRefs, err = cs.getKustomizationInventory(ctx, client, msg.Name, msg.Namespace) + // if err != nil { + // return nil, fmt.Errorf("failed getting kustomization inventory: %w", err) + // } default: gvk, err := cs.primaryKinds.Lookup(msg.Kind) if err != nil { return nil, err } - inventoryRefs, err = cs.getUnknownInventory(ctx, client, msg.Name, msg.Namespace, *gvk) + inventoryRefs, err = cs.getUnstructedInventory(ctx, client, msg.Name, msg.Namespace, *gvk) if err != nil { return nil, fmt.Errorf("failed getting %s inventory: %w", msg.Kind, err) } @@ -118,7 +118,7 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient c return objects, nil } -func (cs *coreServer) getUnknownInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { +func (cs *coreServer) getUnstructedInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { // Create an unstructured object with the desired GVK (GroupVersionKind) obj := &unstructured.Unstructured{} obj.SetGroupVersionKind(gvk) @@ -152,8 +152,8 @@ func (cs *coreServer) getUnknownInventory(ctx context.Context, k8sClient client. continue } - id, _, _ := unstructured.NestedString(entry, "ID") - version, _, _ := unstructured.NestedString(entry, "Version") + id, _, _ := unstructured.NestedString(entry, "id") + version, _, _ := unstructured.NestedString(entry, "v") obj, err := ResourceRefToUnstructured(id, version) if err != nil { cs.logger.Error(err, "failed converting inventory entry", "entry", entry) From 5bc52cff2eb1fa3c82ee81f3ba898adee1d39a4f Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Fri, 20 Oct 2023 12:55:34 +0200 Subject: [PATCH 05/17] Adds support for syncing/suspending/inventorying generic resources --- core/fluxsync/adapters.go | 64 +++++++++++- core/server/inventory.go | 131 ++++++++++++----------- core/server/inventory_test.go | 188 ++++++++++++++++++++++++++++++++++ core/server/suspend.go | 14 +-- core/server/sync.go | 48 +++------ 5 files changed, 336 insertions(+), 109 deletions(-) diff --git a/core/fluxsync/adapters.go b/core/fluxsync/adapters.go index f403db2e9f..c8591fc8cc 100644 --- a/core/fluxsync/adapters.go +++ b/core/fluxsync/adapters.go @@ -1,8 +1,6 @@ package fluxsync import ( - "errors" - helmv2 "github.com/fluxcd/helm-controller/api/v2beta1" imgautomationv1 "github.com/fluxcd/image-automation-controller/api/v1beta1" reflectorv1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" @@ -10,6 +8,9 @@ import ( "github.com/fluxcd/pkg/apis/meta" sourcev1 "github.com/fluxcd/source-controller/api/v1" sourcev1b2 "github.com/fluxcd/source-controller/api/v1beta2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -302,6 +303,49 @@ func (obj ImageUpdateAutomationAdapter) DeepCopyClientObject() client.Object { return obj.DeepCopy() } +type UnstructuredAdapter struct { + *unstructured.Unstructured +} + +func (obj UnstructuredAdapter) GetLastHandledReconcileRequest() string { + if val, found, _ := unstructured.NestedString(obj.Object, "status", "lastHandledReconcileAt"); found { + return val + } + return "" +} + +func (obj UnstructuredAdapter) GetConditions() []metav1.Condition { + conditionsSlice, found, err := unstructured.NestedSlice(obj.Object, "status", "conditions") + if !found || err != nil { + return nil + } + + var conditions []metav1.Condition + for _, c := range conditionsSlice { + var condition metav1.Condition + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(c.(map[string]interface{}), &condition); err != nil { + continue + } + conditions = append(conditions, condition) + } + + return conditions +} + +func (obj UnstructuredAdapter) AsClientObject() client.Object { + // Important for the client reflection stuff to work + // We can't return just `obj` here as it seems to break stuff. + return obj.Unstructured +} + +func (obj UnstructuredAdapter) SetSuspended(suspend bool) { + unstructured.SetNestedField(obj.Object, suspend, "spec", "suspend") +} + +func (obj UnstructuredAdapter) DeepCopyClientObject() client.Object { + return obj.DeepCopy() +} + type sRef struct { apiVersion string name string @@ -325,8 +369,8 @@ func (s sRef) Kind() string { return s.kind } -func ToReconcileable(kind string) (client.ObjectList, Reconcilable, error) { - switch kind { +func ToReconcileable(gvk schema.GroupVersionKind) (client.ObjectList, Reconcilable, error) { + switch gvk.Kind { case kustomizev1.KustomizationKind: return &kustomizev1.KustomizationList{}, NewReconcileable(&kustomizev1.Kustomization{}), nil @@ -355,5 +399,15 @@ func ToReconcileable(kind string) (client.ObjectList, Reconcilable, error) { return &imgautomationv1.ImageUpdateAutomationList{}, NewReconcileable(&imgautomationv1.ImageUpdateAutomation{}), nil } - return nil, nil, errors.New("could not find source type") + return ToUnstructuredReconcilable(gvk) +} + +func ToUnstructuredReconcilable(gvk schema.GroupVersionKind) (client.ObjectList, Reconcilable, error) { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + + objList := &unstructured.UnstructuredList{} + objList.SetGroupVersionKind(gvk) + + return objList, UnstructuredAdapter{Unstructured: obj}, nil } diff --git a/core/server/inventory.go b/core/server/inventory.go index 67140eca39..b1e3255408 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -6,6 +6,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "io" "strings" @@ -53,17 +54,17 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ if err != nil { return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) } - // case kustomizev1.KustomizationKind: - // inventoryRefs, err = cs.getKustomizationInventory(ctx, client, msg.Name, msg.Namespace) - // if err != nil { - // return nil, fmt.Errorf("failed getting kustomization inventory: %w", err) - // } + case kustomizev1.KustomizationKind: + inventoryRefs, err = cs.getKustomizationInventory(ctx, client, msg.Name, msg.Namespace) + if err != nil { + return nil, fmt.Errorf("failed getting kustomization inventory: %w", err) + } default: gvk, err := cs.primaryKinds.Lookup(msg.Kind) if err != nil { return nil, err } - inventoryRefs, err = cs.getUnstructedInventory(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 %s inventory: %w", msg.Kind, err) } @@ -109,55 +110,7 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient c for _, ref := range ks.Status.Inventory.Entries { obj, err := ResourceRefToUnstructured(ref.ID, ref.Version) if err != nil { - cs.logger.Error(err, "failed converting inventory entry", "entry", ref) - return nil, err - } - objects = append(objects, &obj) - } - - return objects, nil -} - -func (cs *coreServer) getUnstructedInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { - // Create an unstructured object with the desired GVK (GroupVersionKind) - obj := &unstructured.Unstructured{} - obj.SetGroupVersionKind(gvk) - obj.SetName(name) - obj.SetNamespace(namespace) - - // Get the object from the Kubernetes cluster - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { - return nil, fmt.Errorf("failed to get kustomization: %w", err) - } - - content := obj.UnstructuredContent() - - // Check if status.inventory is present - inventory, found, err := unstructured.NestedMap(content, "status", "inventory") - if err != nil || !found { - return nil, nil - } - - // Check if status.inventory.entries is present - entries, found, err := unstructured.NestedSlice(inventory, "entries") - if err != nil || !found { - return nil, nil - } - - objects := []*unstructured.Unstructured{} - for _, entryInterface := range entries { - entry, ok := entryInterface.(map[string]interface{}) - if !ok { - // Handle error, the type is not as expected - continue - } - - id, _, _ := unstructured.NestedString(entry, "id") - version, _, _ := unstructured.NestedString(entry, "v") - obj, err := ResourceRefToUnstructured(id, version) - if err != nil { - cs.logger.Error(err, "failed converting inventory entry", "entry", entry) - return nil, err + return nil, fmt.Errorf("failed converting inventory entry: %w", err) } objects = append(objects, &obj) } @@ -182,13 +135,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 } @@ -257,6 +203,67 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel return nil, fmt.Errorf("failed to read the Helm storage object for HelmRelease '%s': %w", helmRelease.Name, err) } + // FIXME: do we need this? + for _, obj := range objects { + if obj.GetNamespace() == "" { + obj.SetNamespace(helmRelease.Namespace) + } + } + + return objects, nil +} + +// GetFluxLikeInventory returns the inventory on a resource if +// it matches the structure of the flux inventory format (e.g. kustomizations) +// It returns an error if the inventory is not as expected +func GetFluxLikeInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { + // Create an unstructured object with the desired GVK (GroupVersionKind) + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + obj.SetName(name) + obj.SetNamespace(namespace) + + // Get the object from the Kubernetes cluster + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { + return nil, fmt.Errorf("failed to get kustomization: %w", err) + } + + return ParseInventoryFromUnstructured(obj) +} + +// Parse the inventory from an unstructured object +// It returns an error if the inventory is not as expected (should look like a kustomization's inventory) +func ParseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { + content := obj.UnstructuredContent() + + // Check if status.inventory is present + inventory, found, err := unstructured.NestedMap(content, "status", "inventory") + if err != nil || !found { + return nil, errors.New("no status.inventory found on resource, it hasn't been synced yet or is not queryable from this endpoint") + } + + // Check if status.inventory.entries is present + entries, found, err := unstructured.NestedSlice(inventory, "entries") + if err != nil || !found { + return nil, nil + } + + objects := []*unstructured.Unstructured{} + for _, entryInterface := range entries { + entry, ok := entryInterface.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("failed converting inventory entry to map[string]interface{}: %+v", entry) + } + + id, _, _ := unstructured.NestedString(entry, "id") + version, _, _ := unstructured.NestedString(entry, "v") + obj, err := ResourceRefToUnstructured(id, version) + if err != nil { + return nil, fmt.Errorf("failed converting inventory entry: %w", err) + } + objects = append(objects, &obj) + } + return objects, nil } diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 16cc1138b6..8882ffb7f7 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "testing" @@ -13,12 +14,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 +320,188 @@ func TestGetInventoryHelmReleaseWithKubeconfig(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(res.Entries).To(HaveLen(0)) } + +func TestGetFluxLikeInventory(t *testing.T) { + g := NewGomegaWithT(t) + + ctx := context.Background() + + ks := &kustomizev1.Kustomization{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-kustomization", + Namespace: "my-namespace", + }, + Spec: kustomizev1.KustomizationSpec{ + SourceRef: kustomizev1.CrossNamespaceSourceReference{ + Kind: sourcev1.GitRepositoryKind, + }, + }, + Status: kustomizev1.KustomizationStatus{ + Inventory: &kustomizev1.ResourceInventory{ + Entries: []kustomizev1.ResourceRef{ + { + ID: "my-namespace_my-deployment_apps_Deployment", + Version: "v1", + }, + }, + }, + }, + } + + scheme, err := kube.CreateScheme() + g.Expect(err).To(BeNil()) + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ks).Build() + + gvk := kustomizev1.GroupVersion.WithKind("Kustomization") + entries, err := server.GetFluxLikeInventory(ctx, k8sClient, ks.Name, ks.Namespace, gvk) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(entries).To(HaveLen(1)) + + expected := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "my-deployment", + "namespace": "my-namespace", + }, + }, + } + + g.Expect(entries[0]).To(Equal(expected)) +} + +func TestParseInventoryFromUnstructured(t *testing.T) { + // inv lives at status.inventory.entries + stdErr := errors.New("no status.inventory found on resource, it hasn't been synced yet or is not queryable from this endpoint") + + testCases := []struct { + name string + obj *unstructured.Unstructured + expected []*unstructured.Unstructured + expectedErr error + }{ + { + name: "no status field", + obj: &unstructured.Unstructured{}, + expected: nil, + expectedErr: stdErr, + }, + { + name: "empty status", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{}, + }, + }, + expected: nil, + expectedErr: stdErr, + }, + { + name: "empty inventory", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{}, + }, + }, + }, + expected: nil, + }, + { + name: "empty entry item", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{ + "entries": []interface{}{ + map[string]interface{}{}, + }, + }, + }, + }, + }, + expected: nil, + expectedErr: errors.New("unable to parse stored object metadata: "), + }, + { + name: "invalid inventory", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{ + "entries": []interface{}{ + map[string]interface{}{ + "v": "v1", + "id": "foo", + }, + }, + }, + }, + }, + }, + expected: nil, + expectedErr: errors.New("unable to parse stored object metadata: foo"), + }, + { + name: "valid inventory", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{ + "entries": []interface{}{ + map[string]interface{}{ + "v": "v1", + "id": "my-namespace_my-deployment_apps_Deployment", + }, + map[string]interface{}{ + "v": "v1", + "id": "my-other-namespace_my-configmap__ConfigMap", + }, + }, + }, + }, + }, + }, + expected: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "my-deployment", + "namespace": "my-namespace", + }, + }, + }, + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "my-configmap", + "namespace": "my-other-namespace", + }, + }, + }, + }, + expectedErr: nil, + }, + } + + for _, tc := range testCases { + // subtests... + t.Run(tc.name, func(tt *testing.T) { + gg := NewGomegaWithT(tt) + // Parse inventory from unstructured + entries, err := server.ParseInventoryFromUnstructured(tc.obj) + + if err != nil || tc.expectedErr != nil { + gg.Expect(err).To(MatchError(tc.expectedErr)) + } + + gg.Expect(entries).To(ConsistOf(tc.expected)) + }) + } +} diff --git a/core/server/suspend.go b/core/server/suspend.go index 3f3411adc1..9ec761f8f8 100644 --- a/core/server/suspend.go +++ b/core/server/suspend.go @@ -33,7 +33,13 @@ func (cs *coreServer) ToggleSuspendResource(ctx context.Context, msg *pb.ToggleS Namespace: obj.Namespace, } - obj, err := getReconcilableObject(obj.Kind) + gvk, err := cs.primaryKinds.Lookup(obj.Kind) + if err != nil { + respErrors = *multierror.Append(fmt.Errorf("looking up GVK for %q: %w", obj.Kind, err), respErrors.Errors...) + continue + } + + _, obj, err := fluxsync.ToReconcileable(*gvk) if err != nil { respErrors = *multierror.Append(fmt.Errorf("converting to reconcilable source: %w", err), respErrors.Errors...) continue @@ -68,9 +74,3 @@ func (cs *coreServer) ToggleSuspendResource(ctx context.Context, msg *pb.ToggleS return &pb.ToggleSuspendResourceResponse{}, respErrors.ErrorOrNil() } - -func getReconcilableObject(kind string) (fluxsync.Reconcilable, error) { - _, s, err := fluxsync.ToReconcileable(kind) - - return s, err -} diff --git a/core/server/sync.go b/core/server/sync.go index d54007f58c..5ffd86c0c0 100644 --- a/core/server/sync.go +++ b/core/server/sync.go @@ -4,12 +4,6 @@ import ( "context" "fmt" - helmv2 "github.com/fluxcd/helm-controller/api/v2beta1" - imgautomationv1 "github.com/fluxcd/image-automation-controller/api/v1beta1" - reflectorv1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" - kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" - sourcev1 "github.com/fluxcd/source-controller/api/v1" - sourcev1b2 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/hashicorp/go-multierror" "github.com/weaveworks/weave-gitops/core/fluxsync" pb "github.com/weaveworks/weave-gitops/pkg/api/core" @@ -39,7 +33,13 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject Namespace: sync.Namespace, } - obj, err := getFluxObject(sync.Kind) + gvk, err := cs.primaryKinds.Lookup(sync.Kind) + if err != nil { + respErrors = *multierror.Append(fmt.Errorf("looking up GVK for %q: %w", sync.Kind, err), respErrors.Errors...) + continue + } + + _, obj, err := fluxsync.ToReconcileable(*gvk) if err != nil { respErrors = *multierror.Append(fmt.Errorf("error converting to object: %w", err), respErrors.Errors...) continue @@ -54,8 +54,12 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject if msg.WithSource && isAutomation { sourceRef := automation.SourceRef() - _, sourceObj, err := fluxsync.ToReconcileable(sourceRef.Kind()) + sourceGVK, err := cs.primaryKinds.Lookup(sourceRef.Kind()) + if err != nil { + return nil, err + } + _, sourceObj, err := fluxsync.ToReconcileable(*sourceGVK) if err != nil { respErrors = *multierror.Append(fmt.Errorf("getting source type for %q: %w", sourceRef.Kind(), err), respErrors.Errors...) continue @@ -105,8 +109,7 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject ) log.Info("Syncing resource") - gvk := obj.GroupVersionKind() - if err := fluxsync.RequestReconciliation(ctx, c, key, gvk); err != nil { + if err := fluxsync.RequestReconciliation(ctx, c, key, *gvk); err != nil { respErrors = *multierror.Append(fmt.Errorf("requesting reconciliation: %w", err), respErrors.Errors...) continue } @@ -119,28 +122,3 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject return &pb.SyncFluxObjectResponse{}, respErrors.ErrorOrNil() } - -func getFluxObject(kind string) (fluxsync.Reconcilable, error) { - switch kind { - case kustomizev1.KustomizationKind: - return &fluxsync.KustomizationAdapter{Kustomization: &kustomizev1.Kustomization{}}, nil - case helmv2.HelmReleaseKind: - return &fluxsync.HelmReleaseAdapter{HelmRelease: &helmv2.HelmRelease{}}, nil - case sourcev1.GitRepositoryKind: - return &fluxsync.GitRepositoryAdapter{GitRepository: &sourcev1.GitRepository{}}, nil - case sourcev1b2.BucketKind: - return &fluxsync.BucketAdapter{Bucket: &sourcev1b2.Bucket{}}, nil - case sourcev1b2.HelmChartKind: - return &fluxsync.HelmChartAdapter{HelmChart: &sourcev1b2.HelmChart{}}, nil - case sourcev1b2.HelmRepositoryKind: - return &fluxsync.HelmRepositoryAdapter{HelmRepository: &sourcev1b2.HelmRepository{}}, nil - case sourcev1b2.OCIRepositoryKind: - return &fluxsync.OCIRepositoryAdapter{OCIRepository: &sourcev1b2.OCIRepository{}}, nil - case reflectorv1.ImageRepositoryKind: - return &fluxsync.ImageRepositoryAdapter{ImageRepository: &reflectorv1.ImageRepository{}}, nil - case imgautomationv1.ImageUpdateAutomationKind: - return &fluxsync.ImageUpdateAutomationAdapter{ImageUpdateAutomation: &imgautomationv1.ImageUpdateAutomation{}}, nil - } - - return nil, fmt.Errorf("not supported kind: %s", kind) -} From 78b576ed978cc35127472870c7a5004aee6eea2a Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Fri, 20 Oct 2023 13:04:43 +0200 Subject: [PATCH 06/17] Add missing test file oops --- core/fluxsync/adapters_test.go | 115 +++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 core/fluxsync/adapters_test.go diff --git a/core/fluxsync/adapters_test.go b/core/fluxsync/adapters_test.go new file mode 100644 index 0000000000..18be2f865d --- /dev/null +++ b/core/fluxsync/adapters_test.go @@ -0,0 +1,115 @@ +package fluxsync + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestGetLastHandledReconcileRequest(t *testing.T) { + g := NewGomegaWithT(t) + + obj := &UnstructuredAdapter{ + Unstructured: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "lastHandledReconcileAt": "2023-10-20T10:10:10Z", + }, + }, + }, + } + + expected := "2023-10-20T10:10:10Z" + got := obj.GetLastHandledReconcileRequest() + g.Expect(got).To(Equal(expected)) +} + +func TestGetConditions(t *testing.T) { + g := NewGomegaWithT(t) + + condition := v1.Condition{ + Type: "Ready", + Status: "True", + } + unstructuredCondition, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(&condition) + + obj := &UnstructuredAdapter{ + Unstructured: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "conditions": []interface{}{unstructuredCondition}, + }, + }, + }, + } + + conditions := obj.GetConditions() + g.Expect(conditions).To(HaveLen(1)) + g.Expect(conditions[0].Type).To(Equal(condition.Type)) + g.Expect(conditions[0].Status).To(Equal(condition.Status)) +} + +func TestSetSuspended(t *testing.T) { + g := NewGomegaWithT(t) + + obj := &UnstructuredAdapter{ + Unstructured: &unstructured.Unstructured{ + Object: make(map[string]interface{}), + }, + } + + obj.SetSuspended(true) + suspend, _, _ := unstructured.NestedBool(obj.Object, "spec", "suspend") + g.Expect(suspend).To(BeTrue()) +} + +func TestDeepCopyClientObject(t *testing.T) { + g := NewGomegaWithT(t) + + obj := &UnstructuredAdapter{ + Unstructured: &unstructured.Unstructured{ + Object: map[string]interface{}{"key": "value"}, + }, + } + + copy := obj.DeepCopyClientObject().(*unstructured.Unstructured) + g.Expect(copy.Object).To(Equal(obj.Object)) + g.Expect(copy).ToNot(BeIdenticalTo(obj)) +} + +func TestAsClientObjectCompatibilityWithTestClient(t *testing.T) { + g := NewGomegaWithT(t) + + scheme := runtime.NewScheme() + + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + + obj := &UnstructuredAdapter{ + Unstructured: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + "namespace": "default", + }, + }, + }, + } + + err := cl.Create(context.TODO(), obj.AsClientObject()) + g.Expect(err).NotTo(HaveOccurred()) + + retrieved := &unstructured.Unstructured{} + retrieved.SetAPIVersion("v1") + retrieved.SetKind("ConfigMap") + err = cl.Get(context.TODO(), client.ObjectKey{Namespace: "default", Name: "test-cm"}, retrieved) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(retrieved.GetName()).To(Equal("test-cm")) +} From fc3a210e99b8609eb2d81965fa4defb00d561a91 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Fri, 20 Oct 2023 14:45:24 +0200 Subject: [PATCH 07/17] Revert refactor in other branch for now --- core/server/inventory.go | 340 ++++++++++++++++++--------------------- 1 file changed, 157 insertions(+), 183 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index b1e3255408..3b8acad8b5 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -15,10 +15,8 @@ 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" @@ -29,12 +27,6 @@ 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 { @@ -46,39 +38,32 @@ 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 inventoryRefs []*unstructured.Unstructured + var entries []*pb.InventoryEntry switch msg.Kind { - case helmv2.HelmReleaseKind: - inventoryRefs, err = cs.getHelmReleaseInventory(ctx, client, msg.Name, msg.Namespace) - if err != nil { - return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) - } case kustomizev1.KustomizationKind: - inventoryRefs, err = cs.getKustomizationInventory(ctx, client, msg.Name, msg.Namespace) + entries, err = cs.getKustomizationInventory(ctx, msg.ClusterName, client, msg.Name, msg.Namespace, msg.WithChildren) if err != nil { return nil, fmt.Errorf("failed getting kustomization inventory: %w", err) } + case helmv2.HelmReleaseKind: + entries, err = cs.getHelmReleaseInventory(ctx, msg.ClusterName, client, msg.Name, msg.Namespace, msg.WithChildren) + if err != nil { + return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) + } default: gvk, err := cs.primaryKinds.Lookup(msg.Kind) if err != nil { return nil, err } - inventoryRefs, 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 %s inventory: %w", msg.Kind, err) + return nil, fmt.Errorf("failed getting flux like inventory: %w", err) + } + entries, err = cs.getInventoryResources(ctx, msg.ClusterName, client, inventoryRefs, msg.Namespace, msg.WithChildren) + if err != nil { + return nil, fmt.Errorf("failed getting inventory resources: %w", err) } - } - - objsWithChildren, err := GetObjectsWithChildren(ctx, inventoryRefs, client, msg.WithChildren, cs.logger) - if err != nil { - return nil, fmt.Errorf("failed getting objects with children: %w", err) - } - - 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) } return &pb.GetInventoryResponse{ @@ -86,39 +71,39 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ }, nil } -func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { - ks := &kustomizev1.Kustomization{ +func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { + kust := &kustomizev1.Kustomization{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, } - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(ks), ks); err != nil { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(kust), kust); err != nil { return nil, fmt.Errorf("failed to get kustomization: %w", err) } - if ks.Status.Inventory == nil { + if kust.Status.Inventory == nil { return nil, nil } - if ks.Status.Inventory.Entries == nil { + if kust.Status.Inventory.Entries == nil { return nil, nil } objects := []*unstructured.Unstructured{} - for _, ref := range ks.Status.Inventory.Entries { - obj, err := ResourceRefToUnstructured(ref.ID, ref.Version) + for _, e := range kust.Status.Inventory.Entries { + obj, err := resourceRefToUnstructured(e) if err != nil { return nil, fmt.Errorf("failed converting inventory entry: %w", err) } objects = append(objects, &obj) } - return objects, nil + return cs.getInventoryResources(ctx, clusterName, k8sClient, objects, namespace, withChildren) } -func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { +func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { release := &helmv2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -135,7 +120,46 @@ func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, k8sClient cli return nil, fmt.Errorf("failed to get helm release objects: %w", err) } - return objects, nil + if len(objects) == 0 { + return []*pb.InventoryEntry{}, nil + } + + return cs.getInventoryResources(ctx, clusterName, k8sClient, objects, namespace, withChildren) +} + +func (cs *coreServer) getInventoryResources(ctx context.Context, clusterName string, k8sClient client.Client, objects []*unstructured.Unstructured, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { + + 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) + } + + wg.Wait() + + return result, nil } // Returns the list of resources applied in the helm chart. @@ -206,159 +230,60 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel // FIXME: do we need this? for _, obj := range objects { if obj.GetNamespace() == "" { - obj.SetNamespace(helmRelease.Namespace) + obj.SetNamespace(helmRelease.GetNamespace()) } } return objects, nil } -// GetFluxLikeInventory returns the inventory on a resource if -// it matches the structure of the flux inventory format (e.g. kustomizations) -// It returns an error if the inventory is not as expected -func GetFluxLikeInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { - // Create an unstructured object with the desired GVK (GroupVersionKind) - obj := &unstructured.Unstructured{} - obj.SetGroupVersionKind(gvk) - obj.SetName(name) - obj.SetNamespace(namespace) - - // Get the object from the Kubernetes cluster - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { - return nil, fmt.Errorf("failed to get kustomization: %w", err) - } - - return ParseInventoryFromUnstructured(obj) -} - -// Parse the inventory from an unstructured object -// It returns an error if the inventory is not as expected (should look like a kustomization's inventory) -func ParseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { - content := obj.UnstructuredContent() +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 - // Check if status.inventory is present - inventory, found, err := unstructured.NestedMap(content, "status", "inventory") - if err != nil || !found { - return nil, errors.New("no status.inventory found on resource, it hasn't been synced yet or is not queryable from this endpoint") - } - - // Check if status.inventory.entries is present - entries, found, err := unstructured.NestedSlice(inventory, "entries") - if err != nil || !found { - return nil, nil - } - - objects := []*unstructured.Unstructured{} - for _, entryInterface := range entries { - entry, ok := entryInterface.(map[string]interface{}) - if !ok { - return nil, fmt.Errorf("failed converting inventory entry to map[string]interface{}: %+v", entry) - } - - id, _, _ := unstructured.NestedString(entry, "id") - version, _, _ := unstructured.NestedString(entry, "v") - obj, err := ResourceRefToUnstructured(id, version) + if unstructuredObj.GetKind() == "Secret" { + unstructuredObj, err = sanitizeUnstructuredSecret(unstructuredObj) if err != nil { - return nil, fmt.Errorf("failed converting inventory entry: %w", err) + return nil, fmt.Errorf("error sanitizing secrets: %w", err) } - objects = append(objects, &obj) } - return objects, nil -} + children := []*pb.InventoryEntry{} -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 withChildren { + children, err = cs.getChildren(ctx, clusterName, k8sClient, unstructuredObj, ns) if err != nil { - return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) - } - - tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNamespaces) - - 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) - if err != nil { - return nil, fmt.Errorf("failed converting child inventory entry: %w", err) - } - - entry := &pb.InventoryEntry{ - Payload: string(bytes), - Tenant: tenant, - ClusterName: clusterName, - Children: children, - Health: &pb.HealthStatus{ - Status: string(health.Status), - Message: health.Message, - }, + return nil, err } - - entries = append(entries, entry) } - 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{} - - 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 - } - } + bytes, err := unstructuredObj.MarshalJSON() + if err != nil { + return nil, err + } - entry := &ObjectWithChildren{ - Object: &obj, - Children: children, - } + clusterUserNss := cs.clustersManager.GetUserNamespaces(auth.Principal(ctx)) + tenant := GetTenant(unstructuredObj.GetNamespace(), clusterName, clusterUserNss) - resultMu.Lock() - result = append(result, entry) - resultMu.Unlock() - }(*o) + health, err := cs.healthChecker.Check(unstructuredObj) + if err != nil { + return nil, err } - wg.Wait() + entry := &pb.InventoryEntry{ + Payload: string(bytes), + Tenant: tenant, + ClusterName: clusterName, + Children: children, + Health: &pb.HealthStatus{ + Status: string(health.Status), + Message: health.Message, + }, + } - return result, nil + return entry, nil } -func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstructured.Unstructured) ([]*ObjectWithChildren, error) { +func (cs *coreServer) getChildren(ctx context.Context, clusterName string, k8sClient client.Client, parentObj unstructured.Unstructured, ns string) ([]*pb.InventoryEntry, error) { listResult := unstructured.UnstructuredList{} switch parentObj.GetObjectKind().GroupVersionKind().Kind { @@ -375,10 +300,10 @@ func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc Kind: "Pod", }) default: - return []*ObjectWithChildren{}, nil + return []*pb.InventoryEntry{}, nil } - if err := k8sClient.List(ctx, &listResult, client.InNamespace(parentObj.GetNamespace())); err != nil { + if err := k8sClient.List(ctx, &listResult, client.InNamespace(ns)); err != nil { return nil, fmt.Errorf("could not get unstructured object: %s", err) } @@ -398,30 +323,24 @@ func getChildren(ctx context.Context, k8sClient client.Client, parentObj unstruc } } - children := []*ObjectWithChildren{} + children := []*pb.InventoryEntry{} for _, c := range unstructuredChildren { - var err error - children, err = getChildren(ctx, k8sClient, c) + entry, err := cs.unstructuredToInventoryEntry(ctx, clusterName, k8sClient, c, ns, true) if err != nil { return nil, err } - entry := &ObjectWithChildren{ - Object: &c, - Children: children, - } children = append(children, entry) } 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) { +func resourceRefToUnstructured(entry kustomizev1.ResourceRef) (unstructured.Unstructured, error) { u := unstructured.Unstructured{} - objMetadata, err := object.ParseObjMetadata(id) + objMetadata, err := object.ParseObjMetadata(entry.ID) if err != nil { return u, err } @@ -429,7 +348,7 @@ func ResourceRefToUnstructured(id, version string) (unstructured.Unstructured, e u.SetGroupVersionKind(schema.GroupVersionKind{ Group: objMetadata.GroupKind.Group, Kind: objMetadata.GroupKind.Kind, - Version: version, + Version: entry.Version, }) u.SetName(objMetadata.Name) u.SetNamespace(objMetadata.Namespace) @@ -437,8 +356,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) { +func sanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Unstructured, error) { redactedUnstructured := unstructured.Unstructured{} s := &v1.Secret{} @@ -458,3 +376,59 @@ func SanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Uns return redactedUnstructured, nil } + +// GetFluxLikeInventory returns the inventory on a resource if +// it matches the structure of the flux inventory format (e.g. kustomizations) +// It returns an error if the inventory is not as expected +func GetFluxLikeInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { + // Create an unstructured object with the desired GVK (GroupVersionKind) + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + obj.SetName(name) + obj.SetNamespace(namespace) + + // Get the object from the Kubernetes cluster + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { + return nil, fmt.Errorf("failed to get kustomization: %w", err) + } + + return ParseInventoryFromUnstructured(obj) +} + +// Parse the inventory from an unstructured object +// It returns an error if the inventory is not as expected (should look like a kustomization's inventory) +func ParseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { + content := obj.UnstructuredContent() + + // Check if status.inventory is present + inventory, found, err := unstructured.NestedMap(content, "status", "inventory") + if err != nil || !found { + return nil, errors.New("no status.inventory found on resource, it hasn't been synced yet or is not queryable from this endpoint") + } + + // Check if status.inventory.entries is present + entries, found, err := unstructured.NestedSlice(inventory, "entries") + if err != nil || !found { + return nil, nil + } + + objects := []*unstructured.Unstructured{} + for _, entryInterface := range entries { + entry, ok := entryInterface.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("failed converting inventory entry to map[string]interface{}: %+v", entry) + } + ref := kustomizev1.ResourceRef{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(entry, ref) + if err != nil { + return nil, fmt.Errorf("failed converting inventory entry: %w", err) + } + invEntry, err := resourceRefToUnstructured(ref) + if err != nil { + return nil, fmt.Errorf("failed converting inventory entry: %w", err) + } + objects = append(objects, &invEntry) + } + + return objects, nil +} From afe57735750f631206c45de1735b1e19715ec1f6 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Fri, 20 Oct 2023 14:52:47 +0200 Subject: [PATCH 08/17] Share a little bit more code --- core/server/inventory.go | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 3b8acad8b5..e91268dc20 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -38,16 +38,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 []*pb.InventoryEntry + var entries []*unstructured.Unstructured switch msg.Kind { case kustomizev1.KustomizationKind: - entries, err = cs.getKustomizationInventory(ctx, msg.ClusterName, client, msg.Name, msg.Namespace, msg.WithChildren) + entries, err = cs.getKustomizationInventory(ctx, msg.ClusterName, 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, msg.ClusterName, client, msg.Name, msg.Namespace, msg.WithChildren) + entries, err = cs.getHelmReleaseInventory(ctx, msg.ClusterName, client, msg.Name, msg.Namespace) if err != nil { return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) } @@ -56,22 +56,23 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ if err != nil { return nil, err } - inventoryRefs, err := GetFluxLikeInventory(ctx, client, msg.Name, msg.Namespace, *gvk) + entries, err = GetFluxLikeInventory(ctx, client, msg.Name, msg.Namespace, *gvk) if err != nil { return nil, fmt.Errorf("failed getting flux like inventory: %w", err) } - entries, err = cs.getInventoryResources(ctx, msg.ClusterName, client, inventoryRefs, msg.Namespace, msg.WithChildren) - if err != nil { - return nil, fmt.Errorf("failed getting inventory resources: %w", err) - } + } + + resources, err := cs.getInventoryResources(ctx, msg.ClusterName, client, entries, msg.Namespace, msg.WithChildren) + if err != nil { + return nil, fmt.Errorf("failed getting inventory resources: %w", err) } return &pb.GetInventoryResponse{ - Entries: entries, + Entries: resources, }, nil } -func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { +func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { kust := &kustomizev1.Kustomization{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -100,10 +101,10 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName objects = append(objects, &obj) } - return cs.getInventoryResources(ctx, clusterName, k8sClient, objects, namespace, withChildren) + return objects, nil } -func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { +func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { release := &helmv2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -120,15 +121,10 @@ func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName s return nil, fmt.Errorf("failed to get helm release objects: %w", err) } - if len(objects) == 0 { - return []*pb.InventoryEntry{}, nil - } - - return cs.getInventoryResources(ctx, clusterName, k8sClient, objects, namespace, withChildren) + return objects, nil } func (cs *coreServer) getInventoryResources(ctx context.Context, clusterName string, k8sClient client.Client, objects []*unstructured.Unstructured, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { - result := []*pb.InventoryEntry{} resultMu := sync.Mutex{} From a9c9e4f9547de29c81eddddc359ab5a4e373c8d2 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Fri, 20 Oct 2023 15:02:37 +0200 Subject: [PATCH 09/17] Fixes inv parsing --- core/server/inventory.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index e91268dc20..24ac5687d2 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -414,12 +414,12 @@ func ParseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstruct if !ok { return nil, fmt.Errorf("failed converting inventory entry to map[string]interface{}: %+v", entry) } - ref := kustomizev1.ResourceRef{} + ref := &kustomizev1.ResourceRef{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(entry, ref) if err != nil { return nil, fmt.Errorf("failed converting inventory entry: %w", err) } - invEntry, err := resourceRefToUnstructured(ref) + invEntry, err := resourceRefToUnstructured(*ref) if err != nil { return nil, fmt.Errorf("failed converting inventory entry: %w", err) } From 85cc01b5c9209f1f96c8d42ebdb251da5503c338 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Fri, 20 Oct 2023 15:44:21 +0200 Subject: [PATCH 10/17] Fixes some static lint checks --- core/fluxsync/adapters.go | 33 +++++++++++++++++++++------------ core/fluxsync/adapters_test.go | 3 ++- core/server/inventory.go | 17 +++++++---------- core/server/suspend.go | 5 ++++- 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/core/fluxsync/adapters.go b/core/fluxsync/adapters.go index c8591fc8cc..65848291d6 100644 --- a/core/fluxsync/adapters.go +++ b/core/fluxsync/adapters.go @@ -24,7 +24,7 @@ type Reconcilable interface { GetLastHandledReconcileRequest() string AsClientObject() client.Object GroupVersionKind() schema.GroupVersionKind - SetSuspended(suspend bool) + SetSuspended(suspend bool) error DeepCopyClientObject() client.Object } @@ -83,8 +83,9 @@ func (obj GitRepositoryAdapter) GroupVersionKind() schema.GroupVersionKind { return sourcev1.GroupVersion.WithKind(sourcev1.GitRepositoryKind) } -func (obj GitRepositoryAdapter) SetSuspended(suspend bool) { +func (obj GitRepositoryAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj GitRepositoryAdapter) DeepCopyClientObject() client.Object { @@ -107,8 +108,9 @@ func (obj BucketAdapter) GroupVersionKind() schema.GroupVersionKind { return sourcev1b2.GroupVersion.WithKind(sourcev1b2.BucketKind) } -func (obj BucketAdapter) SetSuspended(suspend bool) { +func (obj BucketAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj BucketAdapter) DeepCopyClientObject() client.Object { @@ -131,8 +133,9 @@ func (obj HelmChartAdapter) GroupVersionKind() schema.GroupVersionKind { return sourcev1b2.GroupVersion.WithKind(sourcev1b2.HelmChartKind) } -func (obj HelmChartAdapter) SetSuspended(suspend bool) { +func (obj HelmChartAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj HelmChartAdapter) DeepCopyClientObject() client.Object { @@ -155,8 +158,9 @@ func (obj HelmRepositoryAdapter) GroupVersionKind() schema.GroupVersionKind { return sourcev1b2.GroupVersion.WithKind(sourcev1b2.HelmRepositoryKind) } -func (obj HelmRepositoryAdapter) SetSuspended(suspend bool) { +func (obj HelmRepositoryAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj HelmRepositoryAdapter) DeepCopyClientObject() client.Object { @@ -179,8 +183,9 @@ func (obj OCIRepositoryAdapter) GroupVersionKind() schema.GroupVersionKind { return sourcev1b2.GroupVersion.WithKind(sourcev1b2.OCIRepositoryKind) } -func (obj OCIRepositoryAdapter) SetSuspended(suspend bool) { +func (obj OCIRepositoryAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj OCIRepositoryAdapter) DeepCopyClientObject() client.Object { @@ -214,8 +219,9 @@ func (obj HelmReleaseAdapter) GroupVersionKind() schema.GroupVersionKind { return helmv2.GroupVersion.WithKind(helmv2.HelmReleaseKind) } -func (obj HelmReleaseAdapter) SetSuspended(suspend bool) { +func (obj HelmReleaseAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj HelmReleaseAdapter) DeepCopyClientObject() client.Object { @@ -247,8 +253,9 @@ func (obj KustomizationAdapter) GroupVersionKind() schema.GroupVersionKind { return kustomizev1.GroupVersion.WithKind(kustomizev1.KustomizationKind) } -func (obj KustomizationAdapter) SetSuspended(suspend bool) { +func (obj KustomizationAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj KustomizationAdapter) DeepCopyClientObject() client.Object { @@ -271,8 +278,9 @@ func (obj ImageRepositoryAdapter) GroupVersionKind() schema.GroupVersionKind { return reflectorv1.GroupVersion.WithKind(reflectorv1.ImageRepositoryKind) } -func (obj ImageRepositoryAdapter) SetSuspended(suspend bool) { +func (obj ImageRepositoryAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj ImageRepositoryAdapter) DeepCopyClientObject() client.Object { @@ -295,8 +303,9 @@ func (obj ImageUpdateAutomationAdapter) GroupVersionKind() schema.GroupVersionKi return imgautomationv1.GroupVersion.WithKind(imgautomationv1.ImageUpdateAutomationKind) } -func (obj ImageUpdateAutomationAdapter) SetSuspended(suspend bool) { +func (obj ImageUpdateAutomationAdapter) SetSuspended(suspend bool) error { obj.Spec.Suspend = suspend + return nil } func (obj ImageUpdateAutomationAdapter) DeepCopyClientObject() client.Object { @@ -338,8 +347,8 @@ func (obj UnstructuredAdapter) AsClientObject() client.Object { return obj.Unstructured } -func (obj UnstructuredAdapter) SetSuspended(suspend bool) { - unstructured.SetNestedField(obj.Object, suspend, "spec", "suspend") +func (obj UnstructuredAdapter) SetSuspended(suspend bool) error { + return unstructured.SetNestedField(obj.Object, suspend, "spec", "suspend") } func (obj UnstructuredAdapter) DeepCopyClientObject() client.Object { diff --git a/core/fluxsync/adapters_test.go b/core/fluxsync/adapters_test.go index 18be2f865d..0a41f0cacf 100644 --- a/core/fluxsync/adapters_test.go +++ b/core/fluxsync/adapters_test.go @@ -64,7 +64,8 @@ func TestSetSuspended(t *testing.T) { }, } - obj.SetSuspended(true) + err := obj.SetSuspended(true) + g.Expect(err).NotTo(HaveOccurred()) suspend, _, _ := unstructured.NestedBool(obj.Object, "spec", "suspend") g.Expect(suspend).To(BeTrue()) } diff --git a/core/server/inventory.go b/core/server/inventory.go index 24ac5687d2..f959194f2a 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -42,12 +42,12 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ switch msg.Kind { case kustomizev1.KustomizationKind: - entries, err = cs.getKustomizationInventory(ctx, msg.ClusterName, client, msg.Name, msg.Namespace) + entries, 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, msg.ClusterName, client, msg.Name, msg.Namespace) + entries, err = cs.getHelmReleaseInventory(ctx, client, msg.Name, msg.Namespace) if err != nil { return nil, fmt.Errorf("failed getting helm Release inventory: %w", err) } @@ -62,17 +62,14 @@ func (cs *coreServer) GetInventory(ctx context.Context, msg *pb.GetInventoryRequ } } - resources, err := cs.getInventoryResources(ctx, msg.ClusterName, client, entries, msg.Namespace, msg.WithChildren) - if err != nil { - return nil, fmt.Errorf("failed getting inventory resources: %w", err) - } + resources := cs.getInventoryResources(ctx, msg.ClusterName, client, entries, msg.Namespace, msg.WithChildren) return &pb.GetInventoryResponse{ Entries: resources, }, nil } -func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { +func (cs *coreServer) getKustomizationInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { kust := &kustomizev1.Kustomization{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -104,7 +101,7 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName return objects, nil } -func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName string, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { +func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, k8sClient client.Client, name, namespace string) ([]*unstructured.Unstructured, error) { release := &helmv2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -124,7 +121,7 @@ func (cs *coreServer) getHelmReleaseInventory(ctx context.Context, clusterName s return objects, nil } -func (cs *coreServer) getInventoryResources(ctx context.Context, clusterName string, k8sClient client.Client, objects []*unstructured.Unstructured, namespace string, withChildren bool) ([]*pb.InventoryEntry, error) { +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{} @@ -155,7 +152,7 @@ func (cs *coreServer) getInventoryResources(ctx context.Context, clusterName str wg.Wait() - return result, nil + return result } // Returns the list of resources applied in the helm chart. diff --git a/core/server/suspend.go b/core/server/suspend.go index 9ec761f8f8..3462b1728a 100644 --- a/core/server/suspend.go +++ b/core/server/suspend.go @@ -59,7 +59,10 @@ func (cs *coreServer) ToggleSuspendResource(ctx context.Context, msg *pb.ToggleS patch := client.MergeFrom(obj.DeepCopyClientObject()) - obj.SetSuspended(msg.Suspend) + err = obj.SetSuspended(msg.Suspend) + if err != nil { + return nil, err + } if msg.Suspend { log.Info("Suspending resource") From 7f5395dd45dc869c8298657bb2a731777e34a809 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 23 Oct 2023 09:35:38 +0200 Subject: [PATCH 11/17] Fixes another lint warning, don't shadow `copy` --- core/fluxsync/adapters_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/fluxsync/adapters_test.go b/core/fluxsync/adapters_test.go index 0a41f0cacf..51620e1f0c 100644 --- a/core/fluxsync/adapters_test.go +++ b/core/fluxsync/adapters_test.go @@ -79,9 +79,9 @@ func TestDeepCopyClientObject(t *testing.T) { }, } - copy := obj.DeepCopyClientObject().(*unstructured.Unstructured) - g.Expect(copy.Object).To(Equal(obj.Object)) - g.Expect(copy).ToNot(BeIdenticalTo(obj)) + objCopy := obj.DeepCopyClientObject().(*unstructured.Unstructured) + g.Expect(objCopy.Object).To(Equal(obj.Object)) + g.Expect(objCopy).ToNot(BeIdenticalTo(obj)) } func TestAsClientObjectCompatibilityWithTestClient(t *testing.T) { From 5b1f4a6eedb3fc8ae2b307fdbad25080e2cce207 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 23 Oct 2023 09:53:46 +0200 Subject: [PATCH 12/17] Convert the entire inventory from unstructured instead of per-item --- core/server/inventory.go | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index f959194f2a..f01fa93843 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -394,33 +394,24 @@ func ParseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstruct content := obj.UnstructuredContent() // Check if status.inventory is present - inventory, found, err := unstructured.NestedMap(content, "status", "inventory") + unstructuredInventory, found, err := unstructured.NestedMap(content, "status", "inventory") if err != nil || !found { return nil, errors.New("no status.inventory found on resource, it hasn't been synced yet or is not queryable from this endpoint") } - // Check if status.inventory.entries is present - entries, found, err := unstructured.NestedSlice(inventory, "entries") - if err != nil || !found { - return nil, nil + resourceInventory := &kustomizev1.ResourceInventory{} + err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredInventory, resourceInventory) + if err != nil { + return nil, fmt.Errorf("error converting inventory to resource inventory: %w", err) } objects := []*unstructured.Unstructured{} - for _, entryInterface := range entries { - entry, ok := entryInterface.(map[string]interface{}) - if !ok { - return nil, fmt.Errorf("failed converting inventory entry to map[string]interface{}: %+v", entry) - } - ref := &kustomizev1.ResourceRef{} - err := runtime.DefaultUnstructuredConverter.FromUnstructured(entry, ref) + for _, entry := range resourceInventory.Entries { + u, err := resourceRefToUnstructured(entry) if err != nil { - return nil, fmt.Errorf("failed converting inventory entry: %w", err) - } - invEntry, err := resourceRefToUnstructured(*ref) - if err != nil { - return nil, fmt.Errorf("failed converting inventory entry: %w", err) + return nil, fmt.Errorf("error converting resource ref to unstructured: %w", err) } - objects = append(objects, &invEntry) + objects = append(objects, &u) } return objects, nil From 8a3ec8c4af17c36102e57cc837899543bf79bb11 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 23 Oct 2023 14:20:28 +0200 Subject: [PATCH 13/17] Improves test to check AsClientObject retrieval - Which is the case that breaks sometimes w/ reflection if not handled carefully w/ the adpator --- core/fluxsync/adapters.go | 3 ++- core/fluxsync/adapters_test.go | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/core/fluxsync/adapters.go b/core/fluxsync/adapters.go index 65848291d6..bfa4d241e4 100644 --- a/core/fluxsync/adapters.go +++ b/core/fluxsync/adapters.go @@ -343,7 +343,8 @@ func (obj UnstructuredAdapter) GetConditions() []metav1.Condition { func (obj UnstructuredAdapter) AsClientObject() client.Object { // Important for the client reflection stuff to work - // We can't return just `obj` here as it seems to break stuff. + // We can't return just `obj` here otherwise we get a: + // panic: reflect: call of reflect.Value.Elem on struct Value return obj.Unstructured } diff --git a/core/fluxsync/adapters_test.go b/core/fluxsync/adapters_test.go index 51620e1f0c..036a146487 100644 --- a/core/fluxsync/adapters_test.go +++ b/core/fluxsync/adapters_test.go @@ -100,6 +100,7 @@ func TestAsClientObjectCompatibilityWithTestClient(t *testing.T) { "name": "test-cm", "namespace": "default", }, + "data": map[string]interface{}{"key": "value"}, }, }, } @@ -107,10 +108,18 @@ func TestAsClientObjectCompatibilityWithTestClient(t *testing.T) { err := cl.Create(context.TODO(), obj.AsClientObject()) g.Expect(err).NotTo(HaveOccurred()) - retrieved := &unstructured.Unstructured{} - retrieved.SetAPIVersion("v1") - retrieved.SetKind("ConfigMap") - err = cl.Get(context.TODO(), client.ObjectKey{Namespace: "default", Name: "test-cm"}, retrieved) + retrieved := &UnstructuredAdapter{ + Unstructured: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + }, + }, + } + err = cl.Get(context.TODO(), client.ObjectKey{Namespace: "default", Name: "test-cm"}, retrieved.AsClientObject()) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(retrieved.GetName()).To(Equal("test-cm")) + + // check the data key + data, _, _ := unstructured.NestedStringMap(retrieved.Object, "data") + g.Expect(data).To(Equal(map[string]string{"key": "value"})) } From aa84121b068d9da7bda53688a25b793f01a28876 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 23 Oct 2023 16:04:14 +0200 Subject: [PATCH 14/17] Review feedback, - Move some tests around and revert Exposed fns to be internal - Cleanup the ToReconcileable fn interface, doesn't error anymore, we don't use the list value it returns - More explicit error handling for looking up inventory --- core/fluxsync/adapters.go | 78 +++------ core/server/inventory.go | 28 ++-- core/server/inventory_internal_test.go | 211 +++++++++++++++++++++++++ core/server/inventory_test.go | 188 ---------------------- core/server/suspend.go | 6 +- core/server/sync.go | 14 +- core/server/sync_test.go | 6 +- 7 files changed, 251 insertions(+), 280 deletions(-) create mode 100644 core/server/inventory_internal_test.go diff --git a/core/fluxsync/adapters.go b/core/fluxsync/adapters.go index bfa4d241e4..c1d845f991 100644 --- a/core/fluxsync/adapters.go +++ b/core/fluxsync/adapters.go @@ -43,30 +43,6 @@ type Automation interface { SourceRef() SourceRef } -func NewReconcileable(obj client.Object) Reconcilable { - switch o := obj.(type) { - case *kustomizev1.Kustomization: - return KustomizationAdapter{Kustomization: o} - case *helmv2.HelmRelease: - return HelmReleaseAdapter{HelmRelease: o} - case *sourcev1.GitRepository: - return GitRepositoryAdapter{GitRepository: o} - case *sourcev1b2.HelmRepository: - return HelmRepositoryAdapter{HelmRepository: o} - case *sourcev1b2.Bucket: - return BucketAdapter{Bucket: o} - case *sourcev1b2.HelmChart: - return HelmChartAdapter{HelmChart: o} - case *sourcev1b2.OCIRepository: - return OCIRepositoryAdapter{OCIRepository: o} - case *reflectorv1.ImageRepository: - return ImageRepositoryAdapter{ImageRepository: o} - case *imgautomationv1.ImageUpdateAutomation: - return ImageUpdateAutomationAdapter{ImageUpdateAutomation: o} - } - return nil -} - type GitRepositoryAdapter struct { *sourcev1.GitRepository } @@ -312,6 +288,8 @@ func (obj ImageUpdateAutomationAdapter) DeepCopyClientObject() client.Object { return obj.DeepCopy() } +// UnstructuredAdapter implements the Reconcilable interface for unstructured resources. +// The underlying resource gvk should have the standard flux object sync/suspend fields type UnstructuredAdapter struct { *unstructured.Unstructured } @@ -342,8 +320,8 @@ func (obj UnstructuredAdapter) GetConditions() []metav1.Condition { } func (obj UnstructuredAdapter) AsClientObject() client.Object { - // Important for the client reflection stuff to work - // We can't return just `obj` here otherwise we get a: + // Important for the controller-runtime type reflection to work + // We can't return just `obj` here otherwise we get a // panic: reflect: call of reflect.Value.Elem on struct Value return obj.Unstructured } @@ -379,45 +357,39 @@ func (s sRef) Kind() string { return s.kind } -func ToReconcileable(gvk schema.GroupVersionKind) (client.ObjectList, Reconcilable, error) { +// ToReconcileable takes a GVK and returns a "Reconcilable" for it. +// The reconcilable can be passed to a controller-runtime client to fetch it +// from the cluster. Once fetched we can query it for the last sync time, whether +// its suspended etc, using the Reconcilable interface. +// +// The generic unstructured case handles "flux like" objects that we don't explicitly +// know about, but which follow the same patterns for suspend/sync as a stadard flux object. +// E.g. `spec.suspend` and `status.lastHandledReconcileRequest` etc. +func ToReconcileable(gvk schema.GroupVersionKind) Reconcilable { switch gvk.Kind { case kustomizev1.KustomizationKind: - return &kustomizev1.KustomizationList{}, NewReconcileable(&kustomizev1.Kustomization{}), nil - + return KustomizationAdapter{Kustomization: &kustomizev1.Kustomization{}} case helmv2.HelmReleaseKind: - return &helmv2.HelmReleaseList{}, NewReconcileable(&helmv2.HelmRelease{}), nil - + return HelmReleaseAdapter{HelmRelease: &helmv2.HelmRelease{}} + // TODO: remove all these and let them fall through to the Unstructured case? case sourcev1.GitRepositoryKind: - return &sourcev1.GitRepositoryList{}, NewReconcileable(&sourcev1.GitRepository{}), nil - + return GitRepositoryAdapter{GitRepository: &sourcev1.GitRepository{}} case sourcev1b2.BucketKind: - return &sourcev1b2.BucketList{}, NewReconcileable(&sourcev1b2.Bucket{}), nil - + return BucketAdapter{Bucket: &sourcev1b2.Bucket{}} case sourcev1b2.HelmRepositoryKind: - return &sourcev1b2.HelmRepositoryList{}, NewReconcileable(&sourcev1b2.HelmRepository{}), nil - + return HelmRepositoryAdapter{HelmRepository: &sourcev1b2.HelmRepository{}} case sourcev1b2.HelmChartKind: - return &sourcev1b2.HelmChartList{}, NewReconcileable(&sourcev1b2.HelmChart{}), nil - + return HelmChartAdapter{HelmChart: &sourcev1b2.HelmChart{}} case sourcev1b2.OCIRepositoryKind: - return &sourcev1b2.OCIRepositoryList{}, NewReconcileable(&sourcev1b2.OCIRepository{}), nil - + return OCIRepositoryAdapter{OCIRepository: &sourcev1b2.OCIRepository{}} case reflectorv1.ImageRepositoryKind: - return &reflectorv1.ImageRepositoryList{}, NewReconcileable(&reflectorv1.ImageRepository{}), nil - + return ImageRepositoryAdapter{ImageRepository: &reflectorv1.ImageRepository{}} case imgautomationv1.ImageUpdateAutomationKind: - return &imgautomationv1.ImageUpdateAutomationList{}, NewReconcileable(&imgautomationv1.ImageUpdateAutomation{}), nil + return ImageUpdateAutomationAdapter{ImageUpdateAutomation: &imgautomationv1.ImageUpdateAutomation{}} } - return ToUnstructuredReconcilable(gvk) -} - -func ToUnstructuredReconcilable(gvk schema.GroupVersionKind) (client.ObjectList, Reconcilable, error) { + // Return the UnstructuredAdapter for flux-like resources obj := &unstructured.Unstructured{} obj.SetGroupVersionKind(gvk) - - objList := &unstructured.UnstructuredList{} - objList.SetGroupVersionKind(gvk) - - return objList, UnstructuredAdapter{Unstructured: obj}, nil + return UnstructuredAdapter{Unstructured: obj} } diff --git a/core/server/inventory.go b/core/server/inventory.go index f01fa93843..2db2c6015c 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -6,7 +6,6 @@ import ( "context" "encoding/base64" "encoding/json" - "errors" "fmt" "io" "strings" @@ -56,7 +55,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) + entries, err = getFluxLikeInventory(ctx, client, msg.Name, msg.Namespace, *gvk) if err != nil { return nil, fmt.Errorf("failed getting flux like inventory: %w", err) } @@ -220,13 +219,6 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel return nil, fmt.Errorf("failed to read the Helm storage object for HelmRelease '%s': %w", helmRelease.Name, err) } - // FIXME: do we need this? - for _, obj := range objects { - if obj.GetNamespace() == "" { - obj.SetNamespace(helmRelease.GetNamespace()) - } - } - return objects, nil } @@ -370,10 +362,7 @@ func sanitizeUnstructuredSecret(obj unstructured.Unstructured) (unstructured.Uns return redactedUnstructured, nil } -// GetFluxLikeInventory returns the inventory on a resource if -// it matches the structure of the flux inventory format (e.g. kustomizations) -// It returns an error if the inventory is not as expected -func GetFluxLikeInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { +func getFluxLikeInventory(ctx context.Context, k8sClient client.Client, name, namespace string, gvk schema.GroupVersionKind) ([]*unstructured.Unstructured, error) { // Create an unstructured object with the desired GVK (GroupVersionKind) obj := &unstructured.Unstructured{} obj.SetGroupVersionKind(gvk) @@ -385,18 +374,19 @@ func GetFluxLikeInventory(ctx context.Context, k8sClient client.Client, name, na return nil, fmt.Errorf("failed to get kustomization: %w", err) } - return ParseInventoryFromUnstructured(obj) + return parseInventoryFromUnstructured(obj) } -// Parse the inventory from an unstructured object -// It returns an error if the inventory is not as expected (should look like a kustomization's inventory) -func ParseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { +func parseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { content := obj.UnstructuredContent() // Check if status.inventory is present unstructuredInventory, found, err := unstructured.NestedMap(content, "status", "inventory") - if err != nil || !found { - return nil, errors.New("no status.inventory found on resource, it hasn't been synced yet or is not queryable from this endpoint") + if err != nil { + return nil, fmt.Errorf("error getting status.inventory from object: %w", err) + } + if !found { + return nil, fmt.Errorf("status.inventory not found in object") } resourceInventory := &kustomizev1.ResourceInventory{} diff --git a/core/server/inventory_internal_test.go b/core/server/inventory_internal_test.go new file mode 100644 index 0000000000..2d4c5696c3 --- /dev/null +++ b/core/server/inventory_internal_test.go @@ -0,0 +1,211 @@ +package server + +import ( + "context" + "errors" + "testing" + + kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" + sourcev1 "github.com/fluxcd/source-controller/api/v1" + . "github.com/onsi/gomega" + "github.com/weaveworks/weave-gitops/pkg/kube" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestGetFluxLikeInventory(t *testing.T) { + g := NewGomegaWithT(t) + + ctx := context.Background() + + ks := &kustomizev1.Kustomization{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-kustomization", + Namespace: "my-namespace", + }, + Spec: kustomizev1.KustomizationSpec{ + SourceRef: kustomizev1.CrossNamespaceSourceReference{ + Kind: sourcev1.GitRepositoryKind, + }, + }, + Status: kustomizev1.KustomizationStatus{ + Inventory: &kustomizev1.ResourceInventory{ + Entries: []kustomizev1.ResourceRef{ + { + ID: "my-namespace_my-deployment_apps_Deployment", + Version: "v1", + }, + }, + }, + }, + } + + scheme, err := kube.CreateScheme() + g.Expect(err).To(BeNil()) + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ks).Build() + + gvk := kustomizev1.GroupVersion.WithKind("Kustomization") + entries, err := getFluxLikeInventory(ctx, k8sClient, ks.Name, ks.Namespace, gvk) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(entries).To(HaveLen(1)) + + expected := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "my-deployment", + "namespace": "my-namespace", + }, + }, + } + + g.Expect(entries[0]).To(Equal(expected)) +} + +func TestParseInventoryFromUnstructured(t *testing.T) { + // inv lives at status.inventory.entries + stdErr := errors.New("status.inventory not found in object") + + testCases := []struct { + name string + obj *unstructured.Unstructured + expected []*unstructured.Unstructured + expectedErr error + }{ + { + name: "no status field", + obj: &unstructured.Unstructured{}, + expected: nil, + expectedErr: stdErr, + }, + { + name: "empty status", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{}, + }, + }, + expected: nil, + expectedErr: stdErr, + }, + { + name: "empty inventory", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{}, + }, + }, + }, + expected: nil, + }, + { + name: "mallformed inventory", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": "hi there", + }, + }, + }, + expectedErr: errors.New(".status.inventory accessor error: hi there is of the type string, expected map[string]interface{}"), + }, + { + name: "empty entry item", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{ + "entries": []interface{}{ + map[string]interface{}{}, + }, + }, + }, + }, + }, + expected: nil, + expectedErr: errors.New("unable to parse stored object metadata: "), + }, + { + name: "invalid inventory", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{ + "entries": []interface{}{ + map[string]interface{}{ + "v": "v1", + "id": "foo", + }, + }, + }, + }, + }, + }, + expected: nil, + expectedErr: errors.New("unable to parse stored object metadata: foo"), + }, + { + name: "valid inventory", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "inventory": map[string]interface{}{ + "entries": []interface{}{ + map[string]interface{}{ + "v": "v1", + "id": "my-namespace_my-deployment_apps_Deployment", + }, + map[string]interface{}{ + "v": "v1", + "id": "my-other-namespace_my-configmap__ConfigMap", + }, + }, + }, + }, + }, + }, + expected: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "my-deployment", + "namespace": "my-namespace", + }, + }, + }, + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "my-configmap", + "namespace": "my-other-namespace", + }, + }, + }, + }, + expectedErr: nil, + }, + } + + for _, tt := range testCases { + // subtests... + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + // Parse inventory from unstructured + entries, err := parseInventoryFromUnstructured(tt.obj) + + if err != nil || tt.expectedErr != nil { + g.Expect(err).To(MatchError(tt.expectedErr)) + } + + g.Expect(entries).To(ConsistOf(tt.expected)) + }) + } +} diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 8882ffb7f7..16cc1138b6 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/base64" "encoding/json" - "errors" "fmt" "testing" @@ -14,14 +13,12 @@ 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" ) @@ -320,188 +317,3 @@ func TestGetInventoryHelmReleaseWithKubeconfig(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(res.Entries).To(HaveLen(0)) } - -func TestGetFluxLikeInventory(t *testing.T) { - g := NewGomegaWithT(t) - - ctx := context.Background() - - ks := &kustomizev1.Kustomization{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-kustomization", - Namespace: "my-namespace", - }, - Spec: kustomizev1.KustomizationSpec{ - SourceRef: kustomizev1.CrossNamespaceSourceReference{ - Kind: sourcev1.GitRepositoryKind, - }, - }, - Status: kustomizev1.KustomizationStatus{ - Inventory: &kustomizev1.ResourceInventory{ - Entries: []kustomizev1.ResourceRef{ - { - ID: "my-namespace_my-deployment_apps_Deployment", - Version: "v1", - }, - }, - }, - }, - } - - scheme, err := kube.CreateScheme() - g.Expect(err).To(BeNil()) - - k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ks).Build() - - gvk := kustomizev1.GroupVersion.WithKind("Kustomization") - entries, err := server.GetFluxLikeInventory(ctx, k8sClient, ks.Name, ks.Namespace, gvk) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(entries).To(HaveLen(1)) - - expected := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "my-deployment", - "namespace": "my-namespace", - }, - }, - } - - g.Expect(entries[0]).To(Equal(expected)) -} - -func TestParseInventoryFromUnstructured(t *testing.T) { - // inv lives at status.inventory.entries - stdErr := errors.New("no status.inventory found on resource, it hasn't been synced yet or is not queryable from this endpoint") - - testCases := []struct { - name string - obj *unstructured.Unstructured - expected []*unstructured.Unstructured - expectedErr error - }{ - { - name: "no status field", - obj: &unstructured.Unstructured{}, - expected: nil, - expectedErr: stdErr, - }, - { - name: "empty status", - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{}, - }, - }, - expected: nil, - expectedErr: stdErr, - }, - { - name: "empty inventory", - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{ - "inventory": map[string]interface{}{}, - }, - }, - }, - expected: nil, - }, - { - name: "empty entry item", - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{ - "inventory": map[string]interface{}{ - "entries": []interface{}{ - map[string]interface{}{}, - }, - }, - }, - }, - }, - expected: nil, - expectedErr: errors.New("unable to parse stored object metadata: "), - }, - { - name: "invalid inventory", - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{ - "inventory": map[string]interface{}{ - "entries": []interface{}{ - map[string]interface{}{ - "v": "v1", - "id": "foo", - }, - }, - }, - }, - }, - }, - expected: nil, - expectedErr: errors.New("unable to parse stored object metadata: foo"), - }, - { - name: "valid inventory", - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{ - "inventory": map[string]interface{}{ - "entries": []interface{}{ - map[string]interface{}{ - "v": "v1", - "id": "my-namespace_my-deployment_apps_Deployment", - }, - map[string]interface{}{ - "v": "v1", - "id": "my-other-namespace_my-configmap__ConfigMap", - }, - }, - }, - }, - }, - }, - expected: []*unstructured.Unstructured{ - { - Object: map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "my-deployment", - "namespace": "my-namespace", - }, - }, - }, - { - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "my-configmap", - "namespace": "my-other-namespace", - }, - }, - }, - }, - expectedErr: nil, - }, - } - - for _, tc := range testCases { - // subtests... - t.Run(tc.name, func(tt *testing.T) { - gg := NewGomegaWithT(tt) - // Parse inventory from unstructured - entries, err := server.ParseInventoryFromUnstructured(tc.obj) - - if err != nil || tc.expectedErr != nil { - gg.Expect(err).To(MatchError(tc.expectedErr)) - } - - gg.Expect(entries).To(ConsistOf(tc.expected)) - }) - } -} diff --git a/core/server/suspend.go b/core/server/suspend.go index 3462b1728a..adf8767aed 100644 --- a/core/server/suspend.go +++ b/core/server/suspend.go @@ -39,11 +39,7 @@ func (cs *coreServer) ToggleSuspendResource(ctx context.Context, msg *pb.ToggleS continue } - _, obj, err := fluxsync.ToReconcileable(*gvk) - if err != nil { - respErrors = *multierror.Append(fmt.Errorf("converting to reconcilable source: %w", err), respErrors.Errors...) - continue - } + obj := fluxsync.ToReconcileable(*gvk) log := cs.logger.WithValues( "user", principal.ID, diff --git a/core/server/sync.go b/core/server/sync.go index 5ffd86c0c0..4146aeeb27 100644 --- a/core/server/sync.go +++ b/core/server/sync.go @@ -39,12 +39,7 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject continue } - _, obj, err := fluxsync.ToReconcileable(*gvk) - if err != nil { - respErrors = *multierror.Append(fmt.Errorf("error converting to object: %w", err), respErrors.Errors...) - continue - } - + obj := fluxsync.ToReconcileable(*gvk) if err := c.Get(ctx, key, obj.AsClientObject()); err != nil { respErrors = *multierror.Append(fmt.Errorf("error getting object: %w", err), respErrors.Errors...) continue @@ -59,12 +54,7 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject return nil, err } - _, sourceObj, err := fluxsync.ToReconcileable(*sourceGVK) - if err != nil { - respErrors = *multierror.Append(fmt.Errorf("getting source type for %q: %w", sourceRef.Kind(), err), respErrors.Errors...) - continue - } - + sourceObj := fluxsync.ToReconcileable(*sourceGVK) sourceNs := sourceRef.Namespace() // sourceRef.Namespace is an optional field in flux diff --git a/core/server/sync_test.go b/core/server/sync_test.go index 35eadabd8a..7052185b99 100644 --- a/core/server/sync_test.go +++ b/core/server/sync_test.go @@ -88,7 +88,7 @@ func TestSync(t *testing.T) { WithSource: true, }, reconcilable: fluxsync.HelmReleaseAdapter{HelmRelease: hr}, - source: fluxsync.NewReconcileable(helmRepo), + source: fluxsync.HelmRepositoryAdapter{HelmRepository: helmRepo}, }, { name: "kustomization no source", msg: &pb.SyncFluxObjectRequest{ @@ -105,7 +105,7 @@ func TestSync(t *testing.T) { WithSource: true, }, reconcilable: fluxsync.KustomizationAdapter{Kustomization: kust}, - source: fluxsync.NewReconcileable(gitRepo), + source: fluxsync.GitRepositoryAdapter{GitRepository: gitRepo}, }, { name: "gitrepository", msg: &pb.SyncFluxObjectRequest{ @@ -171,7 +171,7 @@ func TestSync(t *testing.T) { WithSource: true, }, reconcilable: fluxsync.HelmReleaseAdapter{HelmRelease: hr}, - source: fluxsync.NewReconcileable(helmRepo), + source: fluxsync.HelmRepositoryAdapter{HelmRepository: helmRepo}, }} for _, tt := range tests { From 781041512c61c8d40d321372e9695631317cb7b1 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 23 Oct 2023 19:52:56 +0200 Subject: [PATCH 15/17] Modernise our multi-error handling, golang can do it now Don't need 3rd party libs --- core/server/sync.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/core/server/sync.go b/core/server/sync.go index 4146aeeb27..267729a3dd 100644 --- a/core/server/sync.go +++ b/core/server/sync.go @@ -2,9 +2,9 @@ package server import ( "context" + "errors" "fmt" - "github.com/hashicorp/go-multierror" "github.com/weaveworks/weave-gitops/core/fluxsync" pb "github.com/weaveworks/weave-gitops/pkg/api/core" "github.com/weaveworks/weave-gitops/pkg/server/auth" @@ -13,18 +13,18 @@ import ( func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObjectRequest) (*pb.SyncFluxObjectResponse, error) { principal := auth.Principal(ctx) - respErrors := multierror.Error{} + var syncErr error for _, sync := range msg.Objects { clustersClient, err := cs.clustersManager.GetImpersonatedClient(ctx, principal) if err != nil { - respErrors = *multierror.Append(fmt.Errorf("error getting impersonating client: %w", err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("error getting impersonating client: %w", err)) continue } c, err := clustersClient.Scoped(sync.ClusterName) if err != nil { - respErrors = *multierror.Append(fmt.Errorf("getting cluster client: %w", err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("getting cluster client: %w", err)) continue } @@ -35,13 +35,13 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject gvk, err := cs.primaryKinds.Lookup(sync.Kind) if err != nil { - respErrors = *multierror.Append(fmt.Errorf("looking up GVK for %q: %w", sync.Kind, err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("looking up GVK for %q: %w", sync.Kind, err)) continue } obj := fluxsync.ToReconcileable(*gvk) if err := c.Get(ctx, key, obj.AsClientObject()); err != nil { - respErrors = *multierror.Append(fmt.Errorf("error getting object: %w", err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("error getting object: %w", err)) continue } @@ -81,12 +81,12 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject log.Info("Syncing resource") if err := fluxsync.RequestReconciliation(ctx, c, sourceKey, sourceGvk); err != nil { - respErrors = *multierror.Append(fmt.Errorf("requesting source reconciliation: %w", err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("requesting source reconciliation: %w", err)) continue } if err := fluxsync.WaitForSync(ctx, c, sourceKey, sourceObj); err != nil { - respErrors = *multierror.Append(fmt.Errorf("syncing source: %w", err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("syncing source: %w", err)) continue } } @@ -100,15 +100,15 @@ func (cs *coreServer) SyncFluxObject(ctx context.Context, msg *pb.SyncFluxObject log.Info("Syncing resource") if err := fluxsync.RequestReconciliation(ctx, c, key, *gvk); err != nil { - respErrors = *multierror.Append(fmt.Errorf("requesting reconciliation: %w", err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("requesting reconciliation: %w", err)) continue } if err := fluxsync.WaitForSync(ctx, c, key, obj); err != nil { - respErrors = *multierror.Append(fmt.Errorf("syncing automation: %w", err), respErrors.Errors...) + syncErr = errors.Join(syncErr, fmt.Errorf("syncing automation: %w", err)) continue } } - return &pb.SyncFluxObjectResponse{}, respErrors.ErrorOrNil() + return &pb.SyncFluxObjectResponse{}, syncErr } From fa338e368af1c8a91da5f60ed8e34ba340e3488d Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 24 Oct 2023 16:25:31 +0200 Subject: [PATCH 16/17] Expose `useListEvents` via npm module --- ui/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui/index.ts b/ui/index.ts index 219368fd4e..04722b3432 100644 --- a/ui/index.ts +++ b/ui/index.ts @@ -91,6 +91,7 @@ import { } from "./contexts/LinkResolverContext"; import { useListAutomations, useSyncFluxObject } from "./hooks/automations"; import { useDebounce, useRequestState } from "./hooks/common"; +import { useListEvents } from "./hooks/events"; import { useFeatureFlags } from "./hooks/featureflags"; import { useListFluxCrds, @@ -273,6 +274,7 @@ export { useLinkResolver, useListAlerts, useListAutomations, + useListEvents, useListFluxCrds, useListFluxRuntimeObjects, useListObjects, From 7eedc62d41efcba93cfdb3451100b8a489c37dfe Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Thu, 26 Oct 2023 10:56:54 +0200 Subject: [PATCH 17/17] Improve error messages and tidy up error assertions in tests --- core/server/inventory.go | 2 +- core/server/inventory_internal_test.go | 33 ++++++++++++++++---------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 2db2c6015c..cc89592217 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -386,7 +386,7 @@ func parseInventoryFromUnstructured(obj *unstructured.Unstructured) ([]*unstruct return nil, fmt.Errorf("error getting status.inventory from object: %w", err) } if !found { - return nil, fmt.Errorf("status.inventory not found in object") + return nil, fmt.Errorf("status.inventory not found in object %s/%s", obj.GetNamespace(), obj.GetName()) } resourceInventory := &kustomizev1.ResourceInventory{} diff --git a/core/server/inventory_internal_test.go b/core/server/inventory_internal_test.go index 2d4c5696c3..49446d18b7 100644 --- a/core/server/inventory_internal_test.go +++ b/core/server/inventory_internal_test.go @@ -2,7 +2,6 @@ package server import ( "context" - "errors" "testing" kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" @@ -67,17 +66,24 @@ func TestGetFluxLikeInventory(t *testing.T) { func TestParseInventoryFromUnstructured(t *testing.T) { // inv lives at status.inventory.entries - stdErr := errors.New("status.inventory not found in object") - + stdErr := "status.inventory not found in object my-namespace/my-resource" testCases := []struct { name string obj *unstructured.Unstructured expected []*unstructured.Unstructured - expectedErr error + expectedErr string }{ { - name: "no status field", - obj: &unstructured.Unstructured{}, + name: "no status field", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + // include name to make sure its included in the error + "metadata": map[string]interface{}{ + "name": "my-resource", + "namespace": "my-namespace", + }, + }, + }, expected: nil, expectedErr: stdErr, }, @@ -85,6 +91,10 @@ func TestParseInventoryFromUnstructured(t *testing.T) { name: "empty status", obj: &unstructured.Unstructured{ Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "my-resource", + "namespace": "my-namespace", + }, "status": map[string]interface{}{}, }, }, @@ -111,7 +121,7 @@ func TestParseInventoryFromUnstructured(t *testing.T) { }, }, }, - expectedErr: errors.New(".status.inventory accessor error: hi there is of the type string, expected map[string]interface{}"), + expectedErr: ".status.inventory accessor error: hi there is of the type string, expected map[string]interface{}", }, { name: "empty entry item", @@ -127,7 +137,7 @@ func TestParseInventoryFromUnstructured(t *testing.T) { }, }, expected: nil, - expectedErr: errors.New("unable to parse stored object metadata: "), + expectedErr: "unable to parse stored object metadata: ", }, { name: "invalid inventory", @@ -146,7 +156,7 @@ func TestParseInventoryFromUnstructured(t *testing.T) { }, }, expected: nil, - expectedErr: errors.New("unable to parse stored object metadata: foo"), + expectedErr: "unable to parse stored object metadata: foo", }, { name: "valid inventory", @@ -190,7 +200,6 @@ func TestParseInventoryFromUnstructured(t *testing.T) { }, }, }, - expectedErr: nil, }, } @@ -201,8 +210,8 @@ func TestParseInventoryFromUnstructured(t *testing.T) { // Parse inventory from unstructured entries, err := parseInventoryFromUnstructured(tt.obj) - if err != nil || tt.expectedErr != nil { - g.Expect(err).To(MatchError(tt.expectedErr)) + if err != nil || tt.expectedErr != "" { + g.Expect(err).To(MatchError(ContainSubstring(tt.expectedErr))) } g.Expect(entries).To(ConsistOf(tt.expected))