From 5190aefc5ac27e6a4a9b254cbd27cb1716a200aa Mon Sep 17 00:00:00 2001 From: jichenjc Date: Tue, 29 Sep 2020 08:43:54 +0000 Subject: [PATCH] Remove hard code version and hash in manifest --- cmd/clusterctl/client/client_test.go | 4 +- cmd/clusterctl/client/cluster/cert_manager.go | 94 +++++++++++++------ .../client/cluster/cert_manager_test.go | 83 ++++++++++------ cmd/clusterctl/client/cluster/client.go | 6 +- cmd/clusterctl/client/init.go | 14 ++- cmd/clusterctl/client/upgrade.go | 13 ++- 6 files changed, 151 insertions(+), 63 deletions(-) diff --git a/cmd/clusterctl/client/client_test.go b/cmd/clusterctl/client/client_test.go index 3f323f7dbed3..a5ae967a1794 100644 --- a/cmd/clusterctl/client/client_test.go +++ b/cmd/clusterctl/client/client_test.go @@ -253,8 +253,8 @@ func (f fakeClusterClient) Proxy() cluster.Proxy { return f.fakeProxy } -func (f *fakeClusterClient) CertManager() cluster.CertManagerClient { - return f.certManager +func (f *fakeClusterClient) CertManager() (cluster.CertManagerClient, error) { + return f.certManager, nil } func (f fakeClusterClient) ProviderComponents() cluster.ComponentsClient { diff --git a/cmd/clusterctl/client/cluster/cert_manager.go b/cmd/clusterctl/client/cluster/cert_manager.go index 97bc535310d5..2e228d7d959f 100644 --- a/cmd/clusterctl/client/cluster/cert_manager.go +++ b/cmd/clusterctl/client/cluster/cert_manager.go @@ -18,7 +18,9 @@ package cluster import ( "context" + "crypto/sha256" "fmt" + "strings" "time" "github.com/pkg/errors" @@ -49,17 +51,7 @@ const ( certmanagerVersionAnnotation = "certmanager.clusterctl.cluster.x-k8s.io/version" certmanagerHashAnnotation = "certmanager.clusterctl.cluster.x-k8s.io/hash" - // NOTE: // 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. - embeddedCertManagerManifestVersion = "v0.16.1" - - // NOTE: The hash is used to ensure that when the cert-manager.yaml file is updated, - // the version number marker here is _also_ updated. - // 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 = "af8c08a8eb65d102ba98889a89f4ad1d3db5d302edb5b8f8f3e69bb992faa211" + certmanagerVersionLabel = "helm.sh/chart" ) // CertManagerUpgradePlan defines the upgrade plan if cert-manager needs to be @@ -89,21 +81,69 @@ type CertManagerClient interface { // certManagerClient implements CertManagerClient . type certManagerClient struct { - configClient config.Client - proxy Proxy - pollImmediateWaiter PollImmediateWaiter + configClient config.Client + proxy Proxy + pollImmediateWaiter PollImmediateWaiter + embeddedCertManagerManifestVersion string + embeddedCertManagerManifestHash string } // Ensure certManagerClient implements the CertManagerClient interface. var _ CertManagerClient = &certManagerClient{} -// newCertMangerClient returns a certManagerClient. -func newCertMangerClient(configClient config.Client, proxy Proxy, pollImmediateWaiter PollImmediateWaiter) *certManagerClient { - return &certManagerClient{ +func (cm *certManagerClient) setManifestHash() error { + yamlData, err := manifests.Asset(embeddedCertManagerManifestPath) + if err != nil { + return err + } + cm.embeddedCertManagerManifestHash = fmt.Sprintf("%x", sha256.Sum256(yamlData)) + return nil +} + +func (cm *certManagerClient) setManifestVersion() error { + // Gets the cert-manager objects from the embedded assets. + objs, err := cm.getManifestObjs() + if err != nil { + return err + } + found := false + + for i := range objs { + o := objs[i] + if o.GetKind() == "CustomResourceDefinition" { + labels := o.GetLabels() + version, ok := labels[certmanagerVersionLabel] + if ok { + s := strings.Split(version, "-") + cm.embeddedCertManagerManifestVersion = s[2] + found = true + break + } + } + } + if !found { + return errors.Errorf("Failed to detect cert-manager version by searching for label %s in all CRDs", certmanagerVersionLabel) + } + return nil +} + +// newCertManagerClient returns a certManagerClient. +func newCertManagerClient(configClient config.Client, proxy Proxy, pollImmediateWaiter PollImmediateWaiter) (*certManagerClient, error) { + cm := &certManagerClient{ configClient: configClient, proxy: proxy, pollImmediateWaiter: pollImmediateWaiter, } + err := cm.setManifestVersion() + if err != nil { + return nil, err + } + + err = cm.setManifestHash() + if err != nil { + return nil, err + } + return cm, nil } // Images return the list of images required for installing the cert-manager. @@ -134,7 +174,7 @@ func (cm *certManagerClient) EnsureInstalled() error { return nil } - log.Info("Installing cert-manager", "Version", embeddedCertManagerManifestVersion) + log.Info("Installing cert-manager", "Version", cm.embeddedCertManagerManifestVersion) return cm.install() } @@ -178,14 +218,14 @@ func (cm *certManagerClient) PlanUpgrade() (CertManagerUpgradePlan, error) { return CertManagerUpgradePlan{}, errors.Wrap(err, "failed get cert manager components") } - currentVersion, shouldUpgrade, err := shouldUpgrade(objs) + currentVersion, shouldUpgrade, err := cm.shouldUpgrade(objs) if err != nil { return CertManagerUpgradePlan{}, err } return CertManagerUpgradePlan{ From: currentVersion, - To: embeddedCertManagerManifestVersion, + To: cm.embeddedCertManagerManifestVersion, ShouldUpgrade: shouldUpgrade, }, nil } @@ -201,7 +241,7 @@ func (cm *certManagerClient) EnsureLatestVersion() error { return errors.Wrap(err, "failed get cert manager components") } - currentVersion, shouldUpgrade, err := shouldUpgrade(objs) + currentVersion, shouldUpgrade, err := cm.shouldUpgrade(objs) if err != nil { return err } @@ -220,7 +260,7 @@ func (cm *certManagerClient) EnsureLatestVersion() error { } // install the cert-manager version embedded in clusterctl - log.Info("Installing cert-manager", "Version", embeddedCertManagerManifestVersion) + log.Info("Installing cert-manager", "Version", cm.embeddedCertManagerManifestVersion) return cm.install() } @@ -254,7 +294,7 @@ func (cm *certManagerClient) deleteObjs(objs []unstructured.Unstructured) error return nil } -func shouldUpgrade(objs []unstructured.Unstructured) (string, bool, error) { +func (cm *certManagerClient) shouldUpgrade(objs []unstructured.Unstructured) (string, bool, error) { needUpgrade := false currentVersion := "" for i := range objs { @@ -279,7 +319,7 @@ func shouldUpgrade(objs []unstructured.Unstructured) (string, bool, error) { return "", false, errors.Wrapf(err, "failed to parse version for cert-manager component %s/%s", obj.GetKind(), obj.GetName()) } - c, err := objSemVersion.Compare(embeddedCertManagerManifestVersion) + c, err := objSemVersion.Compare(cm.embeddedCertManagerManifestVersion) if err != nil { return "", false, errors.Wrapf(err, "failed to compare version for cert-manager component %s/%s", obj.GetKind(), obj.GetName()) } @@ -292,7 +332,7 @@ func shouldUpgrade(objs []unstructured.Unstructured) (string, bool, error) { 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()[certmanagerHashAnnotation] - if !ok || objHash != embeddedCertManagerManifestHash { + if !ok || objHash != cm.embeddedCertManagerManifestHash { currentVersion = fmt.Sprintf("%s (%s)", objVersion, objHash) needUpgrade = true break @@ -383,8 +423,8 @@ func (cm *certManagerClient) createObj(obj unstructured.Unstructured) error { } // persist the version number of stored resources to make a // future enhancement to add upgrade support possible. - annotations[certmanagerVersionAnnotation] = embeddedCertManagerManifestVersion - annotations[certmanagerHashAnnotation] = embeddedCertManagerManifestHash + annotations[certmanagerVersionAnnotation] = cm.embeddedCertManagerManifestVersion + annotations[certmanagerHashAnnotation] = cm.embeddedCertManagerManifestHash obj.SetAnnotations(annotations) c, err := cm.proxy.NewClient() diff --git a/cmd/clusterctl/client/cluster/cert_manager_test.go b/cmd/clusterctl/client/cluster/cert_manager_test.go index 97e9d06a7392..ba944f8e8714 100644 --- a/cmd/clusterctl/client/cluster/cert_manager_test.go +++ b/cmd/clusterctl/client/cluster/cert_manager_test.go @@ -17,7 +17,6 @@ limitations under the License. package cluster import ( - "crypto/sha256" "fmt" "testing" "time" @@ -35,21 +34,28 @@ import ( "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" ) +const ( + // Those values are dummy for test only + expectedHash = "dummy-hash" + expectedVersion = "v0.11.2" +) + func Test_VersionMarkerUpToDate(t *testing.T) { - yaml, err := manifests.Asset(embeddedCertManagerManifestPath) - if err != nil { - t.Fatalf("Failed to get cert-manager.yaml asset data: %v", err) + pollImmediateWaiter := func(interval, timeout time.Duration, condition wait.ConditionFunc) error { + return nil } + fakeConfigClient := newFakeConfig("") + cm, err := newCertManagerClient(fakeConfigClient, nil, pollImmediateWaiter) - actualHash := fmt.Sprintf("%x", sha256.Sum256(yaml)) g := NewWithT(t) - g.Expect(actualHash).To(Equal(embeddedCertManagerManifestHash), "The cert-manager.yaml asset data has changed, but embeddedCertManagerManifestVersion and embeddedCertManagerManifestHash has not been updated.") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cm.embeddedCertManagerManifestVersion).ToNot(BeEmpty()) + g.Expect(cm.embeddedCertManagerManifestHash).ToNot(BeEmpty()) } func Test_certManagerClient_getManifestObjects(t *testing.T) { @@ -134,7 +140,9 @@ func Test_certManagerClient_getManifestObjects(t *testing.T) { } fakeConfigClient := newFakeConfig("") - cm := newCertMangerClient(fakeConfigClient, nil, pollImmediateWaiter) + cm, err := newCertManagerClient(fakeConfigClient, nil, pollImmediateWaiter) + g.Expect(err).ToNot(HaveOccurred()) + objs, err := cm.getManifestObjs() if tt.expectErr { @@ -180,7 +188,9 @@ func Test_GetTimeout(t *testing.T) { fakeConfigClient := newFakeConfig(tt.timeout) - cm := newCertMangerClient(fakeConfigClient, nil, pollImmediateWaiter) + cm, err := newCertManagerClient(fakeConfigClient, nil, pollImmediateWaiter) + g.Expect(err).ToNot(HaveOccurred()) + tm := cm.getWaitTimeout() g.Expect(tm).To(Equal(tt.want)) @@ -221,15 +231,15 @@ func Test_shouldUpgrade(t *testing.T) { Object: map[string]interface{}{ "metadata": map[string]interface{}{ "annotations": map[string]interface{}{ - certmanagerVersionAnnotation: embeddedCertManagerManifestVersion, - certmanagerHashAnnotation: embeddedCertManagerManifestHash, + certmanagerVersionAnnotation: expectedVersion, + certmanagerHashAnnotation: expectedHash, }, }, }, }, }, }, - wantVersion: embeddedCertManagerManifestVersion, + wantVersion: expectedVersion, want: false, wantErr: false, }, @@ -241,7 +251,7 @@ func Test_shouldUpgrade(t *testing.T) { Object: map[string]interface{}{ "metadata": map[string]interface{}{ "annotations": map[string]interface{}{ - certmanagerVersionAnnotation: embeddedCertManagerManifestVersion, + certmanagerVersionAnnotation: expectedVersion, certmanagerHashAnnotation: "foo", }, }, @@ -249,7 +259,7 @@ func Test_shouldUpgrade(t *testing.T) { }, }, }, - wantVersion: fmt.Sprintf("%s (%s)", embeddedCertManagerManifestVersion, "foo"), + wantVersion: fmt.Sprintf("%s (%s)", expectedVersion, "foo"), want: true, wantErr: false, }, @@ -315,8 +325,18 @@ func Test_shouldUpgrade(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + proxy := test.NewFakeProxy() + fakeConfigClient := newFakeConfig("") + pollImmediateWaiter := func(interval, timeout time.Duration, condition wait.ConditionFunc) error { + return nil + } + cm, err := newCertManagerClient(fakeConfigClient, proxy, pollImmediateWaiter) + // set dummy expected hash + cm.embeddedCertManagerManifestHash = expectedHash + cm.embeddedCertManagerManifestVersion = expectedVersion + g.Expect(err).ToNot(HaveOccurred()) - gotVersion, got, err := shouldUpgrade(tt.args.objs) + gotVersion, got, err := cm.shouldUpgrade(tt.args.objs) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return @@ -521,7 +541,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { expectErr: false, expectedPlan: CertManagerUpgradePlan{ From: "v0.11.0", - To: embeddedCertManagerManifestVersion, + To: expectedVersion, ShouldUpgrade: true, }, }, @@ -536,14 +556,14 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "cert-manager", Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, - Annotations: map[string]string{certmanagerVersionAnnotation: "v0.16.0", certmanagerHashAnnotation: "some-hash"}, + Annotations: map[string]string{certmanagerVersionAnnotation: "v0.10.2", certmanagerHashAnnotation: "some-hash"}, }, }, }, expectErr: false, expectedPlan: CertManagerUpgradePlan{ - From: "v0.16.0", - To: embeddedCertManagerManifestVersion, + From: "v0.10.2", + To: expectedVersion, ShouldUpgrade: true, }, }, @@ -558,14 +578,14 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "cert-manager", Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, - Annotations: map[string]string{certmanagerVersionAnnotation: embeddedCertManagerManifestVersion, certmanagerHashAnnotation: "some-other-hash"}, + Annotations: map[string]string{certmanagerVersionAnnotation: expectedVersion, certmanagerHashAnnotation: "some-other-hash"}, }, }, }, expectErr: false, expectedPlan: CertManagerUpgradePlan{ - From: "v0.16.1 (some-other-hash)", - To: embeddedCertManagerManifestVersion, + From: fmt.Sprintf("%s (some-other-hash)", expectedVersion), + To: expectedVersion, ShouldUpgrade: true, }, }, @@ -580,14 +600,14 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "cert-manager", Labels: map[string]string{clusterctlv1.ClusterctlCoreLabelName: "cert-manager"}, - Annotations: map[string]string{certmanagerVersionAnnotation: embeddedCertManagerManifestVersion, certmanagerHashAnnotation: embeddedCertManagerManifestHash}, + Annotations: map[string]string{certmanagerVersionAnnotation: expectedVersion, certmanagerHashAnnotation: expectedHash}, }, }, }, expectErr: false, expectedPlan: CertManagerUpgradePlan{ - From: embeddedCertManagerManifestVersion, - To: embeddedCertManagerManifestVersion, + From: expectedVersion, + To: expectedVersion, ShouldUpgrade: false, }, }, @@ -619,9 +639,18 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - cm := &certManagerClient{ - proxy: test.NewFakeProxy().WithObjs(tt.objs...), + proxy := test.NewFakeProxy().WithObjs(tt.objs...) + fakeConfigClient := newFakeConfig("") + pollImmediateWaiter := func(interval, timeout time.Duration, condition wait.ConditionFunc) error { + return nil } + cm, err := newCertManagerClient(fakeConfigClient, proxy, pollImmediateWaiter) + // set dummy expected hash + cm.embeddedCertManagerManifestHash = expectedHash + cm.embeddedCertManagerManifestVersion = expectedVersion + + g.Expect(err).ToNot(HaveOccurred()) + actualPlan, err := cm.PlanUpgrade() if tt.expectErr { g.Expect(err).To(HaveOccurred()) diff --git a/cmd/clusterctl/client/cluster/client.go b/cmd/clusterctl/client/cluster/client.go index 4c11efadda78..c6e1cca99371 100644 --- a/cmd/clusterctl/client/cluster/client.go +++ b/cmd/clusterctl/client/cluster/client.go @@ -63,7 +63,7 @@ type Client interface { // CertManager returns a CertManagerClient that can be user for // operating the cert-manager components in the cluster. - CertManager() CertManagerClient + CertManager() (CertManagerClient, error) // ProviderComponents returns a ComponentsClient object that can be user for // operating provider components objects in the management cluster (e.g. the CRDs, controllers, RBAC). @@ -117,8 +117,8 @@ func (c *clusterClient) Proxy() Proxy { return c.proxy } -func (c *clusterClient) CertManager() CertManagerClient { - return newCertMangerClient(c.configClient, c.proxy, c.pollImmediateWaiter) +func (c *clusterClient) CertManager() (CertManagerClient, error) { + return newCertManagerClient(c.configClient, c.proxy, c.pollImmediateWaiter) } func (c *clusterClient) ProviderComponents() ComponentsClient { diff --git a/cmd/clusterctl/client/init.go b/cmd/clusterctl/client/init.go index 8a1507453db3..a571468e9011 100644 --- a/cmd/clusterctl/client/init.go +++ b/cmd/clusterctl/client/init.go @@ -105,7 +105,12 @@ func (c *clusterctlClient) Init(options InitOptions) ([]Components, error) { } // Before installing the providers, ensure the cert-manager Webhook is in place. - if err := cluster.CertManager().EnsureInstalled(); err != nil { + certManager, err := cluster.CertManager() + if err != nil { + return nil, err + } + + if err := certManager.EnsureInstalled(); err != nil { return nil, err } @@ -156,8 +161,13 @@ func (c *clusterctlClient) InitImages(options InitOptions) ([]string, error) { return nil, err } + certManager, err := cluster.CertManager() + if err != nil { + return nil, err + } + // Gets the list of container images required for the cert-manager (if not already installed). - images, err := cluster.CertManager().Images() + images, err := certManager.Images() if err != nil { return nil, err } diff --git a/cmd/clusterctl/client/upgrade.go b/cmd/clusterctl/client/upgrade.go index 5d6bcf58f5cd..6235b3516daa 100644 --- a/cmd/clusterctl/client/upgrade.go +++ b/cmd/clusterctl/client/upgrade.go @@ -38,7 +38,11 @@ func (c *clusterctlClient) PlanCertManagerUpgrade(options PlanUpgradeOptions) (C return CertManagerUpgradePlan{}, err } - plan, err := cluster.CertManager().PlanUpgrade() + certManager, err := cluster.CertManager() + if err != nil { + return CertManagerUpgradePlan{}, err + } + plan, err := certManager.PlanUpgrade() return CertManagerUpgradePlan(plan), err } @@ -122,7 +126,12 @@ func (c *clusterctlClient) ApplyUpgrade(options ApplyUpgradeOptions) error { // 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 { + certManager, err := clusterClient.CertManager() + if err != nil { + return err + } + + if err := certManager.EnsureLatestVersion(); err != nil { return err }