fix: delete command supports kinds that do not match singular/plural …

…names (#1236)

fix: delete command supports Kinds that do not match singular/plural names

If a resource has the kind "HiThere" but its singular is "hi-there" then
deletion was not possible since Tanka operated on the kind and not the
singular/plural defined in the CRD.

This changes the handling to following the `TYPE.VERSION.GROUP` format
when possible.

pkg/kubernetes/client/client.go

@@ -20,7 +20,7 @@ type Client interface {
 	DiffServerSide(data manifest.List) (*string, error)
 
 	// Delete the specified object(s) from the cluster
-	Delete(namespace, kind, name string, opts DeleteOpts) error
+	Delete(namespace, apiVersion, kind, name string, opts DeleteOpts) error
 
 	// Namespaces the cluster currently has
 	Namespaces() (map[string]bool, error)

pkg/kubernetes/client/delete.go

@@ -6,14 +6,33 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+
+	"github.com/rs/zerolog/log"
 )
 
+func buildFullType(group, version, kind string) string {
+	output := strings.Builder{}
+	output.WriteString(kind)
+	// Unfortunately, kubectl does not support `Type.Version` for things like
+	// `Service` in v1. In this case, we cannot provide anything but the kind
+	// name:
+	if version != "" && group != "" {
+		output.WriteString(".")
+		output.WriteString(version)
+		output.WriteString(".")
+		output.WriteString(group)
+	}
+	return output.String()
+}
+
 // Test-ability: isolate deleteCtl to build and return exec.Cmd from DeleteOpts
-func (k Kubectl) deleteCtl(namespace, kind, name string, opts DeleteOpts) *exec.Cmd {
+func (k Kubectl) deleteCtl(namespace, group, version, kind, name string, opts DeleteOpts) *exec.Cmd {
+	fullType := buildFullType(group, version, kind)
 	argv := []string{
 		"-n", namespace,
-		kind, name,
+		fullType, name,
 	}
+	log.Debug().Str("name", name).Str("group", group).Str("version", version).Str("kind", kind).Str("namespace", namespace).Msg("Preparing to delete")
 	if opts.Force {
 		argv = append(argv, "--force")
 	}
@@ -27,8 +46,22 @@ func (k Kubectl) deleteCtl(namespace, kind, name string, opts DeleteOpts) *exec.
 }
 
 // Delete deletes the given Kubernetes resource from the cluster
-func (k Kubectl) Delete(namespace, kind, name string, opts DeleteOpts) error {
-	cmd := k.deleteCtl(namespace, kind, name, opts)
+func (k Kubectl) Delete(namespace, apiVersion, kind, name string, opts DeleteOpts) error {
+	apiVersionElements := strings.SplitN(apiVersion, "/", 2)
+	if len(apiVersionElements) < 1 {
+		return fmt.Errorf("apiVersion does not follow the group/version or version format: %s", apiVersion)
+	}
+	var group string
+	var version string
+	if len(apiVersionElements) == 1 {
+		group = ""
+		version = apiVersionElements[0]
+	} else {
+		group = apiVersionElements[0]
+		version = apiVersionElements[1]
+	}
+
+	cmd := k.deleteCtl(namespace, group, version, kind, name, opts)
 
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
@@ -42,6 +75,7 @@ func (k Kubectl) Delete(namespace, kind, name string, opts DeleteOpts) error {
 			print("Delete failed: " + stderr.String())
 			return nil
 		}
+		log.Trace().Msgf("Delete failed: %s", stderr.String())
 		return err
 	}
 	if opts.DryRun != "" {

pkg/kubernetes/client/delete_test.go

@@ -16,10 +16,12 @@ func TestKubectl_deleteCtl(t *testing.T) {
 	}
 
 	type args struct {
-		ns   string
-		kind string
-		name string
-		opts DeleteOpts
+		ns      string
+		group   string
+		version string
+		kind    string
+		name    string
+		opts    DeleteOpts
 	}
 
 	tests := []struct {
@@ -31,10 +33,28 @@ func TestKubectl_deleteCtl(t *testing.T) {
 		{
 			name: "test default",
 			args: args{
-				ns:   "foo-ns",
-				kind: "deploy",
-				name: "foo-deploy",
-				opts: DeleteOpts{},
+				ns:      "foo-ns",
+				group:   "example.org",
+				version: "v1",
+				kind:    "deploy",
+				name:    "foo-deploy",
+				opts:    DeleteOpts{},
+			},
+			expectedArgs:   []string{"--context", info.Kubeconfig.Context.Name, "-n", "foo-ns", "deploy.v1.example.org", "foo-deploy"},
+			unExpectedArgs: []string{"--force", "--dry-run=server"},
+		},
+		{
+			name: "test no apiVersion group",
+			args: args{
+				ns: "foo-ns",
+				// Since there is no group, we should also not include the version since
+				// kubectl does not support something like `Service.v1` or
+				// `Service.v1.core`:
+				group:   "",
+				version: "v1",
+				kind:    "deploy",
+				name:    "foo-deploy",
+				opts:    DeleteOpts{},
 			},
 			expectedArgs:   []string{"--context", info.Kubeconfig.Context.Name, "-n", "foo-ns", "deploy", "foo-deploy"},
 			unExpectedArgs: []string{"--force", "--dry-run=server"},
@@ -60,7 +80,7 @@ func TestKubectl_deleteCtl(t *testing.T) {
 			k := Kubectl{
 				info: info,
 			}
-			got := k.deleteCtl(tt.args.ns, tt.args.kind, tt.args.name, tt.args.opts)
+			got := k.deleteCtl(tt.args.ns, tt.args.group, tt.args.version, tt.args.kind, tt.args.name, tt.args.opts)
 			gotSet := sets.NewString(got.Args...)
 			if !gotSet.HasAll(tt.expectedArgs...) {
 				t.Errorf("Kubectl.applyCtl() = %v doesn't have (all) expectedArgs='%v'", got.Args, tt.expectedArgs)

pkg/kubernetes/delete.go

@@ -16,7 +16,7 @@ func (k *Kubernetes) Delete(state manifest.List, opts DeleteOpts) error {
 	}
 
 	for _, m := range state {
-		if err := k.ctl.Delete(m.Metadata().Namespace(), m.Kind(), m.Metadata().Name(), client.DeleteOpts(opts)); err != nil {
+		if err := k.ctl.Delete(m.Metadata().Namespace(), m.APIVersion(), m.Kind(), m.Metadata().Name(), client.DeleteOpts(opts)); err != nil {
 			return err
 		}
 	}