From 95cf4bdf21658e3b0f6d6772001c14d2ba5c464c Mon Sep 17 00:00:00 2001 From: Moritz Sanft <58110325+msanft@users.noreply.github.com> Date: Thu, 14 Sep 2023 11:51:20 +0200 Subject: [PATCH] cli: perform upgrades in-place in Terraform workspace (#2317) * perform upgrades in-place in terraform workspace Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * update buildfiles Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * add iam upgrade apply test Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * update buildfiles Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * fix linter Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * make config fetcher stubbable Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * change workspace restoring behaviour Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * allow overwriting existing Terraform files Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * allow overwrites of TF variables Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * fix iam upgrade apply Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * fix embed directive Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * make loader test less brittle Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * pass upgrade ID to user Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * naming nit Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * use upgradeDir Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> * tidy Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> --------- Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> --- cli/internal/cloudcmd/BUILD.bazel | 1 - cli/internal/cloudcmd/clients.go | 2 +- cli/internal/cloudcmd/clusterupgrade.go | 18 +- cli/internal/cloudcmd/clusterupgrade_test.go | 2 +- cli/internal/cloudcmd/iamupgrade.go | 18 +- cli/internal/cloudcmd/iamupgrade_test.go | 136 -------------- cli/internal/cloudcmd/tfupgrade.go | 25 ++- cli/internal/cloudcmd/tfupgrade_test.go | 21 +-- cli/internal/cmd/BUILD.bazel | 1 + cli/internal/cmd/iamupgradeapply.go | 21 ++- cli/internal/cmd/iamupgradeapply_test.go | 181 +++++++++++++++++++ cli/internal/cmd/upgradeapply.go | 12 +- cli/internal/cmd/upgradeapply_test.go | 25 ++- cli/internal/cmd/upgradecheck.go | 18 +- cli/internal/cmd/upgradecheck_test.go | 42 +++-- cli/internal/terraform/BUILD.bazel | 3 + cli/internal/terraform/loader.go | 43 +++-- cli/internal/terraform/loader_test.go | 106 ++++++----- cli/internal/terraform/terraform.go | 21 ++- 19 files changed, 410 insertions(+), 286 deletions(-) delete mode 100644 cli/internal/cloudcmd/iamupgrade_test.go create mode 100644 cli/internal/cmd/iamupgradeapply_test.go diff --git a/cli/internal/cloudcmd/BUILD.bazel b/cli/internal/cloudcmd/BUILD.bazel index 115e37fd5..d1019732e 100644 --- a/cli/internal/cloudcmd/BUILD.bazel +++ b/cli/internal/cloudcmd/BUILD.bazel @@ -52,7 +52,6 @@ go_test( "clusterupgrade_test.go", "create_test.go", "iam_test.go", - "iamupgrade_test.go", "patch_test.go", "rollback_test.go", "terminate_test.go", diff --git a/cli/internal/cloudcmd/clients.go b/cli/internal/cloudcmd/clients.go index 068fb4ecc..bde1c3087 100644 --- a/cli/internal/cloudcmd/clients.go +++ b/cli/internal/cloudcmd/clients.go @@ -45,7 +45,7 @@ type tfIAMClient interface { type tfUpgradePlanner interface { ShowPlan(ctx context.Context, logLevel terraform.LogLevel, output io.Writer) error Plan(ctx context.Context, logLevel terraform.LogLevel) (bool, error) - PrepareUpgradeWorkspace(embeddedPath, oldWorkingDir, backupDir string, vars terraform.Variables) error + PrepareUpgradeWorkspace(embeddedPath, backupDir string, vars terraform.Variables) error } type tfIAMUpgradeClient interface { diff --git a/cli/internal/cloudcmd/clusterupgrade.go b/cli/internal/cloudcmd/clusterupgrade.go index b1d305b9c..ca5073bb2 100644 --- a/cli/internal/cloudcmd/clusterupgrade.go +++ b/cli/internal/cloudcmd/clusterupgrade.go @@ -35,7 +35,7 @@ type ClusterUpgrader struct { func NewClusterUpgrader(ctx context.Context, existingWorkspace, upgradeWorkspace string, logLevel terraform.LogLevel, fileHandler file.Handler, ) (*ClusterUpgrader, error) { - tfClient, err := terraform.New(ctx, filepath.Join(upgradeWorkspace, constants.TerraformUpgradeWorkingDir)) + tfClient, err := terraform.New(ctx, constants.TerraformWorkingDir) if err != nil { return nil, fmt.Errorf("setting up terraform client: %w", err) } @@ -57,11 +57,17 @@ func (u *ClusterUpgrader) PlanClusterUpgrade(ctx context.Context, outWriter io.W return planUpgrade( ctx, u.tf, u.fileHandler, outWriter, u.logLevel, vars, filepath.Join("terraform", strings.ToLower(csp.String())), - u.existingWorkspace, filepath.Join(u.upgradeWorkspace, constants.TerraformUpgradeBackupDir), ) } +// RestoreClusterWorkspace rolls back the existing workspace to the backup directory created when planning an upgrade, +// when the user decides to not apply an upgrade after planning it. +// Note that this will not apply the restored state from the backup. +func (u *ClusterUpgrader) RestoreClusterWorkspace() error { + return restoreBackup(u.fileHandler, u.existingWorkspace, filepath.Join(u.upgradeWorkspace, constants.TerraformUpgradeBackupDir)) +} + // ApplyClusterUpgrade applies the Terraform migrations planned by PlanClusterUpgrade. // On success, the workspace of the Upgrader replaces the existing Terraform workspace. func (u *ClusterUpgrader) ApplyClusterUpgrade(ctx context.Context, csp cloudprovider.Provider) (terraform.ApplyOutput, error) { @@ -75,13 +81,5 @@ func (u *ClusterUpgrader) ApplyClusterUpgrade(ctx context.Context, csp cloudprov } } - if err := moveUpgradeToCurrent( - u.fileHandler, - u.existingWorkspace, - filepath.Join(u.upgradeWorkspace, constants.TerraformUpgradeWorkingDir), - ); err != nil { - return tfOutput, fmt.Errorf("promoting upgrade workspace to current workspace: %w", err) - } - return tfOutput, nil } diff --git a/cli/internal/cloudcmd/clusterupgrade_test.go b/cli/internal/cloudcmd/clusterupgrade_test.go index 8551af8b2..8378ca59d 100644 --- a/cli/internal/cloudcmd/clusterupgrade_test.go +++ b/cli/internal/cloudcmd/clusterupgrade_test.go @@ -198,6 +198,6 @@ func (t *tfClusterUpgradeStub) ApplyCluster(_ context.Context, _ cloudprovider.P return terraform.ApplyOutput{}, t.applyErr } -func (t *tfClusterUpgradeStub) PrepareUpgradeWorkspace(_, _, _ string, _ terraform.Variables) error { +func (t *tfClusterUpgradeStub) PrepareUpgradeWorkspace(_, _ string, _ terraform.Variables) error { return t.prepareWorkspaceErr } diff --git a/cli/internal/cloudcmd/iamupgrade.go b/cli/internal/cloudcmd/iamupgrade.go index 4594930a0..bab789ac2 100644 --- a/cli/internal/cloudcmd/iamupgrade.go +++ b/cli/internal/cloudcmd/iamupgrade.go @@ -42,7 +42,7 @@ type IAMUpgrader struct { func NewIAMUpgrader(ctx context.Context, existingWorkspace, upgradeWorkspace string, logLevel terraform.LogLevel, fileHandler file.Handler, ) (*IAMUpgrader, error) { - tfClient, err := terraform.New(ctx, filepath.Join(upgradeWorkspace, constants.TerraformIAMUpgradeWorkingDir)) + tfClient, err := terraform.New(ctx, constants.TerraformIAMWorkingDir) if err != nil { return nil, fmt.Errorf("setting up terraform client: %w", err) } @@ -62,11 +62,17 @@ func (u *IAMUpgrader) PlanIAMUpgrade(ctx context.Context, outWriter io.Writer, v return planUpgrade( ctx, u.tf, u.fileHandler, outWriter, u.logLevel, vars, filepath.Join("terraform", "iam", strings.ToLower(csp.String())), - u.existingWorkspace, filepath.Join(u.upgradeWorkspace, constants.TerraformIAMUpgradeBackupDir), ) } +// RestoreIAMWorkspace rolls back the existing workspace to the backup directory created when planning an upgrade, +// when the user decides to not apply an upgrade after planning it. +// Note that this will not apply the restored state from the backup. +func (u *IAMUpgrader) RestoreIAMWorkspace() error { + return restoreBackup(u.fileHandler, u.existingWorkspace, filepath.Join(u.upgradeWorkspace, constants.TerraformIAMUpgradeBackupDir)) +} + // ApplyIAMUpgrade applies the Terraform IAM migrations planned by PlanIAMUpgrade. // On success, the workspace of the Upgrader replaces the existing Terraform workspace. func (u *IAMUpgrader) ApplyIAMUpgrade(ctx context.Context, csp cloudprovider.Provider) error { @@ -74,13 +80,5 @@ func (u *IAMUpgrader) ApplyIAMUpgrade(ctx context.Context, csp cloudprovider.Pro return fmt.Errorf("terraform apply: %w", err) } - if err := moveUpgradeToCurrent( - u.fileHandler, - u.existingWorkspace, - filepath.Join(u.upgradeWorkspace, constants.TerraformIAMUpgradeWorkingDir), - ); err != nil { - return fmt.Errorf("promoting upgrade workspace to current workspace: %w", err) - } - return nil } diff --git a/cli/internal/cloudcmd/iamupgrade_test.go b/cli/internal/cloudcmd/iamupgrade_test.go deleted file mode 100644 index ef973d817..000000000 --- a/cli/internal/cloudcmd/iamupgrade_test.go +++ /dev/null @@ -1,136 +0,0 @@ -/* -Copyright (c) Edgeless Systems GmbH - -SPDX-License-Identifier: AGPL-3.0-only -*/ - -package cloudcmd - -import ( - "context" - "io" - "path/filepath" - "testing" - - "github.com/edgelesssys/constellation/v2/cli/internal/terraform" - "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" - "github.com/edgelesssys/constellation/v2/internal/constants" - "github.com/edgelesssys/constellation/v2/internal/file" - "github.com/spf13/afero" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestIAMMigrate(t *testing.T) { - assert := assert.New(t) - upgradeID := "test-upgrade" - upgradeDir := filepath.Join(constants.UpgradeDir, upgradeID, constants.TerraformIAMUpgradeWorkingDir) - fs, file := setupMemFSAndFileHandler(t, []string{"terraform.tfvars", "terraform.tfstate"}, []byte("OLD")) - csp := cloudprovider.AWS - - // act - fakeTfClient := &tfIAMUpgradeStub{upgradeID: upgradeID, file: file} - sut := &IAMUpgrader{ - tf: fakeTfClient, - logLevel: terraform.LogLevelDebug, - existingWorkspace: constants.TerraformIAMWorkingDir, - upgradeWorkspace: filepath.Join(constants.UpgradeDir, upgradeID), - fileHandler: file, - } - hasDiff, err := sut.PlanIAMUpgrade(context.Background(), io.Discard, &terraform.QEMUVariables{}, csp) - - // assert - assert.NoError(err) - assert.False(hasDiff) - assertFileExists(t, fs, filepath.Join(upgradeDir, "terraform.tfvars")) - assertFileExists(t, fs, filepath.Join(upgradeDir, "terraform.tfstate")) - - // act - err = sut.ApplyIAMUpgrade(context.Background(), csp) - assert.NoError(err) - - // assert - assertFileReadsContent(t, file, filepath.Join(constants.TerraformIAMWorkingDir, "terraform.tfvars"), "NEW") - assertFileReadsContent(t, file, filepath.Join(constants.TerraformIAMWorkingDir, "terraform.tfstate"), "NEW") - assertFileDoesntExist(t, fs, filepath.Join(upgradeDir)) -} - -func assertFileReadsContent(t *testing.T, file file.Handler, path string, expectedContent string) { - t.Helper() - bt, err := file.Read(path) - assert.NoError(t, err) - assert.Equal(t, expectedContent, string(bt)) -} - -func assertFileExists(t *testing.T, fs afero.Fs, path string) { - t.Helper() - res, err := fs.Stat(path) - assert.NoError(t, err) - assert.NotNil(t, res) -} - -func assertFileDoesntExist(t *testing.T, fs afero.Fs, path string) { - t.Helper() - res, err := fs.Stat(path) - assert.Error(t, err) - assert.Nil(t, res) -} - -// setupMemFSAndFileHandler sets up a file handler with a memory file system and writes the given files with the given content. -func setupMemFSAndFileHandler(t *testing.T, files []string, content []byte) (afero.Fs, file.Handler) { - fs := afero.NewMemMapFs() - file := file.NewHandler(fs) - err := file.MkdirAll(constants.TerraformIAMWorkingDir) - require.NoError(t, err) - - for _, f := range files { - err := file.Write(filepath.Join(constants.TerraformIAMWorkingDir, f), content) - require.NoError(t, err) - } - return fs, file -} - -type tfIAMUpgradeStub struct { - upgradeID string - file file.Handler - applyErr error - planErr error - planDiff bool - showErr error - prepareWorkspaceErr error -} - -func (t *tfIAMUpgradeStub) Plan(_ context.Context, _ terraform.LogLevel) (bool, error) { - return t.planDiff, t.planErr -} - -func (t *tfIAMUpgradeStub) ShowPlan(_ context.Context, _ terraform.LogLevel, _ io.Writer) error { - return t.showErr -} - -func (t *tfIAMUpgradeStub) ApplyIAM(_ context.Context, _ cloudprovider.Provider, _ terraform.LogLevel) (terraform.IAMOutput, error) { - if t.applyErr != nil { - return terraform.IAMOutput{}, t.applyErr - } - - upgradeDir := filepath.Join(constants.UpgradeDir, t.upgradeID, constants.TerraformIAMUpgradeWorkingDir) - if err := t.file.Write(filepath.Join(upgradeDir, "terraform.tfvars"), []byte("NEW"), file.OptOverwrite); err != nil { - return terraform.IAMOutput{}, err - } - if err := t.file.Write(filepath.Join(upgradeDir, "terraform.tfstate"), []byte("NEW"), file.OptOverwrite); err != nil { - return terraform.IAMOutput{}, err - } - return terraform.IAMOutput{}, nil -} - -func (t *tfIAMUpgradeStub) PrepareUpgradeWorkspace(_, _, _ string, _ terraform.Variables) error { - if t.prepareWorkspaceErr != nil { - return t.prepareWorkspaceErr - } - - upgradeDir := filepath.Join(constants.UpgradeDir, t.upgradeID, constants.TerraformIAMUpgradeWorkingDir) - if err := t.file.Write(filepath.Join(upgradeDir, "terraform.tfvars"), []byte("OLD")); err != nil { - return err - } - return t.file.Write(filepath.Join(upgradeDir, "terraform.tfstate"), []byte("OLD")) -} diff --git a/cli/internal/cloudcmd/tfupgrade.go b/cli/internal/cloudcmd/tfupgrade.go index dcc776991..6f5f225ef 100644 --- a/cli/internal/cloudcmd/tfupgrade.go +++ b/cli/internal/cloudcmd/tfupgrade.go @@ -21,16 +21,15 @@ import ( func planUpgrade( ctx context.Context, tfClient tfUpgradePlanner, fileHandler file.Handler, outWriter io.Writer, logLevel terraform.LogLevel, vars terraform.Variables, - templateDir, existingWorkspace, backupDir string, + templateDir, backupDir string, ) (bool, error) { if err := ensureFileNotExist(fileHandler, backupDir); err != nil { - return false, fmt.Errorf("workspace is not clean: %w", err) + return false, fmt.Errorf("backup directory %s already exists: %w", backupDir, err) } - // Prepare the new Terraform workspace and backup the old one + // Backup the old Terraform workspace and move the embedded Terraform files into the workspace. err := tfClient.PrepareUpgradeWorkspace( templateDir, - existingWorkspace, backupDir, vars, ) @@ -52,20 +51,20 @@ func planUpgrade( return hasDiff, nil } -// moveUpgradeToCurrent replaces the an existing Terraform workspace with a workspace holding migrated Terraform resources. -func moveUpgradeToCurrent(fileHandler file.Handler, existingWorkspace, upgradeWorkingDir string) error { - if err := fileHandler.RemoveAll(existingWorkspace); err != nil { - return fmt.Errorf("removing old terraform directory: %w", err) +// restoreBackup replaces the existing Terraform workspace with the backup. +func restoreBackup(fileHandler file.Handler, workingDir, backupDir string) error { + if err := fileHandler.RemoveAll(workingDir); err != nil { + return fmt.Errorf("removing existing workspace: %w", err) } if err := fileHandler.CopyDir( - upgradeWorkingDir, - existingWorkspace, + backupDir, + workingDir, ); err != nil { - return fmt.Errorf("replacing old terraform directory with new one: %w", err) + return fmt.Errorf("replacing terraform workspace with backup: %w", err) } - if err := fileHandler.RemoveAll(upgradeWorkingDir); err != nil { - return fmt.Errorf("removing terraform upgrade directory: %w", err) + if err := fileHandler.RemoveAll(backupDir); err != nil { + return fmt.Errorf("removing backup directory: %w", err) } return nil } diff --git a/cli/internal/cloudcmd/tfupgrade_test.go b/cli/internal/cloudcmd/tfupgrade_test.go index b8869e251..1ee0ea948 100644 --- a/cli/internal/cloudcmd/tfupgrade_test.go +++ b/cli/internal/cloudcmd/tfupgrade_test.go @@ -87,7 +87,7 @@ func TestPlanUpgrade(t *testing.T) { hasDiff, err := planUpgrade( context.Background(), tc.tf, fs, io.Discard, terraform.LogLevelDebug, &terraform.QEMUVariables{}, - "existing", "upgrade", "backup", + "test", "backup", ) if tc.wantErr { assert.Error(err) @@ -99,9 +99,9 @@ func TestPlanUpgrade(t *testing.T) { } } -func TestMoveUpgradeToCurrent(t *testing.T) { +func TestRestoreBackup(t *testing.T) { existingWorkspace := "foo" - upgradeWorkingDir := "bar" + backupDir := "bar" testCases := map[string]struct { prepareFs func(require *require.Assertions) file.Handler @@ -111,18 +111,18 @@ func TestMoveUpgradeToCurrent(t *testing.T) { prepareFs: func(require *require.Assertions) file.Handler { fs := file.NewHandler(afero.NewMemMapFs()) require.NoError(fs.MkdirAll(existingWorkspace)) - require.NoError(fs.MkdirAll(upgradeWorkingDir)) + require.NoError(fs.MkdirAll(backupDir)) return fs }, }, - "old workspace does not exist": { + "existing workspace does not exist": { prepareFs: func(require *require.Assertions) file.Handler { fs := file.NewHandler(afero.NewMemMapFs()) - require.NoError(fs.MkdirAll(upgradeWorkingDir)) + require.NoError(fs.MkdirAll(backupDir)) return fs }, }, - "upgrade working dir does not exist": { + "backup dir does not exist": { prepareFs: func(require *require.Assertions) file.Handler { fs := file.NewHandler(afero.NewMemMapFs()) require.NoError(fs.MkdirAll(existingWorkspace)) @@ -135,8 +135,7 @@ func TestMoveUpgradeToCurrent(t *testing.T) { memFS := afero.NewMemMapFs() fs := file.NewHandler(memFS) require.NoError(fs.MkdirAll(existingWorkspace)) - require.NoError(fs.MkdirAll(upgradeWorkingDir)) - + require.NoError(fs.MkdirAll(backupDir)) return file.NewHandler(afero.NewReadOnlyFs(memFS)) }, wantErr: true, @@ -148,7 +147,7 @@ func TestMoveUpgradeToCurrent(t *testing.T) { assert := assert.New(t) fs := tc.prepareFs(require.New(t)) - err := moveUpgradeToCurrent(fs, existingWorkspace, upgradeWorkingDir) + err := restoreBackup(fs, existingWorkspace, backupDir) if tc.wantErr { assert.Error(err) return @@ -209,7 +208,7 @@ type stubUpgradePlanner struct { showPlanErr error } -func (s *stubUpgradePlanner) PrepareUpgradeWorkspace(_, _ string, _ string, _ terraform.Variables) error { +func (s *stubUpgradePlanner) PrepareUpgradeWorkspace(_, _ string, _ terraform.Variables) error { return s.prepareWorkspaceErr } diff --git a/cli/internal/cmd/BUILD.bazel b/cli/internal/cmd/BUILD.bazel index a188e396c..e6e2bf439 100644 --- a/cli/internal/cmd/BUILD.bazel +++ b/cli/internal/cmd/BUILD.bazel @@ -115,6 +115,7 @@ go_test( "create_test.go", "iamcreate_test.go", "iamdestroy_test.go", + "iamupgradeapply_test.go", "init_test.go", "recover_test.go", "spinner_test.go", diff --git a/cli/internal/cmd/iamupgradeapply.go b/cli/internal/cmd/iamupgradeapply.go index 4642a8f8a..08c632511 100644 --- a/cli/internal/cmd/iamupgradeapply.go +++ b/cli/internal/cmd/iamupgradeapply.go @@ -48,8 +48,8 @@ func newIAMUpgradeApplyCmd() *cobra.Command { type iamUpgradeApplyCmd struct { fileHandler file.Handler - configFetcher attestationconfigapi.Fetcher log debugLog + configFetcher attestationconfigapi.Fetcher } func runIAMUpgradeApply(cmd *cobra.Command, _ []string) error { @@ -58,10 +58,9 @@ func runIAMUpgradeApply(cmd *cobra.Command, _ []string) error { return fmt.Errorf("parsing force argument: %w", err) } fileHandler := file.NewHandler(afero.NewOsFs()) - configFetcher := attestationconfigapi.NewFetcher() - upgradeID := generateUpgradeID(upgradeCmdKindIAM) upgradeDir := filepath.Join(constants.UpgradeDir, upgradeID) + configFetcher := attestationconfigapi.NewFetcher() iamMigrateCmd, err := cloudcmd.NewIAMUpgrader( cmd.Context(), constants.TerraformIAMWorkingDir, @@ -85,8 +84,8 @@ func runIAMUpgradeApply(cmd *cobra.Command, _ []string) error { i := iamUpgradeApplyCmd{ fileHandler: fileHandler, - configFetcher: configFetcher, log: log, + configFetcher: configFetcher, } return i.iamUpgradeApply(cmd, iamMigrateCmd, upgradeDir, force, yes) @@ -108,7 +107,7 @@ func (i iamUpgradeApplyCmd) iamUpgradeApply(cmd *cobra.Command, iamUpgrader iamU } hasDiff, err := iamUpgrader.PlanIAMUpgrade(cmd.Context(), cmd.OutOrStderr(), vars, conf.GetProvider()) if err != nil { - return err + return fmt.Errorf("planning terraform migrations: %w", err) } if !hasDiff && !force { cmd.Println("No IAM migrations necessary.") @@ -124,9 +123,14 @@ func (i iamUpgradeApplyCmd) iamUpgradeApply(cmd *cobra.Command, iamUpgrader iamU } if !ok { cmd.Println("Aborting upgrade.") - // Remove the upgrade directory - if err := i.fileHandler.RemoveAll(upgradeDir); err != nil { - return fmt.Errorf("cleaning up upgrade directory %s: %w", upgradeDir, err) + // User doesn't expect to see any changes in his workspace after aborting an "upgrade apply", + // therefore, roll back to the backed up state. + if err := iamUpgrader.RestoreIAMWorkspace(); err != nil { + return fmt.Errorf( + "restoring Terraform workspace: %w, restore the Terraform workspace manually from %s ", + err, + filepath.Join(upgradeDir, constants.TerraformIAMUpgradeBackupDir), + ) } return errors.New("IAM upgrade aborted by user") } @@ -144,4 +148,5 @@ func (i iamUpgradeApplyCmd) iamUpgradeApply(cmd *cobra.Command, iamUpgrader iamU type iamUpgrader interface { PlanIAMUpgrade(ctx context.Context, outWriter io.Writer, vars terraform.Variables, csp cloudprovider.Provider) (bool, error) ApplyIAMUpgrade(ctx context.Context, csp cloudprovider.Provider) error + RestoreIAMWorkspace() error } diff --git a/cli/internal/cmd/iamupgradeapply_test.go b/cli/internal/cmd/iamupgradeapply_test.go new file mode 100644 index 000000000..ba49c25ca --- /dev/null +++ b/cli/internal/cmd/iamupgradeapply_test.go @@ -0,0 +1,181 @@ +/* +Copyright (c) Edgeless Systems GmbH +SPDX-License-Identifier: AGPL-3.0-only +*/ + +package cmd + +import ( + "context" + "io" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/edgelesssys/constellation/v2/cli/internal/terraform" + "github.com/edgelesssys/constellation/v2/internal/api/attestationconfigapi" + "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" + "github.com/edgelesssys/constellation/v2/internal/config" + "github.com/edgelesssys/constellation/v2/internal/constants" + "github.com/edgelesssys/constellation/v2/internal/file" + "github.com/edgelesssys/constellation/v2/internal/logger" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIamUpgradeApply(t *testing.T) { + setupFs := func(createConfig, createTerraformVars bool) file.Handler { + fs := afero.NewMemMapFs() + fh := file.NewHandler(fs) + if createConfig { + cfg := defaultConfigWithExpectedMeasurements(t, config.Default(), cloudprovider.Azure) + require.NoError(t, fh.WriteYAML(constants.ConfigFilename, cfg)) + } + if createTerraformVars { + require.NoError(t, fh.Write( + filepath.Join(constants.TerraformIAMWorkingDir, "terraform.tfvars"), + []byte( + "region = \"foo\"\n"+ + "resource_group_name = \"bar\"\n"+ + "service_principal_name = \"baz\"\n", + ), + )) + } + return fh + } + + testCases := map[string]struct { + fh file.Handler + iamUpgrader *stubIamUpgrader + configFetcher *stubConfigFetcher + yesFlag bool + input string + wantErr bool + }{ + "success": { + fh: setupFs(true, true), + configFetcher: &stubConfigFetcher{}, + iamUpgrader: &stubIamUpgrader{}, + }, + "abort": { + fh: setupFs(true, true), + iamUpgrader: &stubIamUpgrader{}, + configFetcher: &stubConfigFetcher{}, + input: "no", + yesFlag: true, + }, + "config missing": { + fh: setupFs(false, true), + iamUpgrader: &stubIamUpgrader{}, + configFetcher: &stubConfigFetcher{}, + yesFlag: true, + wantErr: true, + }, + "iam vars missing": { + fh: setupFs(true, false), + iamUpgrader: &stubIamUpgrader{}, + configFetcher: &stubConfigFetcher{}, + yesFlag: true, + wantErr: true, + }, + "plan error": { + fh: setupFs(true, true), + iamUpgrader: &stubIamUpgrader{ + planErr: assert.AnError, + }, + configFetcher: &stubConfigFetcher{}, + yesFlag: true, + wantErr: true, + }, + "apply error": { + fh: setupFs(true, true), + iamUpgrader: &stubIamUpgrader{ + hasDiff: true, + applyErr: assert.AnError, + }, + configFetcher: &stubConfigFetcher{}, + yesFlag: true, + wantErr: true, + }, + "restore error": { + fh: setupFs(true, true), + iamUpgrader: &stubIamUpgrader{ + hasDiff: true, + rollbackErr: assert.AnError, + }, + configFetcher: &stubConfigFetcher{}, + input: "no\n", + wantErr: true, + }, + "config fetcher err": { + fh: setupFs(true, true), + iamUpgrader: &stubIamUpgrader{ + rollbackErr: assert.AnError, + }, + configFetcher: &stubConfigFetcher{ + fetchLatestErr: assert.AnError, + }, + yesFlag: true, + wantErr: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + cmd := newIAMUpgradeApplyCmd() + cmd.SetIn(strings.NewReader(tc.input)) + + iamUpgradeApplyCmd := &iamUpgradeApplyCmd{ + fileHandler: tc.fh, + log: logger.NewTest(t), + configFetcher: tc.configFetcher, + } + + err := iamUpgradeApplyCmd.iamUpgradeApply(cmd, tc.iamUpgrader, "", false, tc.yesFlag) + if tc.wantErr { + assert.Error(err) + } else { + assert.NoError(err) + } + }) + } +} + +type stubIamUpgrader struct { + hasDiff bool + planErr error + applyErr error + rollbackErr error +} + +func (u *stubIamUpgrader) PlanIAMUpgrade(context.Context, io.Writer, terraform.Variables, cloudprovider.Provider) (bool, error) { + return u.hasDiff, u.planErr +} + +func (u *stubIamUpgrader) ApplyIAMUpgrade(context.Context, cloudprovider.Provider) error { + return u.applyErr +} + +func (u *stubIamUpgrader) RestoreIAMWorkspace() error { + return u.rollbackErr +} + +type stubConfigFetcher struct { + fetchLatestErr error +} + +func (s *stubConfigFetcher) FetchAzureSEVSNPVersion(context.Context, attestationconfigapi.AzureSEVSNPVersionAPI) (attestationconfigapi.AzureSEVSNPVersionAPI, error) { + panic("not implemented") +} + +func (s *stubConfigFetcher) FetchAzureSEVSNPVersionList(context.Context, attestationconfigapi.AzureSEVSNPVersionList) (attestationconfigapi.AzureSEVSNPVersionList, error) { + panic("not implemented") +} + +func (s *stubConfigFetcher) FetchAzureSEVSNPVersionLatest(context.Context, time.Time) (attestationconfigapi.AzureSEVSNPVersionAPI, error) { + return attestationconfigapi.AzureSEVSNPVersionAPI{}, s.fetchLatestErr +} diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 6fa6e6fdc..7f1d371c2 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -301,9 +301,14 @@ func (u *upgradeApplyCmd) migrateTerraform(cmd *cobra.Command, conf *config.Conf } if !ok { cmd.Println("Aborting upgrade.") - // Remove the upgrade directory - if err := u.fileHandler.RemoveAll(upgradeDir); err != nil { - return res, fmt.Errorf("cleaning up upgrade directory %s: %w", upgradeDir, err) + // User doesn't expect to see any changes in his workspace after aborting an "upgrade apply", + // therefore, roll back to the backed up state. + if err := u.clusterUpgrader.RestoreClusterWorkspace(); err != nil { + return res, fmt.Errorf( + "restoring Terraform workspace: %w, restore the Terraform workspace manually from %s ", + err, + filepath.Join(upgradeDir, constants.TerraformUpgradeBackupDir), + ) } return res, fmt.Errorf("cluster upgrade aborted by user") } @@ -636,4 +641,5 @@ type kubernetesUpgrader interface { type clusterUpgrader interface { PlanClusterUpgrade(ctx context.Context, outWriter io.Writer, vars terraform.Variables, csp cloudprovider.Provider) (bool, error) ApplyClusterUpgrade(ctx context.Context, csp cloudprovider.Provider) (terraform.ApplyOutput, error) + RestoreClusterWorkspace() error } diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index a489de300..29ec6bf4c 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -83,6 +83,15 @@ func TestUpgradeApply(t *testing.T) { wantErr: true, stdin: "no\n", }, + "abort, restore terraform err": { + kubeUpgrader: &stubKubernetesUpgrader{ + currentConfig: config.DefaultForAzureSEVSNP(), + }, + helmUpgrader: stubApplier{}, + terraformUpgrader: &stubTerraformUpgrader{terraformDiff: true, rollbackWorkspaceErr: assert.AnError}, + wantErr: true, + stdin: "no\n", + }, "plan terraform error": { kubeUpgrader: &stubKubernetesUpgrader{ currentConfig: config.DefaultForAzureSEVSNP(), @@ -220,9 +229,10 @@ func (u *stubKubernetesUpgrader) RemoveHelmKeepAnnotation(_ context.Context) err } type stubTerraformUpgrader struct { - terraformDiff bool - planTerraformErr error - applyTerraformErr error + terraformDiff bool + planTerraformErr error + applyTerraformErr error + rollbackWorkspaceErr error } func (u stubTerraformUpgrader) PlanClusterUpgrade(_ context.Context, _ io.Writer, _ terraform.Variables, _ cloudprovider.Provider) (bool, error) { @@ -233,6 +243,10 @@ func (u stubTerraformUpgrader) ApplyClusterUpgrade(_ context.Context, _ cloudpro return terraform.ApplyOutput{}, u.applyTerraformErr } +func (u stubTerraformUpgrader) RestoreClusterWorkspace() error { + return u.rollbackWorkspaceErr +} + type mockTerraformUpgrader struct { mock.Mock } @@ -247,6 +261,11 @@ func (m *mockTerraformUpgrader) ApplyClusterUpgrade(ctx context.Context, provide return args.Get(0).(terraform.ApplyOutput), args.Error(1) } +func (m *mockTerraformUpgrader) RestoreClusterWorkspace() error { + args := m.Called() + return args.Error(0) +} + type mockApplier struct { mock.Mock } diff --git a/cli/internal/cmd/upgradecheck.go b/cli/internal/cmd/upgradecheck.go index d2b05eda6..5eba7a66f 100644 --- a/cli/internal/cmd/upgradecheck.go +++ b/cli/internal/cmd/upgradecheck.go @@ -108,12 +108,13 @@ func runUpgradeCheck(cmd *cobra.Command, _ []string) error { log: log, versionsapi: versionfetcher, }, + upgradeDir: upgradeDir, terraformChecker: tfClient, fileHandler: fileHandler, log: log, } - return up.upgradeCheck(cmd, attestationconfigapi.NewFetcher(), upgradeDir, flags) + return up.upgradeCheck(cmd, attestationconfigapi.NewFetcher(), flags) } func parseUpgradeCheckFlags(cmd *cobra.Command) (upgradeCheckFlags, error) { @@ -154,6 +155,7 @@ func parseUpgradeCheckFlags(cmd *cobra.Command) (upgradeCheckFlags, error) { type upgradeCheckCmd struct { canUpgradeCheck bool + upgradeDir string collect collector terraformChecker terraformChecker fileHandler file.Handler @@ -161,7 +163,7 @@ type upgradeCheckCmd struct { } // upgradePlan plans an upgrade of a Constellation cluster. -func (u *upgradeCheckCmd) upgradeCheck(cmd *cobra.Command, fetcher attestationconfigapi.Fetcher, upgradeDir string, flags upgradeCheckFlags) error { +func (u *upgradeCheckCmd) upgradeCheck(cmd *cobra.Command, fetcher attestationconfigapi.Fetcher, flags upgradeCheckFlags) error { conf, err := config.New(u.fileHandler, constants.ConfigFilename, fetcher, flags.force) var configValidationErr *config.ValidationError if errors.As(err, &configValidationErr) { @@ -235,9 +237,14 @@ func (u *upgradeCheckCmd) upgradeCheck(cmd *cobra.Command, fetcher attestationco return fmt.Errorf("planning terraform migrations: %w", err) } defer func() { - // Remove the upgrade directory - if err := u.fileHandler.RemoveAll(upgradeDir); err != nil { - u.log.Debugf("Failed to clean up Terraform migrations: %s", err) + // User doesn't expect to see any changes in his workspace after an "upgrade plan", + // therefore, roll back to the backed up state. + if err := u.terraformChecker.RestoreClusterWorkspace(); err != nil { + cmd.PrintErrf( + "restoring Terraform workspace: %s, restore the Terraform workspace manually from %s ", + err, + filepath.Join(u.upgradeDir, constants.TerraformUpgradeBackupDir), + ) } }() @@ -728,6 +735,7 @@ type kubernetesChecker interface { type terraformChecker interface { PlanClusterUpgrade(ctx context.Context, outWriter io.Writer, vars terraform.Variables, csp cloudprovider.Provider) (bool, error) + RestoreClusterWorkspace() error } type versionListFetcher interface { diff --git a/cli/internal/cmd/upgradecheck_test.go b/cli/internal/cmd/upgradecheck_test.go index 816be5f0a..fd9947536 100644 --- a/cli/internal/cmd/upgradecheck_test.go +++ b/cli/internal/cmd/upgradecheck_test.go @@ -24,7 +24,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/constants" "github.com/edgelesssys/constellation/v2/internal/file" "github.com/edgelesssys/constellation/v2/internal/logger" - "github.com/edgelesssys/constellation/v2/internal/semver" consemver "github.com/edgelesssys/constellation/v2/internal/semver" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -47,8 +46,8 @@ func TestBuildString(t *testing.T) { newKubernetes: []string{"v1.24.12", "v1.25.6"}, newCLI: []consemver.Semver{consemver.NewFromInt(2, 5, 0, ""), consemver.NewFromInt(2, 6, 0, "")}, currentServices: consemver.NewFromInt(2, 4, 0, ""), - currentImage: semver.NewFromInt(2, 4, 0, ""), - currentKubernetes: semver.NewFromInt(1, 24, 5, ""), + currentImage: consemver.NewFromInt(2, 4, 0, ""), + currentKubernetes: consemver.NewFromInt(1, 24, 5, ""), currentCLI: consemver.NewFromInt(2, 4, 0, ""), }, expected: "The following updates are available with this CLI:\n Kubernetes: v1.24.5 --> v1.24.12 v1.25.6\n Images:\n v2.4.0 --> v2.5.0\n Includes these measurements:\n 4:\n expected: \"1234123412341234123412341234123412341234123412341234123412341234\"\n warnOnly: false\n 8:\n expected: \"0000000000000000000000000000000000000000000000000000000000000000\"\n warnOnly: false\n 9:\n expected: \"1234123412341234123412341234123412341234123412341234123412341234\"\n warnOnly: false\n 11:\n expected: \"0000000000000000000000000000000000000000000000000000000000000000\"\n warnOnly: false\n 12:\n expected: \"1234123412341234123412341234123412341234123412341234123412341234\"\n warnOnly: false\n 13:\n expected: \"0000000000000000000000000000000000000000000000000000000000000000\"\n warnOnly: false\n 15:\n expected: \"0000000000000000000000000000000000000000000000000000000000000000\"\n warnOnly: false\n \n Services: v2.4.0 --> v2.5.0\n", @@ -70,7 +69,7 @@ func TestBuildString(t *testing.T) { "k8s only": { upgrade: versionUpgrade{ newKubernetes: []string{"v1.24.12", "v1.25.6"}, - currentKubernetes: semver.NewFromInt(1, 24, 5, ""), + currentKubernetes: consemver.NewFromInt(1, 24, 5, ""), }, expected: "The following updates are available with this CLI:\n Kubernetes: v1.24.5 --> v1.24.12 v1.25.6\n", }, @@ -81,8 +80,8 @@ func TestBuildString(t *testing.T) { newKubernetes: []string{}, newCLI: []consemver.Semver{}, currentServices: consemver.NewFromInt(2, 5, 0, ""), - currentImage: semver.NewFromInt(2, 5, 0, ""), - currentKubernetes: semver.NewFromInt(1, 25, 6, ""), + currentImage: consemver.NewFromInt(2, 5, 0, ""), + currentKubernetes: consemver.NewFromInt(1, 25, 6, ""), currentCLI: consemver.NewFromInt(2, 5, 0, ""), }, expected: "You are up to date.\n", @@ -165,8 +164,8 @@ func TestUpgradeCheck(t *testing.T) { }, supportedK8sVersions: []string{"v1.24.5", "v1.24.12", "v1.25.6"}, currentServicesVersions: consemver.NewFromInt(2, 4, 0, ""), - currentImageVersion: semver.NewFromInt(2, 4, 0, ""), - currentK8sVersion: semver.NewFromInt(1, 24, 5, ""), + currentImageVersion: consemver.NewFromInt(2, 4, 0, ""), + currentK8sVersion: consemver.NewFromInt(1, 24, 5, ""), currentCLIVersion: consemver.NewFromInt(2, 4, 0, ""), images: []versionsapi.Version{v2_5}, newCLIVersionsList: []consemver.Semver{consemver.NewFromInt(2, 5, 0, ""), consemver.NewFromInt(2, 6, 0, "")}, @@ -185,15 +184,23 @@ func TestUpgradeCheck(t *testing.T) { csp: cloudprovider.GCP, cliVersion: "v1.0.0", }, - "terraform err": { + "terraform plan err": { collector: collector, checker: stubTerraformChecker{ - err: assert.AnError, + planErr: assert.AnError, }, csp: cloudprovider.GCP, cliVersion: "v1.0.0", wantError: true, }, + "terraform rollback err, log only": { + collector: collector, + checker: stubTerraformChecker{ + rollbackErr: assert.AnError, + }, + csp: cloudprovider.GCP, + cliVersion: "v1.0.0", + }, } for name, tc := range testCases { @@ -214,7 +221,7 @@ func TestUpgradeCheck(t *testing.T) { cmd := newUpgradeCheckCmd() - err := checkCmd.upgradeCheck(cmd, stubAttestationFetcher{}, "test", upgradeCheckFlags{}) + err := checkCmd.upgradeCheck(cmd, stubAttestationFetcher{}, upgradeCheckFlags{}) if tc.wantError { assert.Error(err) return @@ -279,12 +286,17 @@ func (s *stubVersionCollector) filterCompatibleCLIVersions(_ context.Context, _ } type stubTerraformChecker struct { - tfDiff bool - err error + tfDiff bool + planErr error + rollbackErr error } func (s stubTerraformChecker) PlanClusterUpgrade(_ context.Context, _ io.Writer, _ terraform.Variables, _ cloudprovider.Provider) (bool, error) { - return s.tfDiff, s.err + return s.tfDiff, s.planErr +} + +func (s stubTerraformChecker) RestoreClusterWorkspace() error { + return s.rollbackErr } func TestNewCLIVersions(t *testing.T) { @@ -374,7 +386,7 @@ func TestFilterCompatibleCLIVersions(t *testing.T) { t.Run(name, func(t *testing.T) { require := require.New(t) - _, err := tc.verCollector.filterCompatibleCLIVersions(context.Background(), tc.cliPatchVersions, semver.NewFromInt(1, 24, 5, "")) + _, err := tc.verCollector.filterCompatibleCLIVersions(context.Background(), tc.cliPatchVersions, consemver.NewFromInt(1, 24, 5, "")) if tc.wantErr { require.Error(err) return diff --git a/cli/internal/terraform/BUILD.bazel b/cli/internal/terraform/BUILD.bazel index 98f3c7583..3eaf93f2f 100644 --- a/cli/internal/terraform/BUILD.bazel +++ b/cli/internal/terraform/BUILD.bazel @@ -70,6 +70,9 @@ go_library( "terraform/openstack/outputs.tf", "terraform/openstack/variables.tf", "terraform/qemu/modules/instance_group/tdx_domain.xsl", + "terraform/iam/aws/.terraform.lock.hcl", + "terraform/iam/azure/.terraform.lock.hcl", + "terraform/iam/gcp/.terraform.lock.hcl", ], importpath = "github.com/edgelesssys/constellation/v2/cli/internal/terraform", visibility = ["//cli:__subpackages__"], diff --git a/cli/internal/terraform/loader.go b/cli/internal/terraform/loader.go index 2c1bc7f52..092bf28c6 100644 --- a/cli/internal/terraform/loader.go +++ b/cli/internal/terraform/loader.go @@ -25,39 +25,39 @@ var ErrTerraformWorkspaceDifferentFiles = errors.New("creating cluster: trying t //go:embed terraform/* //go:embed terraform/*/.terraform.lock.hcl +//go:embed terraform/iam/*/.terraform.lock.hcl var terraformFS embed.FS +const ( + noOverwrites overwritePolicy = iota + allowOverwrites +) + +type overwritePolicy int + // prepareWorkspace loads the embedded Terraform files, // and writes them into the workspace. func prepareWorkspace(rootDir string, fileHandler file.Handler, workingDir string) error { - return terraformCopier(fileHandler, rootDir, workingDir) + return terraformCopier(fileHandler, rootDir, workingDir, noOverwrites) } -// prepareUpgradeWorkspace takes the Terraform state file from the old workspace and the -// embedded Terraform files and writes them into the new workspace. -func prepareUpgradeWorkspace(rootDir string, fileHandler file.Handler, oldWorkingDir, newWorkingDir, backupDir string) error { +// prepareUpgradeWorkspace backs up the old Terraform workspace from workingDir, and +// copies the embedded Terraform files into workingDir. +func prepareUpgradeWorkspace(rootDir string, fileHandler file.Handler, workingDir, backupDir string) error { // backup old workspace if err := fileHandler.CopyDir( - oldWorkingDir, + workingDir, backupDir, ); err != nil { return fmt.Errorf("backing up old workspace: %w", err) } - // copy state file - if err := fileHandler.CopyFile( - filepath.Join(oldWorkingDir, "terraform.tfstate"), - filepath.Join(newWorkingDir, "terraform.tfstate"), - file.OptMkdirAll, - ); err != nil { - return fmt.Errorf("copying state file: %w", err) - } - - return terraformCopier(fileHandler, rootDir, newWorkingDir) + return terraformCopier(fileHandler, rootDir, workingDir, allowOverwrites) } // terraformCopier copies the embedded Terraform files into the workspace. -func terraformCopier(fileHandler file.Handler, rootDir, workingDir string) error { +// allowOverwrites allows overwriting existing files in the workspace. +func terraformCopier(fileHandler file.Handler, rootDir, workingDir string, overwritePolicy overwritePolicy) error { goEmbedRootDir := filepath.ToSlash(rootDir) return fs.WalkDir(terraformFS, goEmbedRootDir, func(path string, d fs.DirEntry, err error) error { if err != nil { @@ -74,8 +74,15 @@ func terraformCopier(fileHandler file.Handler, rootDir, workingDir string) error } // normalize fileName := strings.Replace(slashpath.Join(workingDir, path), goEmbedRootDir+"/", "", 1) - if err := fileHandler.Write(fileName, content, file.OptMkdirAll); errors.Is(err, afero.ErrFileExists) { - // If a file already exists, check if it is identical. If yes, continue and don't write anything to disk. + opts := []file.Option{ + file.OptMkdirAll, + } + if overwritePolicy == allowOverwrites { + opts = append(opts, file.OptOverwrite) + } + if err := fileHandler.Write(fileName, content, opts...); errors.Is(err, afero.ErrFileExists) { + // If a file already exists and overwritePolicy is set to noOverwrites, + // check if it is identical. If yes, continue and don't write anything to disk. // If no, don't overwrite it and instead throw an error. The affected file could be from a different version, // provider, corrupted or manually modified in general. existingFileContent, err := fileHandler.Read(fileName) diff --git a/cli/internal/terraform/loader_test.go b/cli/internal/terraform/loader_test.go index 116f115d6..3d1b0dcc0 100644 --- a/cli/internal/terraform/loader_test.go +++ b/cli/internal/terraform/loader_test.go @@ -20,6 +20,8 @@ import ( "github.com/stretchr/testify/require" ) +var oldFileContent = []byte("1234") + func TestPrepareWorkspace(t *testing.T) { testCases := map[string]struct { pathBase string @@ -109,20 +111,20 @@ func TestPrepareWorkspace(t *testing.T) { err := prepareWorkspace(path, file, testWorkspace) require.NoError(err) - checkFiles(t, file, func(err error) { assert.NoError(err) }, testWorkspace, tc.fileList) + checkFiles(t, file, func(err error) { assert.NoError(err) }, nil, testWorkspace, tc.fileList) if tc.testAlreadyUnpacked { // Let's try the same again and check if we don't get a "file already exists" error. require.NoError(file.Remove(filepath.Join(testWorkspace, "variables.tf"))) err := prepareWorkspace(path, file, testWorkspace) assert.NoError(err) - checkFiles(t, file, func(err error) { assert.NoError(err) }, testWorkspace, tc.fileList) + checkFiles(t, file, func(err error) { assert.NoError(err) }, nil, testWorkspace, tc.fileList) } err = cleanUpWorkspace(file, testWorkspace) require.NoError(err) - checkFiles(t, file, func(err error) { assert.ErrorIs(err, fs.ErrNotExist) }, testWorkspace, tc.fileList) + checkFiles(t, file, func(err error) { assert.ErrorIs(err, fs.ErrNotExist) }, nil, testWorkspace, tc.fileList) }) } } @@ -131,49 +133,56 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { testCases := map[string]struct { pathBase string provider cloudprovider.Provider - oldWorkingDir string - newWorkingDir string + workingDir string backupDir string - oldWorkspaceFiles []string - newWorkspaceFiles []string + workspaceFiles []string expectedFiles []string + expectedBackupFiles []string testAlreadyUnpacked bool wantErr bool }{ "works": { - pathBase: "terraform", - provider: cloudprovider.AWS, - oldWorkingDir: "old", - newWorkingDir: "new", - backupDir: "backup", - oldWorkspaceFiles: []string{"terraform.tfstate"}, + pathBase: "terraform", + provider: cloudprovider.AWS, + workingDir: "working", + backupDir: "backup", + workspaceFiles: []string{"main.tf", "variables.tf", "outputs.tf"}, expectedFiles: []string{ "main.tf", "variables.tf", "outputs.tf", - "modules", - "terraform.tfstate", + }, + expectedBackupFiles: []string{ + "main.tf", + "variables.tf", + "outputs.tf", }, }, "state file does not exist": { - pathBase: "terraform", - provider: cloudprovider.AWS, - oldWorkingDir: "old", - newWorkingDir: "new", - backupDir: "backup", - oldWorkspaceFiles: []string{}, - expectedFiles: []string{}, - wantErr: true, + pathBase: "terraform", + provider: cloudprovider.AWS, + workingDir: "working", + backupDir: "backup", + workspaceFiles: []string{}, + expectedFiles: []string{}, + wantErr: true, }, - "terraform files already exist in new dir": { - pathBase: "terraform", - provider: cloudprovider.AWS, - oldWorkingDir: "old", - newWorkingDir: "new", - backupDir: "backup", - oldWorkspaceFiles: []string{"terraform.tfstate"}, - newWorkspaceFiles: []string{"main.tf"}, - wantErr: true, + "terraform file already exists in working dir (overwrite)": { + pathBase: "terraform", + provider: cloudprovider.AWS, + workingDir: "working", + backupDir: "backup", + workspaceFiles: []string{"main.tf", "variables.tf", "outputs.tf"}, + expectedFiles: []string{ + "main.tf", + "variables.tf", + "outputs.tf", + }, + expectedBackupFiles: []string{ + "main.tf", + "variables.tf", + "outputs.tf", + }, }, } @@ -186,31 +195,44 @@ func TestPrepareUpgradeWorkspace(t *testing.T) { path := path.Join(tc.pathBase, strings.ToLower(tc.provider.String())) - createFiles(t, file, tc.oldWorkspaceFiles, tc.oldWorkingDir) - createFiles(t, file, tc.newWorkspaceFiles, tc.newWorkingDir) + createFiles(t, file, tc.workspaceFiles, tc.workingDir) - err := prepareUpgradeWorkspace(path, file, tc.oldWorkingDir, tc.newWorkingDir, tc.backupDir) + err := prepareUpgradeWorkspace(path, file, tc.workingDir, tc.backupDir) if tc.wantErr { require.Error(err) } else { require.NoError(err) + checkFiles( + t, file, + func(err error) { assert.NoError(err) }, + func(content []byte) { assert.NotEqual(oldFileContent, content) }, + tc.workingDir, tc.expectedFiles, + ) + checkFiles( + t, file, + func(err error) { assert.NoError(err) }, + func(content []byte) { assert.Equal(oldFileContent, content) }, + tc.backupDir, tc.expectedBackupFiles, + ) } - checkFiles(t, file, func(err error) { assert.NoError(err) }, tc.newWorkingDir, tc.expectedFiles) - checkFiles(t, file, func(err error) { assert.NoError(err) }, - tc.backupDir, - tc.oldWorkspaceFiles, - ) }) } } -func checkFiles(t *testing.T, fileHandler file.Handler, assertion func(error), dir string, files []string) { +func checkFiles(t *testing.T, fileHandler file.Handler, assertion func(error), contentExpection func(content []byte), dir string, files []string) { t.Helper() for _, f := range files { path := filepath.Join(dir, f) _, err := fileHandler.Stat(path) assertion(err) + if err == nil { + content, err := fileHandler.Read(path) + assertion(err) + if contentExpection != nil { + contentExpection(content) + } + } } } @@ -220,7 +242,7 @@ func createFiles(t *testing.T, fileHandler file.Handler, fileList []string, targ for _, f := range fileList { path := filepath.Join(targetDir, f) - err := fileHandler.Write(path, []byte("1234"), file.OptOverwrite, file.OptMkdirAll) + err := fileHandler.Write(path, oldFileContent, file.OptOverwrite, file.OptMkdirAll) require.NoError(err) } } diff --git a/cli/internal/terraform/terraform.go b/cli/internal/terraform/terraform.go index 29d08925c..5688454de 100644 --- a/cli/internal/terraform/terraform.go +++ b/cli/internal/terraform/terraform.go @@ -332,19 +332,18 @@ func (c *Client) PrepareWorkspace(path string, vars Variables) error { return fmt.Errorf("prepare workspace: %w", err) } - return c.writeVars(vars) + return c.writeVars(vars, noOverwrites) } // PrepareUpgradeWorkspace prepares a Terraform workspace for an upgrade. -// It copies the Terraform state from the old working dir and the embedded Terraform files -// into the working dir of the Terraform client. -// Additionally, a backup of the old working dir is created in the backup dir. -func (c *Client) PrepareUpgradeWorkspace(path, oldWorkingDir, backupDir string, vars Variables) error { - if err := prepareUpgradeWorkspace(path, c.file, oldWorkingDir, c.workingDir, backupDir); err != nil { +// It creates a backup of the Terraform workspace in the backupDir, and copies +// the embedded Terraform files into the workingDir. +func (c *Client) PrepareUpgradeWorkspace(path, backupDir string, vars Variables) error { + if err := prepareUpgradeWorkspace(path, c.file, c.workingDir, backupDir); err != nil { return fmt.Errorf("prepare upgrade workspace: %w", err) } - return c.writeVars(vars) + return c.writeVars(vars, allowOverwrites) } // ApplyCluster applies the Terraform configuration of the workspace to create or upgrade a Constellation cluster. @@ -461,13 +460,17 @@ func (c *Client) applyManualStateMigrations(ctx context.Context) error { } // writeVars tries to write the Terraform variables file or, if it exists, checks if it is the same as we are expecting. -func (c *Client) writeVars(vars Variables) error { +func (c *Client) writeVars(vars Variables, overwritePolicy overwritePolicy) error { if vars == nil { return errors.New("creating cluster: vars is nil") } pathToVarsFile := filepath.Join(c.workingDir, terraformVarsFile) - if err := c.file.Write(pathToVarsFile, []byte(vars.String())); errors.Is(err, afero.ErrFileExists) { + opts := []file.Option{} + if overwritePolicy == allowOverwrites { + opts = append(opts, file.OptOverwrite) + } + if err := c.file.Write(pathToVarsFile, []byte(vars.String()), opts...); errors.Is(err, afero.ErrFileExists) { // If a variables file already exists, check if it's the same as we're expecting, so we can continue using it. varsContent, err := c.file.Read(pathToVarsFile) if err != nil {