flyetcl version should ignore talking to admin if host is not configu…

…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]>
austin362667 · May 7, 2024 · d84c92b · d84c92b
1 parent ae51a88
commit d84c92b
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 10 deletions.
diff --git a/flytectl/cmd/core/cmd.go b/flytectl/cmd/core/cmd.go
@@ -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),

diff --git a/flytectl/cmd/core/cmd_test.go b/flytectl/cmd/core/cmd_test.go
@@ -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{}))
+	})
 }
diff --git a/flytectl/cmd/version/version.go b/flytectl/cmd/version/version.go
@@ -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)
@@ -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)

diff --git a/flytectl/cmd/version/version_test.go b/flytectl/cmd/version/version_test.go
@@ -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)
+	})
 }
diff --git a/flytectl/pkg/github/githubutil.go b/flytectl/pkg/github/githubutil.go
@@ -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

diff --git a/flytectl/pkg/github/githubutil_test.go b/flytectl/pkg/github/githubutil_test.go
@@ -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)