Handle extensions duplicates with base commands (#2414)

Author: FilipRudy

gen-docs/main.go

@@ -12,13 +12,11 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
-	"github.com/kyma-project/cli.v3/internal/clierror"
 	"github.com/kyma-project/cli.v3/internal/cmd"
 )
 
 func main() {
-	command, clierr := cmd.NewKymaCMD()
-	clierror.Check(clierr)
+	command := cmd.NewKymaCMD()
 
 	docsTargetDir := "./docs/user/gen-docs"
 

internal/cmd/alpha/alpha.go

@@ -1,7 +1,6 @@
 package alpha
 
 import (
-	"github.com/kyma-project/cli.v3/internal/clierror"
 	"github.com/kyma-project/cli.v3/internal/cmd/alpha/app"
 	"github.com/kyma-project/cli.v3/internal/cmd/alpha/hana"
 	"github.com/kyma-project/cli.v3/internal/cmd/alpha/kubeconfig"
@@ -15,18 +14,15 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func NewAlphaCMD() (*cobra.Command, clierror.Error) {
+func NewAlphaCMD() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:                   "alpha <command> [flags]",
 		Short:                 "Groups command prototypes for which the API may still change",
 		Long:                  `A set of alpha prototypes that may still change. Use them in automation at your own risk.`,
 		DisableFlagsInUseLine: true,
 	}
 
-	kymaConfig, err := cmdcommon.NewKymaConfig(cmd)
-	if err != nil {
-		return nil, err
-	}
+	kymaConfig := cmdcommon.NewKymaConfig(cmd)
 
 	cmd.AddCommand(app.NewAppCMD(kymaConfig))
 	cmd.AddCommand(hana.NewHanaCMD(kymaConfig))
@@ -45,8 +41,11 @@ func NewAlphaCMD() (*cobra.Command, clierror.Error) {
 		// map of available core commands
 		"registry_config":       config.NewConfigCMD,
 		"registry_image-import": imageimport.NewImportCMD,
-	})
+	}, cmd, kymaConfig)
+
+	kymaConfig.DisplayExtensionsErrors(cmd.ErrOrStderr())
+
 	cmd.AddCommand(cmds...)
 
-	return cmd, nil
+	return cmd
 }

internal/cmd/kyma.go

@@ -1,14 +1,13 @@
 package cmd
 
 import (
-	"github.com/kyma-project/cli.v3/internal/clierror"
 	"github.com/kyma-project/cli.v3/internal/cmd/alpha"
 	"github.com/kyma-project/cli.v3/internal/cmd/version"
 	"github.com/kyma-project/cli.v3/internal/cmdcommon"
 	"github.com/spf13/cobra"
 )
 
-func NewKymaCMD() (*cobra.Command, clierror.Error) {
+func NewKymaCMD() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "kyma <command> [flags]",
 		Short: "A simple set of commands to manage a Kyma cluster",
@@ -21,13 +20,10 @@ func NewKymaCMD() (*cobra.Command, clierror.Error) {
 	cmdcommon.AddExtensionsFlags(cmd)
 	cmd.PersistentFlags().BoolP("help", "h", false, "Help for the command")
 
-	alpha, err := alpha.NewAlphaCMD()
-	if err != nil {
-		return nil, err
-	}
+	alpha := alpha.NewAlphaCMD()
 
 	cmd.AddCommand(alpha)
 	cmd.AddCommand(version.NewCmd())
 
-	return cmd, nil
+	return cmd
 }

internal/cmdcommon/extension.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	v1 "k8s.io/api/core/v1"
 	"os"
 	"slices"
 	"strconv"
@@ -19,11 +20,12 @@ import (
 )
 
 type KymaExtensionsConfig struct {
-	kymaConfig *KymaConfig
-	extensions ExtensionList
+	kymaConfig  *KymaConfig
+	extensions  ExtensionList
+	parseErrors error
 }
 
-func newExtensionsConfig(warningWriter io.Writer, config *KymaConfig) *KymaExtensionsConfig {
+func newExtensionsConfig(config *KymaConfig) *KymaExtensionsConfig {
 	extensionsConfig := &KymaExtensionsConfig{
 		kymaConfig: config,
 	}
@@ -35,15 +37,8 @@ func newExtensionsConfig(warningWriter io.Writer, config *KymaConfig) *KymaExten
 		}
 	}
 
-	extensions, err := loadExtensionsFromCluster(config.Ctx, config.KubeClientConfig)
-	if err != nil && getBoolFlagValue("--show-extensions-error") {
-		// print error as warning if expected and continue
-		fmt.Fprintf(warningWriter, "Extensions Warning:\n%s\n\n", err.Error())
-	} else if err != nil {
-		fmt.Fprintf(warningWriter, "Extensions Warning:\nfailed to fetch all extensions from the cluster. Use the '--show-extensions-error' flag to see more details.\n\n")
-	}
+	extensionsConfig.extensions, extensionsConfig.parseErrors = loadExtensionsFromCluster(config.Ctx, config.KubeClientConfig)
 
-	extensionsConfig.extensions = extensions
 	return extensionsConfig
 }
 
@@ -57,17 +52,47 @@ func (kec *KymaExtensionsConfig) GetRawExtensions() ExtensionList {
 	return kec.extensions
 }
 
-func (kec *KymaExtensionsConfig) BuildExtensions(availableTemplateCommands *TemplateCommandsList, availableCoreCommands CoreCommandsMap) []*cobra.Command {
-	cmds := make([]*cobra.Command, len(kec.kymaConfig.extensions))
+func (kec *KymaExtensionsConfig) BuildExtensions(availableTemplateCommands *TemplateCommandsList, availableCoreCommands CoreCommandsMap, cmd *cobra.Command, config *KymaConfig) []*cobra.Command {
+	var cmds []*cobra.Command
+
+	var cms, cmsError = getExtensionConfigMaps(config.Ctx, config.KubeClientConfig)
+	if cmsError != nil {
+		kec.parseErrors = cmsError
+		return nil
+	}
+
+	existingCommands := make(map[string]bool)
+	for _, baseCmd := range cmd.Commands() {
+		existingCommands[baseCmd.Name()] = true
+	}
+
+	for _, cm := range cms.Items {
+		extension, _ := parseResourceExtension(cm.Data)
+		if existingCommands[extension.RootCommand.Name] {
+			kec.parseErrors = errors.Join(
+				kec.parseErrors,
+				fmt.Errorf("failed to validate configmap '%s/%s': base command with name='%s' already exists",
+					cm.GetNamespace(), cm.GetName(), extension.RootCommand.Name),
+			)
+			continue
+		}
 
-	for i, extension := range kec.kymaConfig.extensions {
-		cmds[i] = buildCommandFromExtension(kec.kymaConfig, &extension, availableTemplateCommands, availableCoreCommands)
+		cmds = append(cmds, buildCommandFromExtension(kec.kymaConfig, extension, availableTemplateCommands, availableCoreCommands))
 	}
 
 	return cmds
 }
 
-func loadExtensionsFromCluster(ctx context.Context, clientConfig *KubeClientConfig) ([]Extension, error) {
+func (kec *KymaExtensionsConfig) DisplayExtensionsErrors(warningWriter io.Writer) {
+	if kec.parseErrors != nil && getBoolFlagValue("--show-extensions-error") {
+		// print error as warning if expected and continue
+		fmt.Fprintf(warningWriter, "Extensions Warning:\n%s\n\n", kec.parseErrors.Error())
+	} else if kec.parseErrors != nil {
+		fmt.Fprintf(warningWriter, "Extensions Warning:\nfailed to fetch all extensions from the cluster. Use the '--show-extensions-error' flag to see more details.\n\n")
+	}
+}
+
+func getExtensionConfigMaps(ctx context.Context, clientConfig *KubeClientConfig) (*v1.ConfigMapList, error) {
 	client, clientErr := clientConfig.GetKubeClient()
 	if clientErr != nil {
 		return nil, clientErr
@@ -81,15 +106,24 @@ func loadExtensionsFromCluster(ctx context.Context, clientConfig *KubeClientConf
 		return nil, pkgerrors.Wrapf(err, "failed to load ConfigMaps from cluster with label %s", labelSelector)
 	}
 
+	return cms, nil
+}
+
+func loadExtensionsFromCluster(ctx context.Context, clientConfig *KubeClientConfig) ([]Extension, error) {
+	var cms, cmsError = getExtensionConfigMaps(ctx, clientConfig)
+	if cmsError != nil {
+		return nil, cmsError
+	}
+
 	var extensions []Extension
-	var parseErr error
+	var parseErrors error
 	for _, cm := range cms.Items {
 		extension, err := parseResourceExtension(cm.Data)
 		if err != nil {
 			// if the parse failed add an error to the errors list to take another extension
 			// corrupted extension should not stop parsing the rest of the extensions
-			parseErr = errors.Join(
-				parseErr,
+			parseErrors = errors.Join(
+				parseErrors,
 				pkgerrors.Wrapf(err, "failed to parse configmap '%s/%s'", cm.GetNamespace(), cm.GetName()),
 			)
 			continue
@@ -98,8 +132,8 @@ func loadExtensionsFromCluster(ctx context.Context, clientConfig *KubeClientConf
 		if slices.ContainsFunc(extensions, func(e Extension) bool {
 			return e.RootCommand.Name == extension.RootCommand.Name
 		}) {
-			parseErr = errors.Join(
-				parseErr,
+			parseErrors = errors.Join(
+				parseErrors,
 				fmt.Errorf("failed to validate configmap '%s/%s': extension with rootCommand.name='%s' already exists",
 					cm.GetNamespace(), cm.GetName(), extension.RootCommand.Name),
 			)
@@ -109,7 +143,7 @@ func loadExtensionsFromCluster(ctx context.Context, clientConfig *KubeClientConf
 		extensions = append(extensions, *extension)
 	}
 
-	return extensions, parseErr
+	return extensions, parseErrors
 }
 
 func parseResourceExtension(cmData map[string]string) (*Extension, error) {

internal/cmdcommon/extension_test.go

@@ -3,6 +3,7 @@ package cmdcommon
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"testing"
@@ -17,7 +18,6 @@ import (
 
 func TestListFromCluster(t *testing.T) {
 	t.Run("list extensions from cluster", func(t *testing.T) {
-		warnBuf := bytes.NewBuffer([]byte{})
 		kubeClientConfig := &KubeClientConfig{
 			KubeClient: &fake.KubeClient{
 				TestKubernetesInterface: k8s_fake.NewSimpleClientset(
@@ -39,17 +39,12 @@ func TestListFromCluster(t *testing.T) {
 			KubeClientConfig: kubeClientConfig,
 		}
 
-		got := newExtensionsConfig(warnBuf, kymaConfig)
+		got := newExtensionsConfig(kymaConfig)
 		require.Equal(t, want, got.extensions)
-		require.Empty(t, warnBuf.Bytes())
+		require.Empty(t, got.parseErrors)
 	})
 
 	t.Run("extensions duplications warning", func(t *testing.T) {
-		oldArgs := os.Args
-		os.Args = append(os.Args, "--show-extensions-error")
-		defer func() { os.Args = oldArgs }()
-
-		warnBuf := bytes.NewBuffer([]byte{})
 		kubeClientConfig := &KubeClientConfig{
 			KubeClient: &fake.KubeClient{
 				TestKubernetesInterface: k8s_fake.NewSimpleClientset(
@@ -69,17 +64,17 @@ func TestListFromCluster(t *testing.T) {
 			KubeClientConfig: kubeClientConfig,
 		}
 
-		wantWarning := "Extensions Warning:\n" +
+		wantWarning :=
 			"failed to validate configmap '/test-2': extension with rootCommand.name='test-1' already exists\n" +
-			"failed to validate configmap '/test-3': extension with rootCommand.name='test-1' already exists\n\n"
+				"failed to validate configmap '/test-3': extension with rootCommand.name='test-1' already exists"
+
+		got := newExtensionsConfig(kymaConfig)
 
-		got := newExtensionsConfig(warnBuf, kymaConfig)
 		require.Equal(t, want, got.extensions)
-		require.Equal(t, wantWarning, warnBuf.String())
+		require.Equal(t, wantWarning, got.parseErrors.Error())
 	})
 
 	t.Run("missing rootCommand error", func(t *testing.T) {
-		warnBuf := bytes.NewBuffer([]byte{})
 		kubeClientConfig := &KubeClientConfig{
 			KubeClient: &fake.KubeClient{
 				TestKubernetesInterface: k8s_fake.NewSimpleClientset(
@@ -96,21 +91,20 @@ func TestListFromCluster(t *testing.T) {
 			},
 		}
 
-		wantWarning := "Extensions Warning:\n" +
-			"failed to fetch all extensions from the cluster. Use the '--show-extensions-error' flag to see more details.\n\n"
+		wantWarning :=
+			"failed to parse configmap '/bad-data': missing .data.rootCommand field"
 
 		kymaConfig := &KymaConfig{
 			Ctx:              context.Background(),
 			KubeClientConfig: kubeClientConfig,
 		}
 
-		got := newExtensionsConfig(warnBuf, kymaConfig)
-		require.Equal(t, wantWarning, warnBuf.String())
+		got := newExtensionsConfig(kymaConfig)
+		require.Equal(t, wantWarning, got.parseErrors.Error())
 		require.Empty(t, got.extensions)
 	})
 
 	t.Run("skip optional fields", func(t *testing.T) {
-		warnBuf := bytes.NewBuffer([]byte{})
 		kubeClientConfig := &KubeClientConfig{
 			KubeClient: &fake.KubeClient{
 				TestKubernetesInterface: k8s_fake.NewSimpleClientset(
@@ -148,12 +142,50 @@ descriptionLong: test-description-long
 			KubeClientConfig: kubeClientConfig,
 		}
 
-		got := newExtensionsConfig(warnBuf, kymaConfig)
-		require.Empty(t, warnBuf.Bytes())
+		got := newExtensionsConfig(kymaConfig)
+		require.Empty(t, got.parseErrors)
 		require.Equal(t, want, got.extensions)
 	})
 }
 
+func TestDisplayExtensionsErrors(t *testing.T) {
+	t.Run("extensions warning message display without '--show-extensions-error' flag", func(t *testing.T) {
+		warnBuf := bytes.NewBuffer([]byte{})
+
+		wantWarning :=
+			"Extensions Warning:\nfailed to fetch all extensions from the cluster. Use the '--show-extensions-error' flag to see more details.\n\n"
+
+		kymaExtensionsConfig := &KymaExtensionsConfig{
+			kymaConfig:  &KymaConfig{Ctx: nil, KubeClientConfig: nil},
+			extensions:  ExtensionList{},
+			parseErrors: errors.New("failed to parse configmap '/bad-data': missing .data.rootCommand field"),
+		}
+
+		kymaExtensionsConfig.DisplayExtensionsErrors(warnBuf)
+		require.Equal(t, wantWarning, warnBuf.String())
+	})
+
+	t.Run("extensions warning message display with '--show-extensions-error' flag", func(t *testing.T) {
+		oldArgs := os.Args
+		os.Args = append(os.Args, "--show-extensions-error")
+		defer func() { os.Args = oldArgs }()
+
+		warnBuf := bytes.NewBuffer([]byte{})
+
+		wantWarning :=
+			"Extensions Warning:\nfailed to parse configmap '/bad-data': missing .data.rootCommand field\n\n"
+
+		kymaExtensionsConfig := &KymaExtensionsConfig{
+			kymaConfig:  &KymaConfig{Ctx: nil, KubeClientConfig: nil},
+			extensions:  ExtensionList{},
+			parseErrors: errors.New("failed to parse configmap '/bad-data': missing .data.rootCommand field"),
+		}
+
+		kymaExtensionsConfig.DisplayExtensionsErrors(warnBuf)
+		require.Equal(t, wantWarning, warnBuf.String())
+	})
+}
+
 func fixTestExtensionConfigmap(name string) *corev1.ConfigMap {
 	return fixTestExtensionConfigmapWithCMName(name, name)
 }

internal/cmdcommon/kymaconfig.go

@@ -3,7 +3,6 @@ package cmdcommon
 import (
 	"context"
 
-	"github.com/kyma-project/cli.v3/internal/clierror"
 	"github.com/spf13/cobra"
 )
 
@@ -15,13 +14,13 @@ type KymaConfig struct {
 	Ctx context.Context
 }
 
-func NewKymaConfig(cmd *cobra.Command) (*KymaConfig, clierror.Error) {
+func NewKymaConfig(cmd *cobra.Command) *KymaConfig {
 	ctx := context.Background()
 
 	kymaConfig := &KymaConfig{}
 	kymaConfig.Ctx = ctx
 	kymaConfig.KubeClientConfig = newKubeClientConfig(cmd)
-	kymaConfig.KymaExtensionsConfig = newExtensionsConfig(cmd.OutOrStderr(), kymaConfig)
+	kymaConfig.KymaExtensionsConfig = newExtensionsConfig(kymaConfig)
 
-	return kymaConfig, nil
+	return kymaConfig
 }