From ab8a17e535b9002298ab1925575f22cdd13a0590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Wei=C3=9Fe?= <66256922+daniel-weisse@users.noreply.github.com> Date: Fri, 13 Oct 2023 10:21:21 +0200 Subject: [PATCH] cli: remove old migration steps and id-file references (#2440) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove old migration steps and id-file references * Update codeowners file --------- Signed-off-by: Daniel Weiße --- CODEOWNERS | 1 - cli/internal/clusterid/BUILD.bazel | 20 --- cli/internal/clusterid/id.go | 80 ----------- cli/internal/clusterid/id_test.go | 63 --------- cli/internal/cmd/BUILD.bazel | 2 - cli/internal/cmd/terminate.go | 5 - cli/internal/cmd/upgradeapply.go | 146 +++++--------------- cli/internal/cmd/upgradeapply_test.go | 190 +++++++++----------------- cli/internal/kubecmd/BUILD.bazel | 1 - cli/internal/kubecmd/kubecmd.go | 67 --------- cli/internal/state/BUILD.bazel | 4 - cli/internal/state/state.go | 27 ---- cli/internal/state/state_test.go | 64 --------- internal/constants/constants.go | 2 - 14 files changed, 93 insertions(+), 579 deletions(-) delete mode 100644 cli/internal/clusterid/BUILD.bazel delete mode 100644 cli/internal/clusterid/id.go delete mode 100644 cli/internal/clusterid/id_test.go diff --git a/CODEOWNERS b/CODEOWNERS index 8f712007d9..c9fc06cf56 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -6,7 +6,6 @@ /bazel/sh @katexochen /bootstrapper @3u13r /cli/internal/cloudcmd @daniel-weisse -/cli/internal/clusterid @malt3 /cli/internal/cmd/upgrade* @derpsteb /cli/internal/featureset @malt3 /cli/internal/helm @derpsteb diff --git a/cli/internal/clusterid/BUILD.bazel b/cli/internal/clusterid/BUILD.bazel deleted file mode 100644 index 5d76614387..0000000000 --- a/cli/internal/clusterid/BUILD.bazel +++ /dev/null @@ -1,20 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") -load("//bazel/go:go_test.bzl", "go_test") - -go_library( - name = "clusterid", - srcs = ["id.go"], - importpath = "github.com/edgelesssys/constellation/v2/cli/internal/clusterid", - visibility = ["//cli:__subpackages__"], - deps = [ - "//internal/cloud/cloudprovider", - "//internal/config", - ], -) - -go_test( - name = "clusterid_test", - srcs = ["id_test.go"], - embed = [":clusterid"], - deps = ["@com_github_stretchr_testify//require"], -) diff --git a/cli/internal/clusterid/id.go b/cli/internal/clusterid/id.go deleted file mode 100644 index 826b629b0c..0000000000 --- a/cli/internal/clusterid/id.go +++ /dev/null @@ -1,80 +0,0 @@ -/* -Copyright (c) Edgeless Systems GmbH - -SPDX-License-Identifier: AGPL-3.0-only -*/ - -// package clusterid defines the structure of the Constellation cluster ID file. -// Logic in this package should be kept minimal. -package clusterid - -import ( - "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" - "github.com/edgelesssys/constellation/v2/internal/config" -) - -// File contains state information about a cluster. -// This information is accessible after the creation -// and can be used by further operations such as initialization and upgrades. -type File struct { - // ClusterID is the unique identifier of the cluster. - ClusterID string `json:"clusterID,omitempty"` - // OwnerID is the unique identifier of the owner of the cluster. - OwnerID string `json:"ownerID,omitempty"` - // UID is the unique identifier of the cluster, used for infrastructure management. - UID string `json:"uid,omitempty"` - // CloudProvider is the cloud provider of the cluster. - CloudProvider cloudprovider.Provider `json:"cloudprovider,omitempty"` - // IP is the IP address the cluster can be reached at (often the load balancer). - IP string `json:"ip,omitempty"` - // APIServerCertSANs are subject alternative names (SAN) that are added to - // the TLS certificate of each apiserver instance. - APIServerCertSANs []string `json:"apiServerCertSANs,omitempty"` - // InitSecret is the secret the first Bootstrapper uses to verify the user. - InitSecret []byte `json:"initsecret,omitempty"` - // AttestationURL is the URL of the attestation service. - // It is only set if the cluster is created on Azure. - AttestationURL string `json:"attestationURL,omitempty"` - // MeasurementSalt is the salt generated during cluster init. - MeasurementSalt []byte `json:"measurementSalt,omitempty"` -} - -// Merge merges the other file into the current file and returns the result. -// If a field is set in both files, the value of the other file is used. -// This does in-place changes on the current file. -func (f *File) Merge(other File) *File { - if other.ClusterID != "" { - f.ClusterID = other.ClusterID - } - - if other.OwnerID != "" { - f.OwnerID = other.OwnerID - } - - if other.UID != "" { - f.UID = other.UID - } - - if other.CloudProvider != cloudprovider.Unknown { - f.CloudProvider = other.CloudProvider - } - - if other.IP != "" { - f.IP = other.IP - } - - if other.InitSecret != nil { - f.InitSecret = other.InitSecret - } - - if other.AttestationURL != "" { - f.AttestationURL = other.AttestationURL - } - - return f -} - -// GetClusterName returns the name of the cluster. -func GetClusterName(cfg *config.Config, idFile File) string { - return cfg.Name + "-" + idFile.UID -} diff --git a/cli/internal/clusterid/id_test.go b/cli/internal/clusterid/id_test.go deleted file mode 100644 index a0a2a584e4..0000000000 --- a/cli/internal/clusterid/id_test.go +++ /dev/null @@ -1,63 +0,0 @@ -/* -Copyright (c) Edgeless Systems GmbH - -SPDX-License-Identifier: AGPL-3.0-only -*/ - -package clusterid - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestMerge(t *testing.T) { - testCases := map[string]struct { - current File - other File - want File - }{ - "empty": { - current: File{}, - other: File{}, - want: File{}, - }, - "current empty": { - current: File{}, - other: File{ - ClusterID: "clusterID", - }, - want: File{ - ClusterID: "clusterID", - }, - }, - "other empty": { - current: File{ - ClusterID: "clusterID", - }, - other: File{}, - want: File{ - ClusterID: "clusterID", - }, - }, - "both set": { - current: File{ - ClusterID: "clusterID", - }, - other: File{ - ClusterID: "otherClusterID", - }, - want: File{ - ClusterID: "otherClusterID", - }, - }, - } - - for _, tc := range testCases { - require := require.New(t) - - ret := tc.current.Merge(tc.other) - require.Equal(tc.want, *ret) - } -} diff --git a/cli/internal/cmd/BUILD.bazel b/cli/internal/cmd/BUILD.bazel index 807caeb181..e3dcf2dc0f 100644 --- a/cli/internal/cmd/BUILD.bazel +++ b/cli/internal/cmd/BUILD.bazel @@ -40,7 +40,6 @@ go_library( deps = [ "//bootstrapper/initproto", "//cli/internal/cloudcmd", - "//cli/internal/clusterid", "//cli/internal/cmd/pathprefix", "//cli/internal/featureset", "//cli/internal/helm", @@ -135,7 +134,6 @@ go_test( deps = [ "//bootstrapper/initproto", "//cli/internal/cloudcmd", - "//cli/internal/clusterid", "//cli/internal/cmd/pathprefix", "//cli/internal/helm", "//cli/internal/kubecmd", diff --git a/cli/internal/cmd/terminate.go b/cli/internal/cmd/terminate.go index 573b1c6360..141b755d46 100644 --- a/cli/internal/cmd/terminate.go +++ b/cli/internal/cmd/terminate.go @@ -84,11 +84,6 @@ func terminate(cmd *cobra.Command, terminator cloudTerminator, fileHandler file. removeErr = errors.Join(err, fmt.Errorf("failed to remove file: '%s', please remove it manually", pf.PrefixPrintablePath(constants.AdminConfFilename))) } - // TODO(msanft): Once v2.12.0 is released, remove the ID-file-removal here. - if err := fileHandler.Remove(constants.ClusterIDsFilename); err != nil && !errors.Is(err, fs.ErrNotExist) { - removeErr = errors.Join(err, fmt.Errorf("failed to remove file: '%s', please remove it manually", pf.PrefixPrintablePath(constants.ClusterIDsFilename))) - } - if err := fileHandler.Remove(constants.StateFilename); err != nil && !errors.Is(err, fs.ErrNotExist) { removeErr = errors.Join(err, fmt.Errorf("failed to remove file: '%s', please remove it manually", pf.PrefixPrintablePath(constants.StateFilename))) } diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 7b5ca8dcdb..9e564e4eb4 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -11,13 +11,11 @@ import ( "errors" "fmt" "io" - "io/fs" "path/filepath" "strings" "time" "github.com/edgelesssys/constellation/v2/cli/internal/cloudcmd" - "github.com/edgelesssys/constellation/v2/cli/internal/clusterid" "github.com/edgelesssys/constellation/v2/cli/internal/cmd/pathprefix" "github.com/edgelesssys/constellation/v2/cli/internal/helm" "github.com/edgelesssys/constellation/v2/cli/internal/kubecmd" @@ -31,7 +29,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/kms/uri" - "github.com/edgelesssys/constellation/v2/internal/semver" "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/rogpeppe/go-internal/diff" "github.com/spf13/afero" @@ -114,11 +111,6 @@ func runUpgradeApply(cmd *cobra.Command, _ []string) error { return fmt.Errorf("setting up cluster upgrader: %w", err) } - // Set up terraform client to show existing cluster resources and information required for Helm upgrades - tfShower, err := terraform.New(cmd.Context(), constants.TerraformWorkingDir) - if err != nil { - return fmt.Errorf("setting up terraform client: %w", err) - } helmClient, err := helm.NewClient(constants.AdminConfFilename, log) if err != nil { return fmt.Errorf("creating Helm client: %w", err) @@ -129,7 +121,6 @@ func runUpgradeApply(cmd *cobra.Command, _ []string) error { helmApplier: helmClient, clusterUpgrader: clusterUpgrader, configFetcher: configFetcher, - clusterShower: tfShower, fileHandler: fileHandler, log: log, } @@ -141,15 +132,10 @@ type upgradeApplyCmd struct { kubeUpgrader kubernetesUpgrader clusterUpgrader clusterUpgrader configFetcher attestationconfigapi.Fetcher - clusterShower infrastructureShower fileHandler file.Handler log debugLog } -type infrastructureShower interface { - ShowInfrastructure(ctx context.Context, provider cloudprovider.Provider) (state.Infrastructure, error) -} - func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, flags upgradeApplyFlags) error { conf, err := config.New(u.fileHandler, constants.ConfigFilename, u.configFetcher, flags.force) var configValidationErr *config.ValidationError @@ -178,32 +164,10 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl } stateFile, err := state.ReadFromFile(u.fileHandler, constants.StateFilename) - // TODO(msanft): Remove reading from idFile once v2.12.0 is released and read from state file directly. - // For now, this is only here to ensure upgradability from an id-file to a state file version. - if errors.Is(err, fs.ErrNotExist) { - u.log.Debugf("%s does not exist in current directory, falling back to reading from %s", - constants.StateFilename, constants.ClusterIDsFilename) - var idFile clusterid.File - if err := u.fileHandler.ReadJSON(constants.ClusterIDsFilename, &idFile); err != nil { - return fmt.Errorf("reading cluster ID file: %w", err) - } - // Convert id-file to state file - stateFile = state.NewFromIDFile(idFile, conf) - if stateFile.Infrastructure.Azure != nil { - conf.UpdateMAAURL(stateFile.Infrastructure.Azure.AttestationURL) - } - } else if err != nil { + if err != nil { return fmt.Errorf("reading state file: %w", err) } - // Apply migrations necessary for the upgrade - if err := migrateFrom2_10(cmd.Context(), u.kubeUpgrader); err != nil { - return fmt.Errorf("applying migration for upgrading from v2.10: %w", err) - } - if err := migrateFrom2_11(cmd.Context(), u.kubeUpgrader); err != nil { - return fmt.Errorf("applying migration for upgrading from v2.11: %w", err) - } - if err := u.confirmAndUpgradeAttestationConfig(cmd, conf.GetAttestationConfig(), stateFile.ClusterValues.MeasurementSalt, flags); err != nil { return fmt.Errorf("upgrading measurements: %w", err) } @@ -211,47 +175,37 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl // If infrastructure phase is skipped, we expect the new infrastructure // to be in the Terraform configuration already. Otherwise, perform // the Terraform migrations. - var postMigrationInfraState state.Infrastructure - if flags.skipPhases.contains(skipInfrastructurePhase) { - // TODO(msanft): Once v2.12.0 is released, this should be removed and the state should be read - // from the state file instead, as it will be the only source of truth for the cluster's infrastructure. - postMigrationInfraState, err = u.clusterShower.ShowInfrastructure(cmd.Context(), conf.GetProvider()) + if !flags.skipPhases.contains(skipInfrastructurePhase) { + migrationRequired, err := u.planTerraformMigration(cmd, conf) if err != nil { - return fmt.Errorf("getting Terraform state: %w", err) + return fmt.Errorf("planning Terraform migrations: %w", err) } - } else { - postMigrationInfraState, err = u.migrateTerraform(cmd, conf, upgradeDir, flags) - if err != nil { - return fmt.Errorf("performing Terraform migrations: %w", err) - } - } - // Merge the pre-upgrade state with the post-migration infrastructure values - if _, err := stateFile.Merge( - // temporary state with post-migration infrastructure values - state.New().SetInfrastructure(postMigrationInfraState), - ); err != nil { - return fmt.Errorf("merging pre-upgrade state with post-migration infrastructure values: %w", err) - } + if migrationRequired { + postMigrationInfraState, err := u.migrateTerraform(cmd, conf, upgradeDir, flags) + if err != nil { + return fmt.Errorf("performing Terraform migrations: %w", err) + } - // Write the post-migration state to disk - if err := stateFile.WriteToFile(u.fileHandler, constants.StateFilename); err != nil { - return fmt.Errorf("writing state file: %w", err) - } + // Merge the pre-upgrade state with the post-migration infrastructure values + if _, err := stateFile.Merge( + // temporary state with post-migration infrastructure values + state.New().SetInfrastructure(postMigrationInfraState), + ); err != nil { + return fmt.Errorf("merging pre-upgrade state with post-migration infrastructure values: %w", err) + } - // TODO(msanft): Remove this after v2.12.0 is released, as we do not support - // the id-file starting from v2.13.0. - err = u.fileHandler.RenameFile(constants.ClusterIDsFilename, constants.ClusterIDsFilename+".old") - if !errors.Is(err, fs.ErrNotExist) && err != nil { - return fmt.Errorf("removing cluster ID file: %w", err) + // Write the post-migration state to disk + if err := stateFile.WriteToFile(u.fileHandler, constants.StateFilename); err != nil { + return fmt.Errorf("writing state file: %w", err) + } + } } // extend the clusterConfig cert SANs with any of the supported endpoints: // - (legacy) public IP // - fallback endpoint // - custom (user-provided) endpoint - // TODO(msanft): Remove the comment below once v2.12.0 is released. - // At this point, state file and id-file should have been merged, so we can use the state file. sans := append([]string{stateFile.Infrastructure.ClusterEndpoint, conf.CustomEndpoint}, stateFile.Infrastructure.APIServerCertSANs...) if err := u.kubeUpgrader.ExtendClusterConfigCertSANs(cmd.Context(), sans); err != nil { return fmt.Errorf("extending cert SANs: %w", err) @@ -304,17 +258,13 @@ func diffAttestationCfg(currentAttestationCfg config.AttestationCfg, newAttestat return diff, nil } -// migrateTerraform checks if the Constellation version the cluster is being upgraded to requires a migration -// of cloud resources with Terraform. If so, the migration is performed and the post-migration infrastructure state is returned. -// If no migration is required, the current (pre-upgrade) infrastructure state is returned. -func (u *upgradeApplyCmd) migrateTerraform( - cmd *cobra.Command, conf *config.Config, upgradeDir string, flags upgradeApplyFlags, -) (state.Infrastructure, error) { +// planTerraformMigration checks if the Constellation version the cluster is being upgraded to requires a migration. +func (u *upgradeApplyCmd) planTerraformMigration(cmd *cobra.Command, conf *config.Config) (bool, error) { u.log.Debugf("Planning Terraform migrations") vars, err := cloudcmd.TerraformUpgradeVars(conf) if err != nil { - return state.Infrastructure{}, fmt.Errorf("parsing upgrade variables: %w", err) + return false, fmt.Errorf("parsing upgrade variables: %w", err) } u.log.Debugf("Using Terraform variables:\n%v", vars) @@ -328,15 +278,15 @@ func (u *upgradeApplyCmd) migrateTerraform( // u.upgrader.AddManualStateMigration(migration) // } - hasDiff, err := u.clusterUpgrader.PlanClusterUpgrade(cmd.Context(), cmd.OutOrStdout(), vars, conf.GetProvider()) - if err != nil { - return state.Infrastructure{}, fmt.Errorf("planning terraform migrations: %w", err) - } - if !hasDiff { - u.log.Debugf("No Terraform diff detected") - return u.clusterShower.ShowInfrastructure(cmd.Context(), conf.GetProvider()) - } + return u.clusterUpgrader.PlanClusterUpgrade(cmd.Context(), cmd.OutOrStdout(), vars, conf.GetProvider()) +} +// migrateTerraform checks if the Constellation version the cluster is being upgraded to requires a migration +// of cloud resources with Terraform. If so, the migration is performed and the post-migration infrastructure state is returned. +// If no migration is required, the current (pre-upgrade) infrastructure state is returned. +func (u *upgradeApplyCmd) migrateTerraform( + cmd *cobra.Command, conf *config.Config, upgradeDir string, flags upgradeApplyFlags, +) (state.Infrastructure, error) { // If there are any Terraform migrations to apply, ask for confirmation fmt.Fprintln(cmd.OutOrStdout(), "The upgrade requires a migration of Constellation cloud resources by applying an updated Terraform template. Please manually review the suggested changes below.") if !flags.yes { @@ -513,34 +463,6 @@ func (u *upgradeApplyCmd) handleServiceUpgrade( return nil } -// migrateFrom2_10 applies migrations necessary for upgrading from v2.10 to v2.11 -// TODO(v2.11): Remove this function after v2.11 is released. -func migrateFrom2_10(ctx context.Context, kubeUpgrader kubernetesUpgrader) error { - // Sanity check to make sure we only run migrations on upgrades with CLI version 2.10 < v < 2.12 - if !constants.BinaryVersion().MajorMinorEqual(semver.NewFromInt(2, 11, 0, "")) { - return nil - } - - if err := kubeUpgrader.RemoveAttestationConfigHelmManagement(ctx); err != nil { - return fmt.Errorf("removing helm management from attestation config: %w", err) - } - return nil -} - -// migrateFrom2_11 applies migrations necessary for upgrading from v2.11 to v2.12 -// TODO(v2.12): Remove this function after v2.12 is released. -func migrateFrom2_11(ctx context.Context, kubeUpgrader kubernetesUpgrader) error { - // Sanity check to make sure we only run migrations on upgrades with CLI version 2.11 < v < 2.13 - if !constants.BinaryVersion().MajorMinorEqual(semver.NewFromInt(2, 12, 0, "")) { - return nil - } - - if err := kubeUpgrader.RemoveHelmKeepAnnotation(ctx); err != nil { - return fmt.Errorf("removing helm keep annotation: %w", err) - } - return nil -} - func parseUpgradeApplyFlags(cmd *cobra.Command) (upgradeApplyFlags, error) { workDir, err := cmd.Flags().GetString("workspace") if err != nil { @@ -641,10 +563,6 @@ type kubernetesUpgrader interface { ApplyJoinConfig(ctx context.Context, newAttestConfig config.AttestationCfg, measurementSalt []byte) error BackupCRs(ctx context.Context, crds []apiextensionsv1.CustomResourceDefinition, upgradeDir string) error BackupCRDs(ctx context.Context, upgradeDir string) ([]apiextensionsv1.CustomResourceDefinition, error) - // TODO(v2.11): Remove this function after v2.11 is released. - RemoveAttestationConfigHelmManagement(ctx context.Context) error - // TODO(v2.12): Remove this function after v2.12 is released. - RemoveHelmKeepAnnotation(ctx context.Context) error } type clusterUpgrader interface { diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index e3a2b3e52f..96e4c8688b 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -12,7 +12,6 @@ import ( "io" "testing" - "github.com/edgelesssys/constellation/v2/cli/internal/clusterid" "github.com/edgelesssys/constellation/v2/cli/internal/helm" "github.com/edgelesssys/constellation/v2/cli/internal/kubecmd" "github.com/edgelesssys/constellation/v2/cli/internal/state" @@ -42,16 +41,6 @@ func TestUpgradeApply(t *testing.T) { InitSecret: []byte{0x42}, }). SetClusterValues(state.ClusterValues{MeasurementSalt: []byte{0x41}}) - defaultIDFile := clusterid.File{ - MeasurementSalt: []byte{0x41}, - UID: "uid", - InitSecret: []byte{0x42}, - } - fsWithIDFile := func() file.Handler { - fh := file.NewHandler(afero.NewMemMapFs()) - require.NoError(t, fh.WriteJSON(constants.ClusterIDsFilename, defaultIDFile)) - return fh - } fsWithStateFile := func() file.Handler { fh := file.NewHandler(afero.NewMemMapFs()) require.NoError(t, fh.WriteYAML(constants.StateFilename, defaultState)) @@ -59,55 +48,34 @@ func TestUpgradeApply(t *testing.T) { } testCases := map[string]struct { - helmUpgrader helmApplier - kubeUpgrader *stubKubernetesUpgrader - fh func() file.Handler - fhAssertions func(require *require.Assertions, assert *assert.Assertions, fh file.Handler) - terraformUpgrader clusterUpgrader - infrastructureShower *stubShowInfrastructure - wantErr bool - customK8sVersion string - flags upgradeApplyFlags - stdin string + helmUpgrader helmApplier + kubeUpgrader *stubKubernetesUpgrader + fh func() file.Handler + fhAssertions func(require *require.Assertions, assert *assert.Assertions, fh file.Handler) + terraformUpgrader clusterUpgrader + wantErr bool + customK8sVersion string + flags upgradeApplyFlags + stdin string }{ "success": { - kubeUpgrader: &stubKubernetesUpgrader{currentConfig: config.DefaultForAzureSEVSNP()}, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{}, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, - fhAssertions: func(require *require.Assertions, assert *assert.Assertions, fh file.Handler) { - gotState, err := state.ReadFromFile(fh, constants.StateFilename) - require.NoError(err) - assert.Equal("v1", gotState.Version) - assert.Equal(defaultState, gotState) - }, - }, - "fall back to id file": { - kubeUpgrader: &stubKubernetesUpgrader{currentConfig: config.DefaultForAzureSEVSNP()}, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{}, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithIDFile, + kubeUpgrader: &stubKubernetesUpgrader{currentConfig: config.DefaultForAzureSEVSNP()}, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{}, + flags: upgradeApplyFlags{yes: true}, + fh: fsWithStateFile, fhAssertions: func(require *require.Assertions, assert *assert.Assertions, fh file.Handler) { gotState, err := state.ReadFromFile(fh, constants.StateFilename) require.NoError(err) assert.Equal("v1", gotState.Version) assert.Equal(defaultState, gotState) - var oldIDFile clusterid.File - err = fh.ReadJSON(constants.ClusterIDsFilename+".old", &oldIDFile) - assert.NoError(err) - assert.Equal(defaultIDFile, oldIDFile) }, }, - "id file and state file do not exist": { - kubeUpgrader: &stubKubernetesUpgrader{currentConfig: config.DefaultForAzureSEVSNP()}, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{}, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, + "state file does not exist": { + kubeUpgrader: &stubKubernetesUpgrader{currentConfig: config.DefaultForAzureSEVSNP()}, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{}, + flags: upgradeApplyFlags{yes: true}, fh: func() file.Handler { return file.NewHandler(afero.NewMemMapFs()) }, @@ -118,67 +86,61 @@ func TestUpgradeApply(t *testing.T) { currentConfig: config.DefaultForAzureSEVSNP(), nodeVersionErr: assert.AnError, }, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{}, - wantErr: true, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{}, + wantErr: true, + flags: upgradeApplyFlags{yes: true}, + fh: fsWithStateFile, }, "nodeVersion in progress error": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), nodeVersionErr: kubecmd.ErrInProgress, }, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{}, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{}, + flags: upgradeApplyFlags{yes: true}, + fh: fsWithStateFile, }, "helm other error": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), }, - helmUpgrader: stubApplier{err: assert.AnError}, - terraformUpgrader: &stubTerraformUpgrader{}, - wantErr: true, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + helmUpgrader: stubApplier{err: assert.AnError}, + terraformUpgrader: &stubTerraformUpgrader{}, + wantErr: true, + flags: upgradeApplyFlags{yes: true}, + fh: fsWithStateFile, }, "abort": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), }, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{terraformDiff: true}, - wantErr: true, - stdin: "no\n", - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{terraformDiff: true}, + wantErr: true, + stdin: "no\n", + fh: fsWithStateFile, }, "abort, restore terraform err": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), }, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{terraformDiff: true, rollbackWorkspaceErr: assert.AnError}, - wantErr: true, - stdin: "no\n", - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{terraformDiff: true, rollbackWorkspaceErr: assert.AnError}, + wantErr: true, + stdin: "no\n", + fh: fsWithStateFile, }, "plan terraform error": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), }, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{planTerraformErr: assert.AnError}, - wantErr: true, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{planTerraformErr: assert.AnError}, + wantErr: true, + flags: upgradeApplyFlags{yes: true}, + fh: fsWithStateFile, }, "apply terraform error": { kubeUpgrader: &stubKubernetesUpgrader{ @@ -189,10 +151,9 @@ func TestUpgradeApply(t *testing.T) { applyTerraformErr: assert.AnError, terraformDiff: true, }, - wantErr: true, - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + wantErr: true, + flags: upgradeApplyFlags{yes: true}, + fh: fsWithStateFile, }, "outdated K8s patch version": { kubeUpgrader: &stubKubernetesUpgrader{ @@ -205,21 +166,19 @@ func TestUpgradeApply(t *testing.T) { require.NoError(t, err) return semver.NewFromInt(v.Major(), v.Minor(), v.Patch()-1, "").String() }(), - flags: upgradeApplyFlags{yes: true}, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + flags: upgradeApplyFlags{yes: true}, + fh: fsWithStateFile, }, "outdated K8s version": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), }, - helmUpgrader: stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{}, - customK8sVersion: "v1.20.0", - flags: upgradeApplyFlags{yes: true}, - wantErr: true, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{}, + customK8sVersion: "v1.20.0", + flags: upgradeApplyFlags{yes: true}, + wantErr: true, + fh: fsWithStateFile, }, "skip all upgrade phases": { kubeUpgrader: &stubKubernetesUpgrader{ @@ -231,24 +190,7 @@ func TestUpgradeApply(t *testing.T) { skipPhases: []skipPhase{skipInfrastructurePhase, skipHelmPhase, skipK8sPhase, skipImagePhase}, yes: true, }, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, - }, - "show state err": { - kubeUpgrader: &stubKubernetesUpgrader{ - currentConfig: config.DefaultForAzureSEVSNP(), - }, - helmUpgrader: &stubApplier{}, - terraformUpgrader: &stubTerraformUpgrader{}, - flags: upgradeApplyFlags{ - skipPhases: []skipPhase{skipInfrastructurePhase}, - yes: true, - }, - infrastructureShower: &stubShowInfrastructure{ - showInfraErr: assert.AnError, - }, - wantErr: true, - fh: fsWithStateFile, + fh: fsWithStateFile, }, "skip all phases except node upgrade": { kubeUpgrader: &stubKubernetesUpgrader{ @@ -260,8 +202,7 @@ func TestUpgradeApply(t *testing.T) { skipPhases: []skipPhase{skipInfrastructurePhase, skipHelmPhase, skipK8sPhase}, yes: true, }, - infrastructureShower: &stubShowInfrastructure{}, - fh: fsWithStateFile, + fh: fsWithStateFile, }, } @@ -286,7 +227,6 @@ func TestUpgradeApply(t *testing.T) { clusterUpgrader: tc.terraformUpgrader, log: logger.NewTest(t), configFetcher: stubAttestationFetcher{}, - clusterShower: tc.infrastructureShower, fileHandler: fh, } @@ -408,11 +348,3 @@ func (m *mockApplier) PrepareApply(cfg *config.Config, stateFile *state.State, args := m.Called(cfg, stateFile, helmOpts, str, masterSecret) return args.Get(0).(helm.Applier), args.Bool(1), args.Error(2) } - -type stubShowInfrastructure struct { - showInfraErr error -} - -func (s *stubShowInfrastructure) ShowInfrastructure(context.Context, cloudprovider.Provider) (state.Infrastructure, error) { - return state.Infrastructure{}, s.showInfraErr -} diff --git a/cli/internal/kubecmd/BUILD.bazel b/cli/internal/kubecmd/BUILD.bazel index 075529fdd7..e288b52d33 100644 --- a/cli/internal/kubecmd/BUILD.bazel +++ b/cli/internal/kubecmd/BUILD.bazel @@ -35,7 +35,6 @@ go_library( "@io_k8s_client_go//util/retry", "@io_k8s_kubernetes//cmd/kubeadm/app/apis/kubeadm/v1beta3", "@io_k8s_sigs_yaml//:yaml", - "@sh_helm_helm_v3//pkg/kube", ], ) diff --git a/cli/internal/kubecmd/kubecmd.go b/cli/internal/kubecmd/kubecmd.go index 0b93119b25..7a9a952a95 100644 --- a/cli/internal/kubecmd/kubecmd.go +++ b/cli/internal/kubecmd/kubecmd.go @@ -40,7 +40,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/versions" "github.com/edgelesssys/constellation/v2/internal/versions/components" updatev1alpha1 "github.com/edgelesssys/constellation/v2/operators/constellation-node-operator/v2/api/v1alpha1" - "helm.sh/helm/v3/pkg/kube" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -476,72 +475,6 @@ func joinConfigMap(attestationCfgJSON, measurementSalt []byte) *corev1.ConfigMap } } -// RemoveAttestationConfigHelmManagement removes labels and annotations from the join-config ConfigMap that are added by Helm. -// This is to ensure we can cleanly transition from Helm to Constellation's management of the ConfigMap. -// TODO(v2.11): Remove this function after v2.11 is released. -func (k *KubeCmd) RemoveAttestationConfigHelmManagement(ctx context.Context) error { - const ( - appManagedByLabel = "app.kubernetes.io/managed-by" - appManagedByHelm = "Helm" - helmReleaseNameAnnotation = "meta.helm.sh/release-name" - helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace" - ) - - k.log.Debugf("Checking if join-config ConfigMap needs to be migrated to remove Helm management") - joinConfig, err := k.kubectl.GetConfigMap(ctx, constants.ConstellationNamespace, constants.JoinConfigMap) - if err != nil { - return fmt.Errorf("getting join config: %w", err) - } - - var needUpdate bool - if managedBy, ok := joinConfig.Labels[appManagedByLabel]; ok && managedBy == appManagedByHelm { - delete(joinConfig.Labels, appManagedByLabel) - needUpdate = true - } - if _, ok := joinConfig.Annotations[helmReleaseNameAnnotation]; ok { - delete(joinConfig.Annotations, helmReleaseNameAnnotation) - needUpdate = true - } - if _, ok := joinConfig.Annotations[helmReleaseNamespaceAnnotation]; ok { - delete(joinConfig.Annotations, helmReleaseNamespaceAnnotation) - needUpdate = true - } - - if needUpdate { - // Tell Helm to ignore this resource in the future. - // TODO(v2.11): Remove this annotation from the ConfigMap. - joinConfig.Annotations[kube.ResourcePolicyAnno] = kube.KeepPolicy - - k.log.Debugf("Removing Helm management labels from join-config ConfigMap") - if _, err := k.kubectl.UpdateConfigMap(ctx, joinConfig); err != nil { - return fmt.Errorf("removing Helm management labels from join-config: %w", err) - } - k.log.Debugf("Successfully removed Helm management labels from join-config ConfigMap") - } - - return nil -} - -// RemoveHelmKeepAnnotation removes the Helm Resource Policy annotation from the join-config ConfigMap. -// TODO(v2.12): Remove this function after v2.12 is released. -func (k *KubeCmd) RemoveHelmKeepAnnotation(ctx context.Context) error { - k.log.Debugf("Checking if Helm Resource Policy can be removed from join-config ConfigMap") - joinConfig, err := k.kubectl.GetConfigMap(ctx, constants.ConstellationNamespace, constants.JoinConfigMap) - if err != nil { - return fmt.Errorf("getting join config: %w", err) - } - - if policy, ok := joinConfig.Annotations[kube.ResourcePolicyAnno]; ok && policy == kube.KeepPolicy { - delete(joinConfig.Annotations, kube.ResourcePolicyAnno) - if _, err := k.kubectl.UpdateConfigMap(ctx, joinConfig); err != nil { - return fmt.Errorf("removing Helm Resource Policy from join-config: %w", err) - } - k.log.Debugf("Successfully removed Helm Resource Policy from join-config ConfigMap") - } - - return nil -} - type kubeDoer struct { action func(ctx context.Context) error } diff --git a/cli/internal/state/BUILD.bazel b/cli/internal/state/BUILD.bazel index cbe36bdd70..914e81bd29 100644 --- a/cli/internal/state/BUILD.bazel +++ b/cli/internal/state/BUILD.bazel @@ -7,8 +7,6 @@ go_library( importpath = "github.com/edgelesssys/constellation/v2/cli/internal/state", visibility = ["//cli:__subpackages__"], deps = [ - "//cli/internal/clusterid", - "//internal/config", "//internal/file", "@cat_dario_mergo//:mergo", ], @@ -19,8 +17,6 @@ go_test( srcs = ["state_test.go"], embed = [":state"], deps = [ - "//cli/internal/clusterid", - "//internal/config", "//internal/constants", "//internal/file", "@com_github_spf13_afero//:afero", diff --git a/cli/internal/state/state.go b/cli/internal/state/state.go index ae56b8bdbe..4ea873fc3f 100644 --- a/cli/internal/state/state.go +++ b/cli/internal/state/state.go @@ -11,8 +11,6 @@ import ( "fmt" "dario.cat/mergo" - "github.com/edgelesssys/constellation/v2/cli/internal/clusterid" - "github.com/edgelesssys/constellation/v2/internal/config" "github.com/edgelesssys/constellation/v2/internal/file" ) @@ -44,31 +42,6 @@ func New() *State { } } -// NewFromIDFile creates a new cluster state file from the given ID file and config. -func NewFromIDFile(idFile clusterid.File, cfg *config.Config) *State { - s := New(). - SetClusterValues(ClusterValues{ - OwnerID: idFile.OwnerID, - ClusterID: idFile.ClusterID, - MeasurementSalt: idFile.MeasurementSalt, - }). - SetInfrastructure(Infrastructure{ - UID: idFile.UID, - ClusterEndpoint: idFile.IP, - APIServerCertSANs: idFile.APIServerCertSANs, - InitSecret: idFile.InitSecret, - Name: clusterid.GetClusterName(cfg, idFile), - }) - - if idFile.AttestationURL != "" { - s.Infrastructure.Azure = &Azure{ - AttestationURL: idFile.AttestationURL, - } - } - - return s -} - // SetInfrastructure sets the infrastructure state. func (s *State) SetInfrastructure(infrastructure Infrastructure) *State { s.Infrastructure = infrastructure diff --git a/cli/internal/state/state_test.go b/cli/internal/state/state_test.go index f62929c445..dd7c305bbc 100644 --- a/cli/internal/state/state_test.go +++ b/cli/internal/state/state_test.go @@ -9,8 +9,6 @@ package state import ( "testing" - "github.com/edgelesssys/constellation/v2/cli/internal/clusterid" - "github.com/edgelesssys/constellation/v2/internal/config" "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/spf13/afero" @@ -328,65 +326,3 @@ func TestMerge(t *testing.T) { }) } } - -func TestNewFromIDFile(t *testing.T) { - testCases := map[string]struct { - idFile clusterid.File - cfg *config.Config - expected *State - }{ - "success": { - idFile: clusterid.File{ - ClusterID: "test-cluster-id", - UID: "test-uid", - }, - cfg: config.Default(), - expected: &State{ - Version: Version1, - Infrastructure: Infrastructure{ - UID: "test-uid", - Name: "constell-test-uid", - }, - ClusterValues: ClusterValues{ - ClusterID: "test-cluster-id", - }, - }, - }, - "empty id file": { - idFile: clusterid.File{}, - cfg: config.Default(), - expected: &State{Version: Version1, Infrastructure: Infrastructure{Name: "constell-"}}, - }, - "nested pointer": { - idFile: clusterid.File{ - ClusterID: "test-cluster-id", - UID: "test-uid", - AttestationURL: "test-maaUrl", - }, - cfg: config.Default(), - expected: &State{ - Version: Version1, - Infrastructure: Infrastructure{ - UID: "test-uid", - Azure: &Azure{ - AttestationURL: "test-maaUrl", - }, - Name: "constell-test-uid", - }, - ClusterValues: ClusterValues{ - ClusterID: "test-cluster-id", - }, - }, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - assert := assert.New(t) - - state := NewFromIDFile(tc.idFile, tc.cfg) - - assert.Equal(tc.expected, state) - }) - } -} diff --git a/internal/constants/constants.go b/internal/constants/constants.go index ad7ee7d428..853c45dfdb 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -74,8 +74,6 @@ const ( // Filenames. // - // ClusterIDsFilename filename that contains Constellation clusterID and IP. - ClusterIDsFilename = "constellation-id.json" // StateFilename filename that contains the entire state of the Constellation cluster. StateFilename = "constellation-state.yaml" // ConfigFilename filename of Constellation config file.