From 2a5c4b0320f0423a1f9c26ca43c0562074102dc5 Mon Sep 17 00:00:00 2001 From: zychen5186 Date: Thu, 21 Mar 2024 22:03:37 -0700 Subject: [PATCH 1/5] Add ENV VAR FLTYE_ADMIN_ENDPOINT (#4948) Signed-off-by: zychen5186 --- cmd/core/cmd.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cmd/core/cmd.go b/cmd/core/cmd.go index 2d00adc0..10cd8255 100644 --- a/cmd/core/cmd.go +++ b/cmd/core/cmd.go @@ -3,12 +3,15 @@ package cmdcore import ( "context" "fmt" + "net/url" + "os" "github.com/pkg/errors" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "github.com/flyteorg/flyte/flyteidl/clients/go/admin" + stdConfig "github.com/flyteorg/flyte/flytestdlib/config" "github.com/flyteorg/flytectl/cmd/config" "github.com/flyteorg/flytectl/pkg/pkce" @@ -66,6 +69,18 @@ func generateCommandFunc(cmdEntry CommandEntry) func(cmd *cobra.Command, args [] } adminCfg := admin.GetConfig(ctx) + + if len(os.Getenv("FLTYE_ADMIN_ENDPOINT")) > 0 { + envEndpoint, err := url.Parse(os.Getenv("FLTYE_ADMIN_ENDPOINT")) + if err != nil { + fmt.Println("Error parsing URL:", err) + return err + } + adminCfg.Endpoint = stdConfig.URL{ + URL: *envEndpoint, + } + } + if len(adminCfg.Endpoint.String()) == 0 { return cmdEntry.CmdFunc(ctx, args, CommandContext{}) } From 61acec1f3f2b7ba001224fe72a3ecaed576b9822 Mon Sep 17 00:00:00 2001 From: zychen5186 Date: Fri, 22 Mar 2024 15:11:31 -0700 Subject: [PATCH 2/5] fmt.Errorf error message Signed-off-by: zychen5186 --- cmd/core/cmd.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/core/cmd.go b/cmd/core/cmd.go index 10cd8255..2d4187dc 100644 --- a/cmd/core/cmd.go +++ b/cmd/core/cmd.go @@ -73,8 +73,7 @@ func generateCommandFunc(cmdEntry CommandEntry) func(cmd *cobra.Command, args [] if len(os.Getenv("FLTYE_ADMIN_ENDPOINT")) > 0 { envEndpoint, err := url.Parse(os.Getenv("FLTYE_ADMIN_ENDPOINT")) if err != nil { - fmt.Println("Error parsing URL:", err) - return err + return fmt.Errorf("error parsing url: %v", err) } adminCfg.Endpoint = stdConfig.URL{ URL: *envEndpoint, From 78b795d4200dd530621e42e17ed6d6b375fbeb52 Mon Sep 17 00:00:00 2001 From: zychen5186 Date: Tue, 26 Mar 2024 20:25:48 -0700 Subject: [PATCH 3/5] functionized reading env var to config, add test Signed-off-by: zychen5186 --- cmd/config/config_test.go | 19 +++++++++++++++ cmd/config/env_var_reader.go | 45 ++++++++++++++++++++++++++++++++++++ cmd/core/cmd.go | 14 ----------- cmd/root.go | 2 ++ 4 files changed, 66 insertions(+), 14 deletions(-) create mode 100644 cmd/config/env_var_reader.go diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go index f11ee07a..fbeca871 100644 --- a/cmd/config/config_test.go +++ b/cmd/config/config_test.go @@ -1,8 +1,12 @@ package config import ( + "context" + "net/url" + "os" "testing" + "github.com/flyteorg/flyte/flyteidl/clients/go/admin" "github.com/flyteorg/flytectl/pkg/printer" "github.com/stretchr/testify/assert" ) @@ -28,5 +32,20 @@ func TestInvalidOutputFormat(t *testing.T) { } }() result = c.MustOutputFormat() +} + +func TestUpdateConfigWithEnvVar(t *testing.T) { + originalValue := os.Getenv("FLYTE_ADMIN_ENDPOINT") + defer os.Setenv("FLYTE_ADMIN_ENDPOINT", originalValue) + + dummyUrl := "dns://dummyHost" + os.Setenv("FLYTE_ADMIN_ENDPOINT", dummyUrl) + + parsedDummyUrl, _ := url.Parse(dummyUrl) + + adminCfg := admin.GetConfig(context.Background()) + assert.NotEqual(t, adminCfg.Endpoint.URL, *parsedDummyUrl) + UpdateConfigWithEnvVar() + assert.Equal(t, adminCfg.Endpoint.URL, *parsedDummyUrl) } diff --git a/cmd/config/env_var_reader.go b/cmd/config/env_var_reader.go new file mode 100644 index 00000000..51119f9b --- /dev/null +++ b/cmd/config/env_var_reader.go @@ -0,0 +1,45 @@ +package config + +import ( + "context" + "fmt" + "net/url" + "os" + + "github.com/flyteorg/flyte/flyteidl/clients/go/admin" + "github.com/flyteorg/flyte/flytestdlib/config" +) + +type FuncType func() error + +var funcMap = map[string]FuncType{} + +func init() { + funcMap["FLYTE_ADMIN_ENDPOINT"] = getAdminEndpoint + // TODO add more env vars if needed +} + +func UpdateConfigWithEnvVar() error { + for envVar, f := range funcMap { + if os.Getenv(envVar) != "" { + if err := f(); err != nil { + return fmt.Errorf("error update config with env var: %v", err) + } + } + } + return nil +} + +func getAdminEndpoint() error { + ctx := context.Background() + cfg := admin.GetConfig(ctx) + if len(os.Getenv("FLYTE_ADMIN_ENDPOINT")) > 0 { + envEndpoint, err := url.Parse(os.Getenv("FLYTE_ADMIN_ENDPOINT")) + if err != nil { + return fmt.Errorf("error parsing env var flyte_admin_endpoint: %v", err) + } + cfg.Endpoint = config.URL{URL: *envEndpoint} + admin.SetConfig(cfg) + } + return nil +} diff --git a/cmd/core/cmd.go b/cmd/core/cmd.go index 2d4187dc..2d00adc0 100644 --- a/cmd/core/cmd.go +++ b/cmd/core/cmd.go @@ -3,15 +3,12 @@ package cmdcore import ( "context" "fmt" - "net/url" - "os" "github.com/pkg/errors" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "github.com/flyteorg/flyte/flyteidl/clients/go/admin" - stdConfig "github.com/flyteorg/flyte/flytestdlib/config" "github.com/flyteorg/flytectl/cmd/config" "github.com/flyteorg/flytectl/pkg/pkce" @@ -69,17 +66,6 @@ func generateCommandFunc(cmdEntry CommandEntry) func(cmd *cobra.Command, args [] } adminCfg := admin.GetConfig(ctx) - - if len(os.Getenv("FLTYE_ADMIN_ENDPOINT")) > 0 { - envEndpoint, err := url.Parse(os.Getenv("FLTYE_ADMIN_ENDPOINT")) - if err != nil { - return fmt.Errorf("error parsing url: %v", err) - } - adminCfg.Endpoint = stdConfig.URL{ - URL: *envEndpoint, - } - } - if len(adminCfg.Endpoint.String()) == 0 { return cmdEntry.CmdFunc(ctx, args, CommandContext{}) } diff --git a/cmd/root.go b/cmd/root.go index 418406c0..1a009a8e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -134,6 +134,8 @@ func initConfig(cmd *cobra.Command, _ []string) error { return err } + config.UpdateConfigWithEnvVar() + return nil } From 514e52f9291fcb0b6bbfd7d5840da3cc53ca01fa Mon Sep 17 00:00:00 2001 From: zychen5186 Date: Wed, 27 Mar 2024 19:44:10 -0700 Subject: [PATCH 4/5] modify code stucture, fix lint Signed-off-by: zychen5186 --- cmd/config/config_test.go | 13 +++++++------ cmd/config/env_var_reader.go | 22 +++++++++++----------- cmd/root.go | 4 +++- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go index fbeca871..8fd0674f 100644 --- a/cmd/config/config_test.go +++ b/cmd/config/config_test.go @@ -38,14 +38,15 @@ func TestUpdateConfigWithEnvVar(t *testing.T) { originalValue := os.Getenv("FLYTE_ADMIN_ENDPOINT") defer os.Setenv("FLYTE_ADMIN_ENDPOINT", originalValue) - dummyUrl := "dns://dummyHost" - os.Setenv("FLYTE_ADMIN_ENDPOINT", dummyUrl) + dummyURL := "dns://dummyHost" + os.Setenv("FLYTE_ADMIN_ENDPOINT", dummyURL) - parsedDummyUrl, _ := url.Parse(dummyUrl) + parsedDummyURL, _ := url.Parse(dummyURL) adminCfg := admin.GetConfig(context.Background()) - assert.NotEqual(t, adminCfg.Endpoint.URL, *parsedDummyUrl) - UpdateConfigWithEnvVar() - assert.Equal(t, adminCfg.Endpoint.URL, *parsedDummyUrl) + assert.NotEqual(t, adminCfg.Endpoint.URL, *parsedDummyURL) + err := UpdateConfigWithEnvVar() + assert.Nil(t, err) + assert.Equal(t, adminCfg.Endpoint.URL, *parsedDummyURL) } diff --git a/cmd/config/env_var_reader.go b/cmd/config/env_var_reader.go index 51119f9b..be8c645a 100644 --- a/cmd/config/env_var_reader.go +++ b/cmd/config/env_var_reader.go @@ -10,14 +10,11 @@ import ( "github.com/flyteorg/flyte/flytestdlib/config" ) -type FuncType func() error +const flyteAdminEndpoint = "FLYTE_ADMIN_ENDPOINT" -var funcMap = map[string]FuncType{} +type FuncType func() error -func init() { - funcMap["FLYTE_ADMIN_ENDPOINT"] = getAdminEndpoint - // TODO add more env vars if needed -} +var funcMap = map[string]FuncType{flyteAdminEndpoint: updateAdminEndpoint} func UpdateConfigWithEnvVar() error { for envVar, f := range funcMap { @@ -30,16 +27,19 @@ func UpdateConfigWithEnvVar() error { return nil } -func getAdminEndpoint() error { +func updateAdminEndpoint() error { ctx := context.Background() cfg := admin.GetConfig(ctx) - if len(os.Getenv("FLYTE_ADMIN_ENDPOINT")) > 0 { - envEndpoint, err := url.Parse(os.Getenv("FLYTE_ADMIN_ENDPOINT")) + + if len(os.Getenv(flyteAdminEndpoint)) > 0 { + envEndpoint, err := url.Parse(os.Getenv(flyteAdminEndpoint)) if err != nil { - return fmt.Errorf("error parsing env var flyte_admin_endpoint: %v", err) + return fmt.Errorf("error parsing env var %v: %v", flyteAdminEndpoint, err) } cfg.Endpoint = config.URL{URL: *envEndpoint} - admin.SetConfig(cfg) + if err := admin.SetConfig(cfg); err != nil { + return err + } } return nil } diff --git a/cmd/root.go b/cmd/root.go index 1a009a8e..49ecad01 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -134,7 +134,9 @@ func initConfig(cmd *cobra.Command, _ []string) error { return err } - config.UpdateConfigWithEnvVar() + if err := config.UpdateConfigWithEnvVar(); err != nil { + return err + } return nil } From 15debc2184e98626957ba0e56731a91a47585034 Mon Sep 17 00:00:00 2001 From: zychen5186 Date: Mon, 1 Apr 2024 23:28:35 -0700 Subject: [PATCH 5/5] Modify var names Signed-off-by: zychen5186 --- cmd/config/env_var_reader.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/config/env_var_reader.go b/cmd/config/env_var_reader.go index be8c645a..6ced9035 100644 --- a/cmd/config/env_var_reader.go +++ b/cmd/config/env_var_reader.go @@ -12,14 +12,16 @@ import ( const flyteAdminEndpoint = "FLYTE_ADMIN_ENDPOINT" -type FuncType func() error +type UpdateFunc func(context.Context) error -var funcMap = map[string]FuncType{flyteAdminEndpoint: updateAdminEndpoint} +var envToUpdateFunc = map[string]UpdateFunc{flyteAdminEndpoint: updateAdminEndpoint} func UpdateConfigWithEnvVar() error { - for envVar, f := range funcMap { + ctx := context.Background() + + for envVar, updateFunc := range envToUpdateFunc { if os.Getenv(envVar) != "" { - if err := f(); err != nil { + if err := updateFunc(ctx); err != nil { return fmt.Errorf("error update config with env var: %v", err) } } @@ -27,8 +29,7 @@ func UpdateConfigWithEnvVar() error { return nil } -func updateAdminEndpoint() error { - ctx := context.Background() +func updateAdminEndpoint(ctx context.Context) error { cfg := admin.GetConfig(ctx) if len(os.Getenv(flyteAdminEndpoint)) > 0 {