Merge pull request #330 from paivagustavo/dont-override-aliases

don't override aliases

internal/config/alias.go

@@ -79,13 +79,13 @@ func (a Aliases) Get(k string) (string, bool) {
 }
 
 // Define declares a new alias.
-func (a Aliases) Define(args ...string) {
-	if len(args)%2 != 0 {
-		panic("Invalid alias definition. You must specify pairs")
-	}
-	for i := 0; i < len(args); i += 2 {
-		a.Alias[args[i]] = args[i+1]
+func (a Aliases) Define(command, alias string) {
+	if _, ok := a.Alias[alias]; ok {
+		// Don't override aliases. Take order of alias registration as precedence.
+		return
 	}
+
+	a.Alias[alias] = command
 }
 
 // LoadAliases loads alias from a given file.

internal/config/alias_test.go

@@ -8,22 +8,61 @@ import (
 )
 
 func TestAliasDefine(t *testing.T) {
-	uu := map[string]struct {
-		aa []string
-	}{
-		"one":   {[]string{"blee", "duh"}},
-		"multi": {[]string{"blee", "duh", "fred", "zorg"}},
+	type aliasDef struct {
+		cmd     string
+		aliases []string
 	}
 
-	for k, u := range uu {
-		t.Run(k, func(t *testing.T) {
-			a := config.NewAliases()
-			a.Define(u.aa...)
-			for i := 0; i < len(u.aa); i += 2 {
-				v, ok := a.Get(u.aa[i])
+	tts := []struct {
+		name               string
+		aliases            []aliasDef
+		registeredCommands map[string]string
+	}{
+		{
+			name: "simple aliases",
+			aliases: []aliasDef{
+				{
+					cmd:     "one",
+					aliases: []string{"blee", "duh"},
+				},
+			},
+			registeredCommands: map[string]string{
+				"blee": "one",
+				"duh":  "one",
+			},
+		},
+		{
+			name: "duplicated aliases",
+			aliases: []aliasDef{
+				{
+					cmd:     "one",
+					aliases: []string{"blee", "duh"},
+				}, {
+					cmd:     "two",
+					aliases: []string{"blee", "duh", "fred", "zorg"},
+				},
+			},
+			registeredCommands: map[string]string{
+				"blee": "one",
+				"duh":  "one",
+				"fred": "two",
+				"zorg": "two",
+			},
+		},
+	}
 
+	for _, tt := range tts {
+		t.Run(tt.name, func(t *testing.T) {
+			configAlias := config.NewAliases()
+			for _, aliases := range tt.aliases {
+				for _, a := range aliases.aliases {
+					configAlias.Define(aliases.cmd, a)
+				}
+			}
+			for alias, cmd := range tt.registeredCommands {
+				v, ok := configAlias.Get(alias)
 				assert.True(t, ok)
-				assert.Equal(t, u.aa[i+1], v)
+				assert.Equal(t, cmd, v, "Wrong command for alias "+alias)
 			}
 		})
 	}

internal/resource/base.go

@@ -3,6 +3,7 @@ package resource
 import (
 	"bytes"
 	"context"
+	"errors"
 	"path"
 
 	"github.com/derailed/k9s/internal/k8s"
@@ -100,7 +101,9 @@ func (*Base) NumCols(n string) map[string]bool {
 }
 
 // ExtFields returns extended fields in relation to headers.
-func (*Base) ExtFields(*TypeMeta) {}
+func (*Base) ExtFields() (TypeMeta, error) {
+	return TypeMeta{}, errors.New("Base does not have extended fields.")
+}
 
 // Get a resource by name
 func (b *Base) Get(path string) (Columnar, error) {

internal/resource/crd.go

@@ -1,11 +1,12 @@
 package resource
 
 import (
+	"errors"
 	"time"
 
 	"github.com/derailed/k9s/internal/k8s"
 	"github.com/rs/zerolog/log"
-	yaml "gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
@@ -16,6 +17,8 @@ type CustomResourceDefinition struct {
 	instance *unstructured.Unstructured
 }
 
+var _ Columnar = (*CustomResourceDefinition)(nil)
+
 // NewCustomResourceDefinitionList returns a new resource list.
 func NewCustomResourceDefinitionList(c Connection, ns string) List {
 	return NewList(
@@ -89,11 +92,12 @@ func (r *CustomResourceDefinition) Fields(ns string) Row {
 }
 
 // ExtFields returns extended fields.
-func (r *CustomResourceDefinition) ExtFields(m *TypeMeta) {
+func (r *CustomResourceDefinition) ExtFields() (TypeMeta, error) {
+	m := TypeMeta{}
 	i := r.instance
 	spec, ok := i.Object["spec"].(map[string]interface{})
 	if !ok {
-		return
+		return m, errors.New("missing crd specs")
 	}
 
 	if meta, ok := i.Object["metadata"].(map[string]interface{}); ok {
@@ -103,7 +107,7 @@ func (r *CustomResourceDefinition) ExtFields(m *TypeMeta) {
 	m.Namespaced = isNamespaced(spec["scope"].(string))
 	names, ok := spec["names"].(map[string]interface{})
 	if !ok {
-		return
+		return m, errors.New("missing crd names")
 	}
 	m.Kind = names["kind"].(string)
 	m.Singular, m.Plural = names["singular"].(string), names["plural"].(string)
@@ -114,6 +118,7 @@ func (r *CustomResourceDefinition) ExtFields(m *TypeMeta) {
 	} else {
 		m.ShortNames = nil
 	}
+	return m, nil
 }
 
 func isNamespaced(scope string) bool {

internal/resource/list.go

@@ -90,7 +90,7 @@ type (
 	Columnar interface {
 		Header(ns string) Row
 		Fields(ns string) Row
-		ExtFields(*TypeMeta)
+		ExtFields() (TypeMeta, error)
 		Name() string
 		SetPodMetrics(*mv1beta1.PodMetrics)
 		SetNodeMetrics(*mv1beta1.NodeMetrics)

internal/views/registrar.go

@@ -51,20 +51,23 @@ func allCRDs(c k8s.Connection, vv viewers) {
 	}
 
 	t := time.Now()
-	var meta resource.TypeMeta
 	for _, crd := range crds {
-		crd.ExtFields(&meta)
+		meta, err := crd.ExtFields()
+		if err != nil {
+			log.Error().Err(err).Msgf("Error getting extended fields from %s", crd.Name())
+			continue
+		}
 
 		gvr := k8s.NewGVR(meta.Group, meta.Version, meta.Plural)
 		gvrs := gvr.String()
 		if meta.Plural != "" {
-			aliases.Define(meta.Plural, gvrs)
+			aliases.Define(gvrs, meta.Plural)
 		}
 		if meta.Singular != "" {
-			aliases.Define(meta.Singular, gvrs)
+			aliases.Define(gvrs, meta.Singular)
 		}
 		for _, a := range meta.ShortNames {
-			aliases.Define(a, gvrs)
+			aliases.Define(gvrs, a)
 		}
 
 		vv[gvrs] = viewer{
@@ -141,15 +144,13 @@ func load(c k8s.Connection, vv viewers) {
 			cmd.gvr = gvr.String()
 			vv[gvr.String()] = cmd
 			gvrStr := gvr.String()
-			aliases.Define(
-				strings.ToLower(res.Kind), gvrStr,
-				res.Name, gvrStr,
-			)
+			aliases.Define(gvrStr, strings.ToLower(res.Kind))
+			aliases.Define(gvrStr, res.Name)
 			if len(res.SingularName) > 0 {
-				aliases.Define(res.SingularName, gvrStr)
+				aliases.Define(gvrStr, res.SingularName)
 			}
 			for _, s := range res.ShortNames {
-				aliases.Define(s, gvrStr)
+				aliases.Define(gvrStr, s)
 			}
 		}
 	}