Address review feedback for Add Profile

- removed all `Expect(err).NotTo(BeNil)` before `MatchError()` assertions - refactored how we discover version of available profiles to return it rather than set it on pb profile - pass in all values for MakeHelmRelease to remove Profiles dependency - renamed --port to --profiles-port - use a single const for profiles path (git.ProfilesManifestFileName) instead of models.WegoProfilePath - seed with time at the start of the cmd for late use of rand - improve function name and documentation comment for splitYAML() and GetRandomString() - use Cobra's MarkFlagRequired feature for name, cluster, and config-repo flags - change gitproviders.MergePullRequest() to always use method Merge - uses ConvertStringListToSemanticVersionList() and SortVersions() to sort semver versions of Profile's available versions - checkout package-lock.json to reset it to that of main - removed *resty.Client from addRunE command - moved profiles.yaml const to pkg models - added --kubeconfig flag - renamed GetRepoFiles() to GetRepoDirFiles() - renamed MakeManifestFile() to AppendProfileToFile() - renamed GetAvailableProfile() to GetProfile() - MakeHelmRelease compares HelmReleases with go-cmp (very cool) Co-authored-by: Jake Klein <[email protected]> Co-authored-by: Gergely Brautigam <[email protected]> Co-authored-by: Chetan Patwal <[email protected]>
weaveworks · Feb 2, 2022 · e10b6a3 · e10b6a3
1 parent 0a8c2fa
commit e10b6a3
Show file tree

Hide file tree

Showing 26 changed files with 701 additions and 50,485 deletions.
diff --git a/cmd/gitops/add/cmd.go b/cmd/gitops/add/cmd.go
@@ -22,7 +22,7 @@ gitops add cluster`,
 
 	cmd.AddCommand(clusters.ClusterCommand(endpoint, client))
 	cmd.AddCommand(app.Cmd)
-	cmd.AddCommand(profiles.AddCommand(client))
+	cmd.AddCommand(profiles.AddCommand())
 
 	return cmd
 }
diff --git a/cmd/gitops/add/profiles/cmd.go b/cmd/gitops/add/profiles/cmd.go
@@ -2,14 +2,13 @@ package profiles
 
 import (
 	"context"
-	"errors"
 	"fmt"
+	"math/rand"
 	"os"
 	"path/filepath"
-	"strings"
+	"time"
 
 	"github.com/Masterminds/semver/v3"
-	"github.com/go-resty/resty/v2"
 	"github.com/spf13/cobra"
 	"github.com/weaveworks/weave-gitops/cmd/internal"
 	"github.com/weaveworks/weave-gitops/pkg/flux"
@@ -29,32 +28,41 @@ import (
 var opts profiles.AddOptions
 
 // AddCommand provides support for adding a profile to a cluster.
-func AddCommand(client *resty.Client) *cobra.Command {
+func AddCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:           "profile",
-		Aliases:       []string{"profiles"},
 		Short:         "Add a profile to a cluster",
 		SilenceUsage:  true,
 		SilenceErrors: true,
 		Example: `
 		# Add a profile to a cluster
 		gitops add profile --name=podinfo --cluster=prod --version=1.0.0 --config-repo=ssh://[email protected]/owner/config-repo.git
 		`,
-		RunE: addProfileCmdRunE(client),
+		RunE: addProfileCmdRunE(),
 	}
 
 	cmd.Flags().StringVar(&opts.Name, "name", "", "Name of the profile")
-	cmd.Flags().StringVar(&opts.Version, "version", "latest", "Version 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.Cluster, "cluster", "", "Name of the cluster to add the profile to")
-	cmd.Flags().StringVar(&opts.Port, "port", server.DefaultPort, "Port the profiles API is running on")
+	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 set --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"}
+	for _, f := range requiredFlags {
+		if err := cobra.MarkFlagRequired(cmd.Flags(), f); err != nil {
+			panic(fmt.Errorf("unexpected error: %w", err))
+		}
+
+	}
 
 	return cmd
 }
 
-func addProfileCmdRunE(client *resty.Client) func(*cobra.Command, []string) error {
+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)
@@ -64,13 +72,13 @@ func addProfileCmdRunE(client *resty.Client) func(*cobra.Command, []string) erro
 			return err
 		}
 
-		ns, err := cmd.Parent().Parent().Flags().GetString("namespace")
+		ns, err := cmd.Flags().GetString("namespace")
 		if err != nil {
 			return err
 		}
 		opts.Namespace = ns
 
-		config, err := clientcmd.BuildConfigFromFlags("", filepath.Join(homedir.HomeDir(), ".kube", "config"))
+		config, err := clientcmd.BuildConfigFromFlags("", opts.Kubeconfig)
 		if err != nil {
 			return fmt.Errorf("error initializing kubernetes config: %w", err)
 		}
@@ -96,36 +104,19 @@ func addProfileCmdRunE(client *resty.Client) func(*cobra.Command, []string) erro
 			return fmt.Errorf("failed to get git clients: %w", err)
 		}
 
-		profilesService := profiles.NewService(clientSet, log)
-		return profilesService.Add(ctx, gitProvider, opts)
+		return profiles.NewService(clientSet, log).Add(ctx, gitProvider, opts)
 	}
 }
 
 func validateAddOptions(opts profiles.AddOptions) error {
-	if opts.Name == "" {
-		return errors.New("--name should be provided")
-	}
-
 	if models.ApplicationNameTooLong(opts.Name) {
 		return fmt.Errorf("--name value is too long: %s; must be <= %d characters",
 			opts.Name, models.MaxKubernetesResourceNameLength)
 	}
 
-	if strings.HasPrefix(opts.Name, "wego") {
-		return fmt.Errorf("the prefix 'wego' is used by weave gitops and is not allowed for a profile name")
-	}
-
-	if opts.ConfigRepo == "" {
-		return errors.New("--config-repo should be provided")
-	}
-
-	if opts.Cluster == "" {
-		return errors.New("--cluster should be provided")
-	}
-
 	if opts.Version != "latest" {
 		if _, err := semver.StrictNewVersion(opts.Version); err != nil {
-			return fmt.Errorf("error parsing --version=%s: %s", opts.Version, err)
+			return fmt.Errorf("error parsing --version=%s: %w", opts.Version, err)
 		}
 	}
 	return nil

diff --git a/cmd/gitops/add/profiles/cmd_test.go b/cmd/gitops/add/profiles/cmd_test.go
@@ -17,10 +17,13 @@ var _ = Describe("Add Profiles", func() {
 	BeforeEach(func() {
 		client := resty.New()
 		httpmock.ActivateNonDefault(client.GetClient())
-		defer httpmock.DeactivateAndReset()
 		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{
@@ -39,54 +42,24 @@ var _ = Describe("Add Profiles", func() {
 	})
 
 	When("flags are not valid", func() {
-		It("fails if --name is not provided", func() {
-			cmd.SetArgs([]string{
-				"add", "profile",
-			})
-
-			err := cmd.Execute()
-			Expect(err).To(MatchError("--name should be provided"))
-		})
-
-		When("--name is specified", func() {
-			It("fails if --name value is <= 63 characters in length", func() {
-				cmd.SetArgs([]string{
-					"add", "profile",
-					"--name", "a234567890123456789012345678901234567890123456789012345678901234",
-				})
-				err := cmd.Execute()
-				Expect(err).To(MatchError("--name value is too long: a234567890123456789012345678901234567890123456789012345678901234; must be <= 63 characters"))
-			})
-
-			It("fails if --name is prefixed by 'wego'", func() {
-				cmd.SetArgs([]string{
-					"add", "profile",
-					"--name", "wego-app",
-				})
-				err := cmd.Execute()
-				Expect(err).To(MatchError("the prefix 'wego' is used by weave gitops and is not allowed for a profile name"))
-			})
-		})
-
-		It("fails if --config-repo is not provided", func() {
+		It("fails if --name, --cluster, and --config-repo are not provided", func() {
 			cmd.SetArgs([]string{
 				"add", "profile",
-				"--name", "podinfo",
 			})
 
 			err := cmd.Execute()
-			Expect(err).To(MatchError("--config-repo should be provided"))
+			Expect(err).To(MatchError("required flag(s) \"cluster\", \"config-repo\", \"name\" not set"))
 		})
 
-		It("fails if --config-repo is not provided", func() {
+		It("fails if --name value is <= 63 characters in length", func() {
 			cmd.SetArgs([]string{
 				"add", "profile",
-				"--name", "podinfo",
-				"--config-repo", "ssh://[email protected]/owner/config-repo.git",
+				"--name", "a234567890123456789012345678901234567890123456789012345678901234",
+				"--cluster", "cluster",
+				"--config-repo", "config-repo",
 			})
-
 			err := cmd.Execute()
-			Expect(err).To(MatchError("--cluster should be provided"))
+			Expect(err).To(MatchError("--name value is too long: a234567890123456789012345678901234567890123456789012345678901234; must be <= 63 characters"))
 		})
 
 		It("fails if given version is not valid semver", func() {
@@ -111,7 +84,6 @@ var _ = Describe("Add Profiles", func() {
 			})
 
 			err := cmd.Execute()
-			Expect(err).To(HaveOccurred())
 			Expect(err).To(MatchError("unknown flag: --unknown"))
 		})
 	})