diff --git a/cmd/clusterctl/client/cluster/cert_manager.go b/cmd/clusterctl/client/cluster/cert_manager.go index 170a6a067d88..57efdb41f9c8 100644 --- a/cmd/clusterctl/client/cluster/cert_manager.go +++ b/cmd/clusterctl/client/cluster/cert_manager.go @@ -23,6 +23,8 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/version" + "sigs.k8s.io/controller-runtime/pkg/client" clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3" "sigs.k8s.io/cluster-api/cmd/clusterctl/client/config" @@ -33,20 +35,6 @@ import ( utilyaml "sigs.k8s.io/cluster-api/util/yaml" ) -var ( - // This is declared as a variable mapping a version number to the hash of the - // embedded cert-manager.yaml file. - // The hash is used to ensure that when the cert-manager.yaml file is updated, - // the version number marker here is _also_ updated. - // If the cert-manager.yaml asset is modified, this line **MUST** be updated - // accordingly else future upgrades of the cert-manager component will not - // be possible, as there'll be no record of the version installed. - // You can either generate the SHA256 hash of the file, or alternatively - // run `go test` against this package. THe Test_VersionMarkerUpToDate will output - // the expected hash if it does not match the hash here. - embeddedCertManagerManifestVersion = map[string]string{"v0.16.0": "5770f5f01c10a902355b3522b8ce44508ebb6ec88955efde9a443afe5b3969d7"} -) - const ( embeddedCertManagerManifestPath = "cmd/clusterctl/config/assets/cert-manager.yaml" embeddedCertManagerTestResourcesManifestPath = "cmd/clusterctl/config/assets/cert-manager-test-resources.yaml" @@ -56,6 +44,21 @@ const ( certManagerImageComponent = "cert-manager" timeoutConfigKey = "cert-manager-timeout" + + certmanagerVersionAnnotations = "certmanager.clusterctl.cluster.x-k8s.io/version" + certmanagerHashAnnotations = "certmanager.clusterctl.cluster.x-k8s.io/hash" + + embeddedCertManagerManifestVersion = "v0.16.0" + + // The hash is used to ensure that when the cert-manager.yaml file is updated, + // the version number marker here is _also_ updated. + // If the cert-manager.yaml asset is modified, this line **MUST** be updated + // accordingly else future upgrades of the cert-manager component will not + // be possible, as there'll be no record of the version installed. + // You can either generate the SHA256 hash of the file, or alternatively + // run `go test` against this package. THe Test_VersionMarkerUpToDate will output + // the expected hash if it does not match the hash here. + embeddedCertManagerManifestHash = "5770f5f01c10a902355b3522b8ce44508ebb6ec88955efde9a443afe5b3969d7" ) // CertManagerClient has methods to work with cert-manager components in the cluster. @@ -64,6 +67,10 @@ type CertManagerClient interface { // This is required to install a new provider. EnsureInstalled() error + // EnsureLatestVersion checks the cert-manager version currently installed, and if it is + // older than the version currently embedded in clusterctl, upgrades it. + EnsureLatestVersion() error + // Images return the list of images required for installing the cert-manager. Images() ([]string, error) } @@ -115,21 +122,22 @@ func (cm *certManagerClient) EnsureInstalled() error { return nil } + log.Info("Installing cert-manager") + return cm.install() +} + +func (cm *certManagerClient) install() error { // Gets the cert-manager objects from the embedded assets. objs, err := cm.getManifestObjs() if err != nil { return err } - log.Info("Installing cert-manager") - // Install all cert-manager manifests createCertManagerBackoff := newWriteBackoff() objs = utilresource.SortForCreate(objs) for i := range objs { o := objs[i] - log.V(5).Info("Creating", logf.UnstructuredToValues(o)...) - // Create the Kubernetes object. // Nb. The operation is wrapped in a retry loop to make ensureCerts more resilient to unexpected conditions. if err := retryWithExponentialBackoff(createCertManagerBackoff, func() error { @@ -147,6 +155,108 @@ func (cm *certManagerClient) EnsureInstalled() error { return nil } +// EnsureLatestVersion checks the cert-manager version currently installed, and if it is +// older than the version currently embedded in clusterctl, upgrades it. +func (cm *certManagerClient) EnsureLatestVersion() error { + log := logf.Log + log.Info("Checking cert-manager version...") + + objs, err := cm.proxy.ListResources(map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}) + if err != nil { + return errors.Wrap(err, "failed get cert manager components") + } + + shouldUpgrade, err := shouldUpgrade(objs) + if err != nil { + return err + } + + if !shouldUpgrade { + log.Info("Cert-manager is already up to date") + return nil + } + log.Info("Upgrading cert-manager") + + // delete the cert-manager version currently installed (because it should be upgraded); + // NOTE: CRDs, and namespace are preserved in order to avoid deletion of user objects; + // web-hooks are preserved to avoid a user attempting to CREATE a cert-manager resource while the upgrade is in progress. + if err := cm.deleteObjs(objs); err != nil { + return err + } + + // install the cert-manager version embedded in clusterctl + return cm.install() +} + +func (cm *certManagerClient) deleteObjs(objs []unstructured.Unstructured) error { + for i := range objs { + obj := objs[i] + + // CRDs, and namespace are preserved in order to avoid deletion of user objects; + // web-hooks are preserved to avoid a user attempting to CREATE a cert-manager resource while the upgrade is in progress. + if obj.GetKind() == "CustomResourceDefinition" || + obj.GetKind() == "Namespace" || + obj.GetKind() == "MutatingWebhookConfiguration" || + obj.GetKind() == "ValidatingWebhookConfiguration" { + continue + } + + if err := cm.deleteObj(obj); err != nil { + // tolerate NotFound errors when deleting the resources + if apierrors.IsNotFound(err) { + continue + } + return err + } + } + return nil +} + +func shouldUpgrade(objs []unstructured.Unstructured) (bool, error) { + needUpgrade := false + for i := range objs { + obj := objs[i] + + // if no version then upgrade (v0.11.0) + objVersion, ok := obj.GetAnnotations()[certmanagerVersionAnnotations] + if !ok { + // if there is no version annotation, this means the obj is cert-manager v0.11.0 (installed with older version of clusterctl) + needUpgrade = true + break + } + + objSemVersion, err := version.ParseSemantic(objVersion) + if err != nil { + return false, errors.Wrapf(err, "failed to parse version for cert-manager component %s/%s", obj.GetKind(), obj.GetName()) + } + + c, err := objSemVersion.Compare(embeddedCertManagerManifestVersion) + if err != nil { + return false, errors.Wrapf(err, "failed to compare version for cert-manager component %s/%s", obj.GetKind(), obj.GetName()) + } + + switch { + case c < 0: + // if version < current, then upgrade + needUpgrade = true + break + case c == 0: + // if version == current, check the manifest hash; if it does not exists or if it is different, then upgrade + objHash, ok := obj.GetAnnotations()[certmanagerHashAnnotations] + if !ok || objHash != embeddedCertManagerManifestHash { + needUpgrade = true + break + } + // otherwise we are already at the latest version + continue + case c > 0: + // the installed version is higher than the one embedded in clusterctl, so we are ok + continue + } + } + return needUpgrade, nil +} + func (cm *certManagerClient) getWaitTimeout() time.Duration { log := logf.Log @@ -201,37 +311,62 @@ func getTestResourcesManifestObjs() ([]unstructured.Unstructured, error) { return objs, nil } -func (cm *certManagerClient) createObj(o unstructured.Unstructured) error { - c, err := cm.proxy.NewClient() - if err != nil { - return err - } +func (cm *certManagerClient) createObj(obj unstructured.Unstructured) error { + log := logf.Log - labels := o.GetLabels() + labels := obj.GetLabels() if labels == nil { labels = map[string]string{} } labels[clusterctlv1.ClusterctlCoreLabelName] = "cert-manager" - o.SetLabels(labels) + obj.SetLabels(labels) // persist version marker information as annotations to avoid character and length // restrictions on label values. - annotations := o.GetAnnotations() + annotations := obj.GetAnnotations() if annotations == nil { annotations = map[string]string{} } // persist the version number of stored resources to make a // future enhancement to add upgrade support possible. - version, hash := embeddedCertManagerVersion() - annotations["certmanager.clusterctl.cluster.x-k8s.io/version"] = version - annotations["certmanager.clusterctl.cluster.x-k8s.io/hash"] = hash - o.SetAnnotations(annotations) - - if err = c.Create(ctx, &o); err != nil { - if apierrors.IsAlreadyExists(err) { - return nil + annotations[certmanagerVersionAnnotations] = embeddedCertManagerManifestVersion + annotations[certmanagerHashAnnotations] = embeddedCertManagerManifestHash + obj.SetAnnotations(annotations) + + c, err := cm.proxy.NewClient() + if err != nil { + return err + } + + // check if the component already exists, and eventually update it; otherwise create it + // NOTE: This is required because this func is used also for upgrading cert-manager and during upgrades + // some objects of the previews release are preserved in order to avoid to delete user data (e.g. CRDs). + currentR := &unstructured.Unstructured{} + currentR.SetGroupVersionKind(obj.GroupVersionKind()) + + key := client.ObjectKey{ + Namespace: obj.GetNamespace(), + Name: obj.GetName(), + } + if err := c.Get(ctx, key, currentR); err != nil { + if !apierrors.IsNotFound(err) { + return errors.Wrapf(err, "failed to get cert-manager object %s, %s/%s", obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName()) } - return errors.Wrapf(err, "failed to create cert-manager component: %s, %s/%s", o.GroupVersionKind(), o.GetNamespace(), o.GetName()) + + // if it does not exists, create the component + log.V(5).Info("Creating", logf.UnstructuredToValues(obj)...) + if err := c.Create(ctx, &obj); err != nil { + return errors.Wrapf(err, "failed to create cert-manager component %s, %s/%s", obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName()) + } + return nil + } + + // otherwise update the component + // NB. we are using client.Merge PatchOption so the new objects gets compared with the current one server side + log.V(5).Info("Patching", logf.UnstructuredToValues(obj)...) + obj.SetResourceVersion(currentR.GetResourceVersion()) + if err := c.Patch(ctx, &obj, client.Merge); err != nil { + return errors.Wrapf(err, "failed to patch cert-manager component %s, %s/%s", obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName()) } return nil } @@ -261,7 +396,9 @@ func (cm *certManagerClient) deleteObj(obj unstructured.Unstructured) error { func (cm *certManagerClient) waitForAPIReady(ctx context.Context, retry bool) error { log := logf.Log // Waits for for the cert-manager web-hook to be available. - log.Info("Waiting for cert-manager to be available...") + if retry { + log.Info("Waiting for cert-manager to be available...") + } testObjs, err := getTestResourcesManifestObjs() if err != nil { @@ -270,20 +407,16 @@ func (cm *certManagerClient) waitForAPIReady(ctx context.Context, retry bool) er for i := range testObjs { o := testObjs[i] - log.V(5).Info("Creating", logf.UnstructuredToValues(o)...) // Create the Kubernetes object. // This is wrapped with a retry as the cert-manager API may not be available // yet, so we need to keep retrying until it is. if err := cm.pollImmediateWaiter(waitCertManagerInterval, cm.getWaitTimeout(), func() (bool, error) { if err := cm.createObj(o); err != nil { - log.V(5).Info("Failed to create cert-manager test resource", logf.UnstructuredToValues(o)...) - // If retrying is disabled, return the error here. if !retry { return false, err } - return false, nil } return true, nil @@ -305,16 +438,3 @@ func (cm *certManagerClient) waitForAPIReady(ctx context.Context, retry bool) er return nil } - -// returns the version number and hash of the cert-manager manifest embedded -// into the clusterctl binary -func embeddedCertManagerVersion() (version, hash string) { - if len(embeddedCertManagerManifestVersion) != 1 { - panic("embeddedCertManagerManifestVersion MUST only have one entry") - } - for version, hash := range embeddedCertManagerManifestVersion { - return version, hash - } - // unreachable - return "", "" -} diff --git a/cmd/clusterctl/client/cluster/cert_manager_test.go b/cmd/clusterctl/client/cluster/cert_manager_test.go index fe63899f3b05..46756452e685 100644 --- a/cmd/clusterctl/client/cluster/cert_manager_test.go +++ b/cmd/clusterctl/client/cluster/cert_manager_test.go @@ -24,12 +24,21 @@ import ( . "github.com/onsi/gomega" admissionregistration "k8s.io/api/admissionregistration/v1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" + clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3" "sigs.k8s.io/cluster-api/cmd/clusterctl/client/config" manifests "sigs.k8s.io/cluster-api/cmd/clusterctl/config" "sigs.k8s.io/cluster-api/cmd/clusterctl/internal/scheme" "sigs.k8s.io/cluster-api/cmd/clusterctl/internal/test" + "sigs.k8s.io/controller-runtime/pkg/client" ) func Test_VersionMarkerUpToDate(t *testing.T) { @@ -39,14 +48,12 @@ func Test_VersionMarkerUpToDate(t *testing.T) { } actualHash := fmt.Sprintf("%x", sha256.Sum256(yaml)) - _, embeddedHash := embeddedCertManagerVersion() - if actualHash != embeddedHash { - t.Errorf("The cert-manager.yaml asset data has changed, but the version marker embeddedCertManagerManifestVersion has not been updated. Expected hash to be: %s", actualHash) + if actualHash != embeddedCertManagerManifestHash { + t.Errorf("The cert-manager.yaml asset data has changed, but the version marker embeddedCertManagerManifestHash has not been updated. Expected hash to be: %s", actualHash) } } func Test_certManagerClient_getManifestObjects(t *testing.T) { - tests := []struct { name string expectErr bool @@ -143,7 +150,6 @@ func Test_certManagerClient_getManifestObjects(t *testing.T) { } func Test_GetTimeout(t *testing.T) { - pollImmediateWaiter := func(interval, timeout time.Duration, condition wait.ConditionFunc) error { return nil } @@ -184,6 +190,327 @@ func Test_GetTimeout(t *testing.T) { } +func Test_shouldUpgrade(t *testing.T) { + type args struct { + objs []unstructured.Unstructured + } + tests := []struct { + name string + args args + want bool + wantErr bool + }{ + { + name: "Version is not defined (e.g. cluster created with clusterctl < v0.3.9), should upgrade", + args: args{ + objs: []unstructured.Unstructured{ + { + Object: map[string]interface{}{}, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "Version & hash are equal, should not upgrade", + args: args{ + objs: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + certmanagerVersionAnnotations: embeddedCertManagerManifestVersion, + certmanagerHashAnnotations: embeddedCertManagerManifestHash, + }, + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "Version is equal, hash is different, should upgrade", + args: args{ + objs: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + certmanagerVersionAnnotations: embeddedCertManagerManifestVersion, + certmanagerHashAnnotations: "foo", + }, + }, + }, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "Version is older, should upgrade", + args: args{ + objs: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + certmanagerVersionAnnotations: "v0.11.0", + }, + }, + }, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "Version is newer, should not upgrade", + args: args{ + objs: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + certmanagerVersionAnnotations: "v100.0.0", + }, + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := shouldUpgrade(tt.args.objs) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func Test_certManagerClient_deleteObjs(t *testing.T) { + type fields struct { + objs []runtime.Object + } + type args struct { + objs *unstructured.UnstructuredList + } + tests := []struct { + name string + fields fields + want []string // Define the list of "Kind, Namespace/Name" that should still exist after delete + wantErr bool + }{ + { + name: "CRD should not be deleted", + fields: fields{ + objs: []runtime.Object{ + &apiextensionsv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "CustomResourceDefinition", + APIVersion: apiextensionsv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, + }, + }, + }, + }, + want: []string{"CustomResourceDefinition, /foo"}, + wantErr: false, + }, + { + name: "Namespace should not be deleted", + fields: fields{ + objs: []runtime.Object{ + &corev1.Namespace{ + TypeMeta: metav1.TypeMeta{ + Kind: "Namespace", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, + }, + }, + }, + }, + want: []string{"Namespace, /foo"}, + wantErr: false, + }, + { + name: "MutatingWebhookConfiguration should not be deleted", + fields: fields{ + objs: []runtime.Object{ + &admissionregistration.MutatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "MutatingWebhookConfiguration", + APIVersion: admissionregistration.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, + }, + }, + }, + }, + want: []string{"MutatingWebhookConfiguration, /foo"}, + wantErr: false, + }, + { + name: "ValidatingWebhookConfiguration should not be deleted", + fields: fields{ + objs: []runtime.Object{ + &admissionregistration.ValidatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "ValidatingWebhookConfiguration", + APIVersion: admissionregistration.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, + }, + }, + }, + }, + want: []string{"ValidatingWebhookConfiguration, /foo"}, + wantErr: false, + }, + { + name: "Other resources should be deleted", + fields: fields{ + objs: []runtime.Object{ + &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, + }, + }, + &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: appsv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, + }, + }, + }, + }, + want: nil, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + proxy := test.NewFakeProxy().WithObjs(tt.fields.objs...) + cm := &certManagerClient{ + proxy: proxy, + } + + objBefore, err := proxy.ListResources(map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}) + g.Expect(err).ToNot(HaveOccurred()) + + err = cm.deleteObjs(objBefore) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + for _, obj := range tt.fields.objs { + accessor, err := meta.Accessor(obj) + g.Expect(err).ToNot(HaveOccurred()) + + objShouldStillExist := false + for _, want := range tt.want { + if fmt.Sprintf("%s, %s/%s", obj.GetObjectKind().GroupVersionKind().Kind, accessor.GetNamespace(), accessor.GetName()) == want { + objShouldStillExist = true + } + } + + cl, err := proxy.NewClient() + g.Expect(err).ToNot(HaveOccurred()) + + key, err := client.ObjectKeyFromObject(obj) + g.Expect(err).ToNot(HaveOccurred()) + + err = cl.Get(ctx, key, obj) + switch objShouldStillExist { + case true: + g.Expect(err).ToNot(HaveOccurred()) + case false: + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + } + } + }) + } +} + +func Test_certManagerClient_EnsureLatestVersion(t *testing.T) { + type fields struct { + proxy Proxy + } + tests := []struct { + name string + fields fields + wantErr bool + }{ + { + name: "", + fields: fields{ + proxy: test.NewFakeProxy().WithObjs( + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + }, + ), + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + cm := &certManagerClient{ + proxy: tt.fields.proxy, + } + + err := cm.EnsureLatestVersion() + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} + func newFakeConfig(timeout string) fakeConfigClient { fakeReader := test.NewFakeReader().WithVar("cert-manager-timeout", timeout) diff --git a/cmd/clusterctl/client/upgrade.go b/cmd/clusterctl/client/upgrade.go index f1aee7b1de37..79488ae04a2e 100644 --- a/cmd/clusterctl/client/upgrade.go +++ b/cmd/clusterctl/client/upgrade.go @@ -99,6 +99,14 @@ func (c *clusterctlClient) ApplyUpgrade(options ApplyUpgradeOptions) error { return err } + // Ensures the latest version of cert-manager. + // NOTE: it is safe to upgrade to latest version of cert-manager given that it provides + // conversion web-hooks around Issuer/Certificate kinds, so installing an older versions of providers + // should continue to work with the latest cert-manager. + if err := clusterClient.CertManager().EnsureLatestVersion(); err != nil { + return err + } + // The management group name is derived from the core provider name, so now // convert the reference back into a coreProvider. coreUpgradeItem, err := parseUpgradeItem(options.ManagementGroup, clusterctlv1.CoreProviderType)