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 Nov 9, 2020
1 parent d66eed9 commit 005f66d
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 62 deletions.
4 changes: 2 additions & 2 deletions cmd/clusterctl/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
94 changes: 67 additions & 27 deletions cmd/clusterctl/client/cluster/cert_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package cluster

import (
"context"
"crypto/sha256"
"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 @@ -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.
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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()
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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())
}
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
82 changes: 56 additions & 26 deletions cmd/clusterctl/client/cluster/cert_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,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(Equal(nil))
g.Expect(cm.embeddedCertManagerManifestHash).ToNot(Equal(nil))
}

func Test_certManagerClient_getManifestObjects(t *testing.T) {
Expand Down Expand Up @@ -134,7 +141,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 {
Expand Down Expand Up @@ -180,7 +189,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))
Expand Down Expand Up @@ -221,15 +232,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,
},
Expand All @@ -241,15 +252,15 @@ func Test_shouldUpgrade(t *testing.T) {
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
certmanagerVersionAnnotation: embeddedCertManagerManifestVersion,
certmanagerVersionAnnotation: expectedVersion,
certmanagerHashAnnotation: "foo",
},
},
},
},
},
},
wantVersion: fmt.Sprintf("%s (%s)", embeddedCertManagerManifestVersion, "foo"),
wantVersion: fmt.Sprintf("%s (%s)", expectedVersion, "foo"),
want: true,
wantErr: false,
},
Expand Down Expand Up @@ -315,8 +326,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
Expand Down Expand Up @@ -521,7 +542,7 @@ func Test_certManagerClient_PlanUpgrade(t *testing.T) {
expectErr: false,
expectedPlan: CertManagerUpgradePlan{
From: "v0.11.0",
To: embeddedCertManagerManifestVersion,
To: expectedVersion,
ShouldUpgrade: true,
},
},
Expand All @@ -536,14 +557,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,
},
},
Expand All @@ -558,14 +579,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,
},
},
Expand All @@ -580,14 +601,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,
},
},
Expand Down Expand Up @@ -619,9 +640,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())
Expand Down
Loading

0 comments on commit 005f66d

Please sign in to comment.