Merge pull request #14 from inteon/bugfix

BUGFIX: re-enable install test and fix flags which were not parsed correctly

pkg/factory/factory.go

@@ -31,15 +31,12 @@ import (
 	cmclient "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned"
 )
 
-var (
-	kubeConfigFlags = genericclioptions.NewConfigFlags(true)
-	factory         = util.NewFactory(kubeConfigFlags)
-)
-
 // Factory provides a set of clients and configurations to authenticate and
 // access a target Kubernetes cluster. Factory will ensure that its fields are
 // populated and valid during command execution.
 type Factory struct {
+	factory util.Factory
+
 	// Namespace is the namespace that the user has requested with the
 	// "--namespace" / "-n" flag. Defaults to "default" if the flag was not
 	// provided.
@@ -66,23 +63,32 @@ type Factory struct {
 
 // New returns a new Factory. The supplied command will have flags registered
 // for interacting with the Kubernetes access options. Factory will be
-// populated when the command is executed using the cobra PreRun. If a PreRun
+// populated when the command is executed using the cobra PreRunE. If a PreRunE
 // is already defined, it will be executed _after_ Factory has been populated,
 // making it available.
 func New(ctx context.Context, cmd *cobra.Command) *Factory {
 	f := new(Factory)
 
+	kubeConfigFlags := genericclioptions.NewConfigFlags(true)
+	f.factory = util.NewFactory(kubeConfigFlags)
+
 	kubeConfigFlags.AddFlags(cmd.Flags())
 	cmd.RegisterFlagCompletionFunc("namespace", validArgsListNamespaces(ctx, f))
 
-	// Setup a PreRun to populate the Factory. Catch the existing PreRun command
+	// Setup a PreRunE to populate the Factory. Catch the existing PreRunE command
 	// if one was defined, and execute it second.
-	existingPreRun := cmd.PreRun
-	cmd.PreRun = func(cmd *cobra.Command, args []string) {
-		util.CheckErr(f.complete())
-		if existingPreRun != nil {
-			existingPreRun(cmd, args)
+	// WARNING: Do not set PreRun on the command as cobra will not execute PreRun when
+	// PreRunE is set.
+	existingPreRunE := cmd.PreRunE
+	cmd.PreRunE = func(cmd *cobra.Command, args []string) error {
+		if err := f.complete(); err != nil {
+			return err
+		}
+
+		if existingPreRunE != nil {
+			return existingPreRunE(cmd, args)
 		}
+		return nil
 	}
 
 	return f
@@ -93,12 +99,12 @@ func New(ctx context.Context, cmd *cobra.Command) *Factory {
 func (f *Factory) complete() error {
 	var err error
 
-	f.Namespace, f.EnforceNamespace, err = factory.ToRawKubeConfigLoader().Namespace()
+	f.Namespace, f.EnforceNamespace, err = f.factory.ToRawKubeConfigLoader().Namespace()
 	if err != nil {
 		return err
 	}
 
-	f.RESTConfig, err = factory.ToRESTConfig()
+	f.RESTConfig, err = f.factory.ToRESTConfig()
 	if err != nil {
 		return err
 	}
@@ -113,7 +119,7 @@ func (f *Factory) complete() error {
 		return err
 	}
 
-	f.RESTClientGetter = factory
+	f.RESTClientGetter = f.factory
 
 	return nil
 }

pkg/install/helm/settings.go

@@ -46,7 +46,6 @@ func NewNormalisedEnvSettings() *NormalisedEnvSettings {
 		ActionConfiguration: &action.Configuration{},
 	}
 }
-
 func (n *NormalisedEnvSettings) Namespace() string {
 	return n.Factory.Namespace
 }
@@ -97,23 +96,24 @@ func (n *NormalisedEnvSettings) setupEnvSettings(ctx context.Context, cmd *cobra
 	{
 		// Add a PreRun hook to set the debug value to true if the log level is
 		// >= 3.
-		existingPreRun := cmd.PreRun
-		cmd.PreRun = func(cmd *cobra.Command, args []string) {
+		existingPreRunE := cmd.PreRunE
+		cmd.PreRunE = func(cmd *cobra.Command, args []string) error {
 			if n.logger.V(debugLogLevel).Enabled() {
 				n.EnvSettings.Debug = true
 			}
 
-			if existingPreRun != nil {
-				existingPreRun(cmd, args)
+			if existingPreRunE != nil {
+				return existingPreRunE(cmd, args)
 			}
+			return nil
 		}
 	}
 }
 
 func (n *NormalisedEnvSettings) InitActionConfiguration() error {
 	return n.ActionConfiguration.Init(
 		n.Factory.RESTClientGetter,
-		n.EnvSettings.Namespace(),
+		n.Factory.Namespace,
 		os.Getenv("HELM_DRIVER"),
 		func(format string, v ...interface{}) {
 			n.logger.Info(fmt.Sprintf(format, v...))

test/integration/ctl_install.go → test/integration/ctl_install_test.go

@@ -29,7 +29,7 @@ import (
 
 	"github.com/cert-manager/cmctl/v2/cmd"
 	"github.com/cert-manager/cmctl/v2/test/integration/install_framework"
-	"github.com/cert-manager/cmctl/v2/test/integration/internal/util"
+	logsapi "k8s.io/component-base/logs/api/v1"
 )
 
 func TestCtlInstall(t *testing.T) {
@@ -99,12 +99,11 @@ func executeCommandAndCheckOutput(
 	stdin := bytes.NewBufferString("")
 	stdout := bytes.NewBufferString("")
 
-	chartPath := util.GetTestPath("deploy", "charts", "cert-manager", "cert-manager.tgz")
+	logsapi.ResetForTest(nil)
 	cmd := cmd.NewCertManagerCtlCommand(ctx, stdin, stdout, stdout)
 	cmd.SetArgs(append([]string{
 		fmt.Sprintf("--kubeconfig=%s", kubeConfig),
 		"--wait=false",
-		fmt.Sprintf("--chart-name=%s", chartPath),
 		"x",
 		"install",
 	}, inputArgs...))

test/integration/install_framework/framework.go

@@ -20,11 +20,12 @@ import (
 	"os"
 	"testing"
 
+	"github.com/cert-manager/cert-manager/test/apiserver"
+	"github.com/go-logr/logr/testr"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
-
-	"github.com/cert-manager/cert-manager/test/apiserver"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 type TestInstallApiServer struct {
@@ -39,6 +40,8 @@ type TestInstallApiServer struct {
 type CleanupFunction func()
 
 func NewTestInstallApiServer(t *testing.T) (*TestInstallApiServer, CleanupFunction) {
+	log.SetLogger(testr.New(t))
+
 	env, stopFn := apiserver.RunBareControlPlane(t)
 
 	testUser, err := env.ControlPlane.AddUser(