From 7a17cf9918e761b973326f2723ac244874b2a3a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Wei=C3=9Fe?= Date: Wed, 25 Oct 2023 14:42:59 +0200 Subject: [PATCH] Use the same cloudcmd struct for create and apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Weiße --- cli/internal/cmd/apply.go | 8 ++++-- cli/internal/cmd/applyterraform.go | 6 ++-- cli/internal/cmd/cloud.go | 9 ++---- cli/internal/cmd/cloud_test.go | 23 ++++++++------- cli/internal/cmd/create.go | 7 +++-- cli/internal/cmd/create_test.go | 10 +++---- cli/internal/cmd/init_test.go | 2 +- cli/internal/cmd/miniup.go | 41 ++++++++++++++++++++------- cli/internal/cmd/upgradeapply.go | 10 ------- cli/internal/cmd/upgradeapply_test.go | 32 +++++++-------------- cli/internal/cmd/upgradecheck.go | 13 +++++---- cli/internal/cmd/upgradecheck_test.go | 5 ++-- 12 files changed, 84 insertions(+), 82 deletions(-) diff --git a/cli/internal/cmd/apply.go b/cli/internal/cmd/apply.go index 44a394d763..fae6aa2039 100644 --- a/cli/internal/cmd/apply.go +++ b/cli/internal/cmd/apply.go @@ -214,8 +214,9 @@ func runApply(cmd *cobra.Command, _ []string) error { upgradeID := generateUpgradeID(upgradeCmdKindApply) upgradeDir := filepath.Join(constants.UpgradeDir, upgradeID) - clusterUpgrader, err := cloudcmd.NewClusterUpgrader( + cloudApplier, cleanUp, err := cloudcmd.NewApplier( cmd.Context(), + spinner, constants.TerraformWorkingDir, upgradeDir, flags.tfLogLevel, @@ -224,6 +225,7 @@ func runApply(cmd *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("setting up cluster upgrader: %w", err) } + defer cleanUp() apply := &applyCmd{ fileHandler: fileHandler, @@ -235,7 +237,7 @@ func runApply(cmd *cobra.Command, _ []string) error { newHelmClient: newHelmClient, newDialer: newDialer, newKubeUpgrader: newKubeUpgrader, - clusterUpgrader: clusterUpgrader, + infraApplier: cloudApplier, } ctx, cancel := context.WithTimeout(cmd.Context(), time.Hour) @@ -258,7 +260,7 @@ type applyCmd struct { newHelmClient func(kubeConfigPath string, log debugLog) (helmApplier, error) newDialer func(validator atls.Validator) *dialer.Dialer newKubeUpgrader func(io.Writer, string, debugLog) (kubernetesUpgrader, error) - clusterUpgrader clusterUpgrader + infraApplier cloudApplier } /* diff --git a/cli/internal/cmd/applyterraform.go b/cli/internal/cmd/applyterraform.go index d1006743c2..0a3d5780bf 100644 --- a/cli/internal/cmd/applyterraform.go +++ b/cli/internal/cmd/applyterraform.go @@ -66,7 +66,7 @@ func (a *applyCmd) planTerraformMigration(cmd *cobra.Command, conf *config.Confi // u.infraApplier.AddManualStateMigration(migration) // } - return a.clusterUpgrader.PlanClusterUpgrade(cmd.Context(), cmd.OutOrStdout(), vars, conf.GetProvider()) + return a.infraApplier.Plan(cmd.Context(), conf) } // migrateTerraform migrates an existing Terraform state and the post-migration infrastructure state is returned. @@ -82,7 +82,7 @@ func (a *applyCmd) migrateTerraform(cmd *cobra.Command, conf *config.Config, upg cmd.Println("Aborting upgrade.") // 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 := a.clusterUpgrader.RestoreClusterWorkspace(); err != nil { + if err := a.infraApplier.RestoreWorkspace(); err != nil { return state.Infrastructure{}, fmt.Errorf( "restoring Terraform workspace: %w, restore the Terraform workspace manually from %s ", err, @@ -95,7 +95,7 @@ func (a *applyCmd) migrateTerraform(cmd *cobra.Command, conf *config.Config, upg a.log.Debugf("Applying Terraform migrations") a.spinner.Start("Migrating Terraform resources", false) - infraState, err := a.clusterUpgrader.ApplyClusterUpgrade(cmd.Context(), conf.GetProvider()) + infraState, err := a.infraApplier.Apply(cmd.Context(), conf.GetProvider(), cloudcmd.WithoutRollbackOnError) a.spinner.Stop() if err != nil { return state.Infrastructure{}, fmt.Errorf("applying terraform migrations: %w", err) diff --git a/cli/internal/cmd/cloud.go b/cli/internal/cmd/cloud.go index 4e9ecbed1e..2b46d3a3d5 100644 --- a/cli/internal/cmd/cloud.go +++ b/cli/internal/cmd/cloud.go @@ -17,15 +17,10 @@ import ( "github.com/edgelesssys/constellation/v2/internal/config" ) -type cloudCreator interface { - Create(ctx context.Context, - opts cloudcmd.CreateOptions, - ) (state.Infrastructure, error) -} - type cloudApplier interface { Plan(ctx context.Context, conf *config.Config) (bool, error) - Apply(ctx context.Context, csp cloudprovider.Provider, withRollback bool) (state.Infrastructure, error) + Apply(ctx context.Context, csp cloudprovider.Provider, rollback cloudcmd.RollbackBehavior) (state.Infrastructure, error) + RestoreWorkspace() error } type cloudIAMCreator interface { diff --git a/cli/internal/cmd/cloud_test.go b/cli/internal/cmd/cloud_test.go index 1cbc85f597..1f9352e460 100644 --- a/cli/internal/cmd/cloud_test.go +++ b/cli/internal/cmd/cloud_test.go @@ -27,22 +27,25 @@ func TestMain(m *testing.M) { } type stubCloudCreator struct { - createCalled bool - state state.Infrastructure - createErr error + state state.Infrastructure + planCalled bool + planErr error + applyCalled bool + applyErr error } -func (c *stubCloudCreator) Create(_ context.Context, _ cloudcmd.CreateOptions) (state.Infrastructure, error) { - c.createCalled = true - return c.state, c.createErr +func (c *stubCloudCreator) Plan(_ context.Context, _ *config.Config) (bool, error) { + c.planCalled = true + return false, c.planErr } -func (c *stubCloudCreator) Plan(_ context.Context, _ *config.Config) (bool, error) { - return false, nil +func (c *stubCloudCreator) Apply(_ context.Context, _ cloudprovider.Provider, _ cloudcmd.RollbackBehavior) (state.Infrastructure, error) { + c.applyCalled = true + return c.state, c.applyErr } -func (c *stubCloudCreator) Apply(_ context.Context, _ cloudprovider.Provider, _ bool) (state.Infrastructure, error) { - return state.Infrastructure{}, nil +func (c *stubCloudCreator) RestoreWorkspace() error { + return nil } type stubCloudTerminator struct { diff --git a/cli/internal/cmd/create.go b/cli/internal/cmd/create.go index 9d89f1b858..838a50cb66 100644 --- a/cli/internal/cmd/create.go +++ b/cli/internal/cmd/create.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io/fs" + "os" "path/filepath" "github.com/edgelesssys/constellation/v2/cli/internal/cloudcmd" @@ -190,7 +191,7 @@ func (c *createCmd) create(cmd *cobra.Command, applier cloudApplier, fileHandler if _, err := applier.Plan(cmd.Context(), conf); err != nil { return fmt.Errorf("planning infrastructure creation: %w", err) } - infraState, err := applier.Apply(cmd.Context(), conf.GetProvider(), true) + infraState, err := applier.Apply(cmd.Context(), conf.GetProvider(), cloudcmd.WithRollbackOnError) spinner.Stop() if err != nil { return err @@ -227,9 +228,9 @@ func (c *createCmd) checkDirClean(fileHandler file.Handler) error { ) } c.log.Debugf("Checking terraform working directory") - if clean, err := fileHandler.IsEmpty(constants.TerraformWorkingDir); err != nil { + if clean, err := fileHandler.IsEmpty(constants.TerraformWorkingDir); err != nil && !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("checking if terraform working directory is empty: %w", err) - } else if !clean { + } else if err == nil && !clean { return fmt.Errorf( "directory '%s' already exists and is not empty, run 'constellation terminate' before creating a new one", c.flags.pathPrefixer.PrefixPrintablePath(constants.TerraformWorkingDir), diff --git a/cli/internal/cmd/create_test.go b/cli/internal/cmd/create_test.go index ec478acdec..064cea7bbc 100644 --- a/cli/internal/cmd/create_test.go +++ b/cli/internal/cmd/create_test.go @@ -8,7 +8,6 @@ package cmd import ( "bytes" - "errors" "testing" "github.com/edgelesssys/constellation/v2/cli/internal/state" @@ -45,7 +44,6 @@ func TestCreate(t *testing.T) { return fs } infraState := state.Infrastructure{ClusterEndpoint: "192.0.2.1"} - someErr := errors.New("failed") testCases := map[string]struct { setupFs func(*require.Assertions, cloudprovider.Provider) afero.Fs @@ -125,7 +123,7 @@ func TestCreate(t *testing.T) { }, "create error": { setupFs: fsWithDefaultConfigAndState, - creator: &stubCloudCreator{createErr: someErr}, + creator: &stubCloudCreator{applyErr: assert.AnError}, provider: cloudprovider.GCP, yesFlag: true, wantErr: true, @@ -163,9 +161,11 @@ func TestCreate(t *testing.T) { } else { assert.NoError(err) if tc.wantAbort { - assert.False(tc.creator.createCalled) + assert.False(tc.creator.planCalled) + assert.False(tc.creator.applyCalled) } else { - assert.True(tc.creator.createCalled) + assert.True(tc.creator.planCalled) + assert.True(tc.creator.applyCalled) var gotState state.State expectedState := state.Infrastructure{ diff --git a/cli/internal/cmd/init_test.go b/cli/internal/cmd/init_test.go index 93885a258b..3813cef4a7 100644 --- a/cli/internal/cmd/init_test.go +++ b/cli/internal/cmd/init_test.go @@ -277,7 +277,7 @@ func TestInitialize(t *testing.T) { getClusterAttestationConfigErr: k8serrors.NewNotFound(schema.GroupResource{}, ""), }, nil }, - clusterUpgrader: stubTerraformUpgrader{}, + infraApplier: stubTerraformUpgrader{}, } err := i.apply(cmd, stubAttestationFetcher{}, "test") diff --git a/cli/internal/cmd/miniup.go b/cli/internal/cmd/miniup.go index c5dc3457f5..03c8c2d9a8 100644 --- a/cli/internal/cmd/miniup.go +++ b/cli/internal/cmd/miniup.go @@ -10,6 +10,8 @@ import ( "context" "errors" "fmt" + "os" + "path/filepath" "time" "github.com/edgelesssys/constellation/v2/cli/internal/cloudcmd" @@ -58,7 +60,6 @@ func runUp(cmd *cobra.Command, _ []string) error { return err } defer spinner.Stop() - creator := cloudcmd.NewCreator(spinner) m := &miniUpCmd{ log: log, @@ -68,15 +69,38 @@ func runUp(cmd *cobra.Command, _ []string) error { if err := m.flags.parse(cmd.Flags()); err != nil { return err } + + creator, cleanUp, err := cloudcmd.NewApplier( + cmd.Context(), + spinner, + constants.TerraformWorkingDir, + filepath.Join(constants.UpgradeDir, "create"), // Not used by create + m.flags.tfLogLevel, + m.fileHandler, + ) + if err != nil { + return err + } + defer cleanUp() + return m.up(cmd, creator, spinner) } -func (m *miniUpCmd) up(cmd *cobra.Command, creator cloudCreator, spinner spinnerInterf) error { +func (m *miniUpCmd) up(cmd *cobra.Command, creator cloudApplier, spinner spinnerInterf) error { if err := m.checkSystemRequirements(cmd.ErrOrStderr()); err != nil { return fmt.Errorf("system requirements not met: %w", err) } - // create config if not passed as flag and set default values + if clean, err := m.fileHandler.IsEmpty(constants.TerraformWorkingDir); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("checking if terraform working directory is empty: %w", err) + } else if err == nil && !clean { + return fmt.Errorf( + "directory %q already exists and is not empty, run 'constellation mini down' before creating a new cluster", + m.flags.pathPrefixer.PrefixPrintablePath(constants.TerraformWorkingDir), + ) + } + + // create config if not present in directory and set default values config, err := m.prepareConfig(cmd) if err != nil { return fmt.Errorf("preparing config: %w", err) @@ -159,15 +183,12 @@ func (m *miniUpCmd) prepareExistingConfig(cmd *cobra.Command) (*config.Config, e } // createMiniCluster creates a new cluster using the given config. -func (m *miniUpCmd) createMiniCluster(ctx context.Context, creator cloudCreator, config *config.Config) error { +func (m *miniUpCmd) createMiniCluster(ctx context.Context, creator cloudApplier, config *config.Config) error { m.log.Debugf("Creating mini cluster") - opts := cloudcmd.CreateOptions{ - Provider: cloudprovider.QEMU, - Config: config, - TFWorkspace: constants.TerraformWorkingDir, - TFLogLevel: m.flags.tfLogLevel, + if _, err := creator.Plan(ctx, config); err != nil { + return err } - infraState, err := creator.Create(ctx, opts) + infraState, err := creator.Apply(ctx, config.GetProvider(), cloudcmd.WithoutRollbackOnError) if err != nil { return err } diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 8221e03691..a5ce7e0fa3 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -9,13 +9,9 @@ package cmd import ( "context" "fmt" - "io" "time" - "github.com/edgelesssys/constellation/v2/cli/internal/state" - "github.com/edgelesssys/constellation/v2/cli/internal/terraform" "github.com/edgelesssys/constellation/v2/internal/attestation/variant" - "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" "github.com/edgelesssys/constellation/v2/internal/config" "github.com/rogpeppe/go-internal/diff" "github.com/spf13/cobra" @@ -73,9 +69,3 @@ type kubernetesUpgrader interface { BackupCRs(ctx context.Context, crds []apiextensionsv1.CustomResourceDefinition, upgradeDir string) error BackupCRDs(ctx context.Context, upgradeDir string) ([]apiextensionsv1.CustomResourceDefinition, error) } - -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) (state.Infrastructure, error) - RestoreClusterWorkspace() error -} diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index 227c141874..f07422f222 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -12,10 +12,10 @@ import ( "io" "testing" + "github.com/edgelesssys/constellation/v2/cli/internal/cloudcmd" "github.com/edgelesssys/constellation/v2/cli/internal/helm" "github.com/edgelesssys/constellation/v2/cli/internal/kubecmd" "github.com/edgelesssys/constellation/v2/cli/internal/state" - "github.com/edgelesssys/constellation/v2/cli/internal/terraform" "github.com/edgelesssys/constellation/v2/internal/attestation/variant" "github.com/edgelesssys/constellation/v2/internal/cloud/cloudprovider" "github.com/edgelesssys/constellation/v2/internal/config" @@ -53,7 +53,7 @@ func TestUpgradeApply(t *testing.T) { kubeUpgrader *stubKubernetesUpgrader fh func() file.Handler fhAssertions func(require *require.Assertions, assert *assert.Assertions, fh file.Handler) - terraformUpgrader clusterUpgrader + terraformUpgrader cloudApplier wantErr bool customK8sVersion string flags applyFlags @@ -265,7 +265,7 @@ func TestUpgradeApply(t *testing.T) { newKubeUpgrader: func(_ io.Writer, _ string, _ debugLog) (kubernetesUpgrader, error) { return tc.kubeUpgrader, nil }, - clusterUpgrader: tc.terraformUpgrader, + infraApplier: tc.terraformUpgrader, } err := upgrader.apply(cmd, stubAttestationFetcher{}, "test") if tc.wantErr { @@ -341,16 +341,6 @@ func (u *stubKubernetesUpgrader) ExtendClusterConfigCertSANs(_ context.Context, return nil } -// TODO(v2.11): Remove this function after v2.11 is released. -func (u *stubKubernetesUpgrader) RemoveAttestationConfigHelmManagement(_ context.Context) error { - return nil -} - -// TODO(v2.12): Remove this function. -func (u *stubKubernetesUpgrader) RemoveHelmKeepAnnotation(_ context.Context) error { - return nil -} - type stubTerraformUpgrader struct { terraformDiff bool planTerraformErr error @@ -358,15 +348,15 @@ type stubTerraformUpgrader struct { rollbackWorkspaceErr error } -func (u stubTerraformUpgrader) PlanClusterUpgrade(_ context.Context, _ io.Writer, _ terraform.Variables, _ cloudprovider.Provider) (bool, error) { +func (u stubTerraformUpgrader) Plan(_ context.Context, _ *config.Config) (bool, error) { return u.terraformDiff, u.planTerraformErr } -func (u stubTerraformUpgrader) ApplyClusterUpgrade(_ context.Context, _ cloudprovider.Provider) (state.Infrastructure, error) { +func (u stubTerraformUpgrader) Apply(_ context.Context, _ cloudprovider.Provider, _ cloudcmd.RollbackBehavior) (state.Infrastructure, error) { return state.Infrastructure{}, u.applyTerraformErr } -func (u stubTerraformUpgrader) RestoreClusterWorkspace() error { +func (u stubTerraformUpgrader) RestoreWorkspace() error { return u.rollbackWorkspaceErr } @@ -374,17 +364,17 @@ type mockTerraformUpgrader struct { mock.Mock } -func (m *mockTerraformUpgrader) PlanClusterUpgrade(ctx context.Context, w io.Writer, variables terraform.Variables, provider cloudprovider.Provider) (bool, error) { - args := m.Called(ctx, w, variables, provider) +func (m *mockTerraformUpgrader) Plan(ctx context.Context, conf *config.Config) (bool, error) { + args := m.Called(ctx, conf) return args.Bool(0), args.Error(1) } -func (m *mockTerraformUpgrader) ApplyClusterUpgrade(ctx context.Context, provider cloudprovider.Provider) (state.Infrastructure, error) { - args := m.Called(ctx, provider) +func (m *mockTerraformUpgrader) Apply(ctx context.Context, provider cloudprovider.Provider, rollback cloudcmd.RollbackBehavior) (state.Infrastructure, error) { + args := m.Called(ctx, provider, rollback) return args.Get(0).(state.Infrastructure), args.Error(1) } -func (m *mockTerraformUpgrader) RestoreClusterWorkspace() error { +func (m *mockTerraformUpgrader) RestoreWorkspace() error { args := m.Called() return args.Error(0) } diff --git a/cli/internal/cmd/upgradecheck.go b/cli/internal/cmd/upgradecheck.go index 0d5c381ef1..5ac6f996f9 100644 --- a/cli/internal/cmd/upgradecheck.go +++ b/cli/internal/cmd/upgradecheck.go @@ -20,7 +20,6 @@ import ( "github.com/edgelesssys/constellation/v2/cli/internal/featureset" "github.com/edgelesssys/constellation/v2/cli/internal/helm" "github.com/edgelesssys/constellation/v2/cli/internal/kubecmd" - "github.com/edgelesssys/constellation/v2/cli/internal/terraform" "github.com/edgelesssys/constellation/v2/internal/api/attestationconfigapi" "github.com/edgelesssys/constellation/v2/internal/api/fetcher" "github.com/edgelesssys/constellation/v2/internal/api/versionsapi" @@ -104,8 +103,9 @@ func runUpgradeCheck(cmd *cobra.Command, _ []string) error { upgradeID := generateUpgradeID(upgradeCmdKindCheck) upgradeDir := filepath.Join(constants.UpgradeDir, upgradeID) - tfClient, err := cloudcmd.NewClusterUpgrader( + tfClient, cleanUp, err := cloudcmd.NewApplier( cmd.Context(), + cmd.OutOrStdout(), constants.TerraformWorkingDir, upgradeDir, flags.tfLogLevel, @@ -114,6 +114,7 @@ func runUpgradeCheck(cmd *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("setting up Terraform upgrader: %w", err) } + defer cleanUp() kubeChecker, err := kubecmd.New(cmd.OutOrStdout(), constants.AdminConfFilename, fileHandler, log) if err != nil { @@ -222,14 +223,14 @@ func (u *upgradeCheckCmd) upgradeCheck(cmd *cobra.Command, fetcher attestationco // u.terraformChecker.AddManualStateMigration(migration) // } cmd.Println("The following Terraform migrations are available with this CLI:") - hasDiff, err := u.terraformChecker.PlanClusterUpgrade(cmd.Context(), cmd.OutOrStdout(), vars, conf.GetProvider()) + hasDiff, err := u.terraformChecker.Plan(cmd.Context(), conf) if err != nil { return fmt.Errorf("planning terraform migrations: %w", err) } defer func() { // 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 { + if err := u.terraformChecker.RestoreWorkspace(); err != nil { cmd.PrintErrf( "restoring Terraform workspace: %s, restore the Terraform workspace manually from %s ", err, @@ -720,8 +721,8 @@ type kubernetesChecker interface { } type terraformChecker interface { - PlanClusterUpgrade(ctx context.Context, outWriter io.Writer, vars terraform.Variables, csp cloudprovider.Provider) (bool, error) - RestoreClusterWorkspace() error + Plan(context.Context, *config.Config) (bool, error) + RestoreWorkspace() error } type versionListFetcher interface { diff --git a/cli/internal/cmd/upgradecheck_test.go b/cli/internal/cmd/upgradecheck_test.go index 0037969131..5e6f8329ab 100644 --- a/cli/internal/cmd/upgradecheck_test.go +++ b/cli/internal/cmd/upgradecheck_test.go @@ -15,7 +15,6 @@ import ( "strings" "testing" - "github.com/edgelesssys/constellation/v2/cli/internal/terraform" "github.com/edgelesssys/constellation/v2/internal/api/versionsapi" "github.com/edgelesssys/constellation/v2/internal/attestation/measurements" "github.com/edgelesssys/constellation/v2/internal/attestation/variant" @@ -291,11 +290,11 @@ type stubTerraformChecker struct { rollbackErr error } -func (s stubTerraformChecker) PlanClusterUpgrade(_ context.Context, _ io.Writer, _ terraform.Variables, _ cloudprovider.Provider) (bool, error) { +func (s stubTerraformChecker) Plan(_ context.Context, _ *config.Config) (bool, error) { return s.tfDiff, s.planErr } -func (s stubTerraformChecker) RestoreClusterWorkspace() error { +func (s stubTerraformChecker) RestoreWorkspace() error { return s.rollbackErr }