From bf702ef1fb562be8f1e998e0468dd7e178f20cac Mon Sep 17 00:00:00 2001 From: Horst Gutmann Date: Fri, 22 Nov 2024 08:22:34 +0100 Subject: [PATCH] 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 | 2 +- pkg/kubernetes/client/delete.go | 42 +++++++++++++++++++++++++--- pkg/kubernetes/client/delete_test.go | 38 +++++++++++++++++++------ pkg/kubernetes/delete.go | 2 +- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/pkg/kubernetes/client/client.go b/pkg/kubernetes/client/client.go index d8cb64d4a..0db177ed2 100644 --- a/pkg/kubernetes/client/client.go +++ b/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) diff --git a/pkg/kubernetes/client/delete.go b/pkg/kubernetes/client/delete.go index b6fa37d38..be551c504 100644 --- a/pkg/kubernetes/client/delete.go +++ b/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 != "" { diff --git a/pkg/kubernetes/client/delete_test.go b/pkg/kubernetes/client/delete_test.go index f950e86b6..da9b7f7be 100644 --- a/pkg/kubernetes/client/delete_test.go +++ b/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) diff --git a/pkg/kubernetes/delete.go b/pkg/kubernetes/delete.go index 2d405fbe1..da0ce92c2 100644 --- a/pkg/kubernetes/delete.go +++ b/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 } }