Skip to content

Commit

Permalink
Address review feedback for Add Profile
Browse files Browse the repository at this point in the history 
- 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 <jake@weave.works>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Co-authored-by: Chetan Patwal <cPu1@users.noreply.github.com>
  • Loading branch information
@Skarlso @cPu1
4 people committed Feb 2, 2022
1 parent 82fc301 commit 80fd25f
Show file tree
Hide file tree
Showing 26 changed files with 793 additions and 50,587 deletions.
2 changes: 1 addition & 1 deletion cmd/gitops/add/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
53 changes: 22 additions & 31 deletions cmd/gitops/add/profiles/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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://git@github.com/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().BoolVar(&opts.AutoMerge, "auto-merge", false, "If set, 'gitops add profile' will merge automatically into the set --branch")
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().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)
Expand All @@ -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)
}
Expand All @@ -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
Expand Down
50 changes: 11 additions & 39 deletions cmd/gitops/add/profiles/cmd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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://git@github.com/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() {
Expand All @@ -111,7 +84,6 @@ var _ = Describe("Add Profiles", func() {
})

err := cmd.Execute()
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError("unknown flag: --unknown"))
})
})
Expand Down

0 comments on commit 80fd25f

Please sign in to comment.