From 6997768d2c067f92273310ba0ae22ef0de3dd9ab 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 | 76 +++++++++++++------ .../client/cluster/cert_manager_test.go | 45 +++++------ 2 files changed, 74 insertions(+), 47 deletions(-) diff --git a/cmd/clusterctl/client/cluster/cert_manager.go b/cmd/clusterctl/client/cluster/cert_manager.go index 97bc535310d5..c0550ece6b1f 100644 --- a/cmd/clusterctl/client/cluster/cert_manager.go +++ b/cmd/clusterctl/client/cluster/cert_manager.go @@ -17,8 +17,10 @@ limitations under the License. package cluster import ( + "crypto/sha256" "context" "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" + certmanagerVersionLable = "helm.sh/chart" ) // CertManagerUpgradePlan defines the upgrade plan if cert-manager needs to be @@ -92,18 +84,58 @@ type certManagerClient struct { configClient config.Client proxy Proxy pollImmediateWaiter PollImmediateWaiter + embeddedCertManagerManifestVersion string + embeddedCertManagerManifestHash string } // Ensure certManagerClient implements the CertManagerClient interface. var _ CertManagerClient = &certManagerClient{} +func getManifestHash() string { + yaml, err := manifests.Asset(embeddedCertManagerManifestPath) + if err != nil { + return "" + } + return fmt.Sprintf("%x", sha256.Sum256(yaml)) +} + +// Images return the list of images required for installing the cert-manager. +func setManifestLabel(cm *certManagerClient) error { + // Gets the cert-manager objects from the embedded assets. + objs, err := cm.getManifestObjs() + if err != nil { + return err + } + + for i := range objs { + o := objs[i] + if o.GetKind() == "CustomResourceDefinition" { + labels := o.GetLabels() + version, ok := labels[certmanagerVersionLable] + if ok { + s := strings.Split(version, "-") + cm.embeddedCertManagerManifestVersion = s[2] + break + } + } + } + return nil +} + // newCertMangerClient returns a certManagerClient. func newCertMangerClient(configClient config.Client, proxy Proxy, pollImmediateWaiter PollImmediateWaiter) *certManagerClient { - return &certManagerClient{ + yaml, _ := manifests.Asset(embeddedCertManagerManifestPath) + hash := fmt.Sprintf("%x", sha256.Sum256(yaml)) + + cm := &certManagerClient{ configClient: configClient, proxy: proxy, pollImmediateWaiter: pollImmediateWaiter, + embeddedCertManagerManifestHash: hash, + } + setManifestLabel(cm) + return cm } // Images return the list of images required for installing the cert-manager. @@ -134,7 +166,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 +210,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 +233,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 +252,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 +286,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 +311,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 +324,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 +415,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..2e0a841b0515 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,22 +34,15 @@ 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)) - 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.") -} +const ( + testHash = "af8c08a8eb65d102ba98889a89f4ad1d3db5d302edb5b8f8f3e69bb992faa211" + testVersion = "v0.16.1" +) func Test_certManagerClient_getManifestObjects(t *testing.T) { tests := []struct { @@ -221,15 +213,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 +233,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 +241,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 +307,11 @@ func Test_shouldUpgrade(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + cm := &certManagerClient{ + pollImmediateWaiter: fakePollImmediateWaiter, + } - 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 +516,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { expectErr: false, expectedPlan: CertManagerUpgradePlan{ From: "v0.11.0", - To: embeddedCertManagerManifestVersion, + To: testVersion, ShouldUpgrade: true, }, }, @@ -543,7 +538,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) { expectErr: false, expectedPlan: CertManagerUpgradePlan{ From: "v0.16.0", - To: embeddedCertManagerManifestVersion, + To: testVersion, ShouldUpgrade: true, }, }, @@ -558,14 +553,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 +575,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, }, },