From 1189078c5a9f3d75ca5f0f8a0c8ad60c358217f3 Mon Sep 17 00:00:00 2001 From: katexochen <49727155+katexochen@users.noreply.github.com> Date: Wed, 4 May 2022 08:50:50 +0200 Subject: [PATCH] Replace mutiple args with flags AB#1955 --- cli/cmd/create.go | 137 ++++++++++------ cli/cmd/create_test.go | 180 +++++++++++++-------- cli/cmd/init.go | 1 + cli/cmd/recover.go | 4 +- cli/cmd/root.go | 9 +- cli/cmd/validargs.go | 130 ++------------- cli/cmd/validargs_test.go | 275 -------------------------------- cli/cmd/verify.go | 48 ++++-- cli/cmd/verify_test.go | 132 ++++++++------- internal/constants/constants.go | 7 + 10 files changed, 336 insertions(+), 587 deletions(-) diff --git a/cli/cmd/create.go b/cli/cmd/create.go index b3f130e55..feb91f601 100644 --- a/cli/cmd/create.go +++ b/cli/cmd/create.go @@ -4,8 +4,6 @@ import ( "errors" "fmt" "io/fs" - "strconv" - "strings" "github.com/edgelesssys/constellation/cli/azure" "github.com/edgelesssys/constellation/cli/cloud/cloudcmd" @@ -20,42 +18,39 @@ import ( func newCreateCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "create {aws|gcp|azure} C_COUNT W_COUNT TYPE", + Use: "create {aws|gcp|azure}", Short: "Create instances on a cloud platform for your Constellation.", - Long: `Create instances on a cloud platform for your Constellation. -A Constellation with C_COUNT control-plane nodes and W_COUNT workers is created. -TYPE is the instance type used for all instances.`, + Long: "Create instances on a cloud platform for your Constellation.", Args: cobra.MatchAll( - cobra.ExactArgs(4), - isIntGreaterZeroArg(1), - isIntGreaterZeroArg(2), - isInstanceTypeForProvider(3, 0), + cobra.ExactArgs(1), + isCloudProvider(0), warnAWS(0), ), ValidArgsFunction: createCompletion, RunE: runCreate, } - cmd.Flags().String("name", "constell", "Set this flag to create the Constellation with the specified name.") - cmd.Flags().BoolP("yes", "y", false, "Set this flag to create the Constellation without further confirmation.") - + cmd.Flags().String("name", "constell", "Create the Constellation cluster with the specified name.") + cmd.Flags().BoolP("yes", "y", false, "Create the Constellation cluster without further confirmation.") + cmd.Flags().IntP("control-plane-nodes", "c", 1, "Number of control-plane nodes.") + must(cobra.MarkFlagRequired(cmd.Flags(), "control-plane-nodes")) + cmd.Flags().IntP("worker-nodes", "w", 1, "Number of worker nodes.") + must(cobra.MarkFlagRequired(cmd.Flags(), "worker-nodes")) + cmd.Flags().StringP("instance-type", "t", "", "Instance type of cluster nodes.") + must(cmd.RegisterFlagCompletionFunc("instance-type", instanceTypeCompletion)) return cmd } func runCreate(cmd *cobra.Command, args []string) error { provider := cloudprovider.FromString(args[0]) - countCoord, _ := strconv.Atoi(args[1]) // err checked in args validation - countNode, _ := strconv.Atoi(args[2]) // err checked in args validation - insType := strings.ToLower(args[3]) fileHandler := file.NewHandler(afero.NewOsFs()) creator := cloudcmd.NewCreator(cmd.OutOrStdout()) - return create(cmd, creator, fileHandler, countCoord, countNode, provider, insType) + return create(cmd, creator, fileHandler, provider) } -func create(cmd *cobra.Command, creator cloudCreator, fileHandler file.Handler, - countCoord, countNode int, provider cloudprovider.Provider, insType string, +func create(cmd *cobra.Command, creator cloudCreator, fileHandler file.Handler, provider cloudprovider.Provider, ) (retErr error) { - flags, err := parseCreateFlags(cmd) + flags, err := parseCreateFlags(cmd, provider) if err != nil { return err } @@ -72,8 +67,8 @@ func create(cmd *cobra.Command, creator cloudCreator, fileHandler file.Handler, if !flags.yes { // Ask user to confirm action. cmd.Printf("The following Constellation will be created:\n") - cmd.Printf("%d control-planes nodes of type %s will be created.\n", countCoord, insType) - cmd.Printf("%d worker nodes of type %s will be created.\n", countNode, insType) + cmd.Printf("%d control-planes nodes of type %s will be created.\n", flags.controllerCount, flags.insType) + cmd.Printf("%d worker nodes of type %s will be created.\n", flags.workerCount, flags.insType) ok, err := askToConfirm(cmd, "Do you want to create this Constellation?") if err != nil { return err @@ -84,7 +79,7 @@ func create(cmd *cobra.Command, creator cloudCreator, fileHandler file.Handler, } } - state, err := creator.Create(cmd.Context(), provider, config, flags.name, insType, countCoord, countNode) + state, err := creator.Create(cmd.Context(), provider, config, flags.name, flags.insType, flags.controllerCount, flags.workerCount) if err != nil { return err } @@ -98,7 +93,34 @@ func create(cmd *cobra.Command, creator cloudCreator, fileHandler file.Handler, } // parseCreateFlags parses the flags of the create command. -func parseCreateFlags(cmd *cobra.Command) (createFlags, error) { +func parseCreateFlags(cmd *cobra.Command, provider cloudprovider.Provider) (createFlags, error) { + controllerCount, err := cmd.Flags().GetInt("control-plane-nodes") + if err != nil { + return createFlags{}, err + } + if controllerCount < constants.MinControllerCount { + return createFlags{}, fmt.Errorf("number of control-plane nodes must be at least %d", constants.MinControllerCount) + } + + workerCount, err := cmd.Flags().GetInt("worker-nodes") + if err != nil { + return createFlags{}, err + } + if workerCount < constants.MinWorkerCount { + return createFlags{}, fmt.Errorf("number of worker nodes must be at least %d", constants.MinWorkerCount) + } + + insType, err := cmd.Flags().GetString("instance-type") + if err != nil { + return createFlags{}, err + } + if insType == "" { + insType = defaultInstanceType(provider) + } + if err := validInstanceTypeForProvider(insType, provider); err != nil { + return createFlags{}, err + } + name, err := cmd.Flags().GetString("name") if err != nil { return createFlags{}, err @@ -109,27 +131,47 @@ func parseCreateFlags(cmd *cobra.Command) (createFlags, error) { constellationNameLength, len(name), name, ) } + yes, err := cmd.Flags().GetBool("yes") if err != nil { return createFlags{}, err } + devConfigPath, err := cmd.Flags().GetString("dev-config") if err != nil { return createFlags{}, err } return createFlags{ - name: name, - devConfigPath: devConfigPath, - yes: yes, + controllerCount: controllerCount, + workerCount: workerCount, + insType: insType, + name: name, + devConfigPath: devConfigPath, + yes: yes, }, nil } // createFlags contains the parsed flags of the create command. type createFlags struct { - name string - devConfigPath string - yes bool + controllerCount int + workerCount int + insType string + name string + devConfigPath string + yes bool +} + +// defaultInstanceType returns the default instance type for the given provider. +func defaultInstanceType(provider cloudprovider.Provider) string { + switch provider { + case cloudprovider.GCP: + return gcp.InstanceTypes[0] + case cloudprovider.Azure: + return azure.InstanceTypes[0] + default: + return "" + } } // checkDirClean checks if files of a previous Constellation are left in the current working dir. @@ -153,27 +195,20 @@ func createCompletion(cmd *cobra.Command, args []string, toComplete string) ([]s switch len(args) { case 0: return []string{"aws", "gcp", "azure"}, cobra.ShellCompDirectiveNoFileComp - case 1: - return []string{}, cobra.ShellCompDirectiveNoFileComp - case 2: - return []string{}, cobra.ShellCompDirectiveNoFileComp - case 3: - var instanceTypeList []string - switch args[0] { - case "aws": - instanceTypeList = []string{ - "4xlarge", - "8xlarge", - "12xlarge", - "16xlarge", - "24xlarge", - } - case "gcp": - instanceTypeList = gcp.InstanceTypes - case "azure": - instanceTypeList = azure.InstanceTypes - } - return instanceTypeList, cobra.ShellCompDirectiveNoFileComp + default: + return []string{}, cobra.ShellCompDirectiveError + } +} + +func instanceTypeCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + if len(args) != 1 { + return []string{}, cobra.ShellCompDirectiveError + } + switch args[0] { + case "gcp": + return gcp.InstanceTypes, cobra.ShellCompDirectiveNoFileComp + case "azure": + return azure.InstanceTypes, cobra.ShellCompDirectiveNoFileComp default: return []string{}, cobra.ShellCompDirectiveError } diff --git a/cli/cmd/create_test.go b/cli/cmd/create_test.go index 29ee9d052..e7edb2a12 100644 --- a/cli/cmd/create_test.go +++ b/cli/cmd/create_test.go @@ -3,6 +3,7 @@ package cmd import ( "bytes" "errors" + "fmt" "strings" "testing" @@ -23,29 +24,10 @@ func TestCreateArgumentValidation(t *testing.T) { args []string wantErr bool }{ - "gcp valid create 1": {[]string{"gcp", "3", "3", "n2d-standard-2"}, false}, - "gcp valid create 2": {[]string{"gcp", "3", "7", "n2d-standard-16"}, false}, - "gcp valid create 3": {[]string{"gcp", "1", "2", "n2d-standard-96"}, false}, - "gcp invalid too many arguments": {[]string{"gcp", "3", "2", "n2d-standard-2", "n2d-standard-2"}, true}, - "gcp invalid too many arguments 2": {[]string{"gcp", "3", "2", "n2d-standard-2", "2"}, true}, - "gcp invalid no control planes": {[]string{"gcp", "0", "1", "n2d-standard-2"}, true}, - "gcp invalid no workers": {[]string{"gcp", "1", "0", "n2d-standard-2"}, true}, - "gcp invalid first is no int": {[]string{"gcp", "n2d-standard-2", "1", "n2d-standard-2"}, true}, - "gcp invalid second is no int": {[]string{"gcp", "3", "n2d-standard-2", "n2d-standard-2"}, true}, - "gcp invalid third is no size": {[]string{"gcp", "2", "2", "2"}, true}, - "gcp invalid wrong order": {[]string{"gcp", "n2d-standard-2", "2", "2"}, true}, - "azure valid create 1": {[]string{"azure", "3", "3", "Standard_DC2as_v5"}, false}, - "azure valid create 2": {[]string{"azure", "3", "7", "Standard_DC4as_v5"}, false}, - "azure valid create 3": {[]string{"azure", "1", "2", "Standard_DC8as_v5"}, false}, - "azure invalid to many arguments": {[]string{"azure", "3", "2", "Standard_DC2as_v5", "Standard_DC2as_v5"}, true}, - "azure invalid to many arguments 2": {[]string{"azure", "3", "2", "Standard_DC2as_v5", "2"}, true}, - "azure invalid no control planes": {[]string{"azure", "0", "1", "Standard_DC2as_v5"}, true}, - "azure invalid no workers": {[]string{"azure", "1", "0", "Standard_DC2as_v5"}, true}, - "azure invalid first is no int": {[]string{"azure", "Standard_DC2as_v5", "1", "Standard_DC2as_v5"}, true}, - "azure invalid second is no int": {[]string{"azure", "1", "Standard_DC2as_v5", "Standard_DC2as_v5"}, true}, - "azure invalid third is no size": {[]string{"azure", "2", "2", "2"}, true}, - "azure invalid wrong order": {[]string{"azure", "Standard_DC2as_v5", "2", "2"}, true}, - "aws waring": {[]string{"aws", "1", "2", "4xlarge"}, true}, + "gcp": {[]string{"gcp"}, false}, + "azure": {[]string{"azure"}, false}, + "aws waring": {[]string{"aws"}, true}, + "too many args": {[]string{"gcp", "1", "2"}, true}, } for name, tc := range testCases { @@ -68,15 +50,18 @@ func TestCreate(t *testing.T) { someErr := errors.New("failed") testCases := map[string]struct { - setupFs func(*require.Assertions) afero.Fs - creator *stubCloudCreator - provider cloudprovider.Provider - yesFlag bool - devConfigFlag string - nameFlag string - stdin string - wantErr bool - wantAbbort bool + setupFs func(*require.Assertions) afero.Fs + creator *stubCloudCreator + provider cloudprovider.Provider + yesFlag bool + controllerCountFlag *int + workerCountFlag *int + insTypeFlag string + devConfigFlag string + nameFlag string + stdin string + wantErr bool + wantAbbort bool }{ "create": { setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, @@ -87,7 +72,7 @@ func TestCreate(t *testing.T) { "interactive": { setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, creator: &stubCloudCreator{state: testState}, - provider: cloudprovider.GCP, + provider: cloudprovider.Azure, stdin: "yes\n", }, "interactive abort": { @@ -111,6 +96,43 @@ func TestCreate(t *testing.T) { nameFlag: strings.Repeat("a", constellationNameLength+1), wantErr: true, }, + "flag control-plane-count invalid": { + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + creator: &stubCloudCreator{}, + provider: cloudprovider.GCP, + controllerCountFlag: intPtr(0), + workerCountFlag: intPtr(3), + wantErr: true, + }, + "flag worker-count invalid": { + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + creator: &stubCloudCreator{}, + provider: cloudprovider.GCP, + controllerCountFlag: intPtr(3), + workerCountFlag: intPtr(-1), + wantErr: true, + }, + "flag control-plane-count missing": { + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + creator: &stubCloudCreator{}, + provider: cloudprovider.GCP, + workerCountFlag: intPtr(3), + wantErr: true, + }, + "flag worker-count missing": { + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + creator: &stubCloudCreator{}, + provider: cloudprovider.GCP, + controllerCountFlag: intPtr(3), + wantErr: true, + }, + "flag invalid instance-type": { + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + creator: &stubCloudCreator{}, + provider: cloudprovider.GCP, + insTypeFlag: "invalid", + wantErr: true, + }, "old state in directory": { setupFs: func(require *require.Assertions) afero.Fs { fs := afero.NewMemMapFs() @@ -180,10 +202,10 @@ func TestCreate(t *testing.T) { require := require.New(t) cmd := newCreateCmd() - cmd.Flags().String("dev-config", "", "") // register persisten flag manually cmd.SetOut(&bytes.Buffer{}) cmd.SetErr(&bytes.Buffer{}) cmd.SetIn(bytes.NewBufferString(tc.stdin)) + cmd.Flags().String("dev-config", "", "") // register persisten flag manually if tc.yesFlag { require.NoError(cmd.Flags().Set("yes", "true")) } @@ -193,9 +215,19 @@ func TestCreate(t *testing.T) { if tc.devConfigFlag != "" { require.NoError(cmd.Flags().Set("dev-config", tc.devConfigFlag)) } + if tc.controllerCountFlag != nil { + require.NoError(cmd.Flags().Set("control-plane-nodes", fmt.Sprint(*tc.controllerCountFlag))) + } + if tc.workerCountFlag != nil { + require.NoError(cmd.Flags().Set("worker-nodes", fmt.Sprint(*tc.workerCountFlag))) + } + if tc.insTypeFlag != "" { + require.NoError(cmd.Flags().Set("instance-type", tc.insTypeFlag)) + } + fileHandler := file.NewHandler(tc.setupFs(require)) - err := create(cmd, tc.creator, fileHandler, 3, 3, tc.provider, "type") + err := create(cmd, tc.creator, fileHandler, tc.provider) if tc.wantErr { assert.Error(err) @@ -277,38 +309,7 @@ func TestCreateCompletion(t *testing.T) { wantShellCD: cobra.ShellCompDirectiveNoFileComp, }, "second arg": { - args: []string{"gcp"}, - wantResult: []string{}, - wantShellCD: cobra.ShellCompDirectiveNoFileComp, - }, - "third arg": { - args: []string{"gcp", "1"}, - wantResult: []string{}, - wantShellCD: cobra.ShellCompDirectiveNoFileComp, - }, - "fourth arg aws": { - args: []string{"aws", "1", "2"}, - wantResult: []string{ - "4xlarge", - "8xlarge", - "12xlarge", - "16xlarge", - "24xlarge", - }, - wantShellCD: cobra.ShellCompDirectiveNoFileComp, - }, - "fourth arg gcp": { - args: []string{"gcp", "1", "2"}, - wantResult: gcp.InstanceTypes, - wantShellCD: cobra.ShellCompDirectiveNoFileComp, - }, - "fourth arg azure": { - args: []string{"azure", "1", "2"}, - wantResult: azure.InstanceTypes, - wantShellCD: cobra.ShellCompDirectiveNoFileComp, - }, - "fifth arg": { - args: []string{"aws", "1", "2", "4xlarge"}, + args: []string{"gcp", "foo"}, wantResult: []string{}, wantShellCD: cobra.ShellCompDirectiveError, }, @@ -325,3 +326,48 @@ func TestCreateCompletion(t *testing.T) { }) } } + +func TestInstanceTypeCompletion(t *testing.T) { + testCases := map[string]struct { + args []string + wantResult []string + wantShellCD cobra.ShellCompDirective + }{ + "azure": { + args: []string{"azure"}, + wantResult: azure.InstanceTypes, + wantShellCD: cobra.ShellCompDirectiveNoFileComp, + }, + "gcp": { + args: []string{"gcp"}, + wantResult: gcp.InstanceTypes, + wantShellCD: cobra.ShellCompDirectiveNoFileComp, + }, + "empty args": { + args: []string{}, + wantResult: []string{}, + wantShellCD: cobra.ShellCompDirectiveError, + }, + "unknown provider": { + args: []string{"foo"}, + wantResult: []string{}, + wantShellCD: cobra.ShellCompDirectiveError, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + cmd := &cobra.Command{} + result, shellCD := instanceTypeCompletion(cmd, tc.args, "") + + assert.Equal(tc.wantResult, result) + assert.Equal(tc.wantShellCD, shellCD) + }) + } +} + +func intPtr(i int) *int { + return &i +} diff --git a/cli/cmd/init.go b/cli/cmd/init.go index 0de5bb559..e16dbcdae 100644 --- a/cli/cmd/init.go +++ b/cli/cmd/init.go @@ -43,6 +43,7 @@ func newInitCmd() *cobra.Command { cmd.Flags().String("privatekey", "", "path to your private key.") cmd.Flags().String("master-secret", "", "path to base64 encoded master secret.") cmd.Flags().Bool("wg-autoconfig", false, "enable automatic configuration of WireGuard interface") + must(cmd.Flags().MarkHidden("wg-autoconfig")) cmd.Flags().Bool("autoscale", false, "enable Kubernetes cluster-autoscaler") return cmd } diff --git a/cli/cmd/recover.go b/cli/cmd/recover.go index 51fad7b2f..9cfa05ce9 100644 --- a/cli/cmd/recover.go +++ b/cli/cmd/recover.go @@ -32,9 +32,9 @@ func newRecoverCmd() *cobra.Command { RunE: runRecover, } cmd.Flags().String("ip", "", "Instance IP address.") - _ = cmd.MarkFlagRequired("ip") + must(cmd.MarkFlagRequired("ip")) cmd.Flags().String("disk-uuid", "", "Disk UUID of the encrypted state disk.") - _ = cmd.MarkFlagRequired("disk-uuid") + must(cmd.MarkFlagRequired("disk-uuid")) cmd.Flags().String("master-secret", "", "Path to base64 encoded master secret.") return cmd } diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 66671f79f..19ffe1c7f 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -54,7 +54,8 @@ func init() { cobra.EnableCommandSorting = false // Set output of cmd.Print to stdout. (By default, it's stderr.) rootCmd.SetOut(os.Stdout) - rootCmd.PersistentFlags().String("dev-config", "", "Set this flag to create the Constellation using settings from a development config.") + rootCmd.PersistentFlags().String("dev-config", "", "Set this flag to create the Constellation cluster using settings from a development config.") + must(rootCmd.MarkPersistentFlagFilename("dev-config", "json")) rootCmd.AddCommand(newCreateCmd()) rootCmd.AddCommand(newInitCmd()) rootCmd.AddCommand(newVerifyCmd()) @@ -62,3 +63,9 @@ func init() { rootCmd.AddCommand(newTerminateCmd()) rootCmd.AddCommand(newVersionCmd()) } + +func must(err error) { + if err != nil { + panic(err) + } +} diff --git a/cli/cmd/validargs.go b/cli/cmd/validargs.go index 71a109d2e..0134ac53f 100644 --- a/cli/cmd/validargs.go +++ b/cli/cmd/validargs.go @@ -3,46 +3,13 @@ package cmd import ( "errors" "fmt" - "net" - "strconv" - "strings" "github.com/edgelesssys/constellation/cli/azure" "github.com/edgelesssys/constellation/cli/cloudprovider" - "github.com/edgelesssys/constellation/cli/ec2" "github.com/edgelesssys/constellation/cli/gcp" "github.com/spf13/cobra" ) -// isIntArg checks if argument at position arg is an integer. -func isIntArg(arg int) cobra.PositionalArgs { - return func(cmd *cobra.Command, args []string) error { - if _, err := strconv.Atoi(args[arg]); err != nil { - return fmt.Errorf("argument %d must be an integer", arg) - } - return nil - } -} - -// isIntGreaterArg checks if argument at position arg is an integer and greater i. -func isIntGreaterArg(arg int, i int) cobra.PositionalArgs { - return cobra.MatchAll(isIntArg(arg), func(cmd *cobra.Command, args []string) error { - if v, _ := strconv.Atoi(args[arg]); v <= i { - return fmt.Errorf("argument %d must be greater %d, but it's %d", arg, i, v) - } - return nil - }) -} - -func isIntLessArg(arg int, i int) cobra.PositionalArgs { - return cobra.MatchAll(isIntArg(arg), func(cmd *cobra.Command, args []string) error { - if v, _ := strconv.Atoi(args[arg]); v >= i { - return fmt.Errorf("argument %d must be less %d, but it's %d", arg, i, v) - } - return nil - }) -} - // warnAWS warns that AWS isn't supported. func warnAWS(providerPos int) cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { @@ -53,60 +20,6 @@ func warnAWS(providerPos int) cobra.PositionalArgs { } } -// isIntGreaterZeroArg checks if argument at position arg is a positive non zero integer. -func isIntGreaterZeroArg(arg int) cobra.PositionalArgs { - return isIntGreaterArg(arg, 0) -} - -func isPort(arg int) cobra.PositionalArgs { - return cobra.MatchAll( - isIntGreaterArg(arg, -1), - isIntLessArg(arg, 65536), - ) -} - -func isIP(arg int) cobra.PositionalArgs { - return func(cmd *cobra.Command, args []string) error { - if ip := net.ParseIP(args[arg]); ip == nil { - return fmt.Errorf("argument %s isn't a valid IP address", args[arg]) - } - return nil - } -} - -// isEC2InstanceType checks if argument at position arg is a key in m. -// The argument will always be converted to lower case letters. -func isEC2InstanceType(arg int) cobra.PositionalArgs { - return func(cmd *cobra.Command, args []string) error { - if _, ok := ec2.InstanceTypes[strings.ToLower(args[arg])]; !ok { - return fmt.Errorf("'%s' isn't an AWS EC2 instance type", args[arg]) - } - return nil - } -} - -func isGCPInstanceType(arg int) cobra.PositionalArgs { - return func(cmd *cobra.Command, args []string) error { - for _, instanceType := range gcp.InstanceTypes { - if args[arg] == instanceType { - return nil - } - } - return fmt.Errorf("argument %s isn't a valid GCP instance type", args[arg]) - } -} - -func isAzureInstanceType(arg int) cobra.PositionalArgs { - return func(cmd *cobra.Command, args []string) error { - for _, instanceType := range azure.InstanceTypes { - if args[arg] == instanceType { - return nil - } - } - return fmt.Errorf("argument %s isn't a valid Azure instance type", args[arg]) - } -} - func isCloudProvider(arg int) cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { if provider := cloudprovider.FromString(args[arg]); provider == cloudprovider.Unknown { @@ -116,34 +29,23 @@ func isCloudProvider(arg int) cobra.PositionalArgs { } } -// isInstanceTypeForProvider returns a argument validation function that checks if the argument -// at position typePos is a valid instance type for the cloud provider string at position -// providerPos. -func isInstanceTypeForProvider(typePos, providerPos int) cobra.PositionalArgs { - return func(cmd *cobra.Command, args []string) error { - if len(args) < 2 { - return fmt.Errorf("requires 2 arguments, but only %d are provided", len(args)) +func validInstanceTypeForProvider(insType string, provider cloudprovider.Provider) error { + switch provider { + case cloudprovider.GCP: + for _, instanceType := range gcp.InstanceTypes { + if insType == instanceType { + return nil + } } - if len(args) <= typePos { - return fmt.Errorf( - "%d arguments provided, but index %d of typePos is out of bound", - len(args), typePos, - ) - } - if len(args) <= providerPos { - return fmt.Errorf( - "%d arguments provided, but index %d of providerPos is out of bound", - len(args), providerPos, - ) - } - - switch cloudprovider.FromString(args[providerPos]) { - case cloudprovider.GCP: - return isGCPInstanceType(typePos)(cmd, args) - case cloudprovider.Azure: - return isAzureInstanceType(typePos)(cmd, args) - default: - return fmt.Errorf("argument %s isn't a valid cloud platform", args[providerPos]) + return fmt.Errorf("%s isn't a valid GCP instance type", insType) + case cloudprovider.Azure: + for _, instanceType := range azure.InstanceTypes { + if insType == instanceType { + return nil + } } + return fmt.Errorf("%s isn't a valid Azure instance type", insType) + default: + return fmt.Errorf("%s isn't a valid cloud platform", provider) } } diff --git a/cli/cmd/validargs_test.go b/cli/cmd/validargs_test.go index 35b74907c..508bcfa4c 100644 --- a/cli/cmd/validargs_test.go +++ b/cli/cmd/validargs_test.go @@ -7,243 +7,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestIsIntArg(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "valid int 1": {[]string{"1"}, false}, - "valid int 2": {[]string{"42"}, false}, - "valid int 3": {[]string{"987987498"}, false}, - "valid int and other args": {[]string{"3", "hello"}, false}, - "valid int and other args 2": {[]string{"3", "4"}, false}, - "invalid 1": {[]string{"hello world"}, true}, - "invalid 2": {[]string{"98798d749f8"}, true}, - "invalid 3": {[]string{"three"}, true}, - "invalid 4": {[]string{"0.3"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isIntArg(0)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - -func TestIsIntGreaterArg(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "valid int 1": {[]string{"13"}, false}, - "valid int 2": {[]string{"42"}, false}, - "valid int 3": {[]string{"987987498"}, false}, - "invalid int 1": {[]string{"1"}, true}, - "invalid int and other args": {[]string{"3", "hello"}, true}, - "invalid int and other args 2": {[]string{"-14", "4"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isIntGreaterArg(0, 12)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - -func TestIsIntGreaterZeroArg(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "valid int 1": {[]string{"13"}, false}, - "valid int 2": {[]string{"42"}, false}, - "valid int 3": {[]string{"987987498"}, false}, - "invalid": {[]string{"0"}, true}, - "invalid int 1": {[]string{"-42", "hello"}, true}, - "invalid int and other args": {[]string{"-9487239847", "4"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isIntGreaterZeroArg(0)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - -func TestIsPort(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "valid port 1": {[]string{"80"}, false}, - "valid port 2": {[]string{"8080"}, false}, - "valid port 3": {[]string{"65535"}, false}, - "invalid port 1": {[]string{"foo"}, true}, - "invalid port 2": {[]string{"65536"}, true}, - "invalid port 3": {[]string{"-1"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isPort(0)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - -func TestIsIP(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "valid ip 1": {[]string{"192.168.0.2"}, false}, - "valid ip 2": {[]string{"127.0.0.1"}, false}, - "valid ip 3": {[]string{"8.8.8.8"}, false}, - "invalid ip 1": {[]string{"foo"}, true}, - "invalid ip 2": {[]string{"foo.bar.baz.1"}, true}, - "invalid ip 3": {[]string{"800.800.800.800"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isIP(0)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - -func TestIsEC2InstanceType(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "is instance type 1": {[]string{"4xl"}, false}, - "is instance type 2": {[]string{"12xlarge", "something else"}, false}, - "isn't instance type 1": {[]string{"notAnInstanceType"}, true}, - "isn't instance type 2": {[]string{"Hello World!"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isEC2InstanceType(0)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - -func TestIsGCPInstanceType(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "is instance type 1": {[]string{"n2d-standard-4"}, false}, - "is instance type 2": {[]string{"n2d-standard-16", "something else"}, false}, - "isn't instance type 1": {[]string{"notAnInstanceType"}, true}, - "isn't instance type 2": {[]string{"Hello World!"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isGCPInstanceType(0)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - -func TestIsAzureInstanceType(t *testing.T) { - testCases := map[string]struct { - args []string - wantErr bool - }{ - "is instance type 1": {[]string{"Standard_DC2as_v5"}, false}, - "is instance type 2": {[]string{"Standard_DC8as_v5", "something else"}, false}, - "isn't instance type 1": {[]string{"notAnInstanceType"}, true}, - "isn't instance type 2": {[]string{"Hello World!"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isAzureInstanceType(0)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} - func TestIsCloudProvider(t *testing.T) { testCases := map[string]struct { pos int @@ -273,41 +36,3 @@ func TestIsCloudProvider(t *testing.T) { }) } } - -func TestIsInstanceTypeForProvider(t *testing.T) { - testCases := map[string]struct { - typePos int - providerPos int - args []string - wantErr bool - }{ - "valid gcp type 1": {1, 0, []string{"gcp", "n2d-standard-4"}, false}, - "valid gcp type 2": {1, 0, []string{"gcp", "n2d-standard-16", "foo"}, false}, - "valid azure type 1": {1, 0, []string{"azure", "Standard_DC2as_v5"}, false}, - "valid azure type 2": {1, 0, []string{"azure", "Standard_DC8as_v5", "foo"}, false}, - "mixed order 1": {0, 3, []string{"n2d-standard-4", "", "foo", "gcp"}, false}, - "mixed order 2": {2, 1, []string{"", "gcp", "n2d-standard-4", "foo", "bar"}, false}, - "invalid gcp type": {1, 0, []string{"gcp", "foo"}, true}, - "invalid azure type": {1, 0, []string{"azure", "foo"}, true}, - "args to short": {2, 0, []string{"foo"}, true}, - "provider position invalid": {1, 2, []string{"gcp", "n2d-standard-4"}, true}, - "type position invalid": {2, 0, []string{"gcp", "n2d-standard-4"}, true}, - "unknown provider": {1, 0, []string{"foo", "n2d-standard-4"}, true}, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - testCmd := &cobra.Command{Args: isInstanceTypeForProvider(tc.typePos, tc.providerPos)} - - err := testCmd.ValidateArgs(tc.args) - - if tc.wantErr { - assert.Error(err) - } else { - assert.NoError(err) - } - }) - } -} diff --git a/cli/cmd/verify.go b/cli/cmd/verify.go index a8c11edde..7c9601c76 100644 --- a/cli/cmd/verify.go +++ b/cli/cmd/verify.go @@ -4,12 +4,16 @@ import ( "context" "errors" "fmt" + "net" + "strconv" + "strings" "github.com/edgelesssys/constellation/cli/cloud/cloudcmd" "github.com/edgelesssys/constellation/cli/cloudprovider" "github.com/edgelesssys/constellation/cli/file" "github.com/edgelesssys/constellation/cli/proto" "github.com/edgelesssys/constellation/internal/config" + "github.com/edgelesssys/constellation/internal/constants" "github.com/spf13/afero" "github.com/spf13/cobra" rpcStatus "google.golang.org/grpc/status" @@ -17,33 +21,32 @@ import ( func newVerifyCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "verify {azure|gcp} IP PORT", - Short: "Verify the confidential properties of your Constellation.", - Long: "Verify the confidential properties of your Constellation.", + Use: "verify {aws|azure|gcp}", + Short: "Verify the confidential properties of your Constellation cluster.", + Long: "Verify the confidential properties of your Constellation cluster.", Args: cobra.MatchAll( - cobra.ExactArgs(3), + cobra.ExactArgs(1), isCloudProvider(0), - isIP(1), - isPort(2), + warnAWS(0), ), RunE: runVerify, } - cmd.Flags().String("owner-id", "", "verify the Constellation using the owner identity derived from the master secret.") - cmd.Flags().String("unique-id", "", "verify the Constellation using the unique cluster identity.") + cmd.Flags().String("owner-id", "", "Verify using the owner identity derived from the master secret.") + cmd.Flags().String("unique-id", "", "Verify using the unique cluster identity.") + cmd.Flags().StringP("node-endpoint", "e", "", "Endpoint of the node to verify. Form: HOST[:PORT]") + must(cmd.MarkFlagRequired("node-endpoint")) return cmd } func runVerify(cmd *cobra.Command, args []string) error { provider := cloudprovider.FromString(args[0]) - ip := args[1] - port := args[2] fileHandler := file.NewHandler(afero.NewOsFs()) protoClient := &proto.Client{} defer protoClient.Close() - return verify(cmd.Context(), cmd, provider, ip, port, fileHandler, protoClient) + return verify(cmd.Context(), cmd, provider, fileHandler, protoClient) } -func verify(ctx context.Context, cmd *cobra.Command, provider cloudprovider.Provider, ip, port string, fileHandler file.Handler, protoClient protoClient) error { +func verify(ctx context.Context, cmd *cobra.Command, provider cloudprovider.Provider, fileHandler file.Handler, protoClient protoClient) error { flags, err := parseVerifyFlags(cmd) if err != nil { return err @@ -66,7 +69,7 @@ func verify(ctx context.Context, cmd *cobra.Command, provider cloudprovider.Prov cmd.Print(validators.Warnings()) } - if err := protoClient.Connect(ip, port, validators.V()); err != nil { + if err := protoClient.Connect(flags.nodeHost, flags.nodePort, validators.V()); err != nil { return err } if _, err := protoClient.GetState(ctx); err != nil { @@ -93,12 +96,27 @@ func parseVerifyFlags(cmd *cobra.Command) (verifyFlags, error) { return verifyFlags{}, errors.New("neither owner ID nor unique ID provided to verify the Constellation") } + endpoint, err := cmd.Flags().GetString("node-endpoint") + if err != nil { + return verifyFlags{}, err + } + host, port, err := net.SplitHostPort(endpoint) + if err != nil { + if !strings.Contains(err.Error(), "missing port in address") { + return verifyFlags{}, err + } + host = endpoint + port = strconv.Itoa(constants.CoordinatorPort) + } + devConfigPath, err := cmd.Flags().GetString("dev-config") if err != nil { return verifyFlags{}, err } return verifyFlags{ + nodeHost: host, + nodePort: port, devConfigPath: devConfigPath, ownerID: ownerID, clusterID: clusterID, @@ -106,6 +124,8 @@ func parseVerifyFlags(cmd *cobra.Command) (verifyFlags, error) { } type verifyFlags struct { + nodeHost string + nodePort string ownerID string clusterID string devConfigPath string @@ -117,8 +137,6 @@ func verifyCompletion(cmd *cobra.Command, args []string, toComplete string) ([]s switch len(args) { case 0: return []string{"gcp", "azure"}, cobra.ShellCompDirectiveNoFileComp - case 1, 2: - return []string{}, cobra.ShellCompDirectiveNoFileComp default: return []string{}, cobra.ShellCompDirectiveError } diff --git a/cli/cmd/verify_test.go b/cli/cmd/verify_test.go index a45a0be0d..57dba5ae4 100644 --- a/cli/cmd/verify_test.go +++ b/cli/cmd/verify_test.go @@ -22,15 +22,10 @@ func TestVerifyCmdArgumentValidation(t *testing.T) { args []string wantErr bool }{ - "no args": {[]string{}, true}, - "valid azure": {[]string{"azure", "192.0.2.1", "1234"}, false}, - "valid gcp": {[]string{"gcp", "192.0.2.1", "1234"}, false}, - "invalid provider": {[]string{"invalid", "192.0.2.1", "1234"}, true}, - "invalid ip": {[]string{"gcp", "invalid", "1234"}, true}, - "invalid port": {[]string{"gcp", "192.0.2.1", "invalid"}, true}, - "invalid port 2": {[]string{"gcp", "192.0.2.1", "65536"}, true}, - "not enough arguments": {[]string{"gcp", "192.0.2.1"}, true}, - "too many arguments": {[]string{"gcp", "192.0.2.1", "1234", "5678"}, true}, + "no args": {[]string{}, true}, + "valid azure": {[]string{"azure"}, false}, + "valid gcp": {[]string{"gcp"}, false}, + "invalid provider": {[]string{"invalid", "192.0.2.1", "1234"}, true}, } for name, tc := range testCases { @@ -54,58 +49,81 @@ func TestVerify(t *testing.T) { someErr := errors.New("failed") testCases := map[string]struct { - setupFs func(*require.Assertions) afero.Fs - provider cloudprovider.Provider - protoClient protoClient - devConfigFlag string - ownerIDFlag string - clusterIDFlag string - wantErr bool + setupFs func(*require.Assertions) afero.Fs + provider cloudprovider.Provider + protoClient protoClient + nodeEndpointFlag string + devConfigFlag string + ownerIDFlag string + clusterIDFlag string + wantErr bool }{ "gcp": { - setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, - provider: cloudprovider.GCP, - ownerIDFlag: zeroBase64, - protoClient: &stubProtoClient{}, + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.GCP, + nodeEndpointFlag: "192.0.2.1:1234", + ownerIDFlag: zeroBase64, + protoClient: &stubProtoClient{}, }, "azure": { - setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, - provider: cloudprovider.Azure, - ownerIDFlag: zeroBase64, - protoClient: &stubProtoClient{}, + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.Azure, + nodeEndpointFlag: "192.0.2.1:1234", + ownerIDFlag: zeroBase64, + protoClient: &stubProtoClient{}, + }, + "default port": { + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.GCP, + nodeEndpointFlag: "192.0.2.1", + ownerIDFlag: zeroBase64, + protoClient: &stubProtoClient{}, + }, + "invalid endpoint": { + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.GCP, + nodeEndpointFlag: ":::::", + ownerIDFlag: zeroBase64, + protoClient: &stubProtoClient{}, + wantErr: true, }, "neither owner id nor cluster id set": { - setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, - provider: cloudprovider.GCP, - wantErr: true, + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.GCP, + nodeEndpointFlag: "192.0.2.1:1234", + wantErr: true, }, "dev config file not existing": { - setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, - provider: cloudprovider.GCP, - ownerIDFlag: zeroBase64, - devConfigFlag: "./file", - wantErr: true, + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.GCP, + ownerIDFlag: zeroBase64, + nodeEndpointFlag: "192.0.2.1:1234", + devConfigFlag: "./file", + wantErr: true, }, "error protoClient Connect": { - setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, - provider: cloudprovider.Azure, - ownerIDFlag: zeroBase64, - protoClient: &stubProtoClient{connectErr: someErr}, - wantErr: true, + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.Azure, + nodeEndpointFlag: "192.0.2.1:1234", + ownerIDFlag: zeroBase64, + protoClient: &stubProtoClient{connectErr: someErr}, + wantErr: true, }, "error protoClient GetState": { - setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, - provider: cloudprovider.Azure, - ownerIDFlag: zeroBase64, - protoClient: &stubProtoClient{getStateErr: rpcStatus.Error(codes.Internal, "failed")}, - wantErr: true, + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.Azure, + nodeEndpointFlag: "192.0.2.1:1234", + ownerIDFlag: zeroBase64, + protoClient: &stubProtoClient{getStateErr: rpcStatus.Error(codes.Internal, "failed")}, + wantErr: true, }, "error protoClient GetState not rpc": { - setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, - provider: cloudprovider.Azure, - ownerIDFlag: zeroBase64, - protoClient: &stubProtoClient{getStateErr: someErr}, - wantErr: true, + setupFs: func(require *require.Assertions) afero.Fs { return afero.NewMemMapFs() }, + provider: cloudprovider.Azure, + nodeEndpointFlag: "192.0.2.1:1234", + ownerIDFlag: zeroBase64, + protoClient: &stubProtoClient{getStateErr: someErr}, + wantErr: true, }, } @@ -128,10 +146,13 @@ func TestVerify(t *testing.T) { if tc.clusterIDFlag != "" { require.NoError(cmd.Flags().Set("cluster-id", tc.clusterIDFlag)) } + if tc.nodeEndpointFlag != "" { + require.NoError(cmd.Flags().Set("node-endpoint", tc.nodeEndpointFlag)) + } fileHandler := file.NewHandler(tc.setupFs(require)) ctx := context.Background() - err := verify(ctx, cmd, tc.provider, "192.0.2.1", "1234", fileHandler, tc.protoClient) + err := verify(ctx, cmd, tc.provider, fileHandler, tc.protoClient) if tc.wantErr { assert.Error(err) @@ -156,21 +177,8 @@ func TestVerifyCompletion(t *testing.T) { wantResult: []string{"gcp", "azure"}, wantShellCD: cobra.ShellCompDirectiveNoFileComp, }, - "second arg": { - args: []string{"gcp"}, - toComplete: "192.0.2.1", - wantResult: []string{}, - wantShellCD: cobra.ShellCompDirectiveNoFileComp, - }, - "third arg": { - args: []string{"gcp", "192.0.2.1"}, - toComplete: "443", - wantResult: []string{}, - wantShellCD: cobra.ShellCompDirectiveNoFileComp, - }, "additional arg": { - args: []string{"gcp", "192.0.2.1", "443"}, - toComplete: "./file", + args: []string{"gcp", "foo"}, wantResult: []string{}, wantShellCD: cobra.ShellCompDirectiveError, }, diff --git a/internal/constants/constants.go b/internal/constants/constants.go index d7104af91..c4e6feb35 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -32,6 +32,13 @@ const ( // Cryptographic constants. // StateDiskKeyLength = 32 + + // + // CLI. + // + + MinControllerCount = 1 + MinWorkerCount = 1 ) // CliVersion is the version of the CLI. Left as a separate variable to allow override during build.