Skip to content

Commit

Permalink
flyetcl version should ignore talking to admin if host is not configu…
Browse files Browse the repository at this point in the history
…res (flyteorg#318)

* flyetcl version should ignore talking to admin if host is not configures

Signed-off-by: Kevin Su <[email protected]>

* update test

Signed-off-by: Kevin Su <[email protected]>

* fix test

Signed-off-by: Kevin Su <[email protected]>

* fix test

Signed-off-by: Kevin Su <[email protected]>

* update

Signed-off-by: Kevin Su <[email protected]>

* More tests

Signed-off-by: Kevin Su <[email protected]>
  • Loading branch information
pingsutw authored and robert-ulbrich-mercedes-benz committed Jul 2, 2024
1 parent 296e424 commit 984c95b
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 10 deletions.
4 changes: 4 additions & 0 deletions flytectl/cmd/core/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ func generateCommandFunc(cmdEntry CommandEntry) func(cmd *cobra.Command, args []
}

adminCfg := admin.GetConfig(ctx)
if len(adminCfg.Endpoint.String()) == 0 {
return cmdEntry.CmdFunc(ctx, args, CommandContext{})
}

clientSet, err := admin.ClientSetBuilder().WithConfig(admin.GetConfig(ctx)).
WithTokenCache(pkce.TokenCacheKeyringProvider{
ServiceUser: fmt.Sprintf("%s:%s", adminCfg.Endpoint.String(), pkce.KeyRingServiceUser),
Expand Down
25 changes: 18 additions & 7 deletions flytectl/cmd/core/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,22 @@ func testCommandFunc(ctx context.Context, args []string, cmdCtx CommandContext)
}

func TestGenerateCommandFunc(t *testing.T) {
adminCfg := admin.GetConfig(context.Background())
adminCfg.Endpoint = config.URL{URL: url.URL{Host: "dummyHost"}}
adminCfg.AuthType = admin.AuthTypePkce
rootCmd := &cobra.Command{}
cmdEntry := CommandEntry{CmdFunc: testCommandFunc, ProjectDomainNotRequired: true}
fn := generateCommandFunc(cmdEntry)
assert.Nil(t, fn(rootCmd, []string{}))
t.Run("dummy host name", func(t *testing.T) {
adminCfg := admin.GetConfig(context.Background())
adminCfg.Endpoint = config.URL{URL: url.URL{Host: "dummyHost"}}
adminCfg.AuthType = admin.AuthTypePkce
rootCmd := &cobra.Command{}
cmdEntry := CommandEntry{CmdFunc: testCommandFunc, ProjectDomainNotRequired: true}
fn := generateCommandFunc(cmdEntry)
assert.Nil(t, fn(rootCmd, []string{}))
})

t.Run("host is not configured", func(t *testing.T) {
adminCfg := admin.GetConfig(context.Background())
adminCfg.Endpoint = config.URL{URL: url.URL{Host: ""}}
rootCmd := &cobra.Command{}
cmdEntry := CommandEntry{CmdFunc: testCommandFunc, ProjectDomainNotRequired: true}
fn := generateCommandFunc(cmdEntry)
assert.Nil(t, fn(rootCmd, []string{}))
})
}
6 changes: 6 additions & 0 deletions flytectl/cmd/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func getVersion(ctx context.Context, args []string, cmdCtx cmdCore.CommandContex
}); err != nil {
return err
}

// Print Flyteadmin version if available
if err := getControlPlaneVersion(ctx, cmdCtx); err != nil {
logger.Debug(ctx, err)
Expand All @@ -92,6 +93,11 @@ func printVersion(response versionOutput) error {
}

func getControlPlaneVersion(ctx context.Context, cmdCtx cmdCore.CommandContext) error {
if cmdCtx.ClientSet() == nil {
logger.Debug(ctx, "Ignore talking to admin if host is not configured")
return nil
}

v, err := cmdCtx.AdminClient().GetVersion(ctx, &admin.GetVersionRequest{})
if err != nil || v == nil {
logger.Debugf(ctx, "Failed to get version of control plane %v: \n", err)
Expand Down
7 changes: 6 additions & 1 deletion flytectl/cmd/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,10 @@ func TestVersionUtilFunc(t *testing.T) {
err := getVersion(ctx, []string{}, cmdCtx)
assert.Nil(t, err)
})

t.Run("ClientSet is empty", func(t *testing.T) {
ctx := context.Background()
cmdCtx := cmdCore.CommandContext{}
err := getVersion(ctx, []string{}, cmdCtx)
assert.Nil(t, err)
})
}
4 changes: 3 additions & 1 deletion flytectl/pkg/github/githubutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,10 @@ func GetUpgradeMessage(latest string, goos platformutil.Platform) (string, error
if err != nil {
return "", err
}
message := fmt.Sprintf(commonMessage, stdlibversion.Version, latest)

var message string
if isGreater {
message = fmt.Sprintf(commonMessage, stdlibversion.Version, latest)
symlink, err := CheckBrewInstall(goos)
if err != nil {
return "", err
Expand Down
2 changes: 1 addition & 1 deletion flytectl/pkg/github/githubutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func TestGetUpgradeMessage(t *testing.T) {
version = "v0.2.09"
message, err = GetUpgradeMessage(version, darwin)
assert.Nil(t, err)
assert.Equal(t, 63, len(message))
assert.Equal(t, 0, len(message))

version = "v"
message, err = GetUpgradeMessage(version, darwin)
Expand Down

0 comments on commit 984c95b

Please sign in to comment.