Move logic out of the Configuration container

- It shouldn't be necessary to figure out the namespace if we just pass --version flag
- other small improvements
- The version will now be printed as Info at startup, to prevent confusion in pipelines.

Signed-off-by: Chris <github.account@chrigel.net>

cfg/types.go

@@ -1,10 +1,5 @@
 package cfg
 
-import (
-	"github.com/appuio/seiso/pkg/kubernetes"
-	log "github.com/sirupsen/logrus"
-)
-
 type (
 	// Configuration holds a strongly-typed tree of the configuration
 	Configuration struct {
@@ -47,13 +42,7 @@ type (
 
 // NewDefaultConfig retrieves the hardcoded configs with sane defaults
 func NewDefaultConfig() *Configuration {
-	namespace, err := kubernetes.Namespace()
-	if err != nil {
-		log.Warning("Unable to determine default namespace. Falling back to: default")
-		namespace = "default"
-	}
 	return &Configuration{
-		Namespace: namespace,
 		Git: GitConfig{
 			CommitLimit:  0,
 			RepoPath:     ".",

cmd/common.go

@@ -58,6 +58,12 @@ func toListOptions(labels []string) metav1.ListOptions {
 	}
 }
 
+func showUsageOnError(cmd *cobra.Command, err error) {
+	if err != nil {
+		cmd.Usage()
+	}
+}
+
 func missingLabelSelectorError(namespace, resource string) error {
 	return fmt.Errorf("label selector with --label expected. You can print out available labels with \"kubectl -n %s get %s --show-labels\"", namespace, resource)
 }

cmd/configmaps.go

@@ -7,6 +7,7 @@ import (
 	"github.com/appuio/seiso/pkg/kubernetes"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	"strings"
 )
 
 const (
@@ -22,24 +23,8 @@ var (
 		Aliases:      []string{"configmap", "cm"},
 		Args:         cobra.MaximumNArgs(1),
 		SilenceUsage: true,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			log.SetFormatter(&log.TextFormatter{DisableTimestamp: true})
-			if err := validateConfigMapCommandInput(); err != nil {
-				cmd.Usage()
-				return err
-			}
-
-			coreClient, err := kubernetes.NewCoreV1Client()
-			if err != nil {
-				return fmt.Errorf("cannot initiate kubernetes core client")
-			}
-
-			configMapService := configmap.NewConfigMapsService(
-				coreClient.ConfigMaps(config.Namespace),
-				kubernetes.New(),
-				configmap.ServiceConfiguration{Batch: config.Log.Batch})
-			return executeConfigMapCleanupCommand(configMapService)
-		},
+		PreRunE:      validateConfigMapCommandInput,
+		RunE:         executeConfigMapCleanupCommand,
 	}
 )
 
@@ -48,26 +33,42 @@ func init() {
 	defaults := cfg.NewDefaultConfig()
 
 	configMapCmd.PersistentFlags().BoolP("delete", "d", defaults.Delete, "Effectively delete ConfigMaps found")
-	configMapCmd.PersistentFlags().StringSliceP("label", "l", defaults.Resource.Labels, "Identify the ConfigMap by these labels")
+	configMapCmd.PersistentFlags().StringSliceP("label", "l", defaults.Resource.Labels,
+		"Identify the ConfigMap by these \"key=value\" labels")
 	configMapCmd.PersistentFlags().IntP("keep", "k", defaults.History.Keep,
 		"Keep most current <k> ConfigMaps; does not include currently used ConfigMaps (if detected)")
 	configMapCmd.PersistentFlags().String("older-than", defaults.Resource.OlderThan,
 		"Delete ConfigMaps that are older than the duration, e.g. [1y2mo3w4d5h6m7s]")
 }
 
-func validateConfigMapCommandInput() error {
+func validateConfigMapCommandInput(cmd *cobra.Command, args []string) (returnErr error) {
+	defer showUsageOnError(cmd, returnErr)
 	if len(config.Resource.Labels) == 0 {
 		return missingLabelSelectorError(config.Namespace, "configmaps")
 	}
+	for _, label := range config.Resource.Labels {
+		if !strings.Contains(label, "=") {
+			return fmt.Errorf("incorrect label format does not match expected \"key=value\" format: %s", label)
+		}
+	}
 	if _, err := parseCutOffDateTime(config.Resource.OlderThan); err != nil {
 		return fmt.Errorf("could not parse older-than flag: %w", err)
 	}
 	return nil
 }
 
-func executeConfigMapCleanupCommand(service configmap.Service) error {
+func executeConfigMapCleanupCommand(cmd *cobra.Command, args []string) error {
+	coreClient, err := kubernetes.NewCoreV1Client()
+	if err != nil {
+		return fmt.Errorf("cannot initiate kubernetes client: %w", err)
+	}
+
 	c := config.Resource
 	namespace := config.Namespace
+	service := configmap.NewConfigMapsService(
+		coreClient.ConfigMaps(namespace),
+		kubernetes.New(),
+		configmap.ServiceConfiguration{Batch: config.Log.Batch})
 
 	log.WithField("namespace", namespace).Debug("Getting ConfigMaps")
 	foundConfigMaps, err := service.List(toListOptions(c.Labels))

cmd/history.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"fmt"
-
 	"github.com/appuio/seiso/cfg"
 	"github.com/appuio/seiso/pkg/cleanup"
 	"github.com/appuio/seiso/pkg/git"
@@ -19,13 +18,8 @@ var (
 		Long:         `Clean up excessive image tags matching the commit hashes (prefix) of the git repository`,
 		Args:         cobra.MaximumNArgs(1),
 		SilenceUsage: true,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			if err := validateHistoryCommandInput(args); err != nil {
-				cmd.Usage()
-				return err
-			}
-			return ExecuteHistoryCleanupCommand(args)
-		},
+		PreRunE:      validateHistoryCommandInput,
+		RunE:         ExecuteHistoryCleanupCommand,
 	}
 )
 
@@ -39,21 +33,28 @@ func init() {
 
 }
 
-func validateHistoryCommandInput(args []string) error {
+func validateHistoryCommandInput(cmd *cobra.Command, args []string) (returnErr error) {
+	defer showUsageOnError(cmd, returnErr)
 	if len(args) == 0 {
 		return missingImageNameError(config.Namespace)
 	}
-	if _, _, err := splitNamespaceAndImagestream(args[0]); err != nil {
+	namespace, image, err := splitNamespaceAndImagestream(args[0])
+	if err != nil {
 		return fmt.Errorf("could not parse image name: %w", err)
 	}
 	if config.Git.Tag && !git.IsValidSortValue(config.Git.SortCriteria) {
 		return fmt.Errorf("invalid sort flag provided: %v", config.Git.SortCriteria)
 	}
+	log.WithFields(log.Fields{
+		"namespace": namespace,
+		"image":     image,
+	}).Debug("Using image config")
+	config.Namespace = namespace
 	return nil
 }
 
 // ExecuteHistoryCleanupCommand executes the history cleanup command
-func ExecuteHistoryCleanupCommand(args []string) error {
+func ExecuteHistoryCleanupCommand(cmd *cobra.Command, args []string) error {
 	c := config.History
 	namespace, imageName, _ := splitNamespaceAndImagestream(args[0])
 

cmd/images.go

@@ -24,10 +24,10 @@ func splitNamespaceAndImagestream(repo string) (namespace string, image string,
 	} else {
 		paths := strings.SplitAfter(repo, "/")
 		if len(paths) >= 3 {
-			namespace = paths[1]
+			namespace = strings.TrimSuffix(paths[1], "/")
 			image = paths[2]
 		} else {
-			namespace = paths[0]
+			namespace = strings.TrimSuffix(paths[0], "/")
 			image = paths[1]
 		}
 	}
@@ -37,5 +37,5 @@ func splitNamespaceAndImagestream(repo string) (namespace string, image string,
 	if image == "" {
 		return "", "", errors.New("missing or invalid image name")
 	}
-	return strings.TrimSuffix(namespace, "/"), image, nil
+	return namespace, image, nil
 }

cmd/images_test.go

@@ -0,0 +1,77 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_SplitNamespaceAndImagestream(t *testing.T) {
+	type args struct {
+		repo string
+	}
+	tests := []struct {
+		name              string
+		args              args
+		expectedNamespace string
+		expectedImage     string
+		wantErr           bool
+	}{
+		{
+			name: "ShouldSplit_NamespaceAndImageName",
+			args: args{
+				repo: "namespace/image",
+			},
+			expectedNamespace: "namespace",
+			expectedImage:     "image",
+		},
+		{
+			name: "ShouldReturnActiveNamespace_IfRepoDoesNotContainNamespace",
+			args: args{
+				repo: "image",
+			},
+			expectedNamespace: "currently-active-ns",
+			expectedImage:     "image",
+		},
+		{
+			name: "ShouldThrowError_IfRepoDoesNotContainImage",
+			args: args{
+				repo: "namespace/",
+			},
+			wantErr: true,
+		},
+		{
+			name: "ShouldThrowError_IfRepoIsInvalid",
+			args: args{
+				repo: "/",
+			},
+			wantErr: true,
+		},
+		{
+			name:    "ShouldThrowError_IfRepoIsEmpty",
+			args:    args{},
+			wantErr: true,
+		},
+		{
+			name: "ShouldIgnore_Registry",
+			args: args{
+				repo: "docker.io/namespace/image",
+			},
+			expectedNamespace: "namespace",
+			expectedImage:     "image",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			config.Namespace = "currently-active-ns"
+			namespace, image, err := splitNamespaceAndImagestream(tt.args.repo)
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+			assert.NoError(t, err)
+			assert.Equal(t, namespace, tt.expectedNamespace)
+			assert.Equal(t, image, tt.expectedImage)
+		})
+	}
+}

cmd/orphans.go

@@ -31,13 +31,8 @@ var (
 		Aliases:      []string{"orph", "orphan"},
 		Args:         cobra.MaximumNArgs(1),
 		SilenceUsage: true,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			if err := validateOrphanCommandInput(args); err != nil {
-				cmd.Usage()
-				return err
-			}
-			return ExecuteOrphanCleanupCommand(args)
-		},
+		PreRunE:      validateOrphanCommandInput,
+		RunE:         ExecuteOrphanCleanupCommand,
 	}
 )
 
@@ -52,13 +47,15 @@ func init() {
 		"Delete images that match the regex, defaults to matching Git SHA commits")
 }
 
-func validateOrphanCommandInput(args []string) error {
+func validateOrphanCommandInput(cmd *cobra.Command, args []string) (returnErr error) {
+	defer showUsageOnError(cmd, returnErr)
 	if len(args) == 0 {
 		return missingImageNameError(config.Namespace)
 	}
 	c := config.Orphan
-	if _, _, err := splitNamespaceAndImagestream(args[0]); err != nil {
-		return err
+	namespace, image, err := splitNamespaceAndImagestream(args[0])
+	if err != nil {
+		return fmt.Errorf("could not parse image name: %w", err)
 	}
 	if _, err := parseOrphanDeletionRegex(c.OrphanDeletionRegex); err != nil {
 		return fmt.Errorf("could not parse orphan deletion pattern: %w", err)
@@ -71,11 +68,16 @@ func validateOrphanCommandInput(args []string) error {
 	if config.Git.Tag && !git.IsValidSortValue(config.Git.SortCriteria) {
 		return fmt.Errorf("invalid sort flag provided: %v", config.Git.SortCriteria)
 	}
+	log.WithFields(log.Fields{
+		"namespace": namespace,
+		"image":     image,
+	}).Debug("Using image config")
+	config.Namespace = namespace
 	return nil
 }
 
 // ExecuteOrphanCleanupCommand executes the orphan cleanup command
-func ExecuteOrphanCleanupCommand(args []string) error {
+func ExecuteOrphanCleanupCommand(cmd *cobra.Command, args []string) error {
 	c := config.Orphan
 	namespace, imageName, _ := splitNamespaceAndImagestream(args[0])