From a6203d4e16f3e790ae0f49d9567314553034c86f Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Tue, 8 Feb 2022 14:03:17 +0100 Subject: [PATCH 1/7] Add 'update profile' command - Add cobra CLI command func and required flags - Refactor Add Profile func to re-use some functionality for Update Profile - Add acceptance test --- cmd/gitops/add/profiles/cmd.go | 8 +- cmd/gitops/add/profiles/cmd_test.go | 14 +- cmd/gitops/root/cmd.go | 2 + cmd/gitops/update/cmd.go | 23 ++ cmd/gitops/update/profiles/profiles.go | 104 +++++++++ .../update/profiles/profiles_suite_test.go | 13 ++ cmd/gitops/update/profiles/profiles_test.go | 71 ++++++ pkg/helm/releases.go | 80 +++++++ pkg/helm/releases_test.go | 113 +++++++++- pkg/services/profiles/add.go | 97 +------- pkg/services/profiles/add_test.go | 209 +++++++----------- pkg/services/profiles/get.go | 4 + pkg/services/profiles/get_test.go | 26 ++- pkg/services/profiles/profiles.go | 17 ++ pkg/services/profiles/update.go | 47 ++++ pkg/services/profiles/update_test.go | 130 +++++++++++ test/acceptance/test/profiles_test.go | 17 ++ 17 files changed, 737 insertions(+), 238 deletions(-) create mode 100644 cmd/gitops/update/cmd.go create mode 100644 cmd/gitops/update/profiles/profiles.go create mode 100644 cmd/gitops/update/profiles/profiles_suite_test.go create mode 100644 cmd/gitops/update/profiles/profiles_test.go create mode 100644 pkg/services/profiles/update.go create mode 100644 pkg/services/profiles/update_test.go diff --git a/cmd/gitops/add/profiles/cmd.go b/cmd/gitops/add/profiles/cmd.go index 88721fe13db..4c66e5ff56c 100644 --- a/cmd/gitops/add/profiles/cmd.go +++ b/cmd/gitops/add/profiles/cmd.go @@ -3,10 +3,8 @@ package profiles import ( "context" "fmt" - "math/rand" "os" "path/filepath" - "time" "github.com/Masterminds/semver/v3" "github.com/spf13/cobra" @@ -43,10 +41,10 @@ func AddCommand() *cobra.Command { cmd.Flags().StringVar(&opts.Name, "name", "", "Name of the profile") cmd.Flags().StringVar(&opts.Version, "version", "latest", "Version of the profile specified as semver (e.g.: 0.1.0) or as 'latest'") - cmd.Flags().StringVar(&opts.ConfigRepo, "config-repo", "", "URL of external repository (if any) which will hold automation manifests") + cmd.Flags().StringVar(&opts.ConfigRepo, "config-repo", "", "URL of the external repository that contains the automation manifests") cmd.Flags().StringVar(&opts.Cluster, "cluster", "", "Name of the cluster to add the profile to") cmd.Flags().StringVar(&opts.ProfilesPort, "profiles-port", server.DefaultPort, "Port the Profiles API is running on") - cmd.Flags().BoolVar(&opts.AutoMerge, "auto-merge", false, "If set, 'gitops add profile' will merge automatically into the repository's default branch") + cmd.Flags().BoolVar(&opts.AutoMerge, "auto-merge", false, "If set, 'gitops add profile' will merge automatically into the repository's branch") cmd.Flags().StringVar(&opts.Kubeconfig, "kubeconfig", filepath.Join(homedir.HomeDir(), ".kube", "config"), "Absolute path to the kubeconfig file") requiredFlags := []string{"name", "config-repo", "cluster"} @@ -61,8 +59,6 @@ func AddCommand() *cobra.Command { func addProfileCmdRunE() func(*cobra.Command, []string) error { return func(cmd *cobra.Command, args []string) error { - rand.Seed(time.Now().UnixNano()) - log := internal.NewCLILogger(os.Stdout) fluxClient := flux.New(osys.New(), &runner.CLIRunner{}) factory := services.NewFactory(fluxClient, log) diff --git a/cmd/gitops/add/profiles/cmd_test.go b/cmd/gitops/add/profiles/cmd_test.go index 57755f4c4c1..1c58f2efb31 100644 --- a/cmd/gitops/add/profiles/cmd_test.go +++ b/cmd/gitops/add/profiles/cmd_test.go @@ -2,28 +2,20 @@ package profiles_test import ( "github.com/go-resty/resty/v2" - "github.com/jarcoal/httpmock" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/spf13/cobra" "github.com/weaveworks/weave-gitops/cmd/gitops/root" ) -var _ = Describe("Add Profiles", func() { - var ( - cmd *cobra.Command - ) +var _ = Describe("Add a Profile", func() { + var cmd *cobra.Command BeforeEach(func() { client := resty.New() - httpmock.ActivateNonDefault(client.GetClient()) cmd = root.RootCmd(client) }) - AfterEach(func() { - httpmock.DeactivateAndReset() - }) - When("the flags are valid", func() { It("accepts all known flags for adding a profile", func() { cmd.SetArgs([]string{ @@ -42,7 +34,7 @@ var _ = Describe("Add Profiles", func() { }) When("flags are not valid", func() { - It("fails if --name, --cluster, and --config-repo are not provided", func() { + It("fails if --name, --cluster, or --config-repo are not provided", func() { cmd.SetArgs([]string{ "add", "profile", }) diff --git a/cmd/gitops/root/cmd.go b/cmd/gitops/root/cmd.go index 0bc6af69dc5..dca910f797f 100644 --- a/cmd/gitops/root/cmd.go +++ b/cmd/gitops/root/cmd.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/weaveworks/weave-gitops/cmd/gitops/check" + "github.com/weaveworks/weave-gitops/cmd/gitops/update" "github.com/go-resty/resty/v2" log "github.com/sirupsen/logrus" @@ -140,6 +141,7 @@ func RootCmd(client *resty.Client) *cobra.Command { rootCmd.AddCommand(ui.NewCommand()) rootCmd.AddCommand(get.GetCommand(&options.endpoint, client)) rootCmd.AddCommand(add.GetCommand(&options.endpoint, client)) + rootCmd.AddCommand(update.UpdateCommand(&options.endpoint, client)) rootCmd.AddCommand(delete.DeleteCommand(&options.endpoint, client)) rootCmd.AddCommand(resume.GetCommand()) rootCmd.AddCommand(suspend.GetCommand()) diff --git a/cmd/gitops/update/cmd.go b/cmd/gitops/update/cmd.go new file mode 100644 index 00000000000..cb3c0d43880 --- /dev/null +++ b/cmd/gitops/update/cmd.go @@ -0,0 +1,23 @@ +package update + +import ( + "github.com/weaveworks/weave-gitops/cmd/gitops/update/profiles" + + "github.com/go-resty/resty/v2" + "github.com/spf13/cobra" +) + +func UpdateCommand(endpoint *string, client *resty.Client) *cobra.Command { + cmd := &cobra.Command{ + Use: "update", + Short: "Update a Weave GitOps resource", + Example: ` +# Update an installed profile to a certain version +gitops update profile --name --version --config-repo --cluster --namespace +`, + } + + cmd.AddCommand(profiles.UpdateCommand()) + + return cmd +} diff --git a/cmd/gitops/update/profiles/profiles.go b/cmd/gitops/update/profiles/profiles.go new file mode 100644 index 00000000000..bdad4c02005 --- /dev/null +++ b/cmd/gitops/update/profiles/profiles.go @@ -0,0 +1,104 @@ +package profiles + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/Masterminds/semver/v3" + "github.com/spf13/cobra" + "github.com/weaveworks/weave-gitops/cmd/internal" + "github.com/weaveworks/weave-gitops/pkg/flux" + "github.com/weaveworks/weave-gitops/pkg/kube" + "github.com/weaveworks/weave-gitops/pkg/osys" + "github.com/weaveworks/weave-gitops/pkg/runner" + "github.com/weaveworks/weave-gitops/pkg/server" + "github.com/weaveworks/weave-gitops/pkg/services" + "github.com/weaveworks/weave-gitops/pkg/services/auth" + "github.com/weaveworks/weave-gitops/pkg/services/profiles" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/util/homedir" +) + +var opts profiles.UpdateOptions + +// UpdateCommand provides support for updating an installed profile's version. +func UpdateCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "profile", + Short: "Update a profile to a different version", + SilenceUsage: true, + SilenceErrors: true, + Example: ` + # Update an installed profile to a different version + gitops update profile --name=podinfo --cluster=prod --version=1.0.0 --config-repo=ssh://git@github.com/owner/config-repo.git --namespace=test-namespace + `, + RunE: updateProfileCmdRunE(), + } + + cmd.Flags().StringVar(&opts.Name, "name", "", "Name of the profile") + cmd.Flags().StringVar(&opts.Version, "version", "latest", "Version of the profile specified as semver (e.g.: 0.1.0) or as 'latest'") + cmd.Flags().StringVar(&opts.ConfigRepo, "config-repo", "", "URL of the external repository that contains the automation manifests") + cmd.Flags().StringVar(&opts.Cluster, "cluster", "", "Name of the cluster where the profile is installed") + cmd.Flags().StringVar(&opts.ProfilesPort, "profiles-port", server.DefaultPort, "Port the Profiles API is running on") + cmd.Flags().BoolVar(&opts.AutoMerge, "auto-merge", false, "If set, 'gitops update profile' will merge automatically into the repository's branch") + cmd.Flags().StringVar(&opts.Kubeconfig, "kubeconfig", filepath.Join(homedir.HomeDir(), ".kube", "config"), "Absolute path to the kubeconfig file") + + requiredFlags := []string{"name", "config-repo", "cluster", "version"} + for _, f := range requiredFlags { + if err := cobra.MarkFlagRequired(cmd.Flags(), f); err != nil { + panic(fmt.Errorf("unexpected error: %w", err)) + } + } + + return cmd +} + +func updateProfileCmdRunE() func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + log := internal.NewCLILogger(os.Stdout) + fluxClient := flux.New(osys.New(), &runner.CLIRunner{}) + factory := services.NewFactory(fluxClient, log) + providerClient := internal.NewGitProviderClient(os.Stdout, os.LookupEnv, auth.NewAuthCLIHandler, log) + + if opts.Version != "latest" { + if _, err := semver.StrictNewVersion(opts.Version); err != nil { + return fmt.Errorf("error parsing --version=%s: %w", opts.Version, err) + } + } + + var err error + if opts.Namespace, err = cmd.Flags().GetString("namespace"); err != nil { + return err + } + + config, err := clientcmd.BuildConfigFromFlags("", opts.Kubeconfig) + if err != nil { + return fmt.Errorf("error initializing kubernetes config: %w", err) + } + + clientSet, err := kubernetes.NewForConfig(config) + if err != nil { + return fmt.Errorf("error initializing kubernetes client: %w", err) + } + + kubeClient, _, err := kube.NewKubeHTTPClient() + if err != nil { + return fmt.Errorf("failed to create kube client: %w", err) + } + + _, gitProvider, err := factory.GetGitClients(context.Background(), kubeClient, providerClient, services.GitConfigParams{ + ConfigRepo: opts.ConfigRepo, + Namespace: opts.Namespace, + IsHelmRepository: true, + DryRun: false, + }) + if err != nil { + return fmt.Errorf("failed to get git clients: %w", err) + } + + return profiles.NewService(clientSet, log).Update(context.Background(), gitProvider, opts) + } +} diff --git a/cmd/gitops/update/profiles/profiles_suite_test.go b/cmd/gitops/update/profiles/profiles_suite_test.go new file mode 100644 index 00000000000..f522ba3a046 --- /dev/null +++ b/cmd/gitops/update/profiles/profiles_suite_test.go @@ -0,0 +1,13 @@ +package profiles_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestProfiles(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Profiles Suite") +} diff --git a/cmd/gitops/update/profiles/profiles_test.go b/cmd/gitops/update/profiles/profiles_test.go new file mode 100644 index 00000000000..72648a2d996 --- /dev/null +++ b/cmd/gitops/update/profiles/profiles_test.go @@ -0,0 +1,71 @@ +package profiles_test + +import ( + "github.com/weaveworks/weave-gitops/cmd/gitops/root" + + "github.com/go-resty/resty/v2" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/spf13/cobra" +) + +var _ = Describe("Update Profile(s)", func() { + var cmd *cobra.Command + + BeforeEach(func() { + cmd = root.RootCmd(resty.New()) + }) + + When("the flags are valid", func() { + It("accepts all known flags for updating a profile", func() { + cmd.SetArgs([]string{ + "update", "profile", + "--name", "podinfo", + "--version", "0.0.1", + "--cluster", "prod", + "--namespace", "test-namespace", + "--config-repo", "https://ssh@github:test/test.git", + "--auto-merge", "true", + }) + + err := cmd.Execute() + Expect(err.Error()).NotTo(ContainSubstring("unknown flag")) + }) + }) + + When("flags are not valid", func() { + It("fails if --name, --cluster, --version or --config-repo are not provided", func() { + cmd.SetArgs([]string{ + "update", "profile", + }) + + err := cmd.Execute() + Expect(err).To(MatchError("required flag(s) \"cluster\", \"config-repo\", \"name\", \"version\" not set")) + }) + + It("fails if given version is not valid semver", func() { + cmd.SetArgs([]string{ + "update", "profile", + "--name", "podinfo", + "--config-repo", "ssh://git@github.com/owner/config-repo.git", + "--cluster", "prod", + "--version", "&%*/v", + }) + + err := cmd.Execute() + Expect(err).To(MatchError("error parsing --version=&%*/v: Invalid Semantic Version")) + }) + }) + + When("a flag is unknown", func() { + It("fails", func() { + cmd.SetArgs([]string{ + "update", "profile", + "--unknown", "param", + }) + + err := cmd.Execute() + Expect(err).To(MatchError("unknown flag: --unknown")) + }) + }) +}) diff --git a/pkg/helm/releases.go b/pkg/helm/releases.go index a70b1107806..60b7e602d7d 100644 --- a/pkg/helm/releases.go +++ b/pkg/helm/releases.go @@ -1,12 +1,19 @@ package helm import ( + "bytes" + "fmt" + "io" "time" + "github.com/fluxcd/go-git-providers/gitprovider" + "github.com/fluxcd/helm-controller/api/v2beta1" helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" + "gopkg.in/yaml.v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + kyaml "sigs.k8s.io/yaml" ) // MakeHelmRelease returns a HelmRelease object given a name, version, cluster, namespace, and HelmRepository's name and namespace. @@ -37,3 +44,76 @@ func MakeHelmRelease(name, version, cluster, namespace string, helmRepository ty }, } } + +// AppendHelmReleaseToFile appends a HelmRelease to a gitProvider CommitFile given a condition, file name and file path. +func AppendHelmReleaseToFile(files []*gitprovider.CommitFile, newRelease *v2beta1.HelmRelease, condition func(r, newRelease v2beta1.HelmRelease) error, fileName, filePath string) (gitprovider.CommitFile, error) { + var content string + + for _, f := range files { + if f.Path != nil && *f.Path == filePath { + if f.Content == nil || *f.Content == "" { + break + } + + manifestByteSlice, err := splitYAML([]byte(*f.Content)) + if err != nil { + return gitprovider.CommitFile{}, fmt.Errorf("error splitting %s: %w", fileName, err) + } + + for _, manifestBytes := range manifestByteSlice { + var r v2beta1.HelmRelease + if err := kyaml.Unmarshal(manifestBytes, &r); err != nil { + return gitprovider.CommitFile{}, fmt.Errorf("error unmarshaling %s: %w", fileName, err) + } + + if err := condition(r, *newRelease); err != nil { + return gitprovider.CommitFile{}, err + } + } + + content = *f.Content + + break + } + } + + helmReleaseManifest, err := kyaml.Marshal(newRelease) + if err != nil { + return gitprovider.CommitFile{}, fmt.Errorf("failed to marshal new HelmRelease: %w", err) + } + + content += "\n---\n" + string(helmReleaseManifest) + + return gitprovider.CommitFile{ + Path: &filePath, + Content: &content, + }, nil +} + +// splitYAML splits a manifest file that may contain multiple YAML resources separated by '---' +// and validates that each element is YAML. +func splitYAML(resources []byte) ([][]byte, error) { + var splitResources [][]byte + + decoder := yaml.NewDecoder(bytes.NewReader(resources)) + + for { + var value interface{} + if err := decoder.Decode(&value); err != nil { + if err == io.EOF { + break + } + + return nil, err + } + + valueBytes, err := yaml.Marshal(value) + if err != nil { + return nil, err + } + + splitResources = append(splitResources, valueBytes) + } + + return splitResources, nil +} diff --git a/pkg/helm/releases_test.go b/pkg/helm/releases_test.go index 85c2ba73883..10a49983d47 100644 --- a/pkg/helm/releases_test.go +++ b/pkg/helm/releases_test.go @@ -1,16 +1,23 @@ package helm_test import ( + "fmt" "time" + "github.com/weaveworks/weave-gitops/pkg/git" + "github.com/weaveworks/weave-gitops/pkg/helm" + "github.com/weaveworks/weave-gitops/pkg/models" + + "github.com/fluxcd/go-git-providers/gitprovider" + "github.com/fluxcd/helm-controller/api/v2beta1" helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/weaveworks/weave-gitops/pkg/helm" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/yaml" ) var _ = Describe("MakeHelmRelease", func() { @@ -60,3 +67,107 @@ var _ = Describe("MakeHelmRelease", func() { Expect(cmp.Diff(actualHelmRelease, expectedHelmRelease)).To(BeEmpty()) }) }) + +var _ = Describe("AppendHelmReleaseToFile", func() { + var ( + newRelease *v2beta1.HelmRelease + existingFile *gitprovider.CommitFile + path string + content string + ) + + BeforeEach(func() { + newRelease = helm.MakeHelmRelease( + "podinfo", "6.0.0", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + path = git.GetProfilesPath("prod", models.WegoProfilesPath) + }) + + When("the file does not exist", func() { + It("creates one with the new helm release", func() { + file, err := helm.AppendHelmReleaseToFile(makeTestFiles(), newRelease, makeTestCondition, "profiles.yaml", path) + Expect(err).NotTo(HaveOccurred()) + r, err := yaml.Marshal(newRelease) + Expect(err).NotTo(HaveOccurred()) + Expect(*file.Content).To(ContainSubstring(string(r))) + }) + }) + + When("the file exists", func() { + When("the given condition succeeds", func() { + It("appends the release to the manifest", func() { + existingRelease := helm.MakeHelmRelease( + "podinfo", "6.0.1", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + r, _ := yaml.Marshal(existingRelease) + content = string(r) + file, err := helm.AppendHelmReleaseToFile([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, newRelease, makeTestCondition, "profiles.yaml", path) + Expect(err).NotTo(HaveOccurred()) + Expect(*file.Content).To(ContainSubstring(string(r))) + }) + }) + + When("the given condition returns an error", func() { + It("fails to add the profile", func() { + existingRelease, _ := yaml.Marshal(newRelease) + content = string(existingRelease) + existingFile = &gitprovider.CommitFile{ + Path: &path, + Content: &content, + } + _, err := helm.AppendHelmReleaseToFile([]*gitprovider.CommitFile{existingFile}, newRelease, makeTestCondition, "profiles.yaml", path) + Expect(err).To(MatchError("err")) + }) + }) + + It("fails if the manifest contains a resource that is not a HelmRelease", func() { + content = "content" + _, err := helm.AppendHelmReleaseToFile([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, newRelease, makeTestCondition, "profiles.yaml", path) + Expect(err).To(MatchError("error unmarshaling profiles.yaml: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type v2beta1.HelmRelease")) + }) + }) +}) + +func makeTestCondition(r, newRelease v2beta1.HelmRelease) error { + if r.Spec.Chart.Spec.Version == newRelease.Spec.Chart.Spec.Version { + return fmt.Errorf("err") + } + + return nil +} + +func makeTestFiles() []*gitprovider.CommitFile { + path0 := ".weave-gitops/clusters/prod/system/wego-system.yaml" + content0 := "machine1 yaml content" + path1 := ".weave-gitops/clusters/prod/system/podinfo-helm-release.yaml" + content1 := "machine2 yaml content" + + files := []gitprovider.CommitFile{ + { + Path: &path0, + Content: &content0, + }, + { + Path: &path1, + Content: &content1, + }, + } + + commitFiles := make([]*gitprovider.CommitFile, 0) + for _, file := range files { + commitFiles = append(commitFiles, &gitprovider.CommitFile{ + Path: file.Path, + Content: file.Content, + }) + } + + return commitFiles +} diff --git a/pkg/services/profiles/add.go b/pkg/services/profiles/add.go index d58dcd95e8d..3db9a1a6e41 100644 --- a/pkg/services/profiles/add.go +++ b/pkg/services/profiles/add.go @@ -1,22 +1,17 @@ package profiles import ( - "bytes" "context" "fmt" - "io" "github.com/google/uuid" "github.com/weaveworks/weave-gitops/pkg/git" "github.com/weaveworks/weave-gitops/pkg/gitproviders" "github.com/weaveworks/weave-gitops/pkg/helm" "github.com/weaveworks/weave-gitops/pkg/models" - "k8s.io/apimachinery/pkg/types" "github.com/fluxcd/go-git-providers/gitprovider" "github.com/fluxcd/helm-controller/api/v2beta1" - "gopkg.in/yaml.v2" - kyaml "sigs.k8s.io/yaml" ) const AddCommitMessage = "Add Profile manifests" @@ -52,7 +47,7 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi return fmt.Errorf("failed to get default branch: %w", err) } - availableProfile, version, err := s.GetProfile(ctx, GetOptions{ + helmRepo, version, err := s.discoverHelmRepository(ctx, GetOptions{ Name: opts.Name, Version: opts.Version, Cluster: opts.Cluster, @@ -60,16 +55,7 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi Port: opts.ProfilesPort, }) if err != nil { - return fmt.Errorf("failed to get profiles from cluster: %w", err) - } - - if availableProfile.GetHelmRepository().GetName() == "" || availableProfile.GetHelmRepository().GetNamespace() == "" { - return fmt.Errorf("failed to discover HelmRepository's name and namespace") - } - - helmRepo := types.NamespacedName{ - Name: availableProfile.HelmRepository.Name, - Namespace: availableProfile.HelmRepository.Namespace, + return fmt.Errorf("failed to discover HelmRepository: %w", err) } newRelease := helm.MakeHelmRelease(opts.Name, version, opts.Cluster, opts.Namespace, helmRepo) @@ -79,7 +65,7 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi return fmt.Errorf("failed to get files in '%s' for config repository %q: %s", git.GetSystemPath(opts.Cluster), configRepoURL, err) } - file, err := AppendProfileToFile(files, newRelease, git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath)) + file, err := helm.AppendHelmReleaseToFile(files, newRelease, profileIsInstalled, models.WegoProfilesPath, git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath)) if err != nil { return fmt.Errorf("failed to append HelmRelease to profiles file: %w", err) } @@ -119,79 +105,10 @@ func (s *ProfilesSvc) printAddSummary(opts AddOptions) { s.Logger.Println("Namespace: %s\n", opts.Namespace) } -// AppendProfileToFile appends a HelmRelease to profiles.yaml if file does not contain other HelmRelease with the same name and namespace. -func AppendProfileToFile(files []*gitprovider.CommitFile, newRelease *v2beta1.HelmRelease, path string) (gitprovider.CommitFile, error) { - var content string - - for _, f := range files { - if f.Path != nil && *f.Path == path { - if f.Content == nil || *f.Content == "" { - break - } - - manifestByteSlice, err := splitYAML([]byte(*f.Content)) - if err != nil { - return gitprovider.CommitFile{}, fmt.Errorf("error splitting %s: %w", models.WegoProfilesPath, err) - } - - for _, manifestBytes := range manifestByteSlice { - var r v2beta1.HelmRelease - if err := kyaml.Unmarshal(manifestBytes, &r); err != nil { - return gitprovider.CommitFile{}, fmt.Errorf("error unmarshaling %s: %w", models.WegoProfilesPath, err) - } - - if profileIsInstalled(r, *newRelease) { - return gitprovider.CommitFile{}, fmt.Errorf("version %s of profile '%s' already exists in namespace %s", r.Spec.Chart.Spec.Version, r.Name, r.Namespace) - } - } - - content = *f.Content - - break - } +func profileIsInstalled(r, newRelease v2beta1.HelmRelease) error { + if r.Name == newRelease.Name && r.Namespace == newRelease.Namespace { + return fmt.Errorf("profile '%s' is already installed in namespace %s; please use 'gitops update profile' if you wish to update it", r.Name, r.Namespace) } - helmReleaseManifest, err := kyaml.Marshal(newRelease) - if err != nil { - return gitprovider.CommitFile{}, fmt.Errorf("failed to marshal new HelmRelease: %w", err) - } - - content += "\n---\n" + string(helmReleaseManifest) - - return gitprovider.CommitFile{ - Path: &path, - Content: &content, - }, nil -} - -// splitYAML splits a manifest file that may contain multiple YAML resources separated by '---' -// and validates that each element is YAML. -func splitYAML(resources []byte) ([][]byte, error) { - var splitResources [][]byte - - decoder := yaml.NewDecoder(bytes.NewReader(resources)) - - for { - var value interface{} - if err := decoder.Decode(&value); err != nil { - if err == io.EOF { - break - } - - return nil, err - } - - valueBytes, err := yaml.Marshal(value) - if err != nil { - return nil, err - } - - splitResources = append(splitResources, valueBytes) - } - - return splitResources, nil -} - -func profileIsInstalled(r, newRelease v2beta1.HelmRelease) bool { - return r.Name == newRelease.Name && r.Namespace == newRelease.Namespace && r.Spec.Chart.Spec.Version == newRelease.Spec.Chart.Spec.Version + return nil } diff --git a/pkg/services/profiles/add_test.go b/pkg/services/profiles/add_test.go index 318ea5b836d..0b9385c2ebc 100644 --- a/pkg/services/profiles/add_test.go +++ b/pkg/services/profiles/add_test.go @@ -13,7 +13,6 @@ import ( "github.com/weaveworks/weave-gitops/pkg/vendorfakes/fakegitprovider" "github.com/fluxcd/go-git-providers/gitprovider" - "github.com/fluxcd/helm-controller/api/v2beta1" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/types" @@ -52,99 +51,121 @@ var _ = Describe("Add", func() { When("the config repository exists", func() { When("the version and HelmRepository name and namespace were discovered", func() { - JustBeforeEach(func() { - gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetDefaultBranchReturns("main", nil) - gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getProfilesResp), nil + When("the HelmRelease was appended to profiles.yaml", func() { + BeforeEach(func() { + gitProviders.RepositoryExistsReturns(true, nil) + gitProviders.GetDefaultBranchReturns("main", nil) + gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil + }) }) - }) - - JustAfterEach(func() { - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) - Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) - Expect(gitProviders.CreatePullRequestCallCount()).To(Equal(1)) - }) - It("creates a helm release with the latest available version of the profile", func() { - fakePR.GetReturns(gitprovider.PullRequestInfo{ - WebURL: "url", + AfterEach(func() { + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) + Expect(gitProviders.CreatePullRequestCallCount()).To(Equal(1)) }) - gitProviders.CreatePullRequestReturns(fakePR, nil) - Expect(profilesSvc.Add(context.TODO(), gitProviders, addOptions)).Should(Succeed()) - }) - When("auto-merge is enabled", func() { - It("merges the PR that was created", func() { + It("creates a helm release with the latest available version of the profile", func() { fakePR.GetReturns(gitprovider.PullRequestInfo{ WebURL: "url", - Number: 42, }) gitProviders.CreatePullRequestReturns(fakePR, nil) - addOptions.AutoMerge = true Expect(profilesSvc.Add(context.TODO(), gitProviders, addOptions)).Should(Succeed()) }) - When("the PR fails to be merged", func() { - It("returns an error", func() { + When("auto-merge is enabled", func() { + It("merges the PR that was created", func() { fakePR.GetReturns(gitprovider.PullRequestInfo{ WebURL: "url", + Number: 42, }) gitProviders.CreatePullRequestReturns(fakePR, nil) - gitProviders.MergePullRequestReturns(fmt.Errorf("err")) addOptions.AutoMerge = true - err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(MatchError("error auto-merging PR: err")) + Expect(profilesSvc.Add(context.TODO(), gitProviders, addOptions)).Should(Succeed()) }) - }) - }) - When("an existing version other than 'latest' is specified", func() { - It("creates a helm release with that version", func() { - addOptions.Version = "6.0.0" - fakePR.GetReturns(gitprovider.PullRequestInfo{ - WebURL: "url", + When("the PR fails to be merged", func() { + It("returns an error", func() { + fakePR.GetReturns(gitprovider.PullRequestInfo{ + WebURL: "url", + }) + gitProviders.CreatePullRequestReturns(fakePR, nil) + gitProviders.MergePullRequestReturns(fmt.Errorf("err")) + addOptions.AutoMerge = true + err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) + Expect(err).To(MatchError("error auto-merging PR: err")) + }) }) - fakePR.GetReturns(gitprovider.PullRequestInfo{ - WebURL: "url", + }) + + When("an existing version other than 'latest' is specified", func() { + It("creates a helm release with that version", func() { + addOptions.Version = "6.0.0" + fakePR.GetReturns(gitprovider.PullRequestInfo{ + WebURL: "url", + }) + gitProviders.CreatePullRequestReturns(fakePR, nil) + err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) + Expect(err).To(BeNil()) }) - gitProviders.CreatePullRequestReturns(fakePR, nil) - err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(BeNil()) }) - }) - When("it fails to create a pull request to write the helm release to the config repo", func() { - It("returns an error when ", func() { - gitProviders.CreatePullRequestReturns(nil, fmt.Errorf("err")) - err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(MatchError("failed to create pull request: err")) + When("it fails to create a pull request to write the helm release to the config repo", func() { + It("returns an error", func() { + gitProviders.CreatePullRequestReturns(nil, fmt.Errorf("err")) + err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) + Expect(err).To(MatchError("failed to create pull request: err")) + }) }) }) }) - When("it fails to get a list of available profiles from the cluster", func() { - JustBeforeEach(func() { + When("profiles.yaml contains a HelmRelease with the same name in that namespace", func() { + BeforeEach(func() { gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) + gitProviders.GetDefaultBranchReturns("main", nil) + + existingRelease := helm.MakeHelmRelease( + "podinfo", "6.0.1", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + r, _ := yaml.Marshal(existingRelease) + content := string(r) + path := git.GetProfilesPath("prod", models.WegoProfilesPath) + gitProviders.GetRepoDirFilesReturns([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, nil) + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil + }) }) - It("fails if it's unable to get a matching available profile from the cluster", func() { + AfterEach(func() { + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) + }) + + It("fails to append the new HelmRelease to profiles.yaml", func() { + gitProviders.RepositoryExistsReturns(true, nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapperWithErr("nope"), nil + return true, newFakeResponseWrapper(getProfilesResp), nil }) err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(MatchError("failed to get profiles from cluster: failed to make GET request to service weave-system/wego-app path \"/v1/profiles\": nope")) - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + Expect(err).To(MatchError("failed to append HelmRelease to profiles file: profile 'prod-podinfo' is already installed in namespace weave-system; please use 'gitops update profile' if you wish to update it")) }) + }) - It("fails if it's unable to discover the HelmRepository's name and namespace values", func() { + When("it fails to discover the HelmRepository name and namespace", func() { + It("fails if it's unable to get a matching available profile from the cluster", func() { + gitProviders.RepositoryExistsReturns(true, nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getRespWithoutHelmRepo()), nil + return true, newFakeResponseWrapperWithErr("nope"), nil }) err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(MatchError("failed to discover HelmRepository's name and namespace")) + Expect(err).To(MatchError("failed to discover HelmRepository: failed to get profiles from cluster: failed to make GET request to service weave-system/wego-app path \"/v1/profiles\": nope")) Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) }) }) @@ -157,13 +178,13 @@ var _ = Describe("Add", func() { }) addOptions.Version = "7.0.0" err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(MatchError("failed to get profiles from cluster: version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) + Expect(err).To(MatchError("failed to discover HelmRepository: failed to get profiles from cluster: version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) }) }) }) - When("the config repository exists", func() { + When("the config repository does not exist", func() { It("fails if the --config-repo url format is wrong", func() { addOptions = profiles.AddOptions{ Name: "foo", @@ -184,76 +205,6 @@ var _ = Describe("Add", func() { }) }) -var _ = Describe("AppendProfileToFile", func() { - var ( - newRelease *v2beta1.HelmRelease - existingFile *gitprovider.CommitFile - path string - content string - ) - - BeforeEach(func() { - newRelease = helm.MakeHelmRelease( - "podinfo", "6.0.0", "prod", "weave-system", - types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, - ) - path = git.GetProfilesPath("prod", models.WegoProfilesPath) - }) - - When("profiles.yaml does not exist", func() { - It("creates one with the new helm release", func() { - file, err := profiles.AppendProfileToFile(makeTestFiles(), newRelease, path) - Expect(err).NotTo(HaveOccurred()) - r, err := yaml.Marshal(newRelease) - Expect(err).NotTo(HaveOccurred()) - Expect(*file.Content).To(ContainSubstring(string(r))) - }) - }) - - When("profiles.yaml exists", func() { - When("the manifest contain a release with the same name in that namespace", func() { - When("the version is different", func() { - It("appends the release to the manifest", func() { - existingRelease := helm.MakeHelmRelease( - "podinfo", "6.0.1", "prod", "weave-system", - types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, - ) - r, _ := yaml.Marshal(existingRelease) - content = string(r) - file, err := profiles.AppendProfileToFile([]*gitprovider.CommitFile{{ - Path: &path, - Content: &content, - }}, newRelease, path) - Expect(err).NotTo(HaveOccurred()) - Expect(*file.Content).To(ContainSubstring(string(r))) - }) - }) - - When("the version is the same", func() { - It("fails to add the profile", func() { - existingRelease, _ := yaml.Marshal(newRelease) - content = string(existingRelease) - existingFile = &gitprovider.CommitFile{ - Path: &path, - Content: &content, - } - _, err := profiles.AppendProfileToFile([]*gitprovider.CommitFile{existingFile}, newRelease, path) - Expect(err).To(MatchError("version 6.0.0 of profile 'prod-podinfo' already exists in namespace weave-system")) - }) - }) - }) - - It("fails if the manifest contains a resource that is not a HelmRelease", func() { - content = "content" - _, err := profiles.AppendProfileToFile([]*gitprovider.CommitFile{{ - Path: &path, - Content: &content, - }}, newRelease, path) - Expect(err).To(MatchError("error unmarshaling profiles.yaml: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type v2beta1.HelmRelease")) - }) - }) -}) - func makeTestFiles() []*gitprovider.CommitFile { path0 := ".weave-gitops/clusters/prod/system/wego-system.yaml" content0 := "machine1 yaml content" diff --git a/pkg/services/profiles/get.go b/pkg/services/profiles/get.go index 62e8e6c12fb..fa21f17fa23 100644 --- a/pkg/services/profiles/get.go +++ b/pkg/services/profiles/get.go @@ -86,6 +86,10 @@ func (s *ProfilesSvc) GetProfile(ctx context.Context, opts GetOptions) (*pb.Prof version = opts.Version } + if p.GetHelmRepository().GetName() == "" || p.GetHelmRepository().GetNamespace() == "" { + return nil, "", fmt.Errorf("HelmRepository's name or namespace is empty") + } + return p, version, nil } } diff --git a/pkg/services/profiles/get_test.go b/pkg/services/profiles/get_test.go index b494b4ff144..0f909e6d4c2 100644 --- a/pkg/services/profiles/get_test.go +++ b/pkg/services/profiles/get_test.go @@ -154,7 +154,7 @@ podinfo Podinfo Helm chart for Kubernetes 6.0.0,6.0.1 Expect(version).To(Equal("6.0.1")) }) - It("it fails to return a list of available profiles from the cluster", func() { + It("fails to return a list of available profiles from the cluster", func() { clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapperWithErr("nope"), nil }) @@ -195,6 +195,30 @@ podinfo Podinfo Helm chart for Kubernetes 6.0.0,6.0.1 _, _, err := profilesSvc.GetProfile(context.TODO(), opts) Expect(err).To(MatchError("no version found for profile 'podinfo' in prod/test-namespace")) }) + + It("fails if the available profile's HelmRepository name or namespace if empty", func() { + badProfileResp := `{ + "profiles": [ + { + "name": "podinfo", + "helmRepository": { + "name": "", + "namespace": "" + }, + "availableVersions": [ + "6.0.0", + "6.0.1" + ] + } + ] + } + ` + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(badProfileResp), nil + }) + _, _, err := profilesSvc.GetProfile(context.TODO(), opts) + Expect(err).To(MatchError("HelmRepository's name or namespace is empty")) + }) }) }) diff --git a/pkg/services/profiles/profiles.go b/pkg/services/profiles/profiles.go index fde640411bf..5e682d74f64 100644 --- a/pkg/services/profiles/profiles.go +++ b/pkg/services/profiles/profiles.go @@ -2,9 +2,12 @@ package profiles import ( "context" + "fmt" "github.com/weaveworks/weave-gitops/pkg/gitproviders" "github.com/weaveworks/weave-gitops/pkg/logger" + + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" ) @@ -21,6 +24,8 @@ type ProfilesService interface { Add(ctx context.Context, gitProvider gitproviders.GitProvider, opts AddOptions) error // Get lists all the available profiles in a cluster Get(ctx context.Context, opts GetOptions) error + // Update updates a profile + Update(ctx context.Context, gitProvider gitproviders.GitProvider, opts UpdateOptions) error } type ProfilesSvc struct { @@ -34,3 +39,15 @@ func NewService(clientSet kubernetes.Interface, log logger.Logger) *ProfilesSvc Logger: log, } } + +func (s *ProfilesSvc) discoverHelmRepository(ctx context.Context, opts GetOptions) (types.NamespacedName, string, error) { + availableProfile, version, err := s.GetProfile(ctx, opts) + if err != nil { + return types.NamespacedName{}, "", fmt.Errorf("failed to get profiles from cluster: %w", err) + } + + return types.NamespacedName{ + Name: availableProfile.HelmRepository.Name, + Namespace: availableProfile.HelmRepository.Namespace, + }, version, nil +} diff --git a/pkg/services/profiles/update.go b/pkg/services/profiles/update.go new file mode 100644 index 00000000000..42dc110eb31 --- /dev/null +++ b/pkg/services/profiles/update.go @@ -0,0 +1,47 @@ +package profiles + +import ( + "context" + "fmt" + + "github.com/weaveworks/weave-gitops/pkg/gitproviders" +) + +type UpdateOptions struct { + Name string + Cluster string + ConfigRepo string + Version string + ProfilesPort string + Namespace string + Kubeconfig string + AutoMerge bool +} + +// Update updates an installed profile +func (s *ProfilesSvc) Update(ctx context.Context, gitProvider gitproviders.GitProvider, opts UpdateOptions) error { + configRepoURL, err := gitproviders.NewRepoURL(opts.ConfigRepo) + if err != nil { + return fmt.Errorf("failed to parse url: %w", err) + } + + repoExists, err := gitProvider.RepositoryExists(ctx, configRepoURL) + if err != nil { + return fmt.Errorf("failed to check whether repository exists: %w", err) + } else if !repoExists { + return fmt.Errorf("repository %q could not be found", configRepoURL) + } + + _, _, err = s.GetProfile(ctx, GetOptions{ + Name: opts.Name, + Version: opts.Version, + Cluster: opts.Cluster, + Namespace: opts.Namespace, + Port: opts.ProfilesPort, + }) + if err != nil { + return fmt.Errorf("failed to get profiles from cluster: %w", err) + } + + return nil +} diff --git a/pkg/services/profiles/update_test.go b/pkg/services/profiles/update_test.go new file mode 100644 index 00000000000..840e193c3b5 --- /dev/null +++ b/pkg/services/profiles/update_test.go @@ -0,0 +1,130 @@ +package profiles_test + +import ( + "context" + + "github.com/fluxcd/go-git-providers/gitprovider" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "k8s.io/client-go/kubernetes/fake" + restclient "k8s.io/client-go/rest" + "k8s.io/client-go/testing" + + "github.com/weaveworks/weave-gitops/pkg/gitproviders/gitprovidersfakes" + "github.com/weaveworks/weave-gitops/pkg/logger/loggerfakes" + "github.com/weaveworks/weave-gitops/pkg/services/profiles" + "github.com/weaveworks/weave-gitops/pkg/vendorfakes/fakegitprovider" +) + +var updateOptions profiles.UpdateOptions + +var _ = Describe("Update", func() { + var ( + gitProviders *gitprovidersfakes.FakeGitProvider + profilesSvc *profiles.ProfilesSvc + clientSet *fake.Clientset + fakeLogger *loggerfakes.FakeLogger + fakePR *fakegitprovider.PullRequest + ) + + BeforeEach(func() { + gitProviders = &gitprovidersfakes.FakeGitProvider{} + clientSet = fake.NewSimpleClientset() + fakeLogger = &loggerfakes.FakeLogger{} + fakePR = &fakegitprovider.PullRequest{} + profilesSvc = profiles.NewService(clientSet, fakeLogger) + + updateOptions = profiles.UpdateOptions{ + ConfigRepo: "ssh://git@github.com/owner/config-repo.git", + Name: "podinfo", + Cluster: "prod", + Namespace: "weave-system", + Version: "latest", + } + }) + + When("the config repository exists", func() { + When("the version and HelmRepository name and namespace were discovered", func() { + JustBeforeEach(func() { + gitProviders.RepositoryExistsReturns(true, nil) + gitProviders.GetDefaultBranchReturns("main", nil) + gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil + }) + }) + + JustAfterEach(func() { + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) + Expect(gitProviders.CreatePullRequestCallCount()).To(Equal(1)) + }) + + It("creates a PR to change the version of a Helm Release", func() { + fakePR.GetReturns(gitprovider.PullRequestInfo{ + WebURL: "url", + }) + gitProviders.CreatePullRequestReturns(fakePR, nil) + Expect(profilesSvc.Update(context.TODO(), gitProviders, updateOptions)).Should(Succeed()) + }) + }) + + When("it fails to get a list of available profiles from the cluster", func() { + JustBeforeEach(func() { + gitProviders.RepositoryExistsReturns(true, nil) + gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) + }) + + It("fails if it's unable to get a matching available profile from the cluster", func() { + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapperWithErr("nope"), nil + }) + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to get profiles from cluster: failed to make GET request to service weave-system/wego-app path \"/v1/profiles\": nope")) + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + }) + + It("fails if it's unable to discover the HelmRepository's name and namespace values", func() { + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getRespWithoutHelmRepo()), nil + }) + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to get profiles from cluster: HelmRepository's name or namespace is empty")) + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + }) + }) + + When("it fails to find a matching version", func() { + It("returns an error", func() { + gitProviders.RepositoryExistsReturns(true, nil) + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil + }) + updateOptions.Version = "7.0.0" + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to get profiles from cluster: version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + }) + }) + }) + + When("the config repository does not exist", func() { + It("fails if the --config-repo url format is wrong", func() { + updateOptions = profiles.UpdateOptions{ + Name: "foo", + ConfigRepo: "{http:/-*wrong-url-827", + Cluster: "prod", + } + + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to parse url: could not get provider name from URL {http:/-*wrong-url-827: could not parse git repo url \"{http:/-*wrong-url-827\": parse \"{http:/-*wrong-url-827\": first path segment in URL cannot contain colon")) + }) + + It("fails if the config repo does not exist", func() { + gitProviders.RepositoryExistsReturns(false, nil) + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("repository \"ssh://git@github.com/owner/config-repo.git\" could not be found")) + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + }) + }) +}) diff --git a/test/acceptance/test/profiles_test.go b/test/acceptance/test/profiles_test.go index 667cae16aad..a4100eb02c6 100644 --- a/test/acceptance/test/profiles_test.go +++ b/test/acceptance/test/profiles_test.go @@ -129,6 +129,23 @@ Namespace: %s`, clusterName, namespace))) resp, statusCode, err = kubernetesDoRequest(namespace, clusterName+"-"+profileName, "9898", "/healthz", clientSet) return statusCode }, "120s", "1s").Should(Equal(http.StatusOK)) + + By("Updating the version of the installed profile") + stdOut, stdErr = runCommandAndReturnStringOutput(fmt.Sprintf("%s update profile --name %s --version 6.0.0 --namespace %s --cluster %s --config-repo %s --auto-merge", gitopsBinaryPath, profileName, namespace, clusterName, appRepoRemoteURL)) + Expect(stdErr).To(BeEmpty()) + Expect(stdOut).To(ContainSubstring( + fmt.Sprintf(`Updating 1 profile: + + Name: podinfo + Version: 6.0.0 + Cluster: %s + Namespace: %s`, clusterName, namespace))) + + By("Verifying that the profile installed in the cluster's namespace was updated to the correct version") + Eventually(func() int { + resp, statusCode, err = kubernetesDoRequest(namespace, clusterName+"-"+profileName, "9898", "/healthz", clientSet) + return statusCode + }, "120s", "1s").Should(Equal(http.StatusOK)) }) It("@skipOnNightly profiles are installed into a different namespace", func() { From 922671e7654c6b942f81cbf4b9d6f12c537aa6d3 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 9 Feb 2022 16:05:33 +0100 Subject: [PATCH 2/7] Add validation for updating profile - Refactor profiles funcs to fail if profiles.yaml contains no HelmReleases - Refactor func that appends HelmRelease to file so that update fails if HelmRelease does not exist --- pkg/helm/releases.go | 63 ++++++++-------- pkg/helm/releases_test.go | 103 ++++----------------------- pkg/services/profiles/add.go | 32 ++++++--- pkg/services/profiles/add_test.go | 41 ++--------- pkg/services/profiles/profiles.go | 15 ++++ pkg/services/profiles/update.go | 42 ++++++++++- pkg/services/profiles/update_test.go | 96 +++++++++++++------------ 7 files changed, 175 insertions(+), 217 deletions(-) diff --git a/pkg/helm/releases.go b/pkg/helm/releases.go index 60b7e602d7d..2b18caa03cb 100644 --- a/pkg/helm/releases.go +++ b/pkg/helm/releases.go @@ -4,9 +4,9 @@ import ( "bytes" "fmt" "io" + "strings" "time" - "github.com/fluxcd/go-git-providers/gitprovider" "github.com/fluxcd/helm-controller/api/v2beta1" helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" @@ -45,49 +45,44 @@ func MakeHelmRelease(name, version, cluster, namespace string, helmRepository ty } } -// AppendHelmReleaseToFile appends a HelmRelease to a gitProvider CommitFile given a condition, file name and file path. -func AppendHelmReleaseToFile(files []*gitprovider.CommitFile, newRelease *v2beta1.HelmRelease, condition func(r, newRelease v2beta1.HelmRelease) error, fileName, filePath string) (gitprovider.CommitFile, error) { - var content string - - for _, f := range files { - if f.Path != nil && *f.Path == filePath { - if f.Content == nil || *f.Content == "" { - break - } +// FindHelmReleaseInString finds all HelmRelease(s) in a given string that can be split into YAML. +func FindHelmReleaseInString(s string, newRelease *v2beta1.HelmRelease) ([]v2beta1.HelmRelease, error) { + manifestByteSlice, err := splitYAML([]byte(s)) + if err != nil { + return nil, fmt.Errorf("error splitting into YAML: %w", err) + } - manifestByteSlice, err := splitYAML([]byte(*f.Content)) - if err != nil { - return gitprovider.CommitFile{}, fmt.Errorf("error splitting %s: %w", fileName, err) - } + found := []v2beta1.HelmRelease{} - for _, manifestBytes := range manifestByteSlice { - var r v2beta1.HelmRelease - if err := kyaml.Unmarshal(manifestBytes, &r); err != nil { - return gitprovider.CommitFile{}, fmt.Errorf("error unmarshaling %s: %w", fileName, err) - } + for _, manifestBytes := range manifestByteSlice { + var r v2beta1.HelmRelease + if err := kyaml.Unmarshal(manifestBytes, &r); err != nil { + return nil, fmt.Errorf("error unmarshaling: %w", err) + } - if err := condition(r, *newRelease); err != nil { - return gitprovider.CommitFile{}, err - } - } + if profileIsInstalled(r, *newRelease) { + found = append(found, r) + } + } - content = *f.Content + return found, nil +} - break - } +// AppendHelmReleaseToString appends a HelmRelease to a string. +func AppendHelmReleaseToString(content string, newRelease *v2beta1.HelmRelease) (string, error) { + var sb strings.Builder + if content != "" { + sb.WriteString(content + "\n") } helmReleaseManifest, err := kyaml.Marshal(newRelease) if err != nil { - return gitprovider.CommitFile{}, fmt.Errorf("failed to marshal new HelmRelease: %w", err) + return "", fmt.Errorf("failed to marshal HelmRelease: %w", err) } - content += "\n---\n" + string(helmReleaseManifest) + sb.WriteString("---\n" + string(helmReleaseManifest)) - return gitprovider.CommitFile{ - Path: &filePath, - Content: &content, - }, nil + return sb.String(), nil } // splitYAML splits a manifest file that may contain multiple YAML resources separated by '---' @@ -117,3 +112,7 @@ func splitYAML(resources []byte) ([][]byte, error) { return splitResources, nil } + +func profileIsInstalled(r, newRelease v2beta1.HelmRelease) bool { + return r.Name == newRelease.Name && r.Namespace == newRelease.Namespace && r.Spec.Chart.Spec.Version == newRelease.Spec.Chart.Spec.Version +} diff --git a/pkg/helm/releases_test.go b/pkg/helm/releases_test.go index 10a49983d47..cb65147af60 100644 --- a/pkg/helm/releases_test.go +++ b/pkg/helm/releases_test.go @@ -1,15 +1,10 @@ package helm_test import ( - "fmt" "time" - "github.com/weaveworks/weave-gitops/pkg/git" "github.com/weaveworks/weave-gitops/pkg/helm" - "github.com/weaveworks/weave-gitops/pkg/models" - "github.com/fluxcd/go-git-providers/gitprovider" - "github.com/fluxcd/helm-controller/api/v2beta1" helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" "github.com/google/go-cmp/cmp" @@ -68,106 +63,34 @@ var _ = Describe("MakeHelmRelease", func() { }) }) -var _ = Describe("AppendHelmReleaseToFile", func() { - var ( - newRelease *v2beta1.HelmRelease - existingFile *gitprovider.CommitFile - path string - content string - ) +var _ = Describe("AppendHelmReleaseToString", func() { + var newRelease *helmv2beta1.HelmRelease BeforeEach(func() { newRelease = helm.MakeHelmRelease( "podinfo", "6.0.0", "prod", "weave-system", types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, ) - path = git.GetProfilesPath("prod", models.WegoProfilesPath) }) When("the file does not exist", func() { It("creates one with the new helm release", func() { - file, err := helm.AppendHelmReleaseToFile(makeTestFiles(), newRelease, makeTestCondition, "profiles.yaml", path) + s, err := helm.AppendHelmReleaseToString("", newRelease) Expect(err).NotTo(HaveOccurred()) r, err := yaml.Marshal(newRelease) Expect(err).NotTo(HaveOccurred()) - Expect(*file.Content).To(ContainSubstring(string(r))) + Expect(s).To(ContainSubstring(string(r))) }) }) +}) - When("the file exists", func() { - When("the given condition succeeds", func() { - It("appends the release to the manifest", func() { - existingRelease := helm.MakeHelmRelease( - "podinfo", "6.0.1", "prod", "weave-system", - types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, - ) - r, _ := yaml.Marshal(existingRelease) - content = string(r) - file, err := helm.AppendHelmReleaseToFile([]*gitprovider.CommitFile{{ - Path: &path, - Content: &content, - }}, newRelease, makeTestCondition, "profiles.yaml", path) - Expect(err).NotTo(HaveOccurred()) - Expect(*file.Content).To(ContainSubstring(string(r))) - }) - }) - - When("the given condition returns an error", func() { - It("fails to add the profile", func() { - existingRelease, _ := yaml.Marshal(newRelease) - content = string(existingRelease) - existingFile = &gitprovider.CommitFile{ - Path: &path, - Content: &content, - } - _, err := helm.AppendHelmReleaseToFile([]*gitprovider.CommitFile{existingFile}, newRelease, makeTestCondition, "profiles.yaml", path) - Expect(err).To(MatchError("err")) - }) - }) - - It("fails if the manifest contains a resource that is not a HelmRelease", func() { - content = "content" - _, err := helm.AppendHelmReleaseToFile([]*gitprovider.CommitFile{{ - Path: &path, - Content: &content, - }}, newRelease, makeTestCondition, "profiles.yaml", path) - Expect(err).To(MatchError("error unmarshaling profiles.yaml: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type v2beta1.HelmRelease")) - }) +var _ = Describe("FindHelmReleaseInString", func() { + It("fails if the manifest contains a resource that is not a HelmRelease", func() { + newRelease := helm.MakeHelmRelease( + "podinfo", "6.0.0", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + _, err := helm.FindHelmReleaseInString("content", newRelease) + Expect(err).To(MatchError("error unmarshaling: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type v2beta1.HelmRelease")) }) }) - -func makeTestCondition(r, newRelease v2beta1.HelmRelease) error { - if r.Spec.Chart.Spec.Version == newRelease.Spec.Chart.Spec.Version { - return fmt.Errorf("err") - } - - return nil -} - -func makeTestFiles() []*gitprovider.CommitFile { - path0 := ".weave-gitops/clusters/prod/system/wego-system.yaml" - content0 := "machine1 yaml content" - path1 := ".weave-gitops/clusters/prod/system/podinfo-helm-release.yaml" - content1 := "machine2 yaml content" - - files := []gitprovider.CommitFile{ - { - Path: &path0, - Content: &content0, - }, - { - Path: &path1, - Content: &content1, - }, - } - - commitFiles := make([]*gitprovider.CommitFile, 0) - for _, file := range files { - commitFiles = append(commitFiles, &gitprovider.CommitFile{ - Path: file.Path, - Content: file.Content, - }) - } - - return commitFiles -} diff --git a/pkg/services/profiles/add.go b/pkg/services/profiles/add.go index 3db9a1a6e41..028bfc99205 100644 --- a/pkg/services/profiles/add.go +++ b/pkg/services/profiles/add.go @@ -4,14 +4,14 @@ import ( "context" "fmt" - "github.com/google/uuid" "github.com/weaveworks/weave-gitops/pkg/git" "github.com/weaveworks/weave-gitops/pkg/gitproviders" "github.com/weaveworks/weave-gitops/pkg/helm" "github.com/weaveworks/weave-gitops/pkg/models" "github.com/fluxcd/go-git-providers/gitprovider" - "github.com/fluxcd/helm-controller/api/v2beta1" + "github.com/google/uuid" + "k8s.io/apimachinery/pkg/types" ) const AddCommitMessage = "Add Profile manifests" @@ -58,25 +58,30 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi return fmt.Errorf("failed to discover HelmRepository: %w", err) } - newRelease := helm.MakeHelmRelease(opts.Name, version, opts.Cluster, opts.Namespace, helmRepo) - files, err := gitProvider.GetRepoDirFiles(ctx, configRepoURL, git.GetSystemPath(opts.Cluster), defaultBranch) if err != nil { return fmt.Errorf("failed to get files in '%s' for config repository %q: %s", git.GetSystemPath(opts.Cluster), configRepoURL, err) } - file, err := helm.AppendHelmReleaseToFile(files, newRelease, profileIsInstalled, models.WegoProfilesPath, git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath)) + fileContent := getGitCommitFileContent(files, git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath)) + + content, err := addHelmRelease(helmRepo, fileContent, version, opts) if err != nil { - return fmt.Errorf("failed to append HelmRelease to profiles file: %w", err) + return fmt.Errorf("failed to add HelmRelease for profile '%s' to %s: %w", opts.Name, models.WegoProfilesPath, err) } + path := git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath) + pr, err := gitProvider.CreatePullRequest(ctx, configRepoURL, gitproviders.PullRequestInfo{ Title: fmt.Sprintf("GitOps add %s", opts.Name), Description: fmt.Sprintf("Add manifest for %s profile", opts.Name), CommitMessage: AddCommitMessage, TargetBranch: defaultBranch, NewBranch: uuid.New().String(), - Files: []gitprovider.CommitFile{file}, + Files: []gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, }) if err != nil { return fmt.Errorf("failed to create pull request: %s", err) @@ -105,10 +110,15 @@ func (s *ProfilesSvc) printAddSummary(opts AddOptions) { s.Logger.Println("Namespace: %s\n", opts.Namespace) } -func profileIsInstalled(r, newRelease v2beta1.HelmRelease) error { - if r.Name == newRelease.Name && r.Namespace == newRelease.Namespace { - return fmt.Errorf("profile '%s' is already installed in namespace %s; please use 'gitops update profile' if you wish to update it", r.Name, r.Namespace) +func addHelmRelease(helmRepo types.NamespacedName, fileContent, version string, opts AddOptions) (string, error) { + newRelease := helm.MakeHelmRelease(opts.Name, version, opts.Cluster, opts.Namespace, helmRepo) + + matchingHelmReleases, err := helm.FindHelmReleaseInString(fileContent, newRelease) + if len(matchingHelmReleases) >= 1 { + return "", fmt.Errorf("profile '%s' is already installed in %s/%s", opts.Name, opts.Namespace, opts.Cluster) + } else if err != nil { + return "", fmt.Errorf("error reading from %s: %w", models.WegoProfilesPath, err) } - return nil + return helm.AppendHelmReleaseToString(fileContent, newRelease) } diff --git a/pkg/services/profiles/add_test.go b/pkg/services/profiles/add_test.go index 0b9385c2ebc..b2fcb766d81 100644 --- a/pkg/services/profiles/add_test.go +++ b/pkg/services/profiles/add_test.go @@ -154,12 +154,12 @@ var _ = Describe("Add", func() { return true, newFakeResponseWrapper(getProfilesResp), nil }) err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(MatchError("failed to append HelmRelease to profiles file: profile 'prod-podinfo' is already installed in namespace weave-system; please use 'gitops update profile' if you wish to update it")) + Expect(err).To(MatchError("failed to add HelmRelease for profile 'podinfo' to profiles.yaml: profile 'podinfo' is already installed in weave-system/prod")) }) }) - When("it fails to discover the HelmRepository name and namespace", func() { - It("fails if it's unable to get a matching available profile from the cluster", func() { + Context("it fails to discover the HelmRepository name and namespace", func() { + It("fails if it's unable to list available profiles from the cluster", func() { gitProviders.RepositoryExistsReturns(true, nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapperWithErr("nope"), nil @@ -168,10 +168,8 @@ var _ = Describe("Add", func() { Expect(err).To(MatchError("failed to discover HelmRepository: failed to get profiles from cluster: failed to make GET request to service weave-system/wego-app path \"/v1/profiles\": nope")) Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) }) - }) - When("it fails to find a matching version", func() { - It("returns an error", func() { + It("fails to find an available profile with the given version", func() { gitProviders.RepositoryExistsReturns(true, nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapper(getProfilesResp), nil @@ -232,34 +230,3 @@ func makeTestFiles() []*gitprovider.CommitFile { return commitFiles } - -func getRespWithoutHelmRepo() string { - return `{ - "profiles": [ - { - "name": "podinfo", - "home": "https://github.com/stefanprodan/podinfo", - "sources": [ - "https://github.com/stefanprodan/podinfo" - ], - "description": "Podinfo Helm chart for Kubernetes", - "keywords": [], - "maintainers": [ - { - "name": "stefanprodan", - "email": "stefanprodan@users.noreply.github.com", - "url": "" - } - ], - "icon": "", - "annotations": {}, - "kubeVersion": ">=1.19.0-0", - "availableVersions": [ - "6.0.0", - "6.0.1" - ] - } - ] - } - ` -} diff --git a/pkg/services/profiles/profiles.go b/pkg/services/profiles/profiles.go index 5e682d74f64..18bb975cb6e 100644 --- a/pkg/services/profiles/profiles.go +++ b/pkg/services/profiles/profiles.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/fluxcd/go-git-providers/gitprovider" "github.com/weaveworks/weave-gitops/pkg/gitproviders" "github.com/weaveworks/weave-gitops/pkg/logger" @@ -51,3 +52,17 @@ func (s *ProfilesSvc) discoverHelmRepository(ctx context.Context, opts GetOption Namespace: availableProfile.HelmRepository.Namespace, }, version, nil } + +func getGitCommitFileContent(files []*gitprovider.CommitFile, filePath string) string { + for _, f := range files { + if f.Path != nil && *f.Path == filePath { + if f.Content == nil || *f.Content == "" { + return "" + } + + return *f.Content + } + } + + return "" +} diff --git a/pkg/services/profiles/update.go b/pkg/services/profiles/update.go index 42dc110eb31..f620847eada 100644 --- a/pkg/services/profiles/update.go +++ b/pkg/services/profiles/update.go @@ -4,7 +4,12 @@ import ( "context" "fmt" + "github.com/weaveworks/weave-gitops/pkg/git" "github.com/weaveworks/weave-gitops/pkg/gitproviders" + "github.com/weaveworks/weave-gitops/pkg/helm" + "github.com/weaveworks/weave-gitops/pkg/models" + + "k8s.io/apimachinery/pkg/types" ) type UpdateOptions struct { @@ -32,7 +37,12 @@ func (s *ProfilesSvc) Update(ctx context.Context, gitProvider gitproviders.GitPr return fmt.Errorf("repository %q could not be found", configRepoURL) } - _, _, err = s.GetProfile(ctx, GetOptions{ + defaultBranch, err := gitProvider.GetDefaultBranch(ctx, configRepoURL) + if err != nil { + return fmt.Errorf("failed to get default branch: %w", err) + } + + helmRepo, version, err := s.discoverHelmRepository(ctx, GetOptions{ Name: opts.Name, Version: opts.Version, Cluster: opts.Cluster, @@ -40,8 +50,36 @@ func (s *ProfilesSvc) Update(ctx context.Context, gitProvider gitproviders.GitPr Port: opts.ProfilesPort, }) if err != nil { - return fmt.Errorf("failed to get profiles from cluster: %w", err) + return fmt.Errorf("failed to discover HelmRepository: %w", err) + } + + files, err := gitProvider.GetRepoDirFiles(ctx, configRepoURL, git.GetSystemPath(opts.Cluster), defaultBranch) + if err != nil { + return fmt.Errorf("failed to get files in '%s' of config repository %q: %s", git.GetSystemPath(opts.Cluster), configRepoURL, err) + } + + fileContent := getGitCommitFileContent(files, git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath)) + if fileContent == "" { + return fmt.Errorf("failed to find installed profiles in '%s' of config repo %q", git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath), configRepoURL) + } + + _, err = updateHelmRelease(helmRepo, fileContent, version, opts) + if err != nil { + return fmt.Errorf("failed to update HelmRelease for profile '%s' in %s: %w", opts.Name, models.WegoProfilesPath, err) } return nil } + +func updateHelmRelease(helmRepo types.NamespacedName, fileContent, version string, opts UpdateOptions) (string, error) { + newRelease := helm.MakeHelmRelease(opts.Name, version, opts.Cluster, opts.Namespace, helmRepo) + + matchingHelmReleases, err := helm.FindHelmReleaseInString(fileContent, newRelease) + if len(matchingHelmReleases) == 0 { + return "", fmt.Errorf("profile '%s' could not be found in %s/%s", opts.Name, opts.Namespace, opts.Cluster) + } else if err != nil { + return "", fmt.Errorf("error reading from %s: %w", models.WegoProfilesPath, err) + } + + return helm.AppendHelmReleaseToString(fileContent, newRelease) +} diff --git a/pkg/services/profiles/update_test.go b/pkg/services/profiles/update_test.go index 840e193c3b5..1f624777780 100644 --- a/pkg/services/profiles/update_test.go +++ b/pkg/services/profiles/update_test.go @@ -4,34 +4,38 @@ import ( "context" "github.com/fluxcd/go-git-providers/gitprovider" + "github.com/weaveworks/weave-gitops/pkg/git" + "github.com/weaveworks/weave-gitops/pkg/gitproviders/gitprovidersfakes" + "github.com/weaveworks/weave-gitops/pkg/helm" + "github.com/weaveworks/weave-gitops/pkg/logger/loggerfakes" + "github.com/weaveworks/weave-gitops/pkg/models" + "github.com/weaveworks/weave-gitops/pkg/services/profiles" + "sigs.k8s.io/yaml" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" restclient "k8s.io/client-go/rest" "k8s.io/client-go/testing" - - "github.com/weaveworks/weave-gitops/pkg/gitproviders/gitprovidersfakes" - "github.com/weaveworks/weave-gitops/pkg/logger/loggerfakes" - "github.com/weaveworks/weave-gitops/pkg/services/profiles" - "github.com/weaveworks/weave-gitops/pkg/vendorfakes/fakegitprovider" ) var updateOptions profiles.UpdateOptions -var _ = Describe("Update", func() { +var _ = Describe("Update Profile(s)", func() { var ( gitProviders *gitprovidersfakes.FakeGitProvider profilesSvc *profiles.ProfilesSvc clientSet *fake.Clientset fakeLogger *loggerfakes.FakeLogger - fakePR *fakegitprovider.PullRequest + // fakePR *fakegitprovider.PullRequest ) BeforeEach(func() { gitProviders = &gitprovidersfakes.FakeGitProvider{} clientSet = fake.NewSimpleClientset() fakeLogger = &loggerfakes.FakeLogger{} - fakePR = &fakegitprovider.PullRequest{} + // fakePR = &fakegitprovider.PullRequest{} profilesSvc = profiles.NewService(clientSet, fakeLogger) updateOptions = profiles.UpdateOptions{ @@ -45,64 +49,66 @@ var _ = Describe("Update", func() { When("the config repository exists", func() { When("the version and HelmRepository name and namespace were discovered", func() { - JustBeforeEach(func() { - gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetDefaultBranchReturns("main", nil) - gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getProfilesResp), nil + When("the file containing the HelmReleases is empty", func() { + It("returns an error", func() { + gitProviders.RepositoryExistsReturns(true, nil) + gitProviders.GetDefaultBranchReturns("main", nil) + gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil + }) + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to find installed profiles in '.weave-gitops/clusters/prod/system/profiles.yaml' of config repo \"ssh://git@github.com/owner/config-repo.git\"")) + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) }) }) - JustAfterEach(func() { - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) - Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) - Expect(gitProviders.CreatePullRequestCallCount()).To(Equal(1)) - }) - - It("creates a PR to change the version of a Helm Release", func() { - fakePR.GetReturns(gitprovider.PullRequestInfo{ - WebURL: "url", + When("the file containing the HelmReleases does not contain a HelmRelease with the given name and namespace", func() { + It("returns an error", func() { + gitProviders.RepositoryExistsReturns(true, nil) + gitProviders.GetDefaultBranchReturns("main", nil) + existingRelease := helm.MakeHelmRelease( + "random-profile", "6.0.1", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + r, _ := yaml.Marshal(existingRelease) + content := string(r) + path := git.GetProfilesPath("prod", models.WegoProfilesPath) + gitProviders.GetRepoDirFilesReturns([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, nil) + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil + }) + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: profile 'podinfo' could not be found in weave-system/prod")) + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) }) - gitProviders.CreatePullRequestReturns(fakePR, nil) - Expect(profilesSvc.Update(context.TODO(), gitProviders, updateOptions)).Should(Succeed()) }) }) - When("it fails to get a list of available profiles from the cluster", func() { - JustBeforeEach(func() { + Context("it fails to discover the HelmRepository name and namespace", func() { + It("fails if it's unable to list available profiles from the cluster", func() { gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) - }) - - It("fails if it's unable to get a matching available profile from the cluster", func() { clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapperWithErr("nope"), nil }) err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to get profiles from cluster: failed to make GET request to service weave-system/wego-app path \"/v1/profiles\": nope")) + Expect(err).To(MatchError("failed to discover HelmRepository: failed to get profiles from cluster: failed to make GET request to service weave-system/wego-app path \"/v1/profiles\": nope")) Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) }) - It("fails if it's unable to discover the HelmRepository's name and namespace values", func() { - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getRespWithoutHelmRepo()), nil - }) - err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to get profiles from cluster: HelmRepository's name or namespace is empty")) - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) - }) - }) - - When("it fails to find a matching version", func() { - It("returns an error", func() { + It("fails to find an available profile with the given version", func() { gitProviders.RepositoryExistsReturns(true, nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapper(getProfilesResp), nil }) updateOptions.Version = "7.0.0" err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to get profiles from cluster: version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) + Expect(err).To(MatchError("failed to discover HelmRepository: failed to get profiles from cluster: version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) }) }) From b20b80ffe6106ccedd2670c4085521ed6391e275 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Mon, 14 Feb 2022 17:50:28 +0100 Subject: [PATCH 3/7] Updating an installed profile's version patches it in a list of HelmReleases --- pkg/helm/releases.go | 64 ++++----- pkg/helm/releases_test.go | 58 +++++++- pkg/services/profiles/add.go | 21 ++- pkg/services/profiles/add_test.go | 6 +- pkg/services/profiles/update.go | 68 ++++++++- pkg/services/profiles/update_test.go | 197 +++++++++++++++++++++------ 6 files changed, 315 insertions(+), 99 deletions(-) diff --git a/pkg/helm/releases.go b/pkg/helm/releases.go index 2b18caa03cb..d78ad685a48 100644 --- a/pkg/helm/releases.go +++ b/pkg/helm/releases.go @@ -7,12 +7,11 @@ import ( "strings" "time" - "github.com/fluxcd/helm-controller/api/v2beta1" helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" - "gopkg.in/yaml.v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + apimachinery "k8s.io/apimachinery/pkg/util/yaml" kyaml "sigs.k8s.io/yaml" ) @@ -45,31 +44,19 @@ func MakeHelmRelease(name, version, cluster, namespace string, helmRepository ty } } -// FindHelmReleaseInString finds all HelmRelease(s) in a given string that can be split into YAML. -func FindHelmReleaseInString(s string, newRelease *v2beta1.HelmRelease) ([]v2beta1.HelmRelease, error) { - manifestByteSlice, err := splitYAML([]byte(s)) - if err != nil { - return nil, fmt.Errorf("error splitting into YAML: %w", err) - } - - found := []v2beta1.HelmRelease{} - - for _, manifestBytes := range manifestByteSlice { - var r v2beta1.HelmRelease - if err := kyaml.Unmarshal(manifestBytes, &r); err != nil { - return nil, fmt.Errorf("error unmarshaling: %w", err) - } - - if profileIsInstalled(r, *newRelease) { - found = append(found, r) +// FindReleaseInNamespace iterates through a slice of HelmReleases to find one with a given name in a given namespace, and returns it with its index. +func FindReleaseInNamespace(existingReleases []helmv2beta1.HelmRelease, name, ns string) (*helmv2beta1.HelmRelease, int, error) { + for i, r := range existingReleases { + if r.Name == name && r.Namespace == ns { + return &r, i, nil } } - return found, nil + return nil, -1, nil } // AppendHelmReleaseToString appends a HelmRelease to a string. -func AppendHelmReleaseToString(content string, newRelease *v2beta1.HelmRelease) (string, error) { +func AppendHelmReleaseToString(content string, newRelease *helmv2beta1.HelmRelease) (string, error) { var sb strings.Builder if content != "" { sb.WriteString(content + "\n") @@ -85,15 +72,14 @@ func AppendHelmReleaseToString(content string, newRelease *v2beta1.HelmRelease) return sb.String(), nil } -// splitYAML splits a manifest file that may contain multiple YAML resources separated by '---' -// and validates that each element is YAML. -func splitYAML(resources []byte) ([][]byte, error) { - var splitResources [][]byte +// SplitHelmReleaseYAML splits a manifest file that contains one or more Helm Releases that may be separated by '---'. +func SplitHelmReleaseYAML(resources []byte) ([]helmv2beta1.HelmRelease, error) { + var helmReleaseList []helmv2beta1.HelmRelease - decoder := yaml.NewDecoder(bytes.NewReader(resources)) + decoder := apimachinery.NewYAMLOrJSONDecoder(bytes.NewReader(resources), 100000000) for { - var value interface{} + var value helmv2beta1.HelmRelease if err := decoder.Decode(&value); err != nil { if err == io.EOF { break @@ -102,17 +88,25 @@ func splitYAML(resources []byte) ([][]byte, error) { return nil, err } - valueBytes, err := yaml.Marshal(value) + helmReleaseList = append(helmReleaseList, value) + } + + return helmReleaseList, nil +} + +func PatchHelmRelease(existingReleases []helmv2beta1.HelmRelease, patchedHelmRelease helmv2beta1.HelmRelease, index int) (string, error) { + existingReleases[index] = patchedHelmRelease + + var sb strings.Builder + + for _, r := range existingReleases { + b, err := kyaml.Marshal(r) if err != nil { - return nil, err + return "", fmt.Errorf("failed to marshal: %w", err) } - splitResources = append(splitResources, valueBytes) + sb.WriteString("---\n" + string(b)) } - return splitResources, nil -} - -func profileIsInstalled(r, newRelease v2beta1.HelmRelease) bool { - return r.Name == newRelease.Name && r.Namespace == newRelease.Namespace && r.Spec.Chart.Spec.Version == newRelease.Spec.Chart.Spec.Version + return sb.String(), nil } diff --git a/pkg/helm/releases_test.go b/pkg/helm/releases_test.go index cb65147af60..c9884f01ff6 100644 --- a/pkg/helm/releases_test.go +++ b/pkg/helm/releases_test.go @@ -5,6 +5,7 @@ import ( "github.com/weaveworks/weave-gitops/pkg/helm" + "github.com/fluxcd/helm-controller/api/v2beta1" helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" "github.com/google/go-cmp/cmp" @@ -13,6 +14,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/yaml" + kyaml "sigs.k8s.io/yaml" ) var _ = Describe("MakeHelmRelease", func() { @@ -59,7 +61,7 @@ var _ = Describe("MakeHelmRelease", func() { Interval: metav1.Duration{Duration: time.Minute}, }, } - Expect(cmp.Diff(actualHelmRelease, expectedHelmRelease)).To(BeEmpty()) + Expect(cmp.Diff(&actualHelmRelease, &expectedHelmRelease)).To(BeEmpty()) }) }) @@ -84,13 +86,57 @@ var _ = Describe("AppendHelmReleaseToString", func() { }) }) -var _ = Describe("FindHelmReleaseInString", func() { - It("fails if the manifest contains a resource that is not a HelmRelease", func() { - newRelease := helm.MakeHelmRelease( +var _ = Describe("FindReleaseInNamespace", func() { + var ( + name = "prod-podinfo" + ns = "weave-system" + ) + + When("it finds a HelmRelease with a matching name and namespace", func() { + It("returns its index in the slice of bytes", func() { + newRelease := helm.MakeHelmRelease( + "podinfo", "6.0.0", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + existingRelease, index, err := helm.FindReleaseInNamespace([]helmv2beta1.HelmRelease{*newRelease}, name, ns) + Expect(err).NotTo(HaveOccurred()) + Expect(index).To(Equal(0)) + Expect(cmp.Diff(*existingRelease, *newRelease)).To(BeEmpty()) + }) + }) + + When("it does not find a HelmRelease with a matching name and namespace", func() { + It("returns an index of -1", func() { + _, index, err := helm.FindReleaseInNamespace([]helmv2beta1.HelmRelease{}, name, ns) + Expect(err).NotTo(HaveOccurred()) + Expect(index).To(Equal(-1)) + }) + }) +}) + +var _ = Describe("PatchHelmReleaseInString", func() { + var ( + r *v2beta1.HelmRelease + ) + + BeforeEach(func() { + r = helm.MakeHelmRelease( + "podinfo", "6.0.1", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + }) + + It("returns a string with an updated list of HelmReleases", func() { + existingRelease := helm.MakeHelmRelease( "podinfo", "6.0.0", "prod", "weave-system", types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, ) - _, err := helm.FindHelmReleaseInString("content", newRelease) - Expect(err).To(MatchError("error unmarshaling: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type v2beta1.HelmRelease")) + + expectedContentBytes, _ := kyaml.Marshal(r) + expectedContent := "---\n" + string(expectedContentBytes) + + patchedContent, err := helm.PatchHelmRelease([]helmv2beta1.HelmRelease{*existingRelease}, *r, 0) + Expect(err).NotTo(HaveOccurred()) + Expect(cmp.Diff(patchedContent, expectedContent)).To(BeEmpty()) }) }) diff --git a/pkg/services/profiles/add.go b/pkg/services/profiles/add.go index 028bfc99205..6ca5666b1da 100644 --- a/pkg/services/profiles/add.go +++ b/pkg/services/profiles/add.go @@ -58,6 +58,8 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi return fmt.Errorf("failed to discover HelmRepository: %w", err) } + opts.Version = version + files, err := gitProvider.GetRepoDirFiles(ctx, configRepoURL, git.GetSystemPath(opts.Cluster), defaultBranch) if err != nil { return fmt.Errorf("failed to get files in '%s' for config repository %q: %s", git.GetSystemPath(opts.Cluster), configRepoURL, err) @@ -65,7 +67,7 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi fileContent := getGitCommitFileContent(files, git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath)) - content, err := addHelmRelease(helmRepo, fileContent, version, opts) + content, err := addHelmRelease(helmRepo, fileContent, opts.Name, opts.Version, opts.Cluster, opts.Namespace) if err != nil { return fmt.Errorf("failed to add HelmRelease for profile '%s' to %s: %w", opts.Name, models.WegoProfilesPath, err) } @@ -87,7 +89,7 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi return fmt.Errorf("failed to create pull request: %s", err) } - s.Logger.Actionf("Pull Request created: %s", pr.Get().WebURL) + s.Logger.Actionf("created Pull Request: %s", pr.Get().WebURL) if opts.AutoMerge { s.Logger.Actionf("auto-merge=true; merging PR number %v", pr.Get().Number) @@ -110,12 +112,17 @@ func (s *ProfilesSvc) printAddSummary(opts AddOptions) { s.Logger.Println("Namespace: %s\n", opts.Namespace) } -func addHelmRelease(helmRepo types.NamespacedName, fileContent, version string, opts AddOptions) (string, error) { - newRelease := helm.MakeHelmRelease(opts.Name, version, opts.Cluster, opts.Namespace, helmRepo) +func addHelmRelease(helmRepo types.NamespacedName, fileContent, name, version, cluster, ns string) (string, error) { + existingReleases, err := helm.SplitHelmReleaseYAML([]byte(fileContent)) + if err != nil { + return "", fmt.Errorf("error splitting into YAML: %w", err) + } + + newRelease := helm.MakeHelmRelease(name, version, cluster, ns, helmRepo) - matchingHelmReleases, err := helm.FindHelmReleaseInString(fileContent, newRelease) - if len(matchingHelmReleases) >= 1 { - return "", fmt.Errorf("profile '%s' is already installed in %s/%s", opts.Name, opts.Namespace, opts.Cluster) + matchingHelmRelease, _, err := helm.FindReleaseInNamespace(existingReleases, newRelease.Name, ns) + if matchingHelmRelease != nil { + return "", fmt.Errorf("found another HelmRelease for profile '%s' in namespace %s", name, ns) } else if err != nil { return "", fmt.Errorf("error reading from %s: %w", models.WegoProfilesPath, err) } diff --git a/pkg/services/profiles/add_test.go b/pkg/services/profiles/add_test.go index b2fcb766d81..a4b6a2066cd 100644 --- a/pkg/services/profiles/add_test.go +++ b/pkg/services/profiles/add_test.go @@ -149,12 +149,8 @@ var _ = Describe("Add", func() { }) It("fails to append the new HelmRelease to profiles.yaml", func() { - gitProviders.RepositoryExistsReturns(true, nil) - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getProfilesResp), nil - }) err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).To(MatchError("failed to add HelmRelease for profile 'podinfo' to profiles.yaml: profile 'podinfo' is already installed in weave-system/prod")) + Expect(err).To(MatchError("failed to add HelmRelease for profile 'podinfo' to profiles.yaml: found another HelmRelease for profile 'podinfo' in namespace weave-system")) }) }) diff --git a/pkg/services/profiles/update.go b/pkg/services/profiles/update.go index f620847eada..f877ab27320 100644 --- a/pkg/services/profiles/update.go +++ b/pkg/services/profiles/update.go @@ -9,9 +9,13 @@ import ( "github.com/weaveworks/weave-gitops/pkg/helm" "github.com/weaveworks/weave-gitops/pkg/models" + "github.com/fluxcd/go-git-providers/gitprovider" + "github.com/google/uuid" "k8s.io/apimachinery/pkg/types" ) +const UpdateCommitMessage = "Update Profile manifests" + type UpdateOptions struct { Name string Cluster string @@ -53,6 +57,8 @@ func (s *ProfilesSvc) Update(ctx context.Context, gitProvider gitproviders.GitPr return fmt.Errorf("failed to discover HelmRepository: %w", err) } + opts.Version = version + files, err := gitProvider.GetRepoDirFiles(ctx, configRepoURL, git.GetSystemPath(opts.Cluster), defaultBranch) if err != nil { return fmt.Errorf("failed to get files in '%s' of config repository %q: %s", git.GetSystemPath(opts.Cluster), configRepoURL, err) @@ -63,23 +69,71 @@ func (s *ProfilesSvc) Update(ctx context.Context, gitProvider gitproviders.GitPr return fmt.Errorf("failed to find installed profiles in '%s' of config repo %q", git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath), configRepoURL) } - _, err = updateHelmRelease(helmRepo, fileContent, version, opts) + content, err := updateHelmRelease(helmRepo, fileContent, opts.Name, opts.Version, opts.Cluster, opts.Namespace) if err != nil { return fmt.Errorf("failed to update HelmRelease for profile '%s' in %s: %w", opts.Name, models.WegoProfilesPath, err) } + path := git.GetProfilesPath(opts.Cluster, models.WegoProfilesPath) + + pr, err := gitProvider.CreatePullRequest(ctx, configRepoURL, gitproviders.PullRequestInfo{ + Title: fmt.Sprintf("GitOps update %s", opts.Name), + Description: fmt.Sprintf("Update manifest for %s profile", opts.Name), + CommitMessage: UpdateCommitMessage, + TargetBranch: defaultBranch, + NewBranch: uuid.New().String(), + Files: []gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, + }) + if err != nil { + return fmt.Errorf("failed to create pull request: %s", err) + } + + s.Logger.Actionf("created Pull Request: %s", pr.Get().WebURL) + + if opts.AutoMerge { + s.Logger.Actionf("auto-merge=true; merging PR number %v", pr.Get().Number) + + if err := gitProvider.MergePullRequest(ctx, configRepoURL, pr.Get().Number, AddCommitMessage); err != nil { + return fmt.Errorf("error auto-merging PR: %w", err) + } + } + + s.printUpdateSummary(opts) + return nil } -func updateHelmRelease(helmRepo types.NamespacedName, fileContent, version string, opts UpdateOptions) (string, error) { - newRelease := helm.MakeHelmRelease(opts.Name, version, opts.Cluster, opts.Namespace, helmRepo) +func (s *ProfilesSvc) printUpdateSummary(opts UpdateOptions) { + s.Logger.Println("Updating profile:\n") + s.Logger.Println("Name: %s", opts.Name) + s.Logger.Println("Version: %s", opts.Version) + s.Logger.Println("Cluster: %s", opts.Cluster) + s.Logger.Println("Namespace: %s\n", opts.Namespace) +} + +func updateHelmRelease(helmRepo types.NamespacedName, fileContent, name, version, cluster, ns string) (string, error) { + existingReleases, err := helm.SplitHelmReleaseYAML([]byte(fileContent)) + if err != nil { + return "", fmt.Errorf("error splitting into YAML: %w", err) + } + + releaseName := cluster + "-" + name - matchingHelmReleases, err := helm.FindHelmReleaseInString(fileContent, newRelease) - if len(matchingHelmReleases) == 0 { - return "", fmt.Errorf("profile '%s' could not be found in %s/%s", opts.Name, opts.Namespace, opts.Cluster) + matchingHelmRelease, index, err := helm.FindReleaseInNamespace(existingReleases, releaseName, ns) + if matchingHelmRelease == nil { + return "", fmt.Errorf("failed to find HelmRelease '%s' in namespace %s", releaseName, ns) } else if err != nil { return "", fmt.Errorf("error reading from %s: %w", models.WegoProfilesPath, err) } - return helm.AppendHelmReleaseToString(fileContent, newRelease) + if matchingHelmRelease.Spec.Chart.Spec.Version == version { + return "", fmt.Errorf("version %s of profile '%s' already installed in %s/%s", version, name, ns, cluster) + } + + matchingHelmRelease.Spec.Chart.Spec.Version = version + + return helm.PatchHelmRelease(existingReleases, *matchingHelmRelease, index) } diff --git a/pkg/services/profiles/update_test.go b/pkg/services/profiles/update_test.go index 1f624777780..3f39112f407 100644 --- a/pkg/services/profiles/update_test.go +++ b/pkg/services/profiles/update_test.go @@ -2,6 +2,7 @@ package profiles_test import ( "context" + "fmt" "github.com/fluxcd/go-git-providers/gitprovider" "github.com/weaveworks/weave-gitops/pkg/git" @@ -10,6 +11,7 @@ import ( "github.com/weaveworks/weave-gitops/pkg/logger/loggerfakes" "github.com/weaveworks/weave-gitops/pkg/models" "github.com/weaveworks/weave-gitops/pkg/services/profiles" + "github.com/weaveworks/weave-gitops/pkg/vendorfakes/fakegitprovider" "sigs.k8s.io/yaml" . "github.com/onsi/ginkgo" @@ -28,14 +30,14 @@ var _ = Describe("Update Profile(s)", func() { profilesSvc *profiles.ProfilesSvc clientSet *fake.Clientset fakeLogger *loggerfakes.FakeLogger - // fakePR *fakegitprovider.PullRequest + fakePR *fakegitprovider.PullRequest ) BeforeEach(func() { gitProviders = &gitprovidersfakes.FakeGitProvider{} clientSet = fake.NewSimpleClientset() fakeLogger = &loggerfakes.FakeLogger{} - // fakePR = &fakegitprovider.PullRequest{} + fakePR = &fakegitprovider.PullRequest{} profilesSvc = profiles.NewService(clientSet, fakeLogger) updateOptions = profiles.UpdateOptions{ @@ -48,68 +50,185 @@ var _ = Describe("Update Profile(s)", func() { }) When("the config repository exists", func() { + BeforeEach(func() { + gitProviders.RepositoryExistsReturns(true, nil) + gitProviders.GetDefaultBranchReturns("main", nil) + }) + + AfterEach(func() { + Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + }) + When("the version and HelmRepository name and namespace were discovered", func() { - When("the file containing the HelmReleases is empty", func() { - It("returns an error", func() { - gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetDefaultBranchReturns("main", nil) - gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getProfilesResp), nil - }) - err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to find installed profiles in '.weave-gitops/clusters/prod/system/profiles.yaml' of config repo \"ssh://git@github.com/owner/config-repo.git\"")) - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) - Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) + BeforeEach(func() { + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil }) }) - When("the file containing the HelmReleases does not contain a HelmRelease with the given name and namespace", func() { - It("returns an error", func() { - gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetDefaultBranchReturns("main", nil) - existingRelease := helm.MakeHelmRelease( - "random-profile", "6.0.1", "prod", "weave-system", - types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, - ) - r, _ := yaml.Marshal(existingRelease) - content := string(r) - path := git.GetProfilesPath("prod", models.WegoProfilesPath) - gitProviders.GetRepoDirFilesReturns([]*gitprovider.CommitFile{{ - Path: &path, - Content: &content, - }}, nil) - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getProfilesResp), nil - }) - err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: profile 'podinfo' could not be found in weave-system/prod")) - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) + When("the file containing the HelmReleases is not empty", func() { + AfterEach(func() { Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) }) + + When("a matching HelmRelease is found", func() { + When("the existing HelmRelease is a different version than the want to update to", func() { + BeforeEach(func() { + existingRelease := helm.MakeHelmRelease( + "podinfo", "6.0.0", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + r, _ := yaml.Marshal(existingRelease) + content := string(r) + path := git.GetProfilesPath("prod", models.WegoProfilesPath) + gitProviders.GetRepoDirFilesReturns([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, nil) + }) + + AfterEach(func() { + Expect(gitProviders.CreatePullRequestCallCount()).To(Equal(1)) + }) + + When("it opens a PR", func() { + It("updates the HelmRelease version", func() { + fakePR.GetReturns(gitprovider.PullRequestInfo{ + WebURL: "url", + }) + gitProviders.CreatePullRequestReturns(fakePR, nil) + Expect(profilesSvc.Update(context.TODO(), gitProviders, updateOptions)).Should(Succeed()) + }) + }) + + When("auto-merge is enabled", func() { + It("merges the PR that was created", func() { + fakePR.GetReturns(gitprovider.PullRequestInfo{ + WebURL: "url", + Number: 42, + }) + gitProviders.CreatePullRequestReturns(fakePR, nil) + updateOptions.AutoMerge = true + Expect(profilesSvc.Update(context.TODO(), gitProviders, updateOptions)).Should(Succeed()) + }) + + When("the PR fails to be merged", func() { + It("returns an error", func() { + fakePR.GetReturns(gitprovider.PullRequestInfo{ + WebURL: "url", + }) + gitProviders.CreatePullRequestReturns(fakePR, nil) + gitProviders.MergePullRequestReturns(fmt.Errorf("err")) + updateOptions.AutoMerge = true + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("error auto-merging PR: err")) + }) + }) + }) + + When("a version other than 'latest' is specified", func() { + It("creates a helm release with that version", func() { + updateOptions.Version = "6.0.1" + fakePR.GetReturns(gitprovider.PullRequestInfo{ + WebURL: "url", + }) + gitProviders.CreatePullRequestReturns(fakePR, nil) + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(BeNil()) + }) + }) + + When("the PR fails to be merged", func() { + It("returns an error", func() { + gitProviders.CreatePullRequestReturns(nil, fmt.Errorf("err")) + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to create pull request: err")) + }) + }) + }) + + When("an existing HelmRelease is the same version as the want to update to", func() { + It("returns an error", func() { + existingRelease := helm.MakeHelmRelease( + "podinfo", "6.0.1", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + r, _ := yaml.Marshal(existingRelease) + content := string(r) + path := git.GetProfilesPath("prod", models.WegoProfilesPath) + gitProviders.GetRepoDirFilesReturns([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, nil) + + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: version 6.0.1 of profile 'podinfo' already installed in weave-system/prod")) + }) + }) + }) + + When("the file containing the HelmReleases does not contain a HelmRelease with the given name and namespace", func() { + It("returns an error", func() { + existingRelease := helm.MakeHelmRelease( + "random-profile", "6.0.1", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + r, _ := yaml.Marshal(existingRelease) + content := string(r) + path := git.GetProfilesPath("prod", models.WegoProfilesPath) + gitProviders.GetRepoDirFilesReturns([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, nil) + + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: failed to find HelmRelease 'prod-podinfo' in namespace weave-system")) + }) + }) + + When("the file containing the HelmRelease contains something other than a HelmRelease", func() { + It("returns an error", func() { + content := "content" + path := git.GetProfilesPath("prod", models.WegoProfilesPath) + gitProviders.GetRepoDirFilesReturns([]*gitprovider.CommitFile{{ + Path: &path, + Content: &content, + }}, nil) + + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: error splitting into YAML: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type v2beta1.HelmRelease")) + }) + }) + + When("the file containing the HelmReleases is empty", func() { + It("returns an error", func() { + gitProviders.GetRepoDirFilesReturns(makeTestFiles(), nil) + + err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) + Expect(err).To(MatchError("failed to find installed profiles in '.weave-gitops/clusters/prod/system/profiles.yaml' of config repo \"ssh://git@github.com/owner/config-repo.git\"")) + + Expect(gitProviders.GetRepoDirFilesCallCount()).To(Equal(1)) + }) + }) }) }) Context("it fails to discover the HelmRepository name and namespace", func() { It("fails if it's unable to list available profiles from the cluster", func() { - gitProviders.RepositoryExistsReturns(true, nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapperWithErr("nope"), nil }) err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) Expect(err).To(MatchError("failed to discover HelmRepository: failed to get profiles from cluster: failed to make GET request to service weave-system/wego-app path \"/v1/profiles\": nope")) - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) }) It("fails to find an available profile with the given version", func() { - gitProviders.RepositoryExistsReturns(true, nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapper(getProfilesResp), nil }) updateOptions.Version = "7.0.0" err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) Expect(err).To(MatchError("failed to discover HelmRepository: failed to get profiles from cluster: version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) - Expect(gitProviders.RepositoryExistsCallCount()).To(Equal(1)) }) }) }) From 62de95204d0d612358bc796ec1aaf2f76b0ad8f4 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Tue, 15 Feb 2022 11:09:22 +0100 Subject: [PATCH 4/7] Update acceptance tests for updating profile --- cmd/gitops/update/cmd.go | 6 +++--- cmd/gitops/update/profiles/profiles.go | 8 +++---- pkg/helm/releases_test.go | 4 ++-- pkg/services/profiles/get_test.go | 2 +- test/acceptance/test/profiles_test.go | 30 +++++++++++++++----------- 5 files changed, 28 insertions(+), 22 deletions(-) diff --git a/cmd/gitops/update/cmd.go b/cmd/gitops/update/cmd.go index cb3c0d43880..09401706e2c 100644 --- a/cmd/gitops/update/cmd.go +++ b/cmd/gitops/update/cmd.go @@ -12,9 +12,9 @@ func UpdateCommand(endpoint *string, client *resty.Client) *cobra.Command { Use: "update", Short: "Update a Weave GitOps resource", Example: ` -# Update an installed profile to a certain version -gitops update profile --name --version --config-repo --cluster --namespace -`, + # Update a profile that is installed on a cluster + gitops update profile --name=podinfo --cluster=prod --config-repo=ssh://git@github.com/owner/config-repo.git --version=1.0.0 + `, } cmd.AddCommand(profiles.UpdateCommand()) diff --git a/cmd/gitops/update/profiles/profiles.go b/cmd/gitops/update/profiles/profiles.go index bdad4c02005..d7bd1a63117 100644 --- a/cmd/gitops/update/profiles/profiles.go +++ b/cmd/gitops/update/profiles/profiles.go @@ -24,16 +24,16 @@ import ( var opts profiles.UpdateOptions -// UpdateCommand provides support for updating an installed profile's version. +// UpdateCommand provides support for updating a profile that is installed on a cluster. func UpdateCommand() *cobra.Command { cmd := &cobra.Command{ Use: "profile", - Short: "Update a profile to a different version", + Short: "Update a profile installation", SilenceUsage: true, SilenceErrors: true, Example: ` - # Update an installed profile to a different version - gitops update profile --name=podinfo --cluster=prod --version=1.0.0 --config-repo=ssh://git@github.com/owner/config-repo.git --namespace=test-namespace + # Update a profile that is installed on a cluster + gitops update profile --name=podinfo --cluster=prod --config-repo=ssh://git@github.com/owner/config-repo.git --version=1.0.0 `, RunE: updateProfileCmdRunE(), } diff --git a/pkg/helm/releases_test.go b/pkg/helm/releases_test.go index c9884f01ff6..1e4c3212fd0 100644 --- a/pkg/helm/releases_test.go +++ b/pkg/helm/releases_test.go @@ -75,8 +75,8 @@ var _ = Describe("AppendHelmReleaseToString", func() { ) }) - When("the file does not exist", func() { - It("creates one with the new helm release", func() { + When("the given string is empty", func() { + It("appends a helm release to it", func() { s, err := helm.AppendHelmReleaseToString("", newRelease) Expect(err).NotTo(HaveOccurred()) r, err := yaml.Marshal(newRelease) diff --git a/pkg/services/profiles/get_test.go b/pkg/services/profiles/get_test.go index 0f909e6d4c2..281a917ed2b 100644 --- a/pkg/services/profiles/get_test.go +++ b/pkg/services/profiles/get_test.go @@ -196,7 +196,7 @@ podinfo Podinfo Helm chart for Kubernetes 6.0.0,6.0.1 Expect(err).To(MatchError("no version found for profile 'podinfo' in prod/test-namespace")) }) - It("fails if the available profile's HelmRepository name or namespace if empty", func() { + It("fails if the available profile's HelmRepository name or namespace are empty", func() { badProfileResp := `{ "profiles": [ { diff --git a/test/acceptance/test/profiles_test.go b/test/acceptance/test/profiles_test.go index a4100eb02c6..aa216a33960 100644 --- a/test/acceptance/test/profiles_test.go +++ b/test/acceptance/test/profiles_test.go @@ -131,21 +131,27 @@ Namespace: %s`, clusterName, namespace))) }, "120s", "1s").Should(Equal(http.StatusOK)) By("Updating the version of the installed profile") - stdOut, stdErr = runCommandAndReturnStringOutput(fmt.Sprintf("%s update profile --name %s --version 6.0.0 --namespace %s --cluster %s --config-repo %s --auto-merge", gitopsBinaryPath, profileName, namespace, clusterName, appRepoRemoteURL)) - Expect(stdErr).To(BeEmpty()) + time.Sleep(4 * time.Minute) + Eventually(func() string { + stdOut, stdErr = runCommandAndReturnStringOutput(fmt.Sprintf("%s update profile --name %s --version 6.0.0 --namespace %s --cluster %s --config-repo %s --auto-merge", gitopsBinaryPath, profileName, namespace, clusterName, appRepoRemoteURL)) + return stdErr + }, "120s", "5s").Should(BeEmpty()) + Expect(stdOut).To(ContainSubstring( - fmt.Sprintf(`Updating 1 profile: - - Name: podinfo - Version: 6.0.0 - Cluster: %s - Namespace: %s`, clusterName, namespace))) + fmt.Sprintf( + `Updating profile: + +Name: podinfo +Version: 6.0.0 +Cluster: %s +Namespace: %s`, clusterName, namespace))) By("Verifying that the profile installed in the cluster's namespace was updated to the correct version") - Eventually(func() int { - resp, statusCode, err = kubernetesDoRequest(namespace, clusterName+"-"+profileName, "9898", "/healthz", clientSet) - return statusCode - }, "120s", "1s").Should(Equal(http.StatusOK)) + Eventually(func() string { + stdOut, stdErr = runCommandAndReturnStringOutput(fmt.Sprintf("kubectl get pods -n %s --selector=app.kubernetes.io/name=%s-%s -o jsonpath='{.items[*].spec.containers[*].image}'", namespace, clusterName, profileName)) + Expect(stdErr).Should(BeEmpty()) + return stdOut + }, "240s", "5s").Should(ContainSubstring("ghcr.io/stefanprodan/podinfo:6.0.0")) }) It("@skipOnNightly profiles are installed into a different namespace", func() { From cd0746d5af935140c85412c8a173b6b46244d1bf Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Tue, 15 Feb 2022 17:09:04 +0100 Subject: [PATCH 5/7] Address 'update profile' review --- cmd/gitops/update/profiles/profiles_test.go | 2 +- pkg/helm/releases.go | 22 ++---- pkg/helm/releases_test.go | 87 +++++++++++---------- pkg/services/profiles/add.go | 16 +++- pkg/services/profiles/update.go | 31 +++++--- pkg/services/profiles/update_test.go | 8 +- test/acceptance/test/profiles_test.go | 2 +- 7 files changed, 89 insertions(+), 79 deletions(-) diff --git a/cmd/gitops/update/profiles/profiles_test.go b/cmd/gitops/update/profiles/profiles_test.go index 72648a2d996..376e67bbe7c 100644 --- a/cmd/gitops/update/profiles/profiles_test.go +++ b/cmd/gitops/update/profiles/profiles_test.go @@ -53,7 +53,7 @@ var _ = Describe("Update Profile(s)", func() { }) err := cmd.Execute() - Expect(err).To(MatchError("error parsing --version=&%*/v: Invalid Semantic Version")) + Expect(err).To(MatchError(ContainSubstring("error parsing --version=&%*/v"))) }) }) diff --git a/pkg/helm/releases.go b/pkg/helm/releases.go index d78ad685a48..87591c4338f 100644 --- a/pkg/helm/releases.go +++ b/pkg/helm/releases.go @@ -44,17 +44,6 @@ func MakeHelmRelease(name, version, cluster, namespace string, helmRepository ty } } -// FindReleaseInNamespace iterates through a slice of HelmReleases to find one with a given name in a given namespace, and returns it with its index. -func FindReleaseInNamespace(existingReleases []helmv2beta1.HelmRelease, name, ns string) (*helmv2beta1.HelmRelease, int, error) { - for i, r := range existingReleases { - if r.Name == name && r.Namespace == ns { - return &r, i, nil - } - } - - return nil, -1, nil -} - // AppendHelmReleaseToString appends a HelmRelease to a string. func AppendHelmReleaseToString(content string, newRelease *helmv2beta1.HelmRelease) (string, error) { var sb strings.Builder @@ -73,8 +62,8 @@ func AppendHelmReleaseToString(content string, newRelease *helmv2beta1.HelmRelea } // SplitHelmReleaseYAML splits a manifest file that contains one or more Helm Releases that may be separated by '---'. -func SplitHelmReleaseYAML(resources []byte) ([]helmv2beta1.HelmRelease, error) { - var helmReleaseList []helmv2beta1.HelmRelease +func SplitHelmReleaseYAML(resources []byte) ([]*helmv2beta1.HelmRelease, error) { + var helmReleaseList []*helmv2beta1.HelmRelease decoder := apimachinery.NewYAMLOrJSONDecoder(bytes.NewReader(resources), 100000000) @@ -88,15 +77,14 @@ func SplitHelmReleaseYAML(resources []byte) ([]helmv2beta1.HelmRelease, error) { return nil, err } - helmReleaseList = append(helmReleaseList, value) + helmReleaseList = append(helmReleaseList, &value) } return helmReleaseList, nil } -func PatchHelmRelease(existingReleases []helmv2beta1.HelmRelease, patchedHelmRelease helmv2beta1.HelmRelease, index int) (string, error) { - existingReleases[index] = patchedHelmRelease - +// MarshalHelmReleases marshals a list of HelmReleases. +func MarshalHelmReleases(existingReleases []*helmv2beta1.HelmRelease) (string, error) { var sb strings.Builder for _, r := range existingReleases { diff --git a/pkg/helm/releases_test.go b/pkg/helm/releases_test.go index 1e4c3212fd0..afd6814784d 100644 --- a/pkg/helm/releases_test.go +++ b/pkg/helm/releases_test.go @@ -5,7 +5,6 @@ import ( "github.com/weaveworks/weave-gitops/pkg/helm" - "github.com/fluxcd/helm-controller/api/v2beta1" helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" "github.com/google/go-cmp/cmp" @@ -76,7 +75,7 @@ var _ = Describe("AppendHelmReleaseToString", func() { }) When("the given string is empty", func() { - It("appends a helm release to it", func() { + It("appends a HelmRelease to it", func() { s, err := helm.AppendHelmReleaseToString("", newRelease) Expect(err).NotTo(HaveOccurred()) r, err := yaml.Marshal(newRelease) @@ -84,59 +83,67 @@ var _ = Describe("AppendHelmReleaseToString", func() { Expect(s).To(ContainSubstring(string(r))) }) }) -}) - -var _ = Describe("FindReleaseInNamespace", func() { - var ( - name = "prod-podinfo" - ns = "weave-system" - ) - When("it finds a HelmRelease with a matching name and namespace", func() { - It("returns its index in the slice of bytes", func() { - newRelease := helm.MakeHelmRelease( - "podinfo", "6.0.0", "prod", "weave-system", + When("the given string is not empty", func() { + It("appends a HelmRelease to it", func() { + b, _ := kyaml.Marshal(helm.MakeHelmRelease( + "another-profile", "7.0.0", "prod", "test-namespace", types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, - ) - existingRelease, index, err := helm.FindReleaseInNamespace([]helmv2beta1.HelmRelease{*newRelease}, name, ns) + )) + s, err := helm.AppendHelmReleaseToString(string(b), newRelease) Expect(err).NotTo(HaveOccurred()) - Expect(index).To(Equal(0)) - Expect(cmp.Diff(*existingRelease, *newRelease)).To(BeEmpty()) - }) - }) - - When("it does not find a HelmRelease with a matching name and namespace", func() { - It("returns an index of -1", func() { - _, index, err := helm.FindReleaseInNamespace([]helmv2beta1.HelmRelease{}, name, ns) + r, err := yaml.Marshal(newRelease) Expect(err).NotTo(HaveOccurred()) - Expect(index).To(Equal(-1)) + Expect(s).To(ContainSubstring(string(r))) }) }) }) -var _ = Describe("PatchHelmReleaseInString", func() { - var ( - r *v2beta1.HelmRelease - ) - - BeforeEach(func() { - r = helm.MakeHelmRelease( - "podinfo", "6.0.1", "prod", "weave-system", +var _ = Describe("MarshalHelmRelease", func() { + It("returns a string with an updated list of HelmReleases", func() { + release1 := helm.MakeHelmRelease( + "random-profile", "7.0.0", "prod", "weave-system", types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, ) - }) + releaseBytes1, _ := kyaml.Marshal(release1) - It("returns a string with an updated list of HelmReleases", func() { - existingRelease := helm.MakeHelmRelease( + release2 := helm.MakeHelmRelease( "podinfo", "6.0.0", "prod", "weave-system", types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, ) + releaseBytes2, _ := kyaml.Marshal(release2) - expectedContentBytes, _ := kyaml.Marshal(r) - expectedContent := "---\n" + string(expectedContentBytes) - - patchedContent, err := helm.PatchHelmRelease([]helmv2beta1.HelmRelease{*existingRelease}, *r, 0) + patchedContent, err := helm.MarshalHelmReleases([]*helmv2beta1.HelmRelease{release1, release2}) Expect(err).NotTo(HaveOccurred()) - Expect(cmp.Diff(patchedContent, expectedContent)).To(BeEmpty()) + Expect(cmp.Diff(patchedContent, "---\n"+string(releaseBytes1)+"---\n"+string(releaseBytes2))).To(BeEmpty()) + }) +}) + +var _ = Describe("SplitHelmReleaseYAML", func() { + When("the resource contains only HelmRelease", func() { + It("returns a slice of HelmReleases", func() { + r1 := helm.MakeHelmRelease( + "podinfo", "6.0.0", "prod", "weave-system", + types.NamespacedName{Name: "helm-repo-name", Namespace: "helm-repo-namespace"}, + ) + r2 := helm.MakeHelmRelease( + "profile", "6.0.1", "prod", "test-namespace", + types.NamespacedName{Name: "helm-repo-name", Namespace: "test-namespace"}, + ) + b1, _ := kyaml.Marshal(r1) + bytes := append(b1, []byte("\n---\n")...) + b2, _ := kyaml.Marshal(r2) + list, err := helm.SplitHelmReleaseYAML(append(bytes, b2...)) + Expect(err).NotTo(HaveOccurred()) + Expect(list).To(ContainElements(r1, r2)) + }) + }) + + When("the resource contains any resource other than a HelmRelease", func() { + It("returns an error", func() { + b, _ := kyaml.Marshal("content") + _, err := helm.SplitHelmReleaseYAML(b) + Expect(err).To(MatchError(ContainSubstring("error unmarshaling JSON"))) + }) }) }) diff --git a/pkg/services/profiles/add.go b/pkg/services/profiles/add.go index 6ca5666b1da..8e7e82c86d2 100644 --- a/pkg/services/profiles/add.go +++ b/pkg/services/profiles/add.go @@ -10,6 +10,7 @@ import ( "github.com/weaveworks/weave-gitops/pkg/models" "github.com/fluxcd/go-git-providers/gitprovider" + helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" "github.com/google/uuid" "k8s.io/apimachinery/pkg/types" ) @@ -120,12 +121,19 @@ func addHelmRelease(helmRepo types.NamespacedName, fileContent, name, version, c newRelease := helm.MakeHelmRelease(name, version, cluster, ns, helmRepo) - matchingHelmRelease, _, err := helm.FindReleaseInNamespace(existingReleases, newRelease.Name, ns) - if matchingHelmRelease != nil { + if releaseIsInNamespace(existingReleases, newRelease.Name, ns) { return "", fmt.Errorf("found another HelmRelease for profile '%s' in namespace %s", name, ns) - } else if err != nil { - return "", fmt.Errorf("error reading from %s: %w", models.WegoProfilesPath, err) } return helm.AppendHelmReleaseToString(fileContent, newRelease) } + +func releaseIsInNamespace(existingReleases []*helmv2beta1.HelmRelease, name, ns string) bool { + for _, r := range existingReleases { + if r.Name == name && r.Namespace == ns { + return true + } + } + + return false +} diff --git a/pkg/services/profiles/update.go b/pkg/services/profiles/update.go index f877ab27320..ada03ac43a7 100644 --- a/pkg/services/profiles/update.go +++ b/pkg/services/profiles/update.go @@ -10,6 +10,7 @@ import ( "github.com/weaveworks/weave-gitops/pkg/models" "github.com/fluxcd/go-git-providers/gitprovider" + helmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1" "github.com/google/uuid" "k8s.io/apimachinery/pkg/types" ) @@ -120,20 +121,26 @@ func updateHelmRelease(helmRepo types.NamespacedName, fileContent, name, version return "", fmt.Errorf("error splitting into YAML: %w", err) } - releaseName := cluster + "-" + name - - matchingHelmRelease, index, err := helm.FindReleaseInNamespace(existingReleases, releaseName, ns) - if matchingHelmRelease == nil { - return "", fmt.Errorf("failed to find HelmRelease '%s' in namespace %s", releaseName, ns) - } else if err != nil { - return "", fmt.Errorf("error reading from %s: %w", models.WegoProfilesPath, err) + updatedReleases, err := patchRelease(existingReleases, cluster+"-"+name, ns, version) + if err != nil { + return "", err } - if matchingHelmRelease.Spec.Chart.Spec.Version == version { - return "", fmt.Errorf("version %s of profile '%s' already installed in %s/%s", version, name, ns, cluster) - } + return helm.MarshalHelmReleases(updatedReleases) +} - matchingHelmRelease.Spec.Chart.Spec.Version = version +func patchRelease(existingReleases []*helmv2beta1.HelmRelease, name, ns, version string) ([]*helmv2beta1.HelmRelease, error) { + for _, r := range existingReleases { + if r.Name == name && r.Namespace == ns { + if r.Spec.Chart.Spec.Version == version { + return nil, fmt.Errorf("version %s of HelmRelease '%s' already installed in namespace '%s'", version, name, ns) + } + + r.Spec.Chart.Spec.Version = version + + return existingReleases, nil + } + } - return helm.PatchHelmRelease(existingReleases, *matchingHelmRelease, index) + return nil, fmt.Errorf("failed to find HelmRelease '%s' in namespace '%s'", name, ns) } diff --git a/pkg/services/profiles/update_test.go b/pkg/services/profiles/update_test.go index 3f39112f407..90399479769 100644 --- a/pkg/services/profiles/update_test.go +++ b/pkg/services/profiles/update_test.go @@ -147,7 +147,7 @@ var _ = Describe("Update Profile(s)", func() { }) }) - When("an existing HelmRelease is the same version as the want to update to", func() { + When("an existing HelmRelease is the same version as the one to update to", func() { It("returns an error", func() { existingRelease := helm.MakeHelmRelease( "podinfo", "6.0.1", "prod", "weave-system", @@ -162,7 +162,7 @@ var _ = Describe("Update Profile(s)", func() { }}, nil) err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: version 6.0.1 of profile 'podinfo' already installed in weave-system/prod")) + Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: version 6.0.1 of HelmRelease 'prod-podinfo' already installed in namespace 'weave-system'")) }) }) }) @@ -182,7 +182,7 @@ var _ = Describe("Update Profile(s)", func() { }}, nil) err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: failed to find HelmRelease 'prod-podinfo' in namespace weave-system")) + Expect(err).To(MatchError("failed to update HelmRelease for profile 'podinfo' in profiles.yaml: failed to find HelmRelease 'prod-podinfo' in namespace 'weave-system'")) }) }) @@ -242,7 +242,7 @@ var _ = Describe("Update Profile(s)", func() { } err := profilesSvc.Update(context.TODO(), gitProviders, updateOptions) - Expect(err).To(MatchError("failed to parse url: could not get provider name from URL {http:/-*wrong-url-827: could not parse git repo url \"{http:/-*wrong-url-827\": parse \"{http:/-*wrong-url-827\": first path segment in URL cannot contain colon")) + Expect(err).To(MatchError(ContainSubstring("failed to parse url: could not get provider name from URL {http:/-*wrong-url-827"))) }) It("fails if the config repo does not exist", func() { diff --git a/test/acceptance/test/profiles_test.go b/test/acceptance/test/profiles_test.go index aa216a33960..879f656befb 100644 --- a/test/acceptance/test/profiles_test.go +++ b/test/acceptance/test/profiles_test.go @@ -131,7 +131,7 @@ Namespace: %s`, clusterName, namespace))) }, "120s", "1s").Should(Equal(http.StatusOK)) By("Updating the version of the installed profile") - time.Sleep(4 * time.Minute) + time.Sleep(3 * time.Minute) Eventually(func() string { stdOut, stdErr = runCommandAndReturnStringOutput(fmt.Sprintf("%s update profile --name %s --version 6.0.0 --namespace %s --cluster %s --config-repo %s --auto-merge", gitopsBinaryPath, profileName, namespace, clusterName, appRepoRemoteURL)) return stdErr From c1633cd795176944f6806b0392aabe6133013f6c Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 16 Feb 2022 10:51:28 +0100 Subject: [PATCH 6/7] Remove sleep from profiles acceptance test --- test/acceptance/test/profiles_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/acceptance/test/profiles_test.go b/test/acceptance/test/profiles_test.go index 879f656befb..be7df52196c 100644 --- a/test/acceptance/test/profiles_test.go +++ b/test/acceptance/test/profiles_test.go @@ -131,11 +131,10 @@ Namespace: %s`, clusterName, namespace))) }, "120s", "1s").Should(Equal(http.StatusOK)) By("Updating the version of the installed profile") - time.Sleep(3 * time.Minute) Eventually(func() string { stdOut, stdErr = runCommandAndReturnStringOutput(fmt.Sprintf("%s update profile --name %s --version 6.0.0 --namespace %s --cluster %s --config-repo %s --auto-merge", gitopsBinaryPath, profileName, namespace, clusterName, appRepoRemoteURL)) return stdErr - }, "120s", "5s").Should(BeEmpty()) + }, "240s", "10s").Should(BeEmpty()) Expect(stdOut).To(ContainSubstring( fmt.Sprintf( From e54bb981b827588b1aa8199794e0d62eb183ea03 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 16 Feb 2022 13:19:51 +0100 Subject: [PATCH 7/7] Address review feedback --- pkg/helm/releases.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/helm/releases.go b/pkg/helm/releases.go index 87591c4338f..30827be185d 100644 --- a/pkg/helm/releases.go +++ b/pkg/helm/releases.go @@ -11,10 +11,12 @@ import ( sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - apimachinery "k8s.io/apimachinery/pkg/util/yaml" + k8syaml "k8s.io/apimachinery/pkg/util/yaml" kyaml "sigs.k8s.io/yaml" ) +const DefaultBufferSize = 2048 + // MakeHelmRelease returns a HelmRelease object given a name, version, cluster, namespace, and HelmRepository's name and namespace. func MakeHelmRelease(name, version, cluster, namespace string, helmRepository types.NamespacedName) *helmv2beta1.HelmRelease { return &helmv2beta1.HelmRelease{ @@ -44,7 +46,8 @@ func MakeHelmRelease(name, version, cluster, namespace string, helmRepository ty } } -// AppendHelmReleaseToString appends a HelmRelease to a string. +// AppendHelmReleaseToString appends "---" and a HelmRelease to string that may or may not be empty. +// This creates the content of a manifest that contains HelmReleases separated by "---". func AppendHelmReleaseToString(content string, newRelease *helmv2beta1.HelmRelease) (string, error) { var sb strings.Builder if content != "" { @@ -65,7 +68,7 @@ func AppendHelmReleaseToString(content string, newRelease *helmv2beta1.HelmRelea func SplitHelmReleaseYAML(resources []byte) ([]*helmv2beta1.HelmRelease, error) { var helmReleaseList []*helmv2beta1.HelmRelease - decoder := apimachinery.NewYAMLOrJSONDecoder(bytes.NewReader(resources), 100000000) + decoder := k8syaml.NewYAMLOrJSONDecoder(bytes.NewReader(resources), DefaultBufferSize) for { var value helmv2beta1.HelmRelease