From ab171cdefc9e096d4cd34e733d144d74c57b01d7 Mon Sep 17 00:00:00 2001 From: Haytham Abuelfutuh Date: Fri, 9 Jul 2021 21:05:21 -0700 Subject: [PATCH] Add pflag binding and fix loading of config (#135) --- flytectl/cmd/config/config.go | 9 +- flytectl/cmd/config/config_flags.go | 57 --------- flytectl/cmd/config/config_flags_test.go | 144 ----------------------- flytectl/cmd/root.go | 15 ++- flytectl/config.yaml | 1 + flytectl/go.mod | 2 +- flytectl/go.sum | 4 +- 7 files changed, 23 insertions(+), 209 deletions(-) delete mode 100755 flytectl/cmd/config/config_flags.go delete mode 100755 flytectl/cmd/config/config_flags_test.go diff --git a/flytectl/cmd/config/config.go b/flytectl/cmd/config/config.go index d038c316f5..c20f2bc2ef 100644 --- a/flytectl/cmd/config/config.go +++ b/flytectl/cmd/config/config.go @@ -9,11 +9,12 @@ import ( "github.com/flyteorg/flytectl/pkg/printer" ) -//go:generate pflags Config --bind-default-var - var ( - defaultConfig = &Config{} - section = config.MustRegisterSection("root", defaultConfig) + defaultConfig = &Config{ + Output: printer.OutputFormatTABLE.String(), + } + + section = config.MustRegisterSection("root", defaultConfig) ) // Config hold configration for flytectl flag diff --git a/flytectl/cmd/config/config_flags.go b/flytectl/cmd/config/config_flags.go deleted file mode 100755 index f219394ffb..0000000000 --- a/flytectl/cmd/config/config_flags.go +++ /dev/null @@ -1,57 +0,0 @@ -// Code generated by go generate; DO NOT EDIT. -// This file was generated by robots. - -package config - -import ( - "encoding/json" - "reflect" - - "fmt" - - "github.com/spf13/pflag" -) - -// If v is a pointer, it will get its element value or the zero value of the element type. -// If v is not a pointer, it will return it as is. -func (Config) elemValueOrNil(v interface{}) interface{} { - if t := reflect.TypeOf(v); t.Kind() == reflect.Ptr { - if reflect.ValueOf(v).IsNil() { - return reflect.Zero(t.Elem()).Interface() - } else { - return reflect.ValueOf(v).Interface() - } - } else if v == nil { - return reflect.Zero(t).Interface() - } - - return v -} - -func (Config) mustJsonMarshal(v interface{}) string { - raw, err := json.Marshal(v) - if err != nil { - panic(err) - } - - return string(raw) -} - -func (Config) mustMarshalJSON(v json.Marshaler) string { - raw, err := v.MarshalJSON() - if err != nil { - panic(err) - } - - return string(raw) -} - -// GetPFlagSet will return strongly types pflags for all fields in Config and its nested types. The format of the -// flags is json-name.json-sub-name... etc. -func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { - cmdFlags := pflag.NewFlagSet("Config", pflag.ExitOnError) - cmdFlags.StringVar(&defaultConfig.Project, fmt.Sprintf("%v%v", prefix, "project"), defaultConfig.Project, "Specifies the project to work on.") - cmdFlags.StringVar(&defaultConfig.Domain, fmt.Sprintf("%v%v", prefix, "domain"), defaultConfig.Domain, "Specified the domain to work on.") - cmdFlags.StringVar(&defaultConfig.Output, fmt.Sprintf("%v%v", prefix, "output"), defaultConfig.Output, "Specified the output type.") - return cmdFlags -} diff --git a/flytectl/cmd/config/config_flags_test.go b/flytectl/cmd/config/config_flags_test.go deleted file mode 100755 index 88446ff3e3..0000000000 --- a/flytectl/cmd/config/config_flags_test.go +++ /dev/null @@ -1,144 +0,0 @@ -// Code generated by go generate; DO NOT EDIT. -// This file was generated by robots. - -package config - -import ( - "encoding/json" - "fmt" - "reflect" - "strings" - "testing" - - "github.com/mitchellh/mapstructure" - "github.com/stretchr/testify/assert" -) - -var dereferencableKindsConfig = map[reflect.Kind]struct{}{ - reflect.Array: {}, reflect.Chan: {}, reflect.Map: {}, reflect.Ptr: {}, reflect.Slice: {}, -} - -// Checks if t is a kind that can be dereferenced to get its underlying type. -func canGetElementConfig(t reflect.Kind) bool { - _, exists := dereferencableKindsConfig[t] - return exists -} - -// This decoder hook tests types for json unmarshaling capability. If implemented, it uses json unmarshal to build the -// object. Otherwise, it'll just pass on the original data. -func jsonUnmarshalerHookConfig(_, to reflect.Type, data interface{}) (interface{}, error) { - unmarshalerType := reflect.TypeOf((*json.Unmarshaler)(nil)).Elem() - if to.Implements(unmarshalerType) || reflect.PtrTo(to).Implements(unmarshalerType) || - (canGetElementConfig(to.Kind()) && to.Elem().Implements(unmarshalerType)) { - - raw, err := json.Marshal(data) - if err != nil { - fmt.Printf("Failed to marshal Data: %v. Error: %v. Skipping jsonUnmarshalHook", data, err) - return data, nil - } - - res := reflect.New(to).Interface() - err = json.Unmarshal(raw, &res) - if err != nil { - fmt.Printf("Failed to umarshal Data: %v. Error: %v. Skipping jsonUnmarshalHook", data, err) - return data, nil - } - - return res, nil - } - - return data, nil -} - -func decode_Config(input, result interface{}) error { - config := &mapstructure.DecoderConfig{ - TagName: "json", - WeaklyTypedInput: true, - Result: result, - DecodeHook: mapstructure.ComposeDecodeHookFunc( - mapstructure.StringToTimeDurationHookFunc(), - mapstructure.StringToSliceHookFunc(","), - jsonUnmarshalerHookConfig, - ), - } - - decoder, err := mapstructure.NewDecoder(config) - if err != nil { - return err - } - - return decoder.Decode(input) -} - -func join_Config(arr interface{}, sep string) string { - listValue := reflect.ValueOf(arr) - strs := make([]string, 0, listValue.Len()) - for i := 0; i < listValue.Len(); i++ { - strs = append(strs, fmt.Sprintf("%v", listValue.Index(i))) - } - - return strings.Join(strs, sep) -} - -func testDecodeJson_Config(t *testing.T, val, result interface{}) { - assert.NoError(t, decode_Config(val, result)) -} - -func testDecodeRaw_Config(t *testing.T, vStringSlice, result interface{}) { - assert.NoError(t, decode_Config(vStringSlice, result)) -} - -func TestConfig_GetPFlagSet(t *testing.T) { - val := Config{} - cmdFlags := val.GetPFlagSet("") - assert.True(t, cmdFlags.HasFlags()) -} - -func TestConfig_SetFlags(t *testing.T) { - actual := Config{} - cmdFlags := actual.GetPFlagSet("") - assert.True(t, cmdFlags.HasFlags()) - - t.Run("Test_project", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("project", testValue) - if vString, err := cmdFlags.GetString("project"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vString), &actual.Project) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) - t.Run("Test_domain", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("domain", testValue) - if vString, err := cmdFlags.GetString("domain"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vString), &actual.Domain) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) - t.Run("Test_output", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("output", testValue) - if vString, err := cmdFlags.GetString("output"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vString), &actual.Output) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) -} diff --git a/flytectl/cmd/root.go b/flytectl/cmd/root.go index 70691d1751..9995b42774 100644 --- a/flytectl/cmd/root.go +++ b/flytectl/cmd/root.go @@ -47,6 +47,8 @@ func newRootCmd() *cobra.Command { rootCmd.PersistentFlags().StringVarP(&cfgFile, "config", "c", "", "config file (default is $HOME/.flyte/config.yaml)") + configAccessor.InitializePflags(rootCmd.PersistentFlags()) + // Due to https://github.com/flyteorg/flyte/issues/341, project flag will have to be specified as // --root.project, this adds a convenience on top to allow --project to be used rootCmd.PersistentFlags().StringVarP(&(config.GetConfig().Project), "project", "p", "", "Specifies the Flyte project.") @@ -69,19 +71,30 @@ func newRootCmd() *cobra.Command { return rootCmd } -func initConfig(_ *cobra.Command, _ []string) error { +func initConfig(cmd *cobra.Command, _ []string) error { configFile := f.FilePathJoin(f.UserHomeDir(), configFileDir, configFileName) if len(os.Getenv("FLYTECTL_CONFIG")) > 0 { configFile = os.Getenv("FLYTECTL_CONFIG") } + if len(cfgFile) > 0 { configFile = cfgFile } + configAccessor = viper.NewAccessor(stdConfig.Options{ StrictMode: true, SearchPaths: []string{configFile}, }) + // persistent flags were initially bound to the root command so we must bind to the same command to avoid + // overriding those initial ones. We need to traverse up to the root command and initialize pflags for that. + rootCmd := cmd + for rootCmd.Parent() != nil { + rootCmd = rootCmd.Parent() + } + + configAccessor.InitializePflags(rootCmd.PersistentFlags()) + err := configAccessor.UpdateConfig(context.TODO()) if err != nil { return err diff --git a/flytectl/config.yaml b/flytectl/config.yaml index 82b0ed8704..6447502e59 100644 --- a/flytectl/config.yaml +++ b/flytectl/config.yaml @@ -2,6 +2,7 @@ admin: # For GRPC endpoints you might want to use dns:///flyte.myexample.com endpoint: dns:///localhost:30081 insecure: true + authType: Pkce logger: show-source: true level: 0 diff --git a/flytectl/go.mod b/flytectl/go.mod index 67f26af2a7..1c518389af 100644 --- a/flytectl/go.mod +++ b/flytectl/go.mod @@ -11,7 +11,7 @@ require ( github.com/docker/go-connections v0.4.0 github.com/enescakir/emoji v1.0.0 github.com/flyteorg/flyteidl v0.19.9 - github.com/flyteorg/flytestdlib v0.3.24 + github.com/flyteorg/flytestdlib v0.3.28 github.com/ghodss/yaml v1.0.0 github.com/golang/protobuf v1.4.3 github.com/google/go-cmp v0.5.6 // indirect diff --git a/flytectl/go.sum b/flytectl/go.sum index 802b4060f5..ed5d175028 100644 --- a/flytectl/go.sum +++ b/flytectl/go.sum @@ -342,8 +342,8 @@ github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4 github.com/flyteorg/flyteidl v0.19.9 h1:1j4/YbV/G1m2hrK017F9K0JYZYxCCwf4qtEkiNnUiEw= github.com/flyteorg/flyteidl v0.19.9/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U= github.com/flyteorg/flytestdlib v0.3.13/go.mod h1:Tz8JCECAbX6VWGwFT6cmEQ+RJpZ/6L9pswu3fzWs220= -github.com/flyteorg/flytestdlib v0.3.24 h1:Eu5TMKch9ihOavPKufgTBI677eVYjJpOAPPg9hfZIzU= -github.com/flyteorg/flytestdlib v0.3.24/go.mod h1:1XG0DwYTUm34Yrffm1Qy9Tdr/pWQypEqTq5dUxw3/cM= +github.com/flyteorg/flytestdlib v0.3.28 h1:bvyldApjlUy9ETxSFpYvLhYLJxxndnMZTf93rVG6a00= +github.com/flyteorg/flytestdlib v0.3.28/go.mod h1:7cDWkY3v7xsoesFcDdu6DSW5Q2U2W5KlHUbUHSwBG1Q= github.com/form3tech-oss/jwt-go v3.2.2+incompatible h1:TcekIExNqud5crz4xD2pavyTgWiPvpYe4Xau31I0PRk= github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4=