add support for objects with generated names, fixes #29 (#42)

Enable support for objects (like jobs and one-off pods) to be created
with generateName set instead of a fixed name. This enables the following:

* show -O displays prefix-<xxxxxx> as the object name for dynamic objects
* duplicate checks do not consider dynamic objects
* the real name of the object is updated on sync to ensure that an object just created is not garbage collected
* GC will delete previously created objects with generated names. There is not yet a way to customize this.
* diffs will correctly show a new object being created and existing object(s) deleted

This commit also fixes a nasty regression in the previous commit wherein deletion wouldn't honor component
name filters.

Makefile

@@ -10,8 +10,8 @@ LD_FLAGS +=  -X "$(LD_FLAGS_PKG).version=$(VERSION)"
 LD_FLAGS +=  -X "$(LD_FLAGS_PKG).commit=$(SHORT_COMMIT)"
 LD_FLAGS +=  -X "$(LD_FLAGS_PKG).goVersion=$(GO_VERSION)"
 
-DEP_FLAGS ?= ""
-LINT_FLAGS ?= ""
+DEP_FLAGS ?=
+LINT_FLAGS ?=
 
 .PHONY: all
 all: get build lint test
@@ -26,11 +26,12 @@ build:
 
 .PHONY: test
 test:
-	go test -v ./...
+	go test -race ./...
 
 .PHONY: lint
 lint:
 	go list ./... | grep -v vendor | xargs go vet
+	go list ./... | grep -v vendor | xargs golint
 	golangci-lint run $(LINT_FLAGS) .
 
 .PHONY: install-ci

examples/test-app/components/test-job.yaml

@@ -0,0 +1,13 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  generateName: tj-
+spec:
+  template:
+    spec:
+      containers:
+        - name: pi
+          image: perl
+          command: ["perl"]
+          args: ["-Mbignum=bpi", "-wle", "print bpi(2000)"]
+      restartPolicy: Never

internal/commands/apply.go

@@ -56,6 +56,15 @@ type applyCommandConfig struct {
 	filterFunc  func() (filterParams, error)
 }
 
+type nameWrap struct {
+	name string
+	model.K8sLocalObject
+}
+
+func (nw nameWrap) GetName() string {
+	return nw.name
+}
+
 func doApply(args []string, config applyCommandConfig) error {
 	if len(args) != 1 {
 		return newUsageError("exactly one environment required")
@@ -88,11 +97,17 @@ func doApply(args []string, config applyCommandConfig) error {
 	// prepare for GC with object list of deletions
 	var lister lister = &stubLister{}
 	var all []model.K8sLocalObject
+	var retainObjects []model.K8sLocalObject
 	if config.gc {
 		all, err = allObjects(config.Config, env)
 		if err != nil {
 			return err
 		}
+		for _, o := range all {
+			if o.GetName() != "" {
+				retainObjects = append(retainObjects, o)
+			}
+		}
 		var scope remote.ListQueryScope
 		lister, scope, err = newRemoteLister(client, all, config.app.DefaultNamespace(env))
 		if err != nil {
@@ -117,11 +132,15 @@ func doApply(args []string, config applyCommandConfig) error {
 
 	var stats applyStats
 	for _, ob := range objects {
-		name := client.DisplayName(ob)
 		res, err := client.Sync(ob, opts)
 		if err != nil {
 			return err
 		}
+		if res.GeneratedName != "" {
+			ob = nameWrap{name: res.GeneratedName, K8sLocalObject: ob}
+			retainObjects = append(retainObjects, ob)
+		}
+		name := client.DisplayName(ob)
 		stats.update(name, res)
 		show := res.Type != remote.SyncObjectsIdentical || config.Verbosity() > 0
 		if show {
@@ -131,7 +150,7 @@ func doApply(args []string, config applyCommandConfig) error {
 	}
 
 	// process deletions
-	deletions, err := lister.deletions(all, fp.Includes)
+	deletions, err := lister.deletions(retainObjects, fp.Includes)
 	if err != nil {
 		return err
 	}

internal/commands/apply_test.go

@@ -43,6 +43,8 @@ func TestApplyBasic(t *testing.T) {
 			return &remote.SyncResult{Type: remote.SyncCreated, Details: "some yaml"}, nil
 		case obj.GetName() == "svc2-deploy":
 			return &remote.SyncResult{Type: remote.SyncObjectsIdentical, Details: "sync skipped"}, nil
+		case obj.GetName() == "":
+			return &remote.SyncResult{Type: remote.SyncCreated, GeneratedName: obj.GetGenerateName() + "1234", Details: "created"}, nil
 		default:
 			return &remote.SyncResult{Type: remote.SyncObjectsIdentical, Details: "sync skipped"}, nil
 		}
@@ -58,7 +60,7 @@ func TestApplyBasic(t *testing.T) {
 	a.EqualValues(remote.SyncOptions{}, captured)
 	a.True(stats["same"].(float64) > 0)
 	a.EqualValues(8, stats["same"])
-	a.EqualValues([]interface{}{"Secret:bar-system:svc2-secret"}, stats["created"])
+	a.EqualValues([]interface{}{"Secret:bar-system:svc2-secret", "Job::tj-1234"}, stats["created"])
 	a.EqualValues([]interface{}{"ConfigMap:bar-system:svc2-cm"}, stats["updated"])
 	a.EqualValues([]interface{}{"Deployment:bar-system:svc2-previous-deploy"}, stats["deleted"])
 	s.assertErrorLineMatch(regexp.MustCompile(`sync ConfigMap:bar-system:svc2-cm`))

internal/commands/component_test.go

@@ -32,10 +32,11 @@ func TestComponentListBasic(t *testing.T) {
 	require.Nil(t, err)
 	lines := strings.Split(strings.Trim(s.stdout(), "\n"), "\n")
 	a := assert.New(t)
-	a.Equal(3, len(lines))
+	a.Equal(4, len(lines))
 	s.assertOutputLineMatch(regexp.MustCompile(`COMPONENT\s+FILE`))
 	s.assertOutputLineMatch(regexp.MustCompile(`cluster-objects\s+components/cluster-objects.yaml`))
 	s.assertOutputLineMatch(regexp.MustCompile(`service2\s+components/service2.jsonnet`))
+	s.assertOutputLineMatch(regexp.MustCompile(`test-job\s+components/test-job.yaml`))
 }
 
 func TestComponentListYAML(t *testing.T) {

internal/commands/delete.go

@@ -58,7 +58,9 @@ func doDelete(args []string, config deleteCommandConfig) error {
 			return err
 		}
 		for _, o := range objects {
-			deletions = append(deletions, o)
+			if o.GetName() != "" {
+				deletions = append(deletions, o)
+			}
 		}
 	} else {
 		all, err := allObjects(config.Config, env)

internal/commands/delete_test.go

@@ -41,6 +41,22 @@ func TestDeleteRemote(t *testing.T) {
 	a.EqualValues([]interface{}{"Deployment:bar-system:svc2-previous-deploy", "Deployment:bar-system:svc2-deploy"}, stats["deleted"])
 }
 
+func TestDeleteRemoteComponentFilter(t *testing.T) {
+	s := newScaffold(t)
+	defer s.reset()
+	d := &dg{cmValue: "baz", secretValue: "baz"}
+	s.client.getFunc = d.get
+	s.client.listFunc = stdLister
+	s.client.deleteFunc = func(obj model.K8sMeta, dryRun bool) (*remote.SyncResult, error) {
+		return &remote.SyncResult{Type: remote.SyncDeleted}, nil
+	}
+	err := s.executeCommand("delete", "dev", "-c", "service2")
+	require.Nil(t, err)
+	stats := s.outputStats()
+	a := assert.New(t)
+	a.EqualValues([]interface{}{"Deployment:bar-system:svc2-previous-deploy"}, stats["deleted"])
+}
+
 func TestDeleteLocal(t *testing.T) {
 	s := newScaffold(t)
 	defer s.reset()

internal/commands/diff.go

@@ -185,7 +185,11 @@ func (d *differ) writeDiff(name string, left, right namedUn) (finalErr error) {
 		if err != nil {
 			return err
 		}
-		rightContent = addLeader(rightContent, "object doesn't exist on the server")
+		leaderComment := "object doesn't exist on the server"
+		if right.obj.GetName() == "" {
+			leaderComment += " (generated name)"
+		}
+		rightContent = addLeader(rightContent, leaderComment)
 		b, err := diff.Strings("", rightContent, fileOpts)
 		if err != nil {
 			return err
@@ -214,11 +218,16 @@ func (d *differ) writeDiff(name string, left, right namedUn) (finalErr error) {
 func (d *differ) diff(ob model.K8sMeta) error {
 	name, leftName, rightName := d.names(ob)
 
-	remoteObject, err := d.client.Get(ob)
-	if err != nil && err != remote.ErrNotFound {
-		d.stats.errors(name)
-		sio.Errorf("error fetching %s, %v\n", name, err)
-		return err
+	var remoteObject *unstructured.Unstructured
+	var err error
+
+	if ob.GetName() != "" {
+		remoteObject, err = d.client.Get(ob)
+		if err != nil && err != remote.ErrNotFound {
+			d.stats.errors(name)
+			sio.Errorf("error fetching %s, %v\n", name, err)
+			return err
+		}
 	}
 
 	fixup := func(u *unstructured.Unstructured) *unstructured.Unstructured {
@@ -233,7 +242,7 @@ func (d *differ) diff(ob model.K8sMeta) error {
 	}
 
 	var left, right *unstructured.Unstructured
-	if err == nil {
+	if remoteObject != nil {
 		var source string
 		left, source = remote.GetPristineVersionForDiff(remoteObject)
 		leftName += " (source: " + source + ")"
@@ -287,11 +296,17 @@ func doDiff(args []string, config diffCommandConfig) error {
 
 	var lister lister = &stubLister{}
 	var all []model.K8sLocalObject
+	var retainObjects []model.K8sLocalObject
 	if config.showDeletions {
 		all, err = allObjects(config.Config, env)
 		if err != nil {
 			return err
 		}
+		for _, o := range all {
+			if o.GetName() != "" {
+				retainObjects = append(retainObjects, o)
+			}
+		}
 		var scope remote.ListQueryScope
 		lister, scope, err = newRemoteLister(client, all, config.app.DefaultNamespace(env))
 		if err != nil {
@@ -328,7 +343,7 @@ func doDiff(args []string, config diffCommandConfig) error {
 
 	var listErr error
 	if dErr == nil {
-		extra, err := lister.deletions(all, fp.Includes)
+		extra, err := lister.deletions(retainObjects, fp.Includes)
 		if err != nil {
 			listErr = err
 		} else {

internal/commands/diff_test.go

@@ -47,6 +47,9 @@ func TestDiffBasic(t *testing.T) {
 	a.True(regexp.MustCompile(`\d+ object\(s\) different`).MatchString(err.Error()))
 	a.EqualValues([]interface{}{"ConfigMap:bar-system:svc2-cm", "Secret:bar-system:svc2-secret"}, stats["changes"])
 	a.EqualValues([]interface{}{"Deployment:bar-system:svc2-previous-deploy"}, stats["deletions"])
+	adds, ok := stats["additions"].([]interface{})
+	require.True(t, ok)
+	a.Contains(adds, "Job::tj-<xxxxx>")
 	secretValue := base64.StdEncoding.EncodeToString([]byte("baz"))
 	redactedValue := base64.RawStdEncoding.EncodeToString([]byte("redacted."))
 	a.Contains(s.stdout(), redactedValue)

internal/commands/filter.go

@@ -52,17 +52,19 @@ func addFilterParams(cmd *cobra.Command, includeKindFilters bool) func() (filter
 		cmd.Flags().StringArrayVarP(&kindExcludes, "exclude-kind", "K", nil, "exclude objects with this kind")
 	}
 	return func() (filterParams, error) {
-		if len(includes) > 0 && len(excludes) > 0 {
-			return filterParams{}, newUsageError("cannot include as well as exclude components, specify one or the other")
-		}
 		of, err := model.NewKindFilter(kindIncludes, kindExcludes)
 		if err != nil {
 			return filterParams{}, newUsageError(err.Error())
 		}
+		cf, err := model.NewComponentFilter(includes, excludes)
+		if err != nil {
+			return filterParams{}, newUsageError(err.Error())
+		}
 		return filterParams{
-			includes:   includes,
-			excludes:   excludes,
-			kindFilter: of,
+			includes:        includes,
+			excludes:        excludes,
+			kindFilter:      of,
+			componentFilter: cf,
 		}, nil
 	}
 }
@@ -92,6 +94,9 @@ func checkDuplicates(objects []model.K8sLocalObject, kf keyFunc) error {
 	}
 	objectsByKey := map[string]model.K8sLocalObject{}
 	for _, o := range objects {
+		if o.GetName() == "" { // generated name
+			continue
+		}
 		key := kf(o)
 		if prev, ok := objectsByKey[key]; ok {
 			return fmt.Errorf("duplicate objects %s and %s", displayName(prev), displayName(o))

internal/commands/show.go

@@ -39,7 +39,7 @@ func (n *metaOnly) MarshalJSON() ([]byte, error) {
 		"component":   n.Component(),
 		"environment": n.Environment(),
 		"kind":        gvk.Kind,
-		"name":        n.GetName(),
+		"name":        model.NameForDisplay(n),
 	}
 	if n.GetNamespace() != "" {
 		m["namespace"] = n.GetNamespace()
@@ -51,7 +51,8 @@ func showNames(objects []model.K8sLocalObject, formatSpecified bool, format stri
 	if !formatSpecified { // render as table
 		fmt.Fprintf(w, "%-30s %-30s %-40s %s\n", "COMPONENT", "KIND", "NAME", "NAMESPACE")
 		for _, o := range objects {
-			fmt.Fprintf(w, "%-30s %-30s %-40s %s\n", o.Component(), o.GroupVersionKind().Kind, o.GetName(), o.GetNamespace())
+			name := model.NameForDisplay(o)
+			fmt.Fprintf(w, "%-30s %-30s %-40s %s\n", o.Component(), o.GroupVersionKind().Kind, name, o.GetNamespace())
 		}
 		return nil
 	}

internal/commands/utils_test.go

@@ -62,10 +62,11 @@ type basicObject struct {
 	env       string
 }
 
-func (b *basicObject) Application() string { return b.app }
-func (b *basicObject) Tag() string         { return b.tag }
-func (b *basicObject) Component() string   { return b.component }
-func (b *basicObject) Environment() string { return b.env }
+func (b *basicObject) Application() string     { return b.app }
+func (b *basicObject) Tag() string             { return b.tag }
+func (b *basicObject) Component() string       { return b.component }
+func (b *basicObject) Environment() string     { return b.env }
+func (b *basicObject) GetGenerateName() string { return "" }
 
 type coll struct {
 	data map[objectKey]model.K8sQbecMeta
@@ -114,7 +115,7 @@ type client struct {
 }
 
 func (c *client) DisplayName(o model.K8sMeta) string {
-	return fmt.Sprintf("%s:%s:%s", o.GetKind(), o.GetNamespace(), o.GetName())
+	return fmt.Sprintf("%s:%s:%s", o.GetKind(), o.GetNamespace(), model.NameForDisplay(o))
 }
 
 func (c *client) IsNamespaced(kind schema.GroupVersionKind) (bool, error) {
@@ -220,14 +221,15 @@ func (s *scaffold) jsonOutput(data interface{}) error {
 	return json.Unmarshal(s.outCapture.Bytes(), &data)
 }
 
-func (s *scaffold) executeCommand(args ...string) error {
+func (s *scaffold) executeCommand(args ...string) (err error) {
 	s.cmd.SetArgs(args)
 	defer func() {
 		if os.Getenv("QBEC_VERBOSE") != "" {
 			l := log.New(os.Stderr, "", 0)
 			l.Println("Command:", args)
-			l.Println("Output:\n" + s.stdout())
-			l.Println("Error:\n" + s.stderr())
+			l.Println("Stdout:\n" + s.stdout())
+			l.Println("Stderr:\n" + s.stderr())
+			l.Println("Err:", err)
 		}
 	}()
 	return s.cmd.Execute()

internal/eval/object-extract.go

@@ -26,8 +26,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
-var _ = unstructured.Unstructured{}
-
 func tolerantJSON(data interface{}) string {
 	b, err := json.MarshalIndent(data, "", "  ")
 	if err != nil {
@@ -78,6 +76,15 @@ func (w *walker) walkObjects(path string, component string, data interface{}) ([
 				}
 				ret = append(ret, objects...)
 			} else {
+				u := unstructured.Unstructured{Object: t}
+				name := u.GetName()
+				genName := u.GetGenerateName()
+				if name == "" && genName == "" {
+					return nil, fmt.Errorf("object (%v) did not have a name at path %q, (json=\n%s)",
+						reflect.TypeOf(data),
+						path,
+						tolerantJSON(data))
+				}
 				ret = append(ret, model.NewK8sLocalObject(t, w.app, w.tag, component, w.env))
 			}
 			return ret, nil