From f832f5814d6e851573742523aae7f523daf882ec Mon Sep 17 00:00:00 2001 From: Filip Strozik Date: Wed, 6 Nov 2024 09:15:49 +0100 Subject: [PATCH] Improve extensions fetcher error handler (#2235) --- internal/cmd/alpha/alpha.go | 3 +- internal/cmdcommon/extension.go | 133 ++++++++++++++++++--------- internal/cmdcommon/extension_test.go | 40 +++++--- internal/cmdcommon/kubeconfig.go | 8 ++ internal/cmdcommon/kymaconfig.go | 24 ++--- 5 files changed, 136 insertions(+), 72 deletions(-) diff --git a/internal/cmd/alpha/alpha.go b/internal/cmd/alpha/alpha.go index 89ca4c74f..648a9aace 100644 --- a/internal/cmd/alpha/alpha.go +++ b/internal/cmd/alpha/alpha.go @@ -24,6 +24,7 @@ func NewAlphaCMD() (*cobra.Command, clierror.Error) { Long: `A set of alpha prototypes that may still change. Use in automations on your own risk.`, DisableFlagsInUseLine: true, } + kymaConfig, err := cmdcommon.NewKymaConfig(cmd) if err != nil { return nil, err @@ -38,7 +39,7 @@ func NewAlphaCMD() (*cobra.Command, clierror.Error) { cmd.AddCommand(add.NewAddCMD(kymaConfig)) cmd.AddCommand(remove.NewRemoveCMD(kymaConfig)) - cmds := cmdcommon.BuildExtensions(kymaConfig, &cmdcommon.TemplateCommandsList{ + cmds := kymaConfig.BuildExtensions(&cmdcommon.TemplateCommandsList{ // list of template commands deffinitions Explain: templates.BuildExplainCommand, }, cmdcommon.CoreCommandsMap{ diff --git a/internal/cmdcommon/extension.go b/internal/cmdcommon/extension.go index ee086f8c1..a265a0fb7 100644 --- a/internal/cmdcommon/extension.go +++ b/internal/cmdcommon/extension.go @@ -4,6 +4,9 @@ import ( "context" "errors" "fmt" + "os" + "strconv" + "strings" "github.com/kyma-project/cli.v3/internal/cmd/alpha/templates" pkgerrors "github.com/pkg/errors" @@ -12,64 +15,46 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func BuildExtensions(config *KymaConfig, availableTemplateCommands *TemplateCommandsList, availableCoreCommands CoreCommandsMap) []*cobra.Command { - cmds := make([]*cobra.Command, len(config.Extensions)) - - for i, extension := range config.Extensions { - cmds[i] = buildCommandFromExtension(config, &extension, availableTemplateCommands, availableCoreCommands) - } - - return cmds +type KymaExtensionsConfig struct { + kymaConfig *KymaConfig + extensions ExtensionList } -func buildCommandFromExtension(config *KymaConfig, extension *Extension, availableTemplateCommands *TemplateCommandsList, availableCoreCommands CoreCommandsMap) *cobra.Command { - cmd := &cobra.Command{ - Use: extension.RootCommand.Name, - Short: extension.RootCommand.Description, - Long: extension.RootCommand.DescriptionLong, - Run: func(cmd *cobra.Command, _ []string) { - if err := cmd.Help(); err != nil { - _ = err - } - }, +func newExtensionsConfig(config *KymaConfig, cmd *cobra.Command) *KymaExtensionsConfig { + extensions, err := loadExtensionsFromCluster(config.Ctx, config.KubeClientConfig) + if err != nil && shouldShowExtensionsError() { + // print error as warning if expected and continue + fmt.Printf("Extensions Warning:\n%s\n\n", err.Error()) } - if extension.TemplateCommands != nil { - addGenericCommands(cmd, extension.TemplateCommands, availableTemplateCommands) + extensionsConfig := &KymaExtensionsConfig{ + kymaConfig: config, + extensions: extensions, } + extensionsConfig.addFlag(cmd) - addCoreCommands(cmd, config, extension.CoreCommands, availableCoreCommands) - - return cmd + return extensionsConfig } -func addGenericCommands(cmd *cobra.Command, genericCommands *TemplateCommands, availableTemplateCommands *TemplateCommandsList) { - if genericCommands.ExplainCommand != nil { - cmd.AddCommand(availableTemplateCommands.Explain(&templates.ExplainOptions{ - Short: genericCommands.ExplainCommand.Description, - Long: genericCommands.ExplainCommand.DescriptionLong, - Output: genericCommands.ExplainCommand.Output, - })) - } +func (kec *KymaExtensionsConfig) addFlag(cmd *cobra.Command) { + // this flag is not operational. it's only to print help description and help cobra with validation + _ = cmd.PersistentFlags().Bool("show-extensions-error", false, "Print possible error when fetching extensions failed.") } -func addCoreCommands(cmd *cobra.Command, config *KymaConfig, extensionCoreCommands []CoreCommandInfo, availableCoreCommands CoreCommandsMap) { - for _, expectedCoreCommand := range extensionCoreCommands { - command, ok := availableCoreCommands[expectedCoreCommand.ActionID] - if !ok { - // commands doesn't exist in this version of cli and we will not process it - continue - } +func (kec *KymaExtensionsConfig) BuildExtensions(availableTemplateCommands *TemplateCommandsList, availableCoreCommands CoreCommandsMap) []*cobra.Command { + cmds := make([]*cobra.Command, len(kec.kymaConfig.extensions)) - cmd.AddCommand(command(config)) + for i, extension := range kec.kymaConfig.extensions { + cmds[i] = buildCommandFromExtension(kec.kymaConfig, &extension, availableTemplateCommands, availableCoreCommands) } + + return cmds } -func listExtensions(ctx context.Context, clientConfig *KubeClientConfig) (ExtensionList, error) { +func loadExtensionsFromCluster(ctx context.Context, clientConfig *KubeClientConfig) ([]Extension, error) { client, clientErr := clientConfig.GetKubeClient() if clientErr != nil { - // skip becuase can't connect to the cluster - return nil, nil + return nil, clientErr } labelSelector := fmt.Sprintf("%s==%s", ExtensionLabelKey, ExtensionResourceLabelValue) @@ -151,3 +136,67 @@ func parseOptionalField[T any](cmData map[string]string, cmKey string) (T, error err := yaml.Unmarshal([]byte(dataBytes), &data) return data, err } + +func buildCommandFromExtension(config *KymaConfig, extension *Extension, availableTemplateCommands *TemplateCommandsList, availableCoreCommands CoreCommandsMap) *cobra.Command { + cmd := &cobra.Command{ + Use: extension.RootCommand.Name, + Short: extension.RootCommand.Description, + Long: extension.RootCommand.DescriptionLong, + Run: func(cmd *cobra.Command, _ []string) { + if err := cmd.Help(); err != nil { + _ = err + } + }, + } + + if extension.TemplateCommands != nil { + addGenericCommands(cmd, extension.TemplateCommands, availableTemplateCommands) + } + + addCoreCommands(cmd, config, extension.CoreCommands, availableCoreCommands) + + return cmd +} + +func addGenericCommands(cmd *cobra.Command, genericCommands *TemplateCommands, availableTemplateCommands *TemplateCommandsList) { + if genericCommands.ExplainCommand != nil { + cmd.AddCommand(availableTemplateCommands.Explain(&templates.ExplainOptions{ + Short: genericCommands.ExplainCommand.Description, + Long: genericCommands.ExplainCommand.DescriptionLong, + Output: genericCommands.ExplainCommand.Output, + })) + } +} + +func addCoreCommands(cmd *cobra.Command, config *KymaConfig, extensionCoreCommands []CoreCommandInfo, availableCoreCommands CoreCommandsMap) { + for _, expectedCoreCommand := range extensionCoreCommands { + command, ok := availableCoreCommands[expectedCoreCommand.ActionID] + if !ok { + // commands doesn't exist in this version of cli and we will not process it + continue + } + + cmd.AddCommand(command(config)) + } +} + +// search os.Args manually to find if user pass --show-extensions-error and return its value +func shouldShowExtensionsError() bool { + for i, arg := range os.Args { + //example: --show-extensions-error true + if arg == "--show-extensions-error" && len(os.Args) > i+1 { + + value, err := strconv.ParseBool(os.Args[i+1]) + if err == nil { + return value + } + } + + // example: --show-extensions-error or --show-extensions-error=true + if strings.HasPrefix(arg, "--show-extensions-error") && !strings.Contains(arg, "false") { + return true + } + } + + return false +} diff --git a/internal/cmdcommon/extension_test.go b/internal/cmdcommon/extension_test.go index ce9fee587..c94584a99 100644 --- a/internal/cmdcommon/extension_test.go +++ b/internal/cmdcommon/extension_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/kyma-project/cli.v3/internal/kube/fake" + "github.com/spf13/cobra" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -14,7 +15,7 @@ import ( func TestListFromCluster(t *testing.T) { t.Run("list extensions from cluster", func(t *testing.T) { - client := &KubeClientConfig{ + kubeClientConfig := &KubeClientConfig{ KubeClient: &fake.FakeKubeClient{ TestKubernetesInterface: k8s_fake.NewSimpleClientset( fixTestExtensionConfigmap("test-1"), @@ -24,19 +25,26 @@ func TestListFromCluster(t *testing.T) { }, } + cmd := &cobra.Command{} + want := ExtensionList{ fixTestExtension("test-1"), fixTestExtension("test-2"), fixTestExtension("test-3"), } - got, err := listExtensions(context.Background(), client) - require.NoError(t, err) - require.Equal(t, want, got) + kymaConfig := &KymaConfig{ + Ctx: context.Background(), + KubeClientConfig: kubeClientConfig, + } + + got := newExtensionsConfig(kymaConfig, cmd) + require.Equal(t, want, got.extensions) + require.True(t, cmd.PersistentFlags().HasFlags()) }) t.Run("missing rootCommand error", func(t *testing.T) { - client := &KubeClientConfig{ + kubeClientConfig := &KubeClientConfig{ KubeClient: &fake.FakeKubeClient{ TestKubernetesInterface: k8s_fake.NewSimpleClientset( &corev1.ConfigMap{ @@ -52,13 +60,17 @@ func TestListFromCluster(t *testing.T) { }, } - got, err := listExtensions(context.Background(), client) - require.ErrorContains(t, err, "failed to parse configmap '/bad-data': missing .data.rootCommand field") - require.Nil(t, got) + kymaConfig := &KymaConfig{ + Ctx: context.Background(), + KubeClientConfig: kubeClientConfig, + } + + got := newExtensionsConfig(kymaConfig, &cobra.Command{}) + require.Empty(t, got.extensions) }) t.Run("skip optional fields", func(t *testing.T) { - client := &KubeClientConfig{ + kubeClientConfig := &KubeClientConfig{ KubeClient: &fake.FakeKubeClient{ TestKubernetesInterface: k8s_fake.NewSimpleClientset( &corev1.ConfigMap{ @@ -90,9 +102,13 @@ descriptionLong: test-description-long }, } - got, err := listExtensions(context.Background(), client) - require.NoError(t, err) - require.Equal(t, want, got) + kymaConfig := &KymaConfig{ + Ctx: context.Background(), + KubeClientConfig: kubeClientConfig, + } + + got := newExtensionsConfig(kymaConfig, &cobra.Command{}) + require.Equal(t, want, got.extensions) }) } diff --git a/internal/cmdcommon/kubeconfig.go b/internal/cmdcommon/kubeconfig.go index 142e9476b..e44bf32c5 100644 --- a/internal/cmdcommon/kubeconfig.go +++ b/internal/cmdcommon/kubeconfig.go @@ -2,6 +2,7 @@ package cmdcommon import ( "os" + "strings" "github.com/kyma-project/cli.v3/internal/clierror" "github.com/kyma-project/cli.v3/internal/kube" @@ -51,9 +52,16 @@ func (kcc *KubeClientConfig) complete() { func getKubeconfigPath() string { path := "" for i, arg := range os.Args { + // example: --kubeconfig /path/to/file if arg == "--kubeconfig" && len(os.Args) > i+1 { path = os.Args[i+1] } + + // example: --kubeconfig=/path/to/file + argFields := strings.Split(arg, "=") + if strings.HasPrefix(arg, "--kubeconfig=") && len(argFields) == 2 { + path = argFields[1] + } } return path diff --git a/internal/cmdcommon/kymaconfig.go b/internal/cmdcommon/kymaconfig.go index 5c5314e88..1a6586381 100644 --- a/internal/cmdcommon/kymaconfig.go +++ b/internal/cmdcommon/kymaconfig.go @@ -2,7 +2,6 @@ package cmdcommon import ( "context" - "fmt" "github.com/kyma-project/cli.v3/internal/clierror" "github.com/spf13/cobra" @@ -11,27 +10,18 @@ import ( // KymaConfig contains data common for all subcommands type KymaConfig struct { *KubeClientConfig + *KymaExtensionsConfig - Ctx context.Context - Extensions ExtensionList + Ctx context.Context } func NewKymaConfig(cmd *cobra.Command) (*KymaConfig, clierror.Error) { ctx := context.Background() - kubeClient := newKubeClientConfig(cmd) + kymaConfig := &KymaConfig{} + kymaConfig.Ctx = ctx + kymaConfig.KubeClientConfig = newKubeClientConfig(cmd) + kymaConfig.KymaExtensionsConfig = newExtensionsConfig(kymaConfig, cmd) - extensions, err := listExtensions(ctx, kubeClient) - if err != nil { - fmt.Printf("DEBUG ERROR: %s\n", err.Error()) - // TODO: think about handling error later - // this error should not stop program - // but I'm not sure what we should do with such information due to it's internal value - } - - return &KymaConfig{ - Ctx: ctx, - KubeClientConfig: kubeClient, - Extensions: extensions, - }, nil + return kymaConfig, nil }