Make uninstalling cert-manager SAFE: don't uninstal the CRDs

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.4
 	golang.org/x/crypto v0.20.0
+	golang.org/x/exp v0.0.0-20231226003508-02704c960a9b
 	golang.org/x/sync v0.6.0
 	helm.sh/helm/v3 v3.14.2
 	k8s.io/api v0.29.2
@@ -155,7 +156,6 @@ require (
 	go.starlark.net v0.0.0-20230525235612-a134d8f9ddca // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.26.0 // indirect
-	golang.org/x/exp v0.0.0-20231226003508-02704c960a9b // indirect
 	golang.org/x/net v0.21.0 // indirect
 	golang.org/x/oauth2 v0.15.0 // indirect
 	golang.org/x/sys v0.17.0 // indirect

pkg/uninstall/uninstall.go

@@ -20,12 +20,19 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"sort"
+	"strings"
 
 	"github.com/spf13/cobra"
+	"golang.org/x/exp/maps"
 	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/release"
+	"helm.sh/helm/v3/pkg/releaseutil"
 	"helm.sh/helm/v3/pkg/storage/driver"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/yaml"
 
 	"github.com/cert-manager/cmctl/v2/pkg/build"
 	"github.com/cert-manager/cmctl/v2/pkg/install/helm"
@@ -47,9 +54,11 @@ const (
 )
 
 func description() string {
-	return build.WithTemplate(`This command uninstalls any Helm-managed release of cert-manager.
+	return build.WithTemplate(`This command safely uninstalls any Helm-managed release of cert-manager.
 
-The CRDs will be deleted if you installed cert-manager with the option --set CRDs=true.
+This command is safe because it will not delete any of the cert-manager CRDs even if they were
+installed as part of the Helm release. This is to avoid accidentally deleting CRDs and custom resources.
+This feature is why this command should always be used instead of 'helm uninstall'.
 
 Most of the features supported by 'helm uninstall' are also supported by this command.
 
@@ -89,6 +98,10 @@ func NewCmd(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.C
 				return nil
 			}
 
+			if res != nil && res.Info != "" {
+				fmt.Fprintln(ioStreams.Out, res.Info)
+			}
+
 			fmt.Fprintf(ioStreams.Out, "release \"%s\" uninstalled\n", options.releaseName)
 			return nil
 		},
@@ -113,6 +126,19 @@ func run(ctx context.Context, o options) (*release.UninstallReleaseResponse, err
 	o.client.DisableHooks = false
 	o.client.DryRun = o.dryRun
 	o.client.Wait = o.wait
+	if o.client.Wait {
+		o.client.DeletionPropagation = "foreground"
+	} else {
+		o.client.DeletionPropagation = "background"
+	}
+	o.client.KeepHistory = false
+	o.client.IgnoreNotFound = true
+
+	if !o.client.DryRun {
+		if err := addCRDAnnotations(ctx, o); err != nil {
+			return nil, err
+		}
+	}
 
 	res, err := o.client.Run(o.releaseName)
 
@@ -122,3 +148,92 @@ func run(ctx context.Context, o options) (*release.UninstallReleaseResponse, err
 
 	return res, nil
 }
+
+func addCRDAnnotations(ctx context.Context, o options) error {
+	if err := o.settings.ActionConfiguration.KubeClient.IsReachable(); err != nil {
+		return err
+	}
+
+	if err := chartutil.ValidateReleaseName(o.releaseName); err != nil {
+		return fmt.Errorf("uninstall: %v", err)
+	}
+
+	lastRelease, err := o.settings.ActionConfiguration.Releases.Last(o.releaseName)
+	if err != nil {
+		return fmt.Errorf("uninstall: %v", err)
+	}
+
+	if lastRelease.Info.Status != release.StatusDeployed {
+		return fmt.Errorf("release %v is in a non-deployed state: %v", o.releaseName, lastRelease.Info.Status)
+	}
+
+	const (
+		customResourceDefinitionApiVersionV1      = "apiextensions.k8s.io/v1"
+		customResourceDefinitionApiVersionV1Beta1 = "apiextensions.k8s.io/v1beta1"
+		customResourceDefinitionKind              = "CustomResourceDefinition"
+	)
+
+	// Check if the release manifest contains CRDs. If it does, we need to modify the
+	// release manifest to add the "helm.sh/resource-policy: keep" annotation to the CRDs.
+	manifests := releaseutil.SplitManifests(lastRelease.Manifest)
+	foundNonAnnotatedCRD := false
+	for key, manifest := range manifests {
+		var entry releaseutil.SimpleHead
+		if err := yaml.Unmarshal([]byte(manifest), &entry); err != nil {
+			return fmt.Errorf("failed to unmarshal manifest: %v", err)
+		}
+
+		if entry.Kind != customResourceDefinitionKind || (entry.Version != customResourceDefinitionApiVersionV1 &&
+			entry.Version != customResourceDefinitionApiVersionV1Beta1) {
+			continue
+		}
+
+		if entry.Metadata != nil && entry.Metadata.Annotations != nil && entry.Metadata.Annotations["helm.sh/resource-policy"] == "keep" {
+			continue
+		}
+
+		foundNonAnnotatedCRD = true
+
+		var object unstructured.Unstructured
+		if err := yaml.Unmarshal([]byte(manifest), &object); err != nil {
+			return fmt.Errorf("failed to unmarshal manifest: %v", err)
+		}
+
+		annotations := object.GetAnnotations()
+		if annotations == nil {
+			annotations = make(map[string]string)
+		}
+		annotations["helm.sh/resource-policy"] = "keep"
+		object.SetAnnotations(annotations)
+
+		updatedManifestJSON, err := object.MarshalJSON()
+		if err != nil {
+			return fmt.Errorf("failed to marshal manifest: %v", err)
+		}
+
+		updatedManifest, err := yaml.JSONToYAML(updatedManifestJSON)
+		if err != nil {
+			return fmt.Errorf("failed to convert manifest to YAML: %v", err)
+		}
+
+		manifests[key] = string(updatedManifest)
+	}
+
+	if foundNonAnnotatedCRD {
+		manifestNames := releaseutil.BySplitManifestsOrder(maps.Keys(manifests))
+		sort.Sort(manifestNames)
+		var fullManifest strings.Builder
+		for _, manifest := range manifestNames {
+			fullManifest.WriteString(manifests[manifest])
+			fullManifest.WriteString("\n---\n")
+		}
+
+		lastRelease.Manifest = fullManifest.String()
+
+		if err := o.settings.ActionConfiguration.Releases.Update(lastRelease); err != nil {
+			o.settings.ActionConfiguration.Log("uninstall: Failed to store updated release: %s", err)
+		}
+	}
+
+	return nil
+}

test/integration/ctl_uninstall_test.go

@@ -25,6 +25,9 @@ import (
 	"time"
 
 	"github.com/cert-manager/cmctl/v2/test/integration/install_framework"
+	"github.com/stretchr/testify/require"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestCtlUninstall(t *testing.T) {
@@ -38,6 +41,8 @@ func TestCtlUninstall(t *testing.T) {
 		inputArgs []string
 		expErr    bool
 		expOutput string
+
+		didInstallCRDs bool
 	}{
 		"install and uninstall cert-manager": {
 			prerun:       true,
@@ -48,6 +53,8 @@ func TestCtlUninstall(t *testing.T) {
 			inputArgs: []string{"x", "uninstall", "--wait=false"},
 			expErr:    false,
 			expOutput: `release "cert-manager" uninstalled`,
+
+			didInstallCRDs: true,
 		},
 		"uninstall cert-manager installed by helm": {
 			prehelm: true,
@@ -66,7 +73,9 @@ func TestCtlUninstall(t *testing.T) {
 
 			inputArgs: []string{"x", "uninstall", "--wait=false"},
 			expErr:    false,
-			expOutput: `release "cert-manager" uninstalled`,
+			expOutput: `These resources were kept due to the resource policy:`,
+
+			didInstallCRDs: true,
 		},
 	}
 
@@ -100,6 +109,15 @@ func TestCtlUninstall(t *testing.T) {
 				test.expErr,
 				test.expOutput,
 			)
+
+			// if we installed CRDs, check that they were not deleted
+			if test.didInstallCRDs {
+				clientset, err := apiextensionsv1.NewForConfig(testApiServer.RestConfig())
+				require.NoError(t, err)
+
+				_, err = clientset.CustomResourceDefinitions().Get(ctx, "certificates.cert-manager.io", metav1.GetOptions{})
+				require.NoError(t, err)
+			}
 		})
 	}
 }