From 1ac5584dadf11d191407c24068f721237540223f Mon Sep 17 00:00:00 2001 From: jichenjc Date: Tue, 29 Sep 2020 08:43:54 +0000 Subject: [PATCH] Remove hard code manifest version and hash --- cmd/clusterctl/client/cluster/cert_manager.go | 95 +++++++++++++------ .../client/cluster/cert_manager_test.go | 58 ++++++----- 2 files changed, 100 insertions(+), 53 deletions(-) diff --git a/cmd/clusterctl/client/cluster/cert_manager.go b/cmd/clusterctl/client/cluster/cert_manager.go index 97bc535310d5..0ba4a4faa0b2 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,68 @@ 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{} +func (cm *certManagerClient) getManifestHash() (string, error) { + yamlData, err := manifests.Asset(embeddedCertManagerManifestPath) + if err != nil { + return "", err + } + return fmt.Sprintf("%x", sha256.Sum256(yamlData)), nil +} + +func (cm *certManagerClient) getManifestVersion() 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 found Label %s in all CRDs", certmanagerVersionLabel) + } + return nil +} + // newCertMangerClient returns a certManagerClient. func newCertMangerClient(configClient config.Client, proxy Proxy, pollImmediateWaiter PollImmediateWaiter) *certManagerClient { - return &certManagerClient{ - configClient: configClient, - proxy: proxy, - pollImmediateWaiter: pollImmediateWaiter, + yaml, _ := manifests.Asset(embeddedCertManagerManifestPath) + hash := fmt.Sprintf("%x", sha256.Sum256(yaml)) + + cm := &certManagerClient{ + configClient: configClient, + proxy: proxy, + pollImmediateWaiter: pollImmediateWaiter, + embeddedCertManagerManifestHash: hash, + } + err := cm.getManifestVersion() + if err != nil { + log := logf.Log + log.Info("failed to set Manifest Version") } + return cm } // Images return the list of images required for installing the cert-manager. @@ -134,7 +173,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 +217,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 := shouldUpgrade(cm, objs) if err != nil { return CertManagerUpgradePlan{}, err } return CertManagerUpgradePlan{ From: currentVersion, - To: embeddedCertManagerManifestVersion, + To: cm.embeddedCertManagerManifestVersion, ShouldUpgrade: shouldUpgrade, }, nil } @@ -201,7 +240,7 @@ func (cm *certManagerClient) EnsureLatestVersion() error { return errors.Wrap(err, "failed get cert manager components") } - currentVersion, shouldUpgrade, err := shouldUpgrade(objs) + currentVersion, shouldUpgrade, err := shouldUpgrade(cm, objs) if err != nil { return err } @@ -220,7 +259,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 +293,7 @@ func (cm *certManagerClient) deleteObjs(objs []unstructured.Unstructured) error return nil } -func shouldUpgrade(objs []unstructured.Unstructured) (string, bool, error) { +func shouldUpgrade(cm *certManagerClient, objs []unstructured.Unstructured) (string, bool, error) { needUpgrade := false currentVersion := "" for i := range objs { @@ -279,7 +318,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 +331,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 +422,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..85d76c67c10f 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,23 +34,24 @@ 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" ) 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) - } - - actualHash := fmt.Sprintf("%x", sha256.Sum256(yaml)) + proxy := test.NewFakeProxy() + fakeConfigClient := newFakeConfig("") + cm := newCertMangerClient(fakeConfigClient, proxy, fakePollImmediateWaiter) 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("").To(Equal(cm.embeddedCertManagerManifestVersion), "The cert-manager.yaml asset data has changed, but embeddedCertManagerManifestVersion and embeddedCertManagerManifestHash has not been updated.") } +const ( + testHash = "af8c08a8eb65d102ba98889a89f4ad1d3db5d302edb5b8f8f3e69bb992faa211" + testVersion = "v0.16.1" +) + func Test_certManagerClient_getManifestObjects(t *testing.T) { tests := []struct { name string @@ -221,15 +221,15 @@ func Test_shouldUpgrade(t *testing.T) { Object: map[string]interface{}{ "metadata": map[string]interface{}{ "annotations": map[string]interface{}{ - certmanagerVersionAnnotation: embeddedCertManagerManifestVersion, - certmanagerHashAnnotation: embeddedCertManagerManifestHash, + certmanagerVersionAnnotation: testVersion, + certmanagerHashAnnotation: testHash, }, }, }, }, }, }, - wantVersion: embeddedCertManagerManifestVersion, + wantVersion: testVersion, want: false, wantErr: false, }, @@ -241,7 +241,7 @@ func Test_shouldUpgrade(t *testing.T) { Object: map[string]interface{}{ "metadata": map[string]interface{}{ "annotations": map[string]interface{}{ - certmanagerVersionAnnotation: embeddedCertManagerManifestVersion, + certmanagerVersionAnnotation: testVersion, certmanagerHashAnnotation: "foo", }, }, @@ -249,7 +249,7 @@ func Test_shouldUpgrade(t *testing.T) { }, }, }, - wantVersion: fmt.Sprintf("%s (%s)", embeddedCertManagerManifestVersion, "foo"), + wantVersion: fmt.Sprintf("%s (%s)", testVersion, "foo"), want: true, wantErr: false, }, @@ -315,8 +315,13 @@ 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("") + cm := newCertMangerClient(fakeConfigClient, proxy, fakePollImmediateWaiter) + cm.embeddedCertManagerManifestVersion = testVersion + cm.embeddedCertManagerManifestHash = testHash - gotVersion, got, err := shouldUpgrade(tt.args.objs) + gotVersion, got, err := shouldUpgrade(cm, tt.args.objs) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return @@ -521,7 +526,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { expectErr: false, expectedPlan: CertManagerUpgradePlan{ From: "v0.11.0", - To: embeddedCertManagerManifestVersion, + To: testVersion, ShouldUpgrade: true, }, }, @@ -543,7 +548,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { expectErr: false, expectedPlan: CertManagerUpgradePlan{ From: "v0.16.0", - To: embeddedCertManagerManifestVersion, + To: testVersion, ShouldUpgrade: true, }, }, @@ -558,14 +563,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: testVersion, certmanagerHashAnnotation: "some-other-hash"}, }, }, }, expectErr: false, expectedPlan: CertManagerUpgradePlan{ From: "v0.16.1 (some-other-hash)", - To: embeddedCertManagerManifestVersion, + To: "v0.16.1", ShouldUpgrade: true, }, }, @@ -580,14 +585,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: testVersion, certmanagerHashAnnotation: testHash}, }, }, }, expectErr: false, expectedPlan: CertManagerUpgradePlan{ - From: embeddedCertManagerManifestVersion, - To: embeddedCertManagerManifestVersion, + From: testVersion, + To: testVersion, ShouldUpgrade: false, }, }, @@ -619,9 +624,12 @@ 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("") + cm := newCertMangerClient(fakeConfigClient, proxy, fakePollImmediateWaiter) + cm.embeddedCertManagerManifestVersion = testVersion + cm.embeddedCertManagerManifestHash = testHash + actualPlan, err := cm.PlanUpgrade() if tt.expectErr { g.Expect(err).To(HaveOccurred())