fix(kumactl) Only warn about version compatibility where it makes sen…

…se (#2828) (#2833)

* fix(kumactl) Add method to check server version compatibility

Also remove logging with the k8s logger

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>

* fix(kumactl) Only warn about version compatibility where it makes sense

Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
(cherry picked from commit 4bf896a)

Signed-off-by: Mike <2266568+michaelbeaumont@users.noreply.github.com>

Co-authored-by: Mike <2266568+michaelbeaumont@users.noreply.github.com>

app/kumactl/cmd/apply/apply.go

@@ -57,6 +57,10 @@ Apply a resource from external URL
 $ kumactl apply -f https://example.com/resource.yaml
 `,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if err := pctx.CheckServerVersionCompatibility(); err != nil {
+				cmd.PrintErrln(err)
+			}
+
 			var b []byte
 			var err error
 

app/kumactl/cmd/delete/delete.go

@@ -27,6 +27,10 @@ func NewDeleteCmd(pctx *kumactl_cmd.RootContext) *cobra.Command {
 		Long:  `Delete Kuma resources.`,
 		Args:  cobra.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if err := pctx.CheckServerVersionCompatibility(); err != nil {
+				cmd.PrintErrln(err)
+			}
+
 			resourceTypeArg := args[0]
 			name := args[1]
 

app/kumactl/cmd/generate/generate.go

@@ -7,14 +7,23 @@ import (
 )
 
 func NewGenerateCmd(pctx *kumactl_cmd.RootContext) *cobra.Command {
-	cmd := &cobra.Command{
+	generateCmd := &cobra.Command{
 		Use:   "generate",
 		Short: "Generate resources, tokens, etc",
 		Long:  `Generate resources, tokens, etc.`,
 	}
+	generateCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
+		if err := kumactl_cmd.RunParentPreRunE(generateCmd, args); err != nil {
+			return err
+		}
+		if err := pctx.CheckServerVersionCompatibility(); err != nil {
+			cmd.PrintErrln(err)
+		}
+		return nil
+	}
 	// sub-commands
-	cmd.AddCommand(NewGenerateDataplaneTokenCmd(pctx))
-	cmd.AddCommand(NewGenerateZoneIngressTokenCmd(pctx))
-	cmd.AddCommand(NewGenerateCertificateCmd(pctx))
-	return cmd
+	generateCmd.AddCommand(NewGenerateDataplaneTokenCmd(pctx))
+	generateCmd.AddCommand(NewGenerateZoneIngressTokenCmd(pctx))
+	generateCmd.AddCommand(NewGenerateCertificateCmd(pctx))
+	return generateCmd
 }

app/kumactl/cmd/get/get.go

@@ -11,18 +11,27 @@ import (
 )
 
 func NewGetCmd(pctx *kumactl_cmd.RootContext) *cobra.Command {
-	cmd := &cobra.Command{
+	getCmd := &cobra.Command{
 		Use:   "get",
 		Short: "Show Kuma resources",
 		Long:  `Show Kuma resources.`,
 	}
+	getCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
+		if err := kumactl_cmd.RunParentPreRunE(getCmd, args); err != nil {
+			return err
+		}
+		if err := pctx.CheckServerVersionCompatibility(); err != nil {
+			cmd.PrintErrln(err)
+		}
+		return nil
+	}
 	// flags
-	cmd.PersistentFlags().StringVarP(&pctx.GetContext.Args.OutputFormat, "output", "o", string(output.TableFormat), kuma_cmd.UsageOptions("output format", output.TableFormat, output.YAMLFormat, output.JSONFormat))
+	getCmd.PersistentFlags().StringVarP(&pctx.GetContext.Args.OutputFormat, "output", "o", string(output.TableFormat), kuma_cmd.UsageOptions("output format", output.TableFormat, output.YAMLFormat, output.JSONFormat))
 	for _, cmdInst := range pctx.Runtime.Registry.ObjectDescriptors(model.HasKumactlEnabled()) {
-		cmd.AddCommand(WithPaginationArgs(NewGetResourcesCmd(pctx, cmdInst), &pctx.ListContext))
-		cmd.AddCommand(NewGetResourceCmd(pctx, cmdInst))
+		getCmd.AddCommand(WithPaginationArgs(NewGetResourcesCmd(pctx, cmdInst), &pctx.ListContext))
+		getCmd.AddCommand(NewGetResourceCmd(pctx, cmdInst))
 	}
-	return cmd
+	return getCmd
 }
 
 func WithPaginationArgs(cmd *cobra.Command, ctx *get_context.ListContext) *cobra.Command {

app/kumactl/cmd/inspect/inspect.go

@@ -9,18 +9,27 @@ import (
 )
 
 func NewInspectCmd(pctx *kumactl_cmd.RootContext) *cobra.Command {
-	cmd := &cobra.Command{
+	inspectCmd := &cobra.Command{
 		Use:   "inspect",
 		Short: "Inspect Kuma resources",
 		Long:  `Inspect Kuma resources.`,
 	}
+	inspectCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
+		if err := kumactl_cmd.RunParentPreRunE(inspectCmd, args); err != nil {
+			return err
+		}
+		if err := pctx.CheckServerVersionCompatibility(); err != nil {
+			cmd.PrintErrln(err)
+		}
+		return nil
+	}
 	// flags
-	cmd.PersistentFlags().StringVarP(&pctx.InspectContext.Args.OutputFormat, "output", "o", string(output.TableFormat), kuma_cmd.UsageOptions("output format", output.TableFormat, output.YAMLFormat, output.JSONFormat))
+	inspectCmd.PersistentFlags().StringVarP(&pctx.InspectContext.Args.OutputFormat, "output", "o", string(output.TableFormat), kuma_cmd.UsageOptions("output format", output.TableFormat, output.YAMLFormat, output.JSONFormat))
 	// sub-commands
-	cmd.AddCommand(newInspectDataplanesCmd(pctx))
-	cmd.AddCommand(newInspectZoneIngressesCmd(pctx))
-	cmd.AddCommand(newInspectZonesCmd(pctx))
-	cmd.AddCommand(newInspectMeshesCmd(pctx))
-	cmd.AddCommand(newInspectServicesCmd(pctx))
-	return cmd
+	inspectCmd.AddCommand(newInspectDataplanesCmd(pctx))
+	inspectCmd.AddCommand(newInspectZoneIngressesCmd(pctx))
+	inspectCmd.AddCommand(newInspectZonesCmd(pctx))
+	inspectCmd.AddCommand(newInspectMeshesCmd(pctx))
+	inspectCmd.AddCommand(newInspectServicesCmd(pctx))
+	return inspectCmd
 }

app/kumactl/cmd/root.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"context"
 	"os"
 
 	"github.com/spf13/cobra"
@@ -18,25 +17,18 @@ import (
 	kumactl_cmd "github.com/kumahq/kuma/app/kumactl/pkg/cmd"
 	kumactl_config "github.com/kumahq/kuma/app/kumactl/pkg/config"
 	kumactl_errors "github.com/kumahq/kuma/app/kumactl/pkg/errors"
-	"github.com/kumahq/kuma/pkg/api-server/types"
 	kuma_cmd "github.com/kumahq/kuma/pkg/cmd"
 	"github.com/kumahq/kuma/pkg/cmd/version"
 	"github.com/kumahq/kuma/pkg/core"
 	kuma_log "github.com/kumahq/kuma/pkg/log"
 
 	// Register gateway resources.
 	_ "github.com/kumahq/kuma/pkg/plugins/runtime/gateway/register"
-	kuma_version "github.com/kumahq/kuma/pkg/version"
 
 	// import Envoy protobuf definitions so (un)marshaling Envoy protobuf works
 	_ "github.com/kumahq/kuma/pkg/xds/envoy"
 )
 
-var (
-	kumactlLog       = core.Log.WithName("kumactl")
-	kumaBuildVersion *types.IndexResponse
-)
-
 // newRootCmd represents the base command when called without any subcommands.
 func NewRootCmd(root *kumactl_cmd.RootContext) *cobra.Command {
 	args := struct {
@@ -74,21 +66,6 @@ func NewRootCmd(root *kumactl_cmd.RootContext) *cobra.Command {
 				return err
 			}
 
-			client, err := root.CurrentApiClient()
-			if err != nil {
-				kumactlLog.Error(err, "Unable to get index client")
-			} else {
-				kumaBuildVersion, err = client.GetVersion(context.Background())
-				if err != nil {
-					kumactlLog.Error(err, "Unable to retrieve server version")
-				}
-			}
-
-			if kumaBuildVersion == nil {
-				cmd.PrintErr("WARNING: Unable to confirm the server supports this kumactl version\n")
-			} else if kumaBuildVersion.Version != kuma_version.Build.Version || kumaBuildVersion.Tagline != kuma_version.Product {
-				cmd.PrintErr("WARNING: You are using kumactl version " + kuma_version.Build.Version + " for " + kuma_version.Product + ", but the server returned version: " + kumaBuildVersion.Tagline + " " + kumaBuildVersion.Version + "\n")
-			}
 			return nil
 		},
 	}

app/kumactl/pkg/cmd/root_context.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"context"
 	"time"
 
 	"github.com/pkg/errors"
@@ -11,11 +12,14 @@ import (
 	"github.com/kumahq/kuma/app/kumactl/pkg/config"
 	kumactl_resources "github.com/kumahq/kuma/app/kumactl/pkg/resources"
 	"github.com/kumahq/kuma/app/kumactl/pkg/tokens"
+	"github.com/kumahq/kuma/pkg/api-server/types"
 	config_proto "github.com/kumahq/kuma/pkg/config/app/kumactl/v1alpha1"
+	"github.com/kumahq/kuma/pkg/core"
 	core_model "github.com/kumahq/kuma/pkg/core/resources/model"
 	"github.com/kumahq/kuma/pkg/core/resources/registry"
 	core_store "github.com/kumahq/kuma/pkg/core/resources/store"
 	util_files "github.com/kumahq/kuma/pkg/util/files"
+	kuma_version "github.com/kumahq/kuma/pkg/version"
 )
 
 type RootArgs struct {
@@ -207,3 +211,29 @@ func (rc *RootContext) CurrentApiClient() (kumactl_resources.ApiServerClient, er
 	}
 	return rc.Runtime.NewAPIServerClient(controlPlane.Coordinates.ApiServer)
 }
+
+func (rc *RootContext) CheckServerVersionCompatibility() error {
+	kumactlLog := core.Log.WithName("kumactl")
+
+	var kumaBuildVersion *types.IndexResponse
+
+	client, err := rc.CurrentApiClient()
+	if err != nil {
+		kumactlLog.Error(err, "Unable to get index client")
+	} else {
+		kumaBuildVersion, err = client.GetVersion(context.Background())
+		if err != nil {
+			kumactlLog.Error(err, "Unable to retrieve server version")
+		}
+	}
+
+	if kumaBuildVersion == nil {
+		return errors.New("WARNING: Unable to confirm the server supports this kumactl version")
+	}
+
+	if kumaBuildVersion.Version != kuma_version.Build.Version || kumaBuildVersion.Tagline != kuma_version.Product {
+		return errors.New("WARNING: You are using kumactl version " + kuma_version.Build.Version + " for " + kuma_version.Product + ", but the server returned version: " + kumaBuildVersion.Tagline + " " + kumaBuildVersion.Version)
+	}
+
+	return nil
+}

app/kumactl/pkg/cmd/util.go

@@ -0,0 +1,16 @@
+package cmd
+
+import "github.com/spf13/cobra"
+
+// RunParentPreRunE checks if the parent command has a PersistentPreRunE set and
+// executes it. This is for use in PersistentPreRun and is necessary because
+// only the first PersistentPreRun* of the command's ancestors is executed by
+// cobra.
+func RunParentPreRunE(cmd *cobra.Command, args []string) error {
+	if p := cmd.Parent(); p != nil && p.PersistentPreRunE != nil {
+		if err := p.PersistentPreRunE(p, args); err != nil {
+			return err
+		}
+	}
+	return nil
+}