Skip to content

Commit

Permalink
Remove hard code manifest version and hash
Browse files Browse the repository at this point in the history
  • Loading branch information
@jichenjc
jichenjc committed Oct 21, 2020
1 parent d66eed9 commit c1e7c59
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 44 deletions.
79 changes: 57 additions & 22 deletions cmd/clusterctl/client/cluster/cert_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ limitations under the License.
package cluster

import (
"crypto/sha256"
"context"
"fmt"
"strings"
"time"

"github.com/pkg/errors"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -92,18 +84,61 @@ type certManagerClient struct {
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{
yaml, _ := manifests.Asset(embeddedCertManagerManifestPath)
hash := fmt.Sprintf("%x", sha256.Sum256(yaml))

cm := &certManagerClient{
configClient: configClient,
proxy: proxy,
pollImmediateWaiter: pollImmediateWaiter,
embeddedCertManagerManifestHash: hash,
}
cm.getManifestVersion()
return cm
}

// Images return the list of images required for installing the cert-manager.
Expand Down Expand Up @@ -134,7 +169,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()
}

Expand Down Expand Up @@ -178,14 +213,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
}
Expand All @@ -201,7 +236,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
}
Expand All @@ -220,7 +255,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()
}

Expand Down Expand Up @@ -254,7 +289,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 {
Expand All @@ -279,7 +314,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())
}
Expand All @@ -292,7 +327,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
Expand Down Expand Up @@ -383,8 +418,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()
Expand Down
47 changes: 25 additions & 22 deletions cmd/clusterctl/client/cluster/cert_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cluster

import (
"crypto/sha256"
"fmt"
"testing"
"time"
Expand All @@ -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().WithObjs(tt.fields.objs...)
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
Expand Down Expand Up @@ -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,
},
Expand All @@ -241,15 +241,15 @@ func Test_shouldUpgrade(t *testing.T) {
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
certmanagerVersionAnnotation: embeddedCertManagerManifestVersion,
certmanagerVersionAnnotation: testVersion,
certmanagerHashAnnotation: "foo",
},
},
},
},
},
},
wantVersion: fmt.Sprintf("%s (%s)", embeddedCertManagerManifestVersion, "foo"),
wantVersion: fmt.Sprintf("%s (%s)", testVersion, "foo"),
want: true,
wantErr: false,
},
Expand Down Expand Up @@ -315,8 +315,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
Expand Down Expand Up @@ -521,7 +524,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) {
expectErr: false,
expectedPlan: CertManagerUpgradePlan{
From: "v0.11.0",
To: embeddedCertManagerManifestVersion,
To: testVersion,
ShouldUpgrade: true,
},
},
Expand All @@ -543,7 +546,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) {
expectErr: false,
expectedPlan: CertManagerUpgradePlan{
From: "v0.16.0",
To: embeddedCertManagerManifestVersion,
To: testVersion,
ShouldUpgrade: true,
},
},
Expand All @@ -558,14 +561,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,
},
},
Expand All @@ -580,14 +583,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,
},
},
Expand Down

0 comments on commit c1e7c59

Please sign in to comment.