move versionchecker tests to test/integration

Since this test requires setup before it can successfully run,
we define it as an integration test and move it here so that on a
fresh checkout a user can always run `go test ./pkg/...` and expect that
it would succeed.

Also involves:

- Exporting the VersionChecker and adding NewWithConfig to enable
  testing
- Some comment changes
- A change to the type returned by New(); see
  https://github.com/golang/go/wiki/CodeReviewComments#interfaces

Ideally I'd not add `NewFromClient` but I think it's the most minimal
change and is preferable to publicly exporting `VersionChecker.client`.

Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>

pkg/util/versionchecker/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
     name = "go_default_library",
@@ -23,32 +23,6 @@ go_library(
     ],
 )
 
-go_test(
-    name = "go_default_test",
-    srcs = [
-        "getpodfromtemplate_test.go",
-        "versionchecker_test.go",
-    ],
-    embed = [":go_default_library"],
-    embedsrcs = ["//pkg/util/versionchecker/testdata:test_manifests.tar"],  # keep
-    deps = [
-        "//pkg/util:go_default_library",
-        "@io_k8s_api//apps/v1:go_default_library",
-        "@io_k8s_api//core/v1:go_default_library",
-        "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1:go_default_library",
-        "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1beta1:go_default_library",
-        "@io_k8s_apimachinery//pkg/api/meta:go_default_library",
-        "@io_k8s_apimachinery//pkg/api/validation:go_default_library",
-        "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
-        "@io_k8s_apimachinery//pkg/apis/meta/v1/unstructured:go_default_library",
-        "@io_k8s_apimachinery//pkg/labels:go_default_library",
-        "@io_k8s_apimachinery//pkg/runtime:go_default_library",
-        "@io_k8s_cli_runtime//pkg/resource:go_default_library",
-        "@io_k8s_client_go//kubernetes/scheme:go_default_library",
-        "@io_k8s_sigs_controller_runtime//pkg/client/fake:go_default_library",
-    ],
-)
-
 filegroup(
     name = "package-srcs",
     srcs = glob(["**"]),
@@ -58,10 +32,7 @@ filegroup(
 
 filegroup(
     name = "all-srcs",
-    srcs = [
-        ":package-srcs",
-        "//pkg/util/versionchecker/testdata:all-srcs",
-    ],
+    srcs = [":package-srcs"],
     tags = ["automanaged"],
     visibility = ["//visibility:public"],
 )

pkg/util/versionchecker/fromcrd.go

@@ -25,7 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func (o *versionChecker) extractVersionFromCrd(ctx context.Context, crdName string) error {
+func (o *VersionChecker) extractVersionFromCrd(ctx context.Context, crdName string) error {
 	crdKey := client.ObjectKey{Name: crdName}
 
 	objv1 := &apiextensionsv1.CustomResourceDefinition{}
@@ -61,7 +61,7 @@ func (o *versionChecker) extractVersionFromCrd(ctx context.Context, crdName stri
 	return ErrCertManagerCRDsNotFound
 }
 
-func (o *versionChecker) extractVersionFromCrdv1(ctx context.Context, crd *apiextensionsv1.CustomResourceDefinition) error {
+func (o *VersionChecker) extractVersionFromCrdv1(ctx context.Context, crd *apiextensionsv1.CustomResourceDefinition) error {
 	if (crd.Spec.Conversion == nil) ||
 		(crd.Spec.Conversion.Webhook == nil) ||
 		(crd.Spec.Conversion.Webhook.ClientConfig == nil) ||
@@ -76,7 +76,7 @@ func (o *versionChecker) extractVersionFromCrdv1(ctx context.Context, crd *apiex
 	)
 }
 
-func (o *versionChecker) extractVersionFromCrdv1beta1(ctx context.Context, crd *apiextensionsv1beta1.CustomResourceDefinition) error {
+func (o *VersionChecker) extractVersionFromCrdv1beta1(ctx context.Context, crd *apiextensionsv1beta1.CustomResourceDefinition) error {
 	if (crd.Spec.Conversion == nil) ||
 		(crd.Spec.Conversion.WebhookClientConfig == nil) ||
 		(crd.Spec.Conversion.WebhookClientConfig.Service == nil) {

pkg/util/versionchecker/fromservice.go

@@ -27,7 +27,7 @@ import (
 
 var imageVersion = regexp.MustCompile(`^quay.io/jetstack/cert-manager-webhook:(v(?:\d+)\.(?:\d+)\.(?:\d+)(?:.*))$`)
 
-func (o *versionChecker) extractVersionFromService(
+func (o *VersionChecker) extractVersionFromService(
 	ctx context.Context,
 	namespace string,
 	serviceName string,

pkg/util/versionchecker/versionchecker.go

@@ -20,11 +20,10 @@ import (
 	"context"
 	"fmt"
 
+	errors "github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
-
-	errors "github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -63,20 +62,24 @@ type Interface interface {
 	Version(context.Context) (*Version, error)
 }
 
-type versionChecker struct {
+// VersionChecker implements a version checker using a controller-runtime client
+type VersionChecker struct {
 	client client.Client
 
 	versionSources map[string]string
 }
 
-// New returns a cert-manager version checker
-func New(restcfg *rest.Config, scheme *runtime.Scheme) (Interface, error) {
+// New returns a cert-manager version checker. Prefer New over NewFromClient
+// since New will ensure the scheme is configured correctly.
+func New(restcfg *rest.Config, scheme *runtime.Scheme) (*VersionChecker, error) {
 	if err := corev1.AddToScheme(scheme); err != nil {
 		return nil, err
 	}
+
 	if err := apiextensionsv1.AddToScheme(scheme); err != nil {
 		return nil, err
 	}
+
 	if err := apiextensionsv1beta1.AddToScheme(scheme); err != nil {
 		return nil, err
 	}
@@ -87,20 +90,30 @@ func New(restcfg *rest.Config, scheme *runtime.Scheme) (Interface, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &versionChecker{
+
+	return &VersionChecker{
 		client:         cl,
 		versionSources: map[string]string{},
 	}, nil
 }
 
-// Determine the installed cert-manager version. First, we start by looking for
+// NewFromClient initialises a VersionChecker using the given client. Prefer New
+// instead, which will ensure that the scheme on the client is configured correctly.
+func NewFromClient(cl client.Client) *VersionChecker {
+	return &VersionChecker{
+		client:         cl,
+		versionSources: map[string]string{},
+	}
+}
+
+// Version determines the installed cert-manager version. First, we look for
 // the "certificates.cert-manager.io" CRD and try to extract the version from that
 // resource's labels. Then, if it uses a webhook, that webhook service resource's
 // labels are checked for a label. Lastly the pods linked to the webhook its labels
 // are checked and the image tag is used to determine the version.
 // If no "certificates.cert-manager.io" CRD is found, the older
 // "certificates.certmanager.k8s.io" CRD is tried too.
-func (o *versionChecker) Version(ctx context.Context) (*Version, error) {
+func (o *VersionChecker) Version(ctx context.Context) (*Version, error) {
 	// Use the "certificates.cert-manager.io" CRD as a starting point
 	err := o.extractVersionFromCrd(ctx, certificatesCertManagerCrdName)
 
@@ -128,11 +141,11 @@ func (o *versionChecker) Version(ctx context.Context) (*Version, error) {
 	return version, err
 }
 
-// Try to determine the version of the cert-manager install based on all found
+// determineVersion attempts to determine the version of the cert-manager install based on all found
 // versions. The function tries to reduce the found versions to 1 correct version.
 // An error is returned if no sources were found or if multiple different versions
 // were found.
-func (o *versionChecker) determineVersion() (*Version, error) {
+func (o *VersionChecker) determineVersion() (*Version, error) {
 	if len(o.versionSources) == 0 {
 		return nil, ErrVersionNotDetected
 	}

test/integration/BUILD.bazel

@@ -15,6 +15,7 @@ filegroup(
         "//test/integration/ctl:all-srcs",
         "//test/integration/framework:all-srcs",
         "//test/integration/validation:all-srcs",
+        "//test/integration/versionchecker:all-srcs",
         "//test/integration/webhook:all-srcs",
     ],
     tags = ["automanaged"],

test/integration/versionchecker/BUILD.bazel

@@ -0,0 +1,44 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_test")
+
+go_test(
+    name = "go_default_test",
+    srcs = [
+        "getpodfromtemplate_test.go",
+        "versionchecker_test.go",
+    ],
+    embedsrcs = ["//test/integration/versionchecker/testdata:test_manifests.tar"],  # keep
+    deps = [
+        "//pkg/util:go_default_library",
+        "//pkg/util/versionchecker:go_default_library",
+        "@io_k8s_api//apps/v1:go_default_library",
+        "@io_k8s_api//core/v1:go_default_library",
+        "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1:go_default_library",
+        "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1beta1:go_default_library",
+        "@io_k8s_apimachinery//pkg/api/meta:go_default_library",
+        "@io_k8s_apimachinery//pkg/api/validation:go_default_library",
+        "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
+        "@io_k8s_apimachinery//pkg/apis/meta/v1/unstructured:go_default_library",
+        "@io_k8s_apimachinery//pkg/labels:go_default_library",
+        "@io_k8s_apimachinery//pkg/runtime:go_default_library",
+        "@io_k8s_cli_runtime//pkg/resource:go_default_library",
+        "@io_k8s_client_go//kubernetes/scheme:go_default_library",
+        "@io_k8s_sigs_controller_runtime//pkg/client/fake:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [
+        ":package-srcs",
+        "//test/integration/versionchecker/testdata:all-srcs",
+    ],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

test/integration/versionchecker/versionchecker_test.go

@@ -34,6 +34,8 @@ import (
 	"k8s.io/cli-runtime/pkg/resource"
 	kubernetesscheme "k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/jetstack/cert-manager/pkg/util/versionchecker"
 )
 
 //go:embed testdata/test_manifests.tar
@@ -109,7 +111,7 @@ func transformObjects(objects []runtime.RawExtension) ([]runtime.Object, error)
 	return transformedObjects, nil
 }
 
-func setupFakeVersionChecker(manifest io.Reader) (*versionChecker, error) {
+func setupFakeVersionChecker(manifest io.Reader) (*versionchecker.VersionChecker, error) {
 	scheme := runtime.NewScheme()
 
 	if err := kubernetesscheme.AddToScheme(scheme); err != nil {
@@ -136,10 +138,7 @@ func setupFakeVersionChecker(manifest io.Reader) (*versionChecker, error) {
 		WithRuntimeObjects(objs...).
 		Build()
 
-	return &versionChecker{
-		client:         cl,
-		versionSources: map[string]string{},
-	}, nil
+	return versionchecker.NewFromClient(cl), nil
 }
 
 func TestVersionChecker(t *testing.T) {