From 40f2b12bb2acab793972e743a0ab92b7dfe0ffe1 Mon Sep 17 00:00:00 2001 From: AlinaGoaga Date: Tue, 19 Sep 2023 10:46:06 +0200 Subject: [PATCH 1/7] Do not check for secrets when retrieving hr inventory if spec kubeconfig is set up --- core/server/helm_release.go | 9 +++++++-- core/server/inventory.go | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/core/server/helm_release.go b/core/server/helm_release.go index 0d85684ee9..c7076ba669 100644 --- a/core/server/helm_release.go +++ b/core/server/helm_release.go @@ -37,8 +37,13 @@ func getHelmReleaseInventory(ctx context.Context, helmRelease helmv2.HelmRelease Namespace: storageNamespace, } - if err := c.Get(ctx, cluster, key, storageSecret); err != nil { - return nil, err + if helmRelease.Spec.KubeConfig != nil { + // helmrelease secret is on another cluster so we cannot inspect it to figure out the inventory and version and other things + return nil, nil + } else { + if err := c.Get(ctx, cluster, key, storageSecret); err != nil { + return nil, err + } } releaseData, releaseFound := storageSecret.Data["release"] diff --git a/core/server/inventory.go b/core/server/inventory.go index 519129dcfb..749dbe70fe 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -195,8 +195,13 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel Namespace: storageNamespace, } - if err := k8sClient.Get(ctx, key, storageSecret); err != nil { - return nil, err + if helmRelease.Spec.KubeConfig != nil { + // helmrelease secret is on another cluster so we cannot inspect it to figure out the inventory and version and other things + return nil, nil + } else { + if err := k8sClient.Get(ctx, key, storageSecret); err != nil { + return nil, err + } } releaseData, releaseFound := storageSecret.Data["release"] From 0b507dbc3501b440a9ebf9b4416d600f930584b0 Mon Sep 17 00:00:00 2001 From: AlinaGoaga Date: Tue, 19 Sep 2023 10:57:20 +0200 Subject: [PATCH 2/7] Refactor getHelmReleaseInventory to use getHelmReleaseObjects --- core/server/helm_release.go | 69 +++---------------------------------- 1 file changed, 4 insertions(+), 65 deletions(-) diff --git a/core/server/helm_release.go b/core/server/helm_release.go index c7076ba669..9340f04f4f 100644 --- a/core/server/helm_release.go +++ b/core/server/helm_release.go @@ -1,86 +1,25 @@ package server import ( - "bytes" - "compress/gzip" "context" - "encoding/base64" - "encoding/json" "fmt" - "io" "strings" helmv2 "github.com/fluxcd/helm-controller/api/v2beta1" - "github.com/fluxcd/pkg/ssa" "github.com/weaveworks/weave-gitops/core/clustersmngr" - "github.com/weaveworks/weave-gitops/core/server/types" pb "github.com/weaveworks/weave-gitops/pkg/api/core" - v1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) func getHelmReleaseInventory(ctx context.Context, helmRelease helmv2.HelmRelease, c clustersmngr.Client, cluster string) ([]*pb.GroupVersionKind, error) { - storageNamespace := helmRelease.GetStorageNamespace() - - storageName := helmRelease.GetReleaseName() - - storageVersion := helmRelease.Status.LastReleaseRevision - if storageVersion < 1 { - // skip release if it failed to install - return nil, nil - } - - storageSecret := &v1.Secret{} - secretName := fmt.Sprintf("sh.helm.release.v1.%s.v%v", storageName, storageVersion) - key := client.ObjectKey{ - Name: secretName, - Namespace: storageNamespace, - } - - if helmRelease.Spec.KubeConfig != nil { - // helmrelease secret is on another cluster so we cannot inspect it to figure out the inventory and version and other things - return nil, nil - } else { - if err := c.Get(ctx, cluster, key, storageSecret); err != nil { - return nil, err - } - } - - releaseData, releaseFound := storageSecret.Data["release"] - if !releaseFound { - return nil, fmt.Errorf("failed to decode the Helm storage object for HelmRelease '%s'", helmRelease.Name) - } - - byteData, err := base64.StdEncoding.DecodeString(string(releaseData)) + k8sClient, err := c.Scoped(cluster) if err != nil { - return nil, err - } - - var magicGzip = []byte{0x1f, 0x8b, 0x08} - if bytes.Equal(byteData[0:3], magicGzip) { - r, err := gzip.NewReader(bytes.NewReader(byteData)) - if err != nil { - return nil, err - } - - defer r.Close() - - uncompressedByteData, err := io.ReadAll(r) - if err != nil { - return nil, err - } - - byteData = uncompressedByteData + return nil, fmt.Errorf("error getting scoped client for cluster=%s: %w", cluster, err) } - storage := types.HelmReleaseStorage{} - if err := json.Unmarshal(byteData, &storage); err != nil { - return nil, fmt.Errorf("failed to decode the Helm storage object for HelmRelease '%s': %w", helmRelease.Name, err) - } + objects, err := getHelmReleaseObjects(ctx, k8sClient, &helmRelease) - objects, err := ssa.ReadObjects(strings.NewReader(storage.Manifest)) if err != nil { - return nil, fmt.Errorf("failed to read the Helm storage object for HelmRelease '%s': %w", helmRelease.Name, err) + return nil, err } var gvk []*pb.GroupVersionKind From 6e0cf2ddf825dec6850b1ab0f8a6da7995c3b58f Mon Sep 17 00:00:00 2001 From: AlinaGoaga Date: Tue, 19 Sep 2023 14:08:11 +0200 Subject: [PATCH 3/7] Add test for spec kubeconfig scenario --- core/server/inventory.go | 8 ++--- core/server/inventory_test.go | 63 ++++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/core/server/inventory.go b/core/server/inventory.go index 749dbe70fe..62215a9e5b 100644 --- a/core/server/inventory.go +++ b/core/server/inventory.go @@ -198,10 +198,10 @@ func getHelmReleaseObjects(ctx context.Context, k8sClient client.Client, helmRel if helmRelease.Spec.KubeConfig != nil { // helmrelease secret is on another cluster so we cannot inspect it to figure out the inventory and version and other things return nil, nil - } else { - if err := k8sClient.Get(ctx, key, storageSecret); err != nil { - return nil, err - } + } + + if err := k8sClient.Get(ctx, key, storageSecret); err != nil { + return nil, err } releaseData, releaseFound := storageSecret.Data["release"] diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index fe07ef165d..1ac97e2c3d 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -9,6 +9,7 @@ import ( helmv2 "github.com/fluxcd/helm-controller/api/v2beta1" kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" + "github.com/fluxcd/pkg/apis/meta" sourcev1 "github.com/fluxcd/source-controller/api/v1" . "github.com/onsi/gomega" "github.com/weaveworks/weave-gitops/core/clustersmngr/cluster" @@ -196,7 +197,7 @@ func TestGetBlankInventoryKustomization(t *testing.T) { g.Expect(res.Entries).To(HaveLen(0)) } -func TestGetInventoryHelmRelease(t *testing.T) { +func TestGetInventoryHelmReleaseWithSecret(t *testing.T) { g := NewGomegaWithT(t) scheme, err := kube.CreateScheme() @@ -270,3 +271,63 @@ func TestGetInventoryHelmRelease(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(res.Entries).To(HaveLen(1)) } + +func TestGetInventoryHelmReleaseWithKubeconfig(t *testing.T) { + g := NewGomegaWithT(t) + + scheme, err := kube.CreateScheme() + g.Expect(err).NotTo(HaveOccurred()) + + ctx := context.Background() + + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + }, + } + helm1 := &helmv2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "first-helm-name", + Namespace: ns.Name, + }, + Spec: helmv2.HelmReleaseSpec{ + KubeConfig: &meta.KubeConfigReference{ + SecretRef: meta.SecretKeyReference{ + Name: "kubeconfig", + }, + }, + }, + Status: helmv2.HelmReleaseStatus{ + LastReleaseRevision: 1, + }, + } + + cm := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + Kind: "ConfigMap", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "config-map", + Namespace: ns.Name, + }, + Data: map[string]string{ + "key": "value", + }, + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ns, helm1, nil, cm).Build() + cfg := makeServerConfig(client, t, "") + c := makeServer(cfg, t) + + res, err := c.GetInventory(ctx, &pb.GetInventoryRequest{ + Namespace: ns.Name, + ClusterName: cluster.DefaultCluster, + Kind: "HelmRelease", + Name: helm1.Name, + WithChildren: true, + }) + + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.Entries).To(HaveLen(1)) +} From 4d57bb9d6f51ec92ba6f32986b211374ecee1838 Mon Sep 17 00:00:00 2001 From: AlinaGoaga Date: Tue, 19 Sep 2023 14:25:28 +0200 Subject: [PATCH 4/7] Add test for spec kubeconfig scenario - updated --- core/server/inventory_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 1ac97e2c3d..743ed64295 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -197,7 +197,7 @@ func TestGetBlankInventoryKustomization(t *testing.T) { g.Expect(res.Entries).To(HaveLen(0)) } -func TestGetInventoryHelmReleaseWithSecret(t *testing.T) { +func TestGetInventoryHelmRelease(t *testing.T) { g := NewGomegaWithT(t) scheme, err := kube.CreateScheme() @@ -316,7 +316,7 @@ func TestGetInventoryHelmReleaseWithKubeconfig(t *testing.T) { }, } - client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ns, helm1, nil, cm).Build() + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ns, helm1, cm).Build() cfg := makeServerConfig(client, t, "") c := makeServer(cfg, t) From 8c822651e237f8430c425819683742d2166819ef Mon Sep 17 00:00:00 2001 From: AlinaGoaga Date: Tue, 19 Sep 2023 15:04:56 +0200 Subject: [PATCH 5/7] Add test for spec kubeconfig scenario - updated2 --- core/server/inventory_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 743ed64295..33e6b480fa 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -329,5 +329,5 @@ func TestGetInventoryHelmReleaseWithKubeconfig(t *testing.T) { }) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(res.Entries).To(HaveLen(1)) + g.Expect(res.Entries).To(HaveLen(0)) } From 32e8b19c93cdef54d2507e3c5769e92d81d31771 Mon Sep 17 00:00:00 2001 From: AlinaGoaga Date: Wed, 20 Sep 2023 15:41:40 +0200 Subject: [PATCH 6/7] Show user info alert when spec kubeconfig is set --- core/server/inventory_test.go | 16 +--------------- ui/components/Alert.tsx | 17 ++++++++++++++--- ui/components/AutomationDetail.tsx | 17 +++++++++++++---- ui/lib/objects.ts | 4 ++++ 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/core/server/inventory_test.go b/core/server/inventory_test.go index 33e6b480fa..16cc1138b6 100644 --- a/core/server/inventory_test.go +++ b/core/server/inventory_test.go @@ -302,21 +302,7 @@ func TestGetInventoryHelmReleaseWithKubeconfig(t *testing.T) { }, } - cm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - Kind: "ConfigMap", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "config-map", - Namespace: ns.Name, - }, - Data: map[string]string{ - "key": "value", - }, - } - - client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ns, helm1, cm).Build() + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(ns, helm1).Build() cfg := makeServerConfig(client, t, "") c := makeServer(cfg, t) diff --git a/ui/components/Alert.tsx b/ui/components/Alert.tsx index c3e49ab0af..7234378769 100644 --- a/ui/components/Alert.tsx +++ b/ui/components/Alert.tsx @@ -4,6 +4,7 @@ import styled from "styled-components"; import Flex from "./Flex"; import Icon, { IconType } from "./Icon"; import Text from "./Text"; +import { ThemeTypes } from "../contexts/AppContext"; /** Alert Properties */ export interface Props { @@ -23,9 +24,7 @@ function UnstyledAlert({ center, title, message, severity, className }: Props) { return ( - } + icon={} severity={severity} > {title} @@ -37,11 +36,23 @@ function UnstyledAlert({ center, title, message, severity, className }: Props) { const Alert = styled(UnstyledAlert)` .MuiAlert-standardError { + svg { + color: ${(props) => props.theme.colors.alertDark}; + } background-color: ${(props) => props.theme.colors.alertLight}; } .MuiAlertTitle-root { color: ${(props) => props.theme.colors.black}; } + .MuiAlert-standardInfo { + svg { + color: ${(props) => props.theme.colors.primary10}; + } + background-color: ${(props) => + props.theme.mode === ThemeTypes.Dark + ? props.theme.colors.primary20 + : null}; + } `; export default Alert; diff --git a/ui/components/AutomationDetail.tsx b/ui/components/AutomationDetail.tsx index d15f0e1fc4..f247b01218 100644 --- a/ui/components/AutomationDetail.tsx +++ b/ui/components/AutomationDetail.tsx @@ -24,6 +24,10 @@ import SyncActions from "./SyncActions"; import Text from "./Text"; import Timestamp from "./Timestamp"; import YamlView from "./YamlView"; +import Alert from "./Alert"; + +const hrInfoMessage = + "spec.Kubeconfig is set on this HelmRelease. Details about reconciled objects are not available."; type Props = { automation: Automation; @@ -87,10 +91,15 @@ function AutomationDetail({ component: () => { return ( - + {automation.type === "HelmRelease" && + (automation as HelmRelease).kubeConfig === "" ? ( + + ) : ( + + )} ); }, diff --git a/ui/lib/objects.ts b/ui/lib/objects.ts index e7d5185902..af6af25b45 100644 --- a/ui/lib/objects.ts +++ b/ui/lib/objects.ts @@ -301,6 +301,10 @@ export class HelmRelease extends FluxObject { get lastAttemptedRevision(): string { return this.obj.status?.lastAttemptedRevision || ""; } + + get kubeConfig(): string { + return this.obj.spec?.kubeConfig?.secretRef?.name || ""; + } } export class Provider extends FluxObject { From b841289190142fd83e563a2c0af2c3a8e90e9c6a Mon Sep 17 00:00:00 2001 From: AlinaGoaga Date: Wed, 20 Sep 2023 15:47:40 +0200 Subject: [PATCH 7/7] Solve linting warnings --- ui/components/Alert.tsx | 2 +- ui/components/AutomationDetail.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/components/Alert.tsx b/ui/components/Alert.tsx index 7234378769..215d2135a3 100644 --- a/ui/components/Alert.tsx +++ b/ui/components/Alert.tsx @@ -1,10 +1,10 @@ import { Alert as MaterialAlert, AlertTitle } from "@material-ui/lab"; import * as React from "react"; import styled from "styled-components"; +import { ThemeTypes } from "../contexts/AppContext"; import Flex from "./Flex"; import Icon, { IconType } from "./Icon"; import Text from "./Text"; -import { ThemeTypes } from "../contexts/AppContext"; /** Alert Properties */ export interface Props { diff --git a/ui/components/AutomationDetail.tsx b/ui/components/AutomationDetail.tsx index f247b01218..9dec01033b 100644 --- a/ui/components/AutomationDetail.tsx +++ b/ui/components/AutomationDetail.tsx @@ -5,6 +5,7 @@ import { createCanaryCondition, useGetInventory } from "../hooks/inventory"; import { Condition, Kind, ObjectRef } from "../lib/api/core/types.pb"; import { Automation, HelmRelease } from "../lib/objects"; import { automationLastUpdated } from "../lib/utils"; +import Alert from "./Alert"; import Collapsible from "./Collapsible"; import DependenciesView from "./DependenciesView"; import EventsTable from "./EventsTable"; @@ -24,7 +25,6 @@ import SyncActions from "./SyncActions"; import Text from "./Text"; import Timestamp from "./Timestamp"; import YamlView from "./YamlView"; -import Alert from "./Alert"; const hrInfoMessage = "spec.Kubeconfig is set on this HelmRelease. Details about reconciled objects are not available.";