Skip to content

Commit

Permalink
Check chart versions against target in users config
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Weiße <[email protected]>
  • Loading branch information
daniel-weisse committed Sep 8, 2023
1 parent 6cb506b commit 0ce695a
Show file tree
Hide file tree
Showing 6 changed files with 316 additions and 24 deletions.
2 changes: 2 additions & 0 deletions cli/internal/helm/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ go_library(
go_test(
name = "helm_test",
srcs = [
"actionfactory_test.go",
"helm_test.go",
"loader_test.go",
],
Expand All @@ -475,6 +476,7 @@ go_test(
"@com_github_stretchr_testify//mock",
"@com_github_stretchr_testify//require",
"@sh_helm_helm_v3//pkg/action",
"@sh_helm_helm_v3//pkg/chart",
"@sh_helm_helm_v3//pkg/chartutil",
"@sh_helm_helm_v3//pkg/engine",
],
Expand Down
46 changes: 32 additions & 14 deletions cli/internal/helm/actionfactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ type actionFactory struct {
versionLister releaseVersionLister
cfg *action.Configuration
kubeClient crdClient
cliVersion semver.Semver
log debugLog
}

Expand All @@ -38,9 +37,8 @@ type crdClient interface {
}

// newActionFactory creates a new action factory for managing helm releases.
func newActionFactory(kubeClient crdClient, lister releaseVersionLister, actionConfig *action.Configuration, cliVersion semver.Semver, log debugLog) *actionFactory {
func newActionFactory(kubeClient crdClient, lister releaseVersionLister, actionConfig *action.Configuration, log debugLog) *actionFactory {
return &actionFactory{
cliVersion: cliVersion,
versionLister: lister,
cfg: actionConfig,
kubeClient: kubeClient,
Expand All @@ -49,10 +47,10 @@ func newActionFactory(kubeClient crdClient, lister releaseVersionLister, actionC
}

// GetActions returns a list of actions to apply the given releases.
func (a actionFactory) GetActions(releases []Release, force, allowDestructive bool) (actions []applyAction, includesUpgrade bool, err error) {
func (a actionFactory) GetActions(releases []Release, configTargetVersion semver.Semver, force, allowDestructive bool) (actions []applyAction, includesUpgrade bool, err error) {
upgradeErrs := []error{}
for _, release := range releases {
err := a.appendNewAction(release, force, allowDestructive, &actions)
err := a.appendNewAction(release, configTargetVersion, force, allowDestructive, &actions)
var invalidUpgrade *compatibility.InvalidUpgradeError
if errors.As(err, &invalidUpgrade) {
upgradeErrs = append(upgradeErrs, err)
Expand All @@ -71,13 +69,21 @@ func (a actionFactory) GetActions(releases []Release, force, allowDestructive bo
return actions, includesUpgrade, errors.Join(upgradeErrs...)
}

func (a actionFactory) appendNewAction(release Release, force, allowDestructive bool, actions *[]applyAction) error {
func (a actionFactory) appendNewAction(release Release, configTargetVersion semver.Semver, force, allowDestructive bool, actions *[]applyAction) error {
newVersion, err := semver.New(release.Chart.Metadata.Version)
if err != nil {
return fmt.Errorf("parsing chart version: %w", err)
}
currentVersion, err := a.versionLister.currentVersion(release.ReleaseName)
if errors.Is(err, errReleaseNotFound) {
// Don't install a new release if the user's config specifies a different version than the CLI offers.
if !force && isCLIVersionedRelease(release.ReleaseName) && configTargetVersion.Compare(newVersion) != 0 {
return fmt.Errorf(
"unable to install release %s at %s: this CLI only supports microservice version %s for upgrading",
release.ReleaseName, configTargetVersion, newVersion,
)
}

a.log.Debugf("Release %s not found, adding to new releases...", release.ReleaseName)
*actions = append(*actions, a.newInstall(release))
return nil
Expand All @@ -88,18 +94,30 @@ func (a actionFactory) appendNewAction(release Release, force, allowDestructive
a.log.Debugf("Current %s version: %s", release.ReleaseName, currentVersion)
a.log.Debugf("New %s version: %s", release.ReleaseName, newVersion)

// This may break for cert-manager or cilium if we decide to upgrade more than one minor version at a time.
// Leaving it as is since it is not clear to me what kind of sanity check we could do.
if !force {
if err := newVersion.IsUpgradeTo(currentVersion); err != nil {
return fmt.Errorf("invalid upgrade for %s: %w", release.ReleaseName, err)
// For charts we package ourselves, the version is equal to the CLI version (charts are embedded in the binary).
// We need to make sure this matches with the version in a user's config, if an upgrade should be applied.
if isCLIVersionedRelease(release.ReleaseName) {
// If target version is not a valid upgrade, don't upgrade any charts.
if err := configTargetVersion.IsUpgradeTo(currentVersion); err != nil {
return fmt.Errorf("invalid upgrade for %s: %w", release.ReleaseName, err)
}
// Target version is newer than current version, so we should perform an upgrade.
// Now make sure the target version is equal to the the CLI version.
if configTargetVersion.Compare(newVersion) != 0 {
return fmt.Errorf(
"unable to upgrade release %s to %s: this CLI only supports microservice version %s for upgrading",
release.ReleaseName, configTargetVersion, newVersion,
)
}
} else {
// This may break for external chart dependencies if we decide to upgrade more than one minor version at a time.
if err := newVersion.IsUpgradeTo(currentVersion); err != nil {
return fmt.Errorf("invalid upgrade for %s: %w", release.ReleaseName, err)
}
}
}

// at this point we conclude that the release should be upgraded. check that this CLI supports the upgrade.
if isCLIVersionedRelease(release.ReleaseName) && a.cliVersion.Compare(newVersion) != 0 {
return fmt.Errorf("this CLI only supports microservice version %s for upgrading", a.cliVersion.String())
}
if !allowDestructive &&
release.ReleaseName == certManagerInfo.releaseName {
return ErrConfirmationMissing
Expand Down
267 changes: 267 additions & 0 deletions cli/internal/helm/actionfactory_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
/*
Copyright (c) Edgeless Systems GmbH
SPDX-License-Identifier: AGPL-3.0-only
*/

package helm

import (
"errors"
"testing"

"github.com/edgelesssys/constellation/v2/internal/compatibility"
"github.com/edgelesssys/constellation/v2/internal/logger"
"github.com/edgelesssys/constellation/v2/internal/semver"
"github.com/stretchr/testify/assert"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/chart"
)

func TestAppendNewAction(t *testing.T) {
assertUpgradeErr := func(assert *assert.Assertions, err error) {
var invalidUpgrade *compatibility.InvalidUpgradeError
assert.True(errors.As(err, &invalidUpgrade))
}

testCases := map[string]struct {
lister stubLister
release Release
configTargetVersion semver.Semver
force bool
allowDestructive bool
wantErr bool
assertErr func(*assert.Assertions, error)
}{
"upgrade release": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
ReleaseName: "test",
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
},
"upgrade to same version": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
ReleaseName: "test",
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.0.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
wantErr: true,
assertErr: assertUpgradeErr,
},
"upgrade to older version": {
lister: stubLister{version: semver.NewFromInt(1, 1, 0, "")},
release: Release{
ReleaseName: "test",
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.0.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 0, 0, ""),
wantErr: true,
assertErr: assertUpgradeErr,
},
"upgrade to older version can be forced": {
lister: stubLister{version: semver.NewFromInt(1, 1, 0, "")},
release: Release{
ReleaseName: "test",
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.0.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 0, 0, ""),
force: true,
},
"non semver in chart metadata": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
ReleaseName: "test",
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "some-version",
},
},
},
wantErr: true,
},
"listing release fails": {
lister: stubLister{err: assert.AnError},
release: Release{
ReleaseName: "test",
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
wantErr: true,
},
"release not installed": {
lister: stubLister{err: errReleaseNotFound},
release: Release{
ReleaseName: "test",
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
},
"destructive release upgrade requires confirmation": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
ReleaseName: certManagerInfo.releaseName,
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
wantErr: true,
assertErr: func(assert *assert.Assertions, err error) {
assert.ErrorIs(err, ErrConfirmationMissing)
},
},
"destructive release upgrade can be accepted": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
ReleaseName: certManagerInfo.releaseName,
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
allowDestructive: true,
},
"config version takes precedence over CLI version": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
ReleaseName: constellationServicesInfo.releaseName,
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 0, 0, ""),
wantErr: true,
assertErr: assertUpgradeErr,
},
"error if CLI version does not match config version on upgrade": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
ReleaseName: constellationServicesInfo.releaseName,
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.5",
},
},
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
wantErr: true,
assertErr: func(assert *assert.Assertions, err error) {
var invalidUpgrade *compatibility.InvalidUpgradeError
assert.False(errors.As(err, &invalidUpgrade))
},
},
"config version matches CLI version on upgrade": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
ReleaseName: constellationServicesInfo.releaseName,
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.5",
},
},
},
configTargetVersion: semver.NewFromInt(1, 1, 5, ""),
},
"config - CLI version mismatch can be forced through": {
lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")},
release: Release{
ReleaseName: constellationServicesInfo.releaseName,
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.5",
},
},
},
configTargetVersion: semver.NewFromInt(1, 1, 0, ""),
force: true,
},
"installing new release requires matching config and CLI version": {
lister: stubLister{err: errReleaseNotFound},
release: Release{
ReleaseName: constellationServicesInfo.releaseName,
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 0, 0, ""),
wantErr: true,
},
"config - CLI version mismatch for new releases can be forced through": {
lister: stubLister{err: errReleaseNotFound},
release: Release{
ReleaseName: constellationServicesInfo.releaseName,
Chart: &chart.Chart{
Metadata: &chart.Metadata{
Version: "1.1.0",
},
},
},
configTargetVersion: semver.NewFromInt(1, 0, 0, ""),
force: true,
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
assert := assert.New(t)

actions := []applyAction{}
actionFactory := newActionFactory(nil, tc.lister, &action.Configuration{}, logger.NewTest(t))

err := actionFactory.appendNewAction(tc.release, tc.configTargetVersion, tc.force, tc.allowDestructive, &actions)
if tc.wantErr {
assert.Error(err)
if tc.assertErr != nil {
tc.assertErr(assert, err)
}
return
}
assert.NoError(err)
assert.Len(actions, 1) // no error == actions gets appended
})
}
}

type stubLister struct {
err error
version semver.Semver
}

func (s stubLister) currentVersion(_ string) (semver.Semver, error) {
return s.version, s.err
}
4 changes: 2 additions & 2 deletions cli/internal/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func NewClient(kubeConfigPath string, log debugLog) (*Client, error) {
}
lister := ReleaseVersionClient{actionConfig}
cliVersion := constants.BinaryVersion()
factory := newActionFactory(kubeClient, lister, actionConfig, cliVersion, log)
factory := newActionFactory(kubeClient, lister, actionConfig, log)
return &Client{factory, cliVersion, log}, nil
}

Expand All @@ -93,7 +93,7 @@ func (h Client) PrepareApply(conf *config.Config, validK8sversion versions.Valid
return nil, false, fmt.Errorf("loading Helm releases: %w", err)
}
h.log.Debugf("Loaded Helm releases")
actions, includesUpgrades, err := h.factory.GetActions(releases, flags.Force, flags.AllowDestructive)
actions, includesUpgrades, err := h.factory.GetActions(releases, conf.MicroserviceVersion, flags.Force, flags.AllowDestructive)
return &ChartApplyExecutor{actions: actions, log: h.log}, includesUpgrades, err
}

Expand Down
Loading

0 comments on commit 0ce695a

Please sign in to comment.