From a6e76d770ae50cea7398e87dd7cc77c822d72e0d Mon Sep 17 00:00:00 2001 From: Yuvraj Date: Mon, 27 Dec 2021 18:42:45 +0530 Subject: [PATCH] Improve flytectl config init UX (#241) * Fix flytectl ux issue Signed-off-by: Yuvraj --- .../config/subcommand/config/init_flags.go | 2 +- flytectl/cmd/configuration/configuration.go | 51 ++++++++++++++++--- .../cmd/configuration/configuration_test.go | 18 ++++++- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/flytectl/cmd/config/subcommand/config/init_flags.go b/flytectl/cmd/config/subcommand/config/init_flags.go index 029b92baf48..cb2047dec68 100755 --- a/flytectl/cmd/config/subcommand/config/init_flags.go +++ b/flytectl/cmd/config/subcommand/config/init_flags.go @@ -3,7 +3,7 @@ package config //go:generate pflags Config --default-var DefaultConfig --bind-default-var var ( DefaultConfig = &Config{ - Insecure: true, + Insecure: false, Storage: false, } ) diff --git a/flytectl/cmd/configuration/configuration.go b/flytectl/cmd/configuration/configuration.go index cbf0b8bf368..bdc2fe83a18 100644 --- a/flytectl/cmd/configuration/configuration.go +++ b/flytectl/cmd/configuration/configuration.go @@ -2,9 +2,12 @@ package configuration import ( "context" + "errors" "fmt" "io" + "net" "os" + "regexp" "strings" "github.com/flyteorg/flytestdlib/logger" @@ -31,18 +34,24 @@ Generates sandbox config. Flyte Sandbox is a fully standalone minimal environmen :: - flytectl configuration config + flytectl config init -Generates remote cluster config. Read more about the remote deployment https://docs.flyte.org/en/latest/deployment/index.html +Generates remote cluster config, By default connection is secure. Read more about the remote deployment https://docs.flyte.org/en/latest/deployment/index.html :: - flytectl configuration config --host=flyte.myexample.com - + flytectl config init --host=flyte.myexample.com + +Generates remote cluster config with insecure connection + +:: + + flytectl config init --host=flyte.myexample.com --insecure + Generates FlyteCTL config with a storage provider :: - flytectl configuration config --host=flyte.myexample.com --storage + flytectl config init --host=flyte.myexample.com --storage ` ) @@ -51,6 +60,8 @@ var prompt = promptui.Select{ Items: []string{"S3", "GCS"}, } +var endpointPrefix = [3]string{"dns://", "http://", "https://"} + // CreateConfigCommand will return configuration command func CreateConfigCommand() *cobra.Command { configCmd := viper.GetConfigCommand() @@ -77,12 +88,18 @@ func initFlytectlConfig(ctx context.Context, reader io.Reader) error { templateValues := configutil.ConfigTemplateSpec{ Host: "dns:///localhost:30081", - Insecure: initConfig.DefaultConfig.Insecure, + Insecure: true, } templateStr := configutil.GetSandboxTemplate() if len(initConfig.DefaultConfig.Host) > 0 { - templateValues.Host = fmt.Sprintf("dns:///%v", initConfig.DefaultConfig.Host) + trimHost := trim(initConfig.DefaultConfig.Host) + host := strings.Split(trimHost, ":") + if !validateEndpointName(host[0]) { + return errors.New("Please use a valid endpoint") + } + templateValues.Host = fmt.Sprintf("dns://%s", trimHost) + templateValues.Insecure = initConfig.DefaultConfig.Insecure templateStr = configutil.AdminConfigTemplate if initConfig.DefaultConfig.Storage { templateStr = configutil.GetAWSCloudTemplate() @@ -114,3 +131,23 @@ func initFlytectlConfig(ctx context.Context, reader io.Reader) error { fmt.Printf("Init flytectl config file at [%s]", configutil.ConfigFile) return nil } + +func trim(hostname string) string { + for _, prefix := range endpointPrefix { + hostname = strings.TrimPrefix(hostname, prefix) + } + return hostname + +} + +func validateEndpointName(domain string) bool { + RegExp := regexp.MustCompile(`^(([a-zA-Z]{1})|([a-zA-Z]{1}[a-zA-Z]{1})|([a-zA-Z]{1}[0-9]{1})|([0-9]{1}[a-zA-Z]{1})|([a-zA-Z0-9][a-zA-Z0-9-_]{1,61}[a-zA-Z0-9]))\.([a-zA-Z]{2,6}|[a-zA-Z0-9-]{2,30}\.[a-zA-Z + ]{2,3})$`) + if RegExp.MatchString(domain) || domain == "localhost" { + return true + } + if net.ParseIP(domain) == nil { + return false + } + return true +} diff --git a/flytectl/cmd/configuration/configuration_test.go b/flytectl/cmd/configuration/configuration_test.go index 5e84f00505f..ce2d2ca1bf4 100644 --- a/flytectl/cmd/configuration/configuration_test.go +++ b/flytectl/cmd/configuration/configuration_test.go @@ -56,8 +56,24 @@ func TestSetupConfigFunc(t *testing.T) { assert.Nil(t, initFlytectlConfig(ctx, yes)) assert.Nil(t, initFlytectlConfig(ctx, yes)) assert.Nil(t, initFlytectlConfig(ctx, no)) - initConfig.DefaultConfig.Host = "test" + initConfig.DefaultConfig.Host = "flyte.org" + assert.Nil(t, initFlytectlConfig(ctx, no)) + initConfig.DefaultConfig.Host = "localhost:30081" assert.Nil(t, initFlytectlConfig(ctx, no)) initConfig.DefaultConfig.Storage = true assert.NotNil(t, initFlytectlConfig(ctx, yes)) } + +func TestTrimFunc(t *testing.T) { + assert.Equal(t, trim("dns://localhost"), "localhost") + assert.Equal(t, trim("http://localhost"), "localhost") + assert.Equal(t, trim("https://localhost"), "localhost") +} + +func TestValidateEndpointName(t *testing.T) { + assert.Equal(t, validateEndpointName("127.0.0.1"), true) + assert.Equal(t, validateEndpointName("127.0.0.1.0"), false) + assert.Equal(t, validateEndpointName("localhost"), true) + assert.Equal(t, validateEndpointName("flyte.org"), true) + assert.Equal(t, validateEndpointName("flyte"), false) +}