Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@emosbaugh
emosbaugh committed Oct 29, 2024
1 parent 30539e4 commit 8b5bf1c
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 63 deletions.
66 changes: 40 additions & 26 deletions pkg/store/kotsstore/downstream_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/replicatedhq/kots/pkg/kotsutil"
"github.com/replicatedhq/kots/pkg/logger"
"github.com/replicatedhq/kots/pkg/persistence"
"github.com/replicatedhq/kots/pkg/store"
"github.com/replicatedhq/kots/pkg/store/types"
"github.com/replicatedhq/kots/pkg/tasks"
"github.com/replicatedhq/kots/pkg/util"
Expand Down Expand Up @@ -427,7 +426,20 @@ func (s *KOTSStore) GetDownstreamVersions(appID string, clusterID string, downlo
if err := s.AddDownstreamVersionDetails(appID, clusterID, v, false); err != nil {
return nil, errors.Wrap(err, "failed to add details to latest downloaded version")
}
v.IsDeployable, v.NonDeployableCause, err = isAppVersionDeployable(s, appID, v, result, license.Spec.IsSemverRequired)
var currentECConfig, newECConfig []byte
if util.IsEmbeddedCluster() {
if result.CurrentVersion != nil {
currentECConfig, err = s.getRawEmbeddedClusterConfigForVersion(appID, result.CurrentVersion.Sequence)
if err != nil {
return nil, errors.Wrapf(err, "failed to get embedded cluster config for current version %d", result.CurrentVersion.Sequence)
}
}
newECConfig, err = s.getRawEmbeddedClusterConfigForVersion(appID, v.Sequence)
if err != nil {
return nil, errors.Wrapf(err, "failed to get embedded cluster config for version %d", v.Sequence)
}
}
v.IsDeployable, v.NonDeployableCause, err = isAppVersionDeployable(appID, v, result, license.Spec.IsSemverRequired, currentECConfig, newECConfig)
if err != nil {
return nil, errors.Wrapf(err, "failed to check if version %s is deployable", v.VersionLabel)
}
Expand Down Expand Up @@ -682,7 +694,20 @@ func (s *KOTSStore) AddDownstreamVersionsDetails(appID string, clusterID string,
}

for _, v := range versions {
v.IsDeployable, v.NonDeployableCause, err = isAppVersionDeployable(s, appID, v, allVersions, license.Spec.IsSemverRequired)
var currentECConfig, newECConfig []byte
if util.IsEmbeddedCluster() {
if allVersions.CurrentVersion != nil {
currentECConfig, err = s.getRawEmbeddedClusterConfigForVersion(appID, allVersions.CurrentVersion.Sequence)
if err != nil {
return errors.Wrapf(err, "failed to get embedded cluster config for current version %d", allVersions.CurrentVersion.Sequence)
}
}
newECConfig, err = s.getRawEmbeddedClusterConfigForVersion(appID, v.Sequence)
if err != nil {
return errors.Wrapf(err, "failed to get embedded cluster config for version %d", v.Sequence)
}
}
v.IsDeployable, v.NonDeployableCause, err = isAppVersionDeployable(appID, v, allVersions, license.Spec.IsSemverRequired, currentECConfig, newECConfig)
if err != nil {
return errors.Wrapf(err, "failed to check if version %s is deployable", v.VersionLabel)
}
Expand Down Expand Up @@ -875,7 +900,10 @@ func isSameUpstreamRelease(v1 *downstreamtypes.DownstreamVersion, v2 *downstream
return v1.Semver.EQ(*v2.Semver)
}

func isAppVersionDeployable(s store.Store, appID string, version *downstreamtypes.DownstreamVersion, appVersions *downstreamtypes.DownstreamVersions, isSemverRequired bool) (bool, string, error) {
func isAppVersionDeployable(
appID string, version *downstreamtypes.DownstreamVersion, appVersions *downstreamtypes.DownstreamVersions,
isSemverRequired bool, currentECConfig []byte, versionECConfig []byte,
) (bool, string, error) {
if version.HasFailingStrictPreflights {
return false, "Deployment is disabled as a strict analyzer in this version's preflight checks has failed or has not been run.", nil
}
Expand Down Expand Up @@ -932,15 +960,11 @@ func isAppVersionDeployable(s store.Store, appID string, version *downstreamtype
break
}

if util.IsEmbeddedCluster() {
if util.IsEmbeddedCluster() && currentECConfig != nil {
// Compare the embedded cluster config of the version specified to the currently
// deployed version to check if it has changed. If it has, then we do not allow
// rollbacks.
changed, err := didECClusterConfigChange(s, appID, version, appVersions.CurrentVersion)
if err != nil {
return false, "", errors.Wrapf(err, "failed to check if embedded cluster config changed for version %d", version.Sequence)
}
if changed {
if !bytes.Equal(currentECConfig, versionECConfig) {
return false, "Rollback is not supported, cluster configuration has changed.", nil
}
}
Expand Down Expand Up @@ -1031,26 +1055,16 @@ ALL_VERSIONS_LOOP:
return true, "", nil
}

// didECClusterConfigChange compares the embedded cluster config of the version specified to the
// currently deployed version to check if it has changed.
func didECClusterConfigChange(s store.Store, appID string, version *downstreamtypes.DownstreamVersion, currentVersion *downstreamtypes.DownstreamVersion) (bool, error) {
currentConf, err := s.GetEmbeddedClusterConfigForVersion(appID, currentVersion.Sequence)
if err != nil {
return false, errors.Wrapf(err, "failed to get embedded cluster config for current version %d", currentVersion.Sequence)
}
currentECConfigBytes, err := json.Marshal(currentConf)
if err != nil {
return false, errors.Wrapf(err, "failed to marshal embedded cluster config for current version %d", currentVersion.Sequence)
}
thisConf, err := s.GetEmbeddedClusterConfigForVersion(appID, version.Sequence)
func (s *KOTSStore) getRawEmbeddedClusterConfigForVersion(appID string, sequence int64) ([]byte, error) {
currentConf, err := s.GetEmbeddedClusterConfigForVersion(appID, sequence)
if err != nil {
return false, errors.Wrap(err, "failed to get embedded cluster config")
return nil, errors.Wrap(err, "failed to get embedded cluster config")
}
ecConfigBytes, err := json.Marshal(thisConf)
b, err := json.Marshal(currentConf)
if err != nil {
return false, errors.Wrap(err, "failed to marshal embedded cluster config")
return nil, errors.Wrap(err, "failed to marshal embedded cluster config")
}
return !bytes.Equal(ecConfigBytes, currentECConfigBytes), nil
return b, nil
}

func getReleaseNotes(appID string, parentSequence int64) (string, error) {
Expand Down
82 changes: 49 additions & 33 deletions pkg/store/kotsstore/downstream_store_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
package kotsstore

import (
"encoding/json"
"testing"

"github.com/blang/semver"
"github.com/golang/mock/gomock"
embeddedclusterv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1"
downstreamtypes "github.com/replicatedhq/kots/pkg/api/downstream/types"
"github.com/replicatedhq/kots/pkg/cursor"
"github.com/replicatedhq/kots/pkg/kotsutil"
mock_store "github.com/replicatedhq/kots/pkg/store/mock"
"github.com/replicatedhq/kots/pkg/store/types"
kotsv1beta1 "github.com/replicatedhq/kotskinds/apis/kots/v1beta1"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -254,7 +253,9 @@ func Test_isAppVersionDeployable(t *testing.T) {
version *downstreamtypes.DownstreamVersion
appVersions *downstreamtypes.DownstreamVersions
isSemverRequired bool
setup func(t *testing.T, mockStore *mock_store.MockStore)
currentECConfig *embeddedclusterv1beta1.Config
versionECConfig *embeddedclusterv1beta1.Config
setup func(t *testing.T)
expectedIsDeployable bool
expectedCause string
wantErr bool
Expand Down Expand Up @@ -3630,19 +3631,8 @@ func Test_isAppVersionDeployable(t *testing.T) {
/* ---- Embedded cluster config tests start here ---- */
{
name: "embedded cluster config change should not allow rollbacks",
setup: func(t *testing.T, mockStore *mock_store.MockStore) {
setup: func(t *testing.T) {
t.Setenv("EMBEDDED_CLUSTER_ID", "1234")

mockStore.EXPECT().GetEmbeddedClusterConfigForVersion("APPID", int64(0)).Return(&embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.0",
},
}, nil)
mockStore.EXPECT().GetEmbeddedClusterConfigForVersion("APPID", int64(1)).Return(&embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.1",
},
}, nil)
},
version: &downstreamtypes.DownstreamVersion{
VersionLabel: "1.0.0",
Expand Down Expand Up @@ -3675,25 +3665,24 @@ func Test_isAppVersionDeployable(t *testing.T) {
},
},
},
currentECConfig: &embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.1",
},
},
versionECConfig: &embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.0",
},
},
expectedIsDeployable: false,
expectedCause: "Rollback is not supported, cluster configuration has changed.",
wantErr: false,
},
{
name: "embedded cluster config no change should allow rollbacks",
setup: func(t *testing.T, mockStore *mock_store.MockStore) {
setup: func(t *testing.T) {
t.Setenv("EMBEDDED_CLUSTER_ID", "1234")

mockStore.EXPECT().GetEmbeddedClusterConfigForVersion("APPID", int64(0)).Return(&embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.0",
},
}, nil)
mockStore.EXPECT().GetEmbeddedClusterConfigForVersion("APPID", int64(1)).Return(&embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.0",
},
}, nil)
},
version: &downstreamtypes.DownstreamVersion{
VersionLabel: "1.0.0",
Expand Down Expand Up @@ -3726,13 +3715,23 @@ func Test_isAppVersionDeployable(t *testing.T) {
},
},
},
currentECConfig: &embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.1",
},
},
versionECConfig: &embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.1",
},
},
expectedIsDeployable: true,
expectedCause: "",
wantErr: false,
},
{
name: "embedded cluster, allowRollback = false should not allow rollbacks",
setup: func(t *testing.T, mockStore *mock_store.MockStore) {
setup: func(t *testing.T) {
t.Setenv("EMBEDDED_CLUSTER_ID", "1234")
},
version: &downstreamtypes.DownstreamVersion{
Expand Down Expand Up @@ -3766,6 +3765,16 @@ func Test_isAppVersionDeployable(t *testing.T) {
},
},
},
currentECConfig: &embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.1",
},
},
versionECConfig: &embeddedclusterv1beta1.Config{
Spec: embeddedclusterv1beta1.ConfigSpec{
Version: "1.0.0-ec.0",
},
},
expectedIsDeployable: false,
expectedCause: "Rollback is not supported.",
wantErr: false,
Expand Down Expand Up @@ -3806,14 +3815,21 @@ func Test_isAppVersionDeployable(t *testing.T) {
}
}

ctrl := gomock.NewController(t)
defer ctrl.Finish()
var currentECConfig, versionECConfig []byte
if test.currentECConfig != nil {
currentECConfig, err = json.Marshal(test.currentECConfig)
require.NoError(t, err)
}
if test.versionECConfig != nil {
versionECConfig, err = json.Marshal(test.versionECConfig)
require.NoError(t, err)
}

mockStore := mock_store.NewMockStore(ctrl)
if test.setup != nil {
test.setup(t, mockStore)
test.setup(t)
}
isDeployable, cause, err := isAppVersionDeployable(mockStore, "APPID", test.version, test.appVersions, test.isSemverRequired)

isDeployable, cause, err := isAppVersionDeployable("APPID", test.version, test.appVersions, test.isSemverRequired, currentECConfig, versionECConfig)
if test.wantErr {
require.Error(t, err)
} else {
Expand Down
3 changes: 0 additions & 3 deletions pkg/store/kotsstore/kots_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
kotsadmtypes "github.com/replicatedhq/kots/pkg/kotsadm/types"
"github.com/replicatedhq/kots/pkg/logger"
"github.com/replicatedhq/kots/pkg/persistence"
"github.com/replicatedhq/kots/pkg/store"
"github.com/replicatedhq/kots/pkg/util"
kotsscheme "github.com/replicatedhq/kotskinds/client/kotsclientset/scheme"
troubleshootscheme "github.com/replicatedhq/troubleshoot/pkg/client/troubleshootclientset/scheme"
Expand All @@ -35,8 +34,6 @@ type KOTSStore struct {
}

func init() {
store.SetStore(StoreFromEnv())

kotsscheme.AddToScheme(scheme.Scheme)
veleroscheme.AddToScheme(scheme.Scheme)
troubleshootscheme.AddToScheme(scheme.Scheme)
Expand Down
10 changes: 9 additions & 1 deletion pkg/store/store.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package store

import (
"github.com/replicatedhq/kots/pkg/store/kotsstore"
"github.com/replicatedhq/kots/pkg/util"
)

Expand All @@ -9,16 +10,23 @@ var (
globalStore Store
)

var _ Store = (*kotsstore.KOTSStore)(nil)

func GetStore() Store {
if util.IsUpgradeService() {
panic("store cannot not be used in the upgrade service")
}
if !hasStore {
panic("store not initialized")
globalStore = storeFromEnv()
hasStore = true
}
return globalStore
}

func storeFromEnv() Store {
return kotsstore.StoreFromEnv()
}

func SetStore(s Store) {
if s == nil {
hasStore = false
Expand Down

0 comments on commit 8b5bf1c

Please sign in to comment.