improve default flag values & other requested changes

Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com>

cmd/ctl/BUILD.bazel

@@ -38,6 +38,7 @@ filegroup(
         "//cmd/ctl/pkg/create:all-srcs",
         "//cmd/ctl/pkg/deny:all-srcs",
         "//cmd/ctl/pkg/experimental:all-srcs",
+        "//cmd/ctl/pkg/flags:all-srcs",
         "//cmd/ctl/pkg/inspect:all-srcs",
         "//cmd/ctl/pkg/install:all-srcs",
         "//cmd/ctl/pkg/renew:all-srcs",

cmd/ctl/cmd/BUILD.bazel

@@ -11,6 +11,7 @@ go_library(
         "//cmd/ctl/pkg/create:go_default_library",
         "//cmd/ctl/pkg/deny:go_default_library",
         "//cmd/ctl/pkg/experimental:go_default_library",
+        "//cmd/ctl/pkg/flags:go_default_library",
         "//cmd/ctl/pkg/inspect:go_default_library",
         "//cmd/ctl/pkg/renew:go_default_library",
         "//cmd/ctl/pkg/status:go_default_library",
@@ -18,8 +19,6 @@ go_library(
         "@com_github_spf13_cobra//:go_default_library",
         "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library",
         "@io_k8s_client_go//plugin/pkg/client/auth:go_default_library",
-        "@io_k8s_klog_v2//:go_default_library",
-        "@io_k8s_kubectl//pkg/cmd/util:go_default_library",
     ],
 )
 

cmd/ctl/cmd/cmd.go

@@ -18,23 +18,19 @@ package cmd
 
 import (
 	"context"
-	"flag"
-	"fmt"
 	"io"
-	"os"
 
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	// Load all auth plugins
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
-	"k8s.io/klog/v2"
-	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/approve"
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/convert"
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/create"
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/deny"
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/experimental"
+	"github.com/jetstack/cert-manager/cmd/ctl/pkg/flags"
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/inspect"
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/renew"
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/status"
@@ -50,20 +46,7 @@ kubectl cert-manager is a CLI tool manage and configure cert-manager resources f
 	}
 	cmds.SetUsageTemplate(usageTemplate)
 
-	kubeConfigFlags := genericclioptions.NewConfigFlags(true)
-	kubeConfigFlags.AddFlags(cmds.PersistentFlags())
-	matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags)
-	matchVersionKubeConfigFlags.AddFlags(cmds.PersistentFlags())
-	factory := cmdutil.NewFactory(matchVersionKubeConfigFlags)
-
-	cmds.Flags().AddGoFlagSet(flag.CommandLine)
-	flag.CommandLine.Parse([]string{})
-	fakefs := flag.NewFlagSet("fake", flag.ExitOnError)
-	klog.InitFlags(fakefs)
-	if err := fakefs.Parse([]string{"-logtostderr=false"}); err != nil {
-		fmt.Fprintf(os.Stderr, "%s\n", err)
-		os.Exit(1)
-	}
+	factory := flags.AddFlags(cmds)
 
 	ioStreams := genericclioptions.IOStreams{In: in, Out: out, ErrOut: err}
 	cmds.AddCommand(version.NewCmdVersion(ctx, ioStreams))
@@ -76,7 +59,7 @@ kubectl cert-manager is a CLI tool manage and configure cert-manager resources f
 	cmds.AddCommand(deny.NewCmdDeny(ctx, ioStreams, factory))
 
 	// Experimental features
-	cmds.AddCommand(experimental.NewCmdExperimental(ctx, ioStreams, factory, kubeConfigFlags))
+	cmds.AddCommand(experimental.NewCmdExperimental(ctx, ioStreams, factory))
 
 	return cmds
 }

cmd/ctl/pkg/experimental/experimental.go

@@ -29,7 +29,7 @@ import (
 	"github.com/jetstack/cert-manager/cmd/ctl/pkg/install"
 )
 
-func NewCmdExperimental(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory, kubeConfigFlags *genericclioptions.ConfigFlags) *cobra.Command {
+func NewCmdExperimental(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory) *cobra.Command {
 	cmds := &cobra.Command{
 		Use:     "experimental",
 		Aliases: []string{"x"},
@@ -40,8 +40,8 @@ func NewCmdExperimental(ctx context.Context, ioStreams genericclioptions.IOStrea
 	create := create.NewCmdCreateBare()
 	create.AddCommand(certificatesigningrequest.NewCmdCreateCSR(ctx, ioStreams, factory))
 	cmds.AddCommand(create)
-	cmds.AddCommand(install.NewCmdInstall(ctx, ioStreams, factory, kubeConfigFlags))
-	cmds.AddCommand(install.NewCmdUninstall(ctx, ioStreams, factory, kubeConfigFlags))
+	cmds.AddCommand(install.NewCmdInstall(ctx, ioStreams, factory))
+	cmds.AddCommand(install.NewCmdUninstall(ctx, ioStreams, factory))
 
 	return cmds
 }

cmd/ctl/pkg/flags/BUILD.bazel

@@ -0,0 +1,28 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["flags.go"],
+    importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/flags",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@com_github_spf13_cobra//:go_default_library",
+        "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library",
+        "@io_k8s_klog_v2//:go_default_library",
+        "@io_k8s_kubectl//pkg/cmd/util:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

cmd/ctl/pkg/flags/flags.go

@@ -0,0 +1,34 @@
+package flags
+
+import (
+	"flag"
+	"fmt"
+	"os"
+
+	"k8s.io/klog/v2"
+
+	"github.com/spf13/cobra"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	cmdutil "k8s.io/kubectl/pkg/cmd/util"
+)
+
+func AddFlags(cmds *cobra.Command) cmdutil.Factory {
+	kubeConfigFlags := genericclioptions.NewConfigFlags(true)
+	kubeConfigFlags.AddFlags(cmds.PersistentFlags())
+
+	matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags)
+	matchVersionKubeConfigFlags.AddFlags(cmds.PersistentFlags())
+
+	factory := cmdutil.NewFactory(matchVersionKubeConfigFlags)
+
+	cmds.Flags().AddGoFlagSet(flag.CommandLine)
+	flag.CommandLine.Parse([]string{})
+	fakefs := flag.NewFlagSet("fake", flag.ExitOnError)
+	klog.InitFlags(fakefs)
+	if err := fakefs.Parse([]string{"-logtostderr=false"}); err != nil {
+		fmt.Fprintf(os.Stderr, "%s\n", err)
+		os.Exit(1)
+	}
+
+	return factory
+}

cmd/ctl/pkg/install/helm/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
         "@com_github_spf13_pflag//:go_default_library",
         "@io_k8s_apimachinery//pkg/api/errors:go_default_library",
         "@io_k8s_apimachinery//pkg/api/meta:go_default_library",
-        "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library",
         "@io_k8s_cli_runtime//pkg/resource:go_default_library",
         "@sh_helm_helm_v3//pkg/action:go_default_library",
         "@sh_helm_helm_v3//pkg/chart:go_default_library",

cmd/ctl/pkg/install/helm/applycrd.go

@@ -35,6 +35,9 @@ const (
 	CreateReplace
 )
 
+// TODO: Currently, only CRDsPolicy.Create is used. In the future,
+// CRDsPolicy.CreateReplace will allow the cli tool to also upgrade crds.
+
 func (policy CRDsPolicy) String() string {
 	return [...]string{"Skip", "Create", "CreateReplace"}[policy]
 }

cmd/ctl/pkg/install/helm/resource.go

@@ -23,7 +23,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/cli-runtime/pkg/resource"
 
-	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/kube"
 )
 
@@ -32,24 +31,10 @@ const (
 	customResourceDefinitionKind  = "CustomResourceDefinition"
 )
 
-// Build a list of resource.Info objects from a chart definition and its rendered manifest.
-// The chart is only used for its CRDObjects() function that returns a list of all files in the /crds folder.
-// The includeCrdFolder option is used to not include the /crds folder. Current versions of the cert-manager chart
-// don't have a crds folder, so this option is only in case this would ever change. The manifest includes
-// all types of resources (not only crds).
-func GetChartResourceInfo(ch *chart.Chart, manifest string, includeCrdFolder bool, kubeClient kube.Interface) ([]*resource.Info, error) {
+// Build a list of resource.Info objects from a rendered manifest.
+func GetChartResourceInfo(manifest string, kubeClient kube.Interface) ([]*resource.Info, error) {
 	resources := make([]*resource.Info, 0)
 
-	if includeCrdFolder {
-		for _, obj := range ch.CRDObjects() {
-			res, err := kubeClient.Build(bytes.NewBuffer(obj.File.Data), false)
-			if err != nil {
-				return nil, fmt.Errorf("failed to parse CRDs from %s: %s", obj.Name, err)
-			}
-			resources = append(resources, res...)
-		}
-	}
-
 	res, err := kubeClient.Build(bytes.NewBufferString(manifest), false)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse CRDs from render: %s", err)

cmd/ctl/pkg/install/helm/settings.go

@@ -17,30 +17,36 @@ limitations under the License.
 package helm
 
 import (
-	flag "github.com/spf13/pflag"
-	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"github.com/spf13/pflag"
 
 	"helm.sh/helm/v3/pkg/cli"
 )
 
-func CopyCliFlags(kubeConfigFlags *genericclioptions.ConfigFlags, cliEnvSettings *cli.EnvSettings) error {
+func CopyCliFlags(flags *pflag.FlagSet, defaults map[string]string, cliEnvSettings *cli.EnvSettings) error {
 	// Pass namespace value through fake flags, because it is a private property
-	fakefs := flag.NewFlagSet("fake", flag.ExitOnError)
+	fakefs := pflag.NewFlagSet("fake", pflag.ExitOnError)
 	cliEnvSettings.AddFlags(fakefs)
-	if err := fakefs.Set("namespace", *kubeConfigFlags.Namespace); err != nil {
+
+	for name, value := range defaults {
+		if err := fakefs.Set(name, value); err != nil {
+			return err
+		}
+	}
+
+	var err error = nil
+	flags.VisitAll(func(flag *pflag.Flag) {
+		if err != nil || !flag.Changed {
+			return
+		}
+		err = fakefs.Set(flag.Name, flag.Value.String())
+	})
+	if err != nil {
 		return err
 	}
+
 	if err := fakefs.Parse([]string{}); err != nil {
 		return err
 	}
 
-	cliEnvSettings.KubeConfig = *kubeConfigFlags.KubeConfig
-	cliEnvSettings.KubeContext = *kubeConfigFlags.Context
-	cliEnvSettings.KubeToken = *kubeConfigFlags.BearerToken
-	cliEnvSettings.KubeAsUser = *kubeConfigFlags.Impersonate
-	cliEnvSettings.KubeAsGroups = *kubeConfigFlags.ImpersonateGroup
-	cliEnvSettings.KubeAPIServer = *kubeConfigFlags.APIServer
-	cliEnvSettings.KubeCaFile = *kubeConfigFlags.CAFile
-
 	return nil
 }

cmd/ctl/pkg/install/install.go

@@ -52,19 +52,20 @@ type InstallOptions struct {
 
 const installCRDsFlagName = "installCRDs"
 const installDesc = `
-This command installs cert-manager. It uses the helm libraries to do so.
+This command installs cert-manager. It uses the Helm libraries to do so.
 
 The latest published cert-manager chart on the "https://charts.jetstack.io" repo is used.
 Most of the features supported by 'helm install' are also supported by this command.
-Additional the the functionallity that the helm command gives you, this command will
-also manage CRD resources.
+In addition his command will always install CRD resources.
 
 Some example uses:
-	$ kubectl cert-manager install -n cert-manager
+	$ kubectl cert-manager install
 or
-	$ kubectl cert-manager install -n cert-manager --version v1.4.0
+	$ kubectl cert-manager install -n new-cert-manager
 or
-	$ kubectl cert-manager install -n cert-manager --set prometheus.enabled=false
+	$ kubectl cert-manager install --version v1.4.0
+or
+	$ kubectl cert-manager install --set prometheus.enabled=false
 
 To override values in the cert-manager chart, use either the '--values' flag and pass in a file
 or use the '--set' flag and pass configuration from the command line, to force
@@ -73,7 +74,7 @@ you want not to use neither '--values' nor '--set', use '--set-file' to read the
 single large value from file.
 `
 
-func NewCmdInstall(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory, kubeConfigFlags *genericclioptions.ConfigFlags) *cobra.Command {
+func NewCmdInstall(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory) *cobra.Command {
 	settings := cli.New()
 	cfg := new(action.Configuration)
 
@@ -85,12 +86,16 @@ func NewCmdInstall(ctx context.Context, ioStreams genericclioptions.IOStreams, f
 		IOStreams: ioStreams,
 	}
 
+	// Set default namespace cli flag value
+	defaults := make(map[string]string)
+	defaults["namespace"] = "cert-manager"
+
 	cmd := &cobra.Command{
 		Use:   "install",
 		Short: "install cert-manager",
 		Long:  installDesc,
-		RunE: func(_ *cobra.Command, args []string) error {
-			if err := helm.CopyCliFlags(kubeConfigFlags, settings); err != nil {
+		RunE: func(cmd *cobra.Command, args []string) error {
+			if err := helm.CopyCliFlags(cmd.Root().PersistentFlags(), defaults, settings); err != nil {
 				return nil
 			}
 			options.client.Namespace = settings.Namespace()
@@ -99,7 +104,7 @@ func NewCmdInstall(ctx context.Context, ioStreams genericclioptions.IOStreams, f
 			if err != nil {
 				return err
 			}
-			return writeRelease(ioStreams.Out, rel, false)
+			return writeRelease(ioStreams.Out, rel, options.DryRun)
 		},
 		SilenceUsage:  true,
 		SilenceErrors: true,
@@ -161,8 +166,12 @@ func (o *InstallOptions) runInstall(ctx context.Context) (*release.Release, erro
 		return nil, err
 	}
 
+	if o.DryRun {
+		return dryRunResult, nil
+	}
+
 	// Extract the resource.Info objects from the helm chart crds (/crds folder) and the manifest
-	resources, err := helm.GetChartResourceInfo(chart, dryRunResult.Manifest, true, o.cfg.KubeClient)
+	resources, err := helm.GetChartResourceInfo(dryRunResult.Manifest, o.cfg.KubeClient)
 	if err != nil {
 		return nil, err
 	}
@@ -182,14 +191,14 @@ func (o *InstallOptions) runInstall(ctx context.Context) (*release.Release, erro
 	}
 
 	// Install CRDs
-	if len(crds) > 0 && !o.DryRun {
+	if len(crds) > 0 {
 		if err := helm.ApplyCRDs(helm.Create, crds, o.cfg); err != nil {
 			return nil, err
 		}
 	}
 
 	// Install chart
-	o.client.DryRun = o.DryRun               // Apply DryRun cli flags
+	o.client.DryRun = false                  // Apply DryRun cli flags
 	o.client.IsUpgrade = false               // Reset value to false
 	o.client.Atomic = true                   // If part of the install fails, also undo other installed resources
 	chartValues[installCRDsFlagName] = false // Do not render crds, as this might cause problems when uninstalling using helm
@@ -197,7 +206,12 @@ func (o *InstallOptions) runInstall(ctx context.Context) (*release.Release, erro
 	return o.client.Run(chart, chartValues)
 }
 
-func writeRelease(out io.Writer, rel *release.Release, debug bool) error {
+func writeRelease(out io.Writer, rel *release.Release, dryRun bool) error {
+	if dryRun {
+		fmt.Fprintf(out, "%s", rel.Manifest)
+		return nil
+	}
+
 	fmt.Fprintf(out, "NAME: %s\n", rel.Name)
 	if !rel.Info.LastDeployed.IsZero() {
 		fmt.Fprintf(out, "LAST DEPLOYED: %s\n", rel.Info.LastDeployed.Format(time.ANSIC))

cmd/ctl/pkg/install/uninstall.go

@@ -52,9 +52,11 @@ the provided chart and chart parameters and are used to determine what resources
 from the kubernetes cluster.
 
 Some example uses:
-	$ kubectl cert-manager uninstall -n cert-manager
+	$ kubectl cert-manager uninstall
 or
-	$ kubectl cert-manager uninstall -n cert-manager --remove-crds
+	$ kubectl cert-manager uninstall --remove-crds
+or
+	$ kubectl cert-manager uninstall -n new-cert-manager
 `
 
 type UninstallOptions struct {
@@ -71,7 +73,7 @@ type UninstallOptions struct {
 }
 
 // TODO: should wait for uninstall (https://github.com/helm/helm/pull/9702)
-func NewCmdUninstall(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory, kubeConfigFlags *genericclioptions.ConfigFlags) *cobra.Command {
+func NewCmdUninstall(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory) *cobra.Command {
 	settings := cli.New()
 	cfg := new(action.Configuration)
 
@@ -84,12 +86,16 @@ func NewCmdUninstall(ctx context.Context, ioStreams genericclioptions.IOStreams,
 		IOStreams:       ioStreams,
 	}
 
+	// Set default namespace cli flag value
+	defaults := make(map[string]string)
+	defaults["namespace"] = "cert-manager"
+
 	cmd := &cobra.Command{
 		Use:   "uninstall",
 		Short: "uninstall cert-manager",
 		Long:  uninstallDesc,
-		RunE: func(_ *cobra.Command, args []string) error {
-			if err := helm.CopyCliFlags(kubeConfigFlags, settings); err != nil {
+		RunE: func(cmd *cobra.Command, args []string) error {
+			if err := helm.CopyCliFlags(cmd.Root().PersistentFlags(), defaults, settings); err != nil {
 				return nil
 			}
 			options.installClient.Namespace = settings.Namespace()
@@ -152,7 +158,7 @@ func (o *UninstallOptions) runUninstall() error {
 	}
 
 	// Extract all resources that are present in the chart
-	resources, err := helm.GetChartResourceInfo(ch, dryRunResult.Manifest, o.RemoveCrds, o.cfg.KubeClient)
+	resources, err := helm.GetChartResourceInfo(dryRunResult.Manifest, o.cfg.KubeClient)
 	if err != nil {
 		return err
 	}

go.mod

@@ -46,7 +46,7 @@ require (
 	gomodules.xyz/jsonpatch/v2 v2.2.0
 	google.golang.org/api v0.20.0
 	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
-	helm.sh/helm/v3 v3.6.1
+	helm.sh/helm/v3 v3.6.2
 	k8s.io/api v0.21.1
 	k8s.io/apiextensions-apiserver v0.21.1
 	k8s.io/apimachinery v0.21.1

go.sum

@@ -1442,8 +1442,8 @@ gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81
 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
 gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0=
 gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
-helm.sh/helm/v3 v3.6.1 h1:TQ6q4pAatXr7qh2fbLcb0oNd0I3J7kv26oo5cExKTtc=
-helm.sh/helm/v3 v3.6.1/go.mod h1:mIIus8EOqj+obtycw3sidsR4ORr2aFDmXMSI3k+oeVY=
+helm.sh/helm/v3 v3.6.2 h1:7YbEhLC6AUgmcDoh5BGFUGNtR/o43pr0AIelSNjhuFU=
+helm.sh/helm/v3 v3.6.2/go.mod h1:mIIus8EOqj+obtycw3sidsR4ORr2aFDmXMSI3k+oeVY=
 honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

hack/build/repos.bzl

@@ -4392,8 +4392,8 @@ def go_repositories():
         build_file_generation = "on",
         build_file_proto_mode = "disable",
         importpath = "helm.sh/helm/v3",
-        sum = "h1:TQ6q4pAatXr7qh2fbLcb0oNd0I3J7kv26oo5cExKTtc=",
-        version = "v3.6.1",
+        sum = "h1:7YbEhLC6AUgmcDoh5BGFUGNtR/o43pr0AIelSNjhuFU=",
+        version = "v3.6.2",
     )
 
     go_repository(