From e630daba4eb088e2d179007a16e1a97fc0b89221 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Mon, 31 Jul 2023 23:46:49 +0700 Subject: [PATCH 1/4] fix npe when inventory object is nil Signed-off-by: Chanwit Kaewkasi --- core/server/inventory.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/server/inventory.go b/core/server/inventory.go index a926cb525e..c04152c62d 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -71,6 +71,10 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName return nil, fmt.Errorf("failed to get kustomization: %w", err) } + if kust.Status.Inventory == nil { + return []*pb.InventoryEntry{}, nil + } + if kust.Status.Inventory.Entries == nil { return []*pb.InventoryEntry{}, nil } From 18ea29a8ba0809a7a174d2c3d658e6a99e3eb4e1 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Tue, 1 Aug 2023 19:50:57 +0700 Subject: [PATCH 2/4] add a test for the case of blank inventory Signed-off-by: Chanwit Kaewkasi --- core/server/inventory_test.go | 97 +++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 839b69af30..971c7adbd5 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -128,6 +128,103 @@ func TestGetInventoryKustomization(t *testing.T) { g.Expect(res.Entries[0].Tenant).To(Equal("tenant")) } +func TestGetBlankInventoryKustomization(t *testing.T) { + g := NewGomegaWithT(t) + + ctx := context.Background() + + automationName := "my-automation" + + ns := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + Labels: map[string]string{ + "toolkit.fluxcd.io/tenant": "tenant", + }, + }, + } + + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment", + Namespace: ns.Name, + UID: "this-is-not-an-uid", + }, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + types.AppLabel: automationName, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{types.AppLabel: automationName}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "nginx", + Image: "nginx", + }}, + }, + }, + }, + } + + rs := &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-123abcd", automationName), + Namespace: ns.Name, + }, + Spec: appsv1.ReplicaSetSpec{ + Template: deployment.Spec.Template, + Selector: deployment.Spec.Selector, + }, + Status: appsv1.ReplicaSetStatus{ + Replicas: 1, + }, + } + + rs.SetOwnerReferences([]metav1.OwnerReference{{ + UID: deployment.UID, + APIVersion: appsv1.SchemeGroupVersion.String(), + Kind: "Deployment", + Name: deployment.Name, + }}) + + kust := &kustomizev1.Kustomization{ + ObjectMeta: metav1.ObjectMeta{ + Name: automationName, + Namespace: ns.Name, + }, + Spec: kustomizev1.KustomizationSpec{ + SourceRef: kustomizev1.CrossNamespaceSourceReference{ + Kind: sourcev1.GitRepositoryKind, + }, + }, + Status: kustomizev1.KustomizationStatus{ + Inventory: nil, // blank inventory + }, + } + + scheme, err := kube.CreateScheme() + g.Expect(err).To(BeNil()) + + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(&ns, kust, deployment, rs).Build() + cfg := makeServerConfig(client, t, "") + c := makeServer(cfg, t) + + res, err := c.GetInventory(ctx, &pb.GetInventoryRequest{ + Namespace: ns.Name, + ClusterName: cluster.DefaultCluster, + Kind: "Kustomization", + Name: kust.Name, + WithChildren: true, + }) + + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.Entries).To(HaveLen(0)) +} + func TestGetInventoryHelmRelease(t *testing.T) { g := NewGomegaWithT(t) From ddf8f9d66984980a246989a05db7003aee8d81a8 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Wed, 16 Aug 2023 19:33:15 +0700 Subject: [PATCH 3/4] return nil instead of the blank array Signed-off-by: Chanwit Kaewkasi --- 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 c04152c62d..519129dcfb 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -72,11 +72,11 @@ func (cs *coreServer) getKustomizationInventory(ctx context.Context, clusterName } if kust.Status.Inventory == nil { - return []*pb.InventoryEntry{}, nil + return nil, nil } if kust.Status.Inventory.Entries == nil { - return []*pb.InventoryEntry{}, nil + return nil, nil } result := []*pb.InventoryEntry{} From fbd097460259dd4687777c89ff862a13d98da147 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Wed, 16 Aug 2023 19:33:33 +0700 Subject: [PATCH 4/4] remove unused test objects Signed-off-by: Chanwit Kaewkasi --- core/server/inventory_test.go | 39 +++++------------------------------ 1 file changed, 5 insertions(+), 34 deletions(-) diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 971c7adbd5..fe07ef165d 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -134,20 +134,12 @@ func TestGetBlankInventoryKustomization(t *testing.T) { ctx := context.Background() automationName := "my-automation" - - ns := corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-namespace", - Labels: map[string]string{ - "toolkit.fluxcd.io/tenant": "tenant", - }, - }, - } + ns := "test-namespace" deployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "my-deployment", - Namespace: ns.Name, + Namespace: ns, UID: "this-is-not-an-uid", }, Spec: appsv1.DeploymentSpec{ @@ -170,31 +162,10 @@ func TestGetBlankInventoryKustomization(t *testing.T) { }, } - rs := &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-123abcd", automationName), - Namespace: ns.Name, - }, - Spec: appsv1.ReplicaSetSpec{ - Template: deployment.Spec.Template, - Selector: deployment.Spec.Selector, - }, - Status: appsv1.ReplicaSetStatus{ - Replicas: 1, - }, - } - - rs.SetOwnerReferences([]metav1.OwnerReference{{ - UID: deployment.UID, - APIVersion: appsv1.SchemeGroupVersion.String(), - Kind: "Deployment", - Name: deployment.Name, - }}) - kust := &kustomizev1.Kustomization{ ObjectMeta: metav1.ObjectMeta{ Name: automationName, - Namespace: ns.Name, + Namespace: ns, }, Spec: kustomizev1.KustomizationSpec{ SourceRef: kustomizev1.CrossNamespaceSourceReference{ @@ -209,12 +180,12 @@ func TestGetBlankInventoryKustomization(t *testing.T) { scheme, err := kube.CreateScheme() g.Expect(err).To(BeNil()) - client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(&ns, kust, deployment, rs).Build() + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(kust, deployment).Build() cfg := makeServerConfig(client, t, "") c := makeServer(cfg, t) res, err := c.GetInventory(ctx, &pb.GetInventoryRequest{ - Namespace: ns.Name, + Namespace: ns, ClusterName: cluster.DefaultCluster, Kind: "Kustomization", Name: kust.Name,