Skip to content

Commit

Permalink
cli: fix iam rollback (#1148)
Browse files Browse the repository at this point in the history 
* AB#2897 rename DestroyCluster

* #AB2897 error if terraform dir exists

* AB#2897 reword DestroyResources
  • Loading branch information
@msanft @derpsteb
msanft authored and derpsteb committed Feb 22, 2023
1 parent f550b8a commit baf03bd
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 32 deletions.
2 changes: 1 addition & 1 deletion cli/internal/cloudcmd/clients.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type terraformClient interface {
PrepareWorkspace(path string, input terraform.Variables) error
CreateCluster(ctx context.Context) (terraform.CreateOutput, error)
CreateIAMConfig(ctx context.Context, provider cloudprovider.Provider) (terraform.IAMOutput, error)
DestroyCluster(ctx context.Context) error
Destroy(ctx context.Context) error
CleanUpWorkspace() error
RemoveInstaller()
}
Expand Down
10 changes: 5 additions & 5 deletions cli/internal/cloudcmd/clients_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ type stubTerraformClient struct {
uid string
cleanUpWorkspaceCalled bool
removeInstallerCalled bool
destroyClusterCalled bool
destroyCalled bool
createClusterErr error
destroyClusterErr error
destroyErr error
prepareWorkspaceErr error
cleanUpWorkspaceErr error
iamOutputErr error
Expand All @@ -56,9 +56,9 @@ func (c *stubTerraformClient) PrepareWorkspace(path string, input terraform.Vari
return c.prepareWorkspaceErr
}

func (c *stubTerraformClient) DestroyCluster(ctx context.Context) error {
c.destroyClusterCalled = true
return c.destroyClusterErr
func (c *stubTerraformClient) Destroy(ctx context.Context) error {
c.destroyCalled = true
return c.destroyErr
}

func (c *stubTerraformClient) CleanUpWorkspace() error {
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cloudcmd/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestCreator(t *testing.T) {
if tc.wantRollback {
cl := tc.tfClient.(*stubTerraformClient)
if tc.wantTerraformRollback {
assert.True(cl.destroyClusterCalled)
assert.True(cl.destroyCalled)
}
assert.True(cl.cleanUpWorkspaceCalled)
if tc.provider == cloudprovider.QEMU {
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/cloudcmd/rollback.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type rollbackerTerraform struct {

func (r *rollbackerTerraform) rollback(ctx context.Context) error {
var err error
err = multierr.Append(err, r.client.DestroyCluster(ctx))
err = multierr.Append(err, r.client.Destroy(ctx))
if err == nil {
err = multierr.Append(err, r.client.CleanUpWorkspace())
}
Expand All @@ -56,7 +56,7 @@ type rollbackerQEMU struct {
func (r *rollbackerQEMU) rollback(ctx context.Context) error {
var err error
if r.createdWorkspace {
err = multierr.Append(err, r.client.DestroyCluster(ctx))
err = multierr.Append(err, r.client.Destroy(ctx))
}
err = multierr.Append(err, r.libvirt.Stop(ctx))
if err == nil {
Expand Down
10 changes: 5 additions & 5 deletions cli/internal/cloudcmd/rollback_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestRollbackTerraform(t *testing.T) {
tfClient: &stubTerraformClient{},
},
"destroy cluster error": {
tfClient: &stubTerraformClient{destroyClusterErr: someErr},
tfClient: &stubTerraformClient{destroyErr: someErr},
wantErr: true,
},
"clean up workspace error": {
Expand All @@ -51,7 +51,7 @@ func TestRollbackTerraform(t *testing.T) {
return
}
assert.NoError(err)
assert.True(tc.tfClient.destroyClusterCalled)
assert.True(tc.tfClient.destroyCalled)
assert.True(tc.tfClient.cleanUpWorkspaceCalled)
})
}
Expand All @@ -78,7 +78,7 @@ func TestRollbackQEMU(t *testing.T) {
},
"destroy cluster error": {
libvirt: &stubLibvirtRunner{stopErr: someErr},
tfClient: &stubTerraformClient{destroyClusterErr: someErr},
tfClient: &stubTerraformClient{destroyErr: someErr},
wantErr: true,
},
"clean up workspace error": {
Expand Down Expand Up @@ -109,9 +109,9 @@ func TestRollbackQEMU(t *testing.T) {
assert.NoError(err)
assert.True(tc.libvirt.stopCalled)
if tc.createdWorkspace {
assert.True(tc.tfClient.destroyClusterCalled)
assert.True(tc.tfClient.destroyCalled)
} else {
assert.False(tc.tfClient.destroyClusterCalled)
assert.False(tc.tfClient.destroyCalled)
}
assert.True(tc.tfClient.cleanUpWorkspaceCalled)
})
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cloudcmd/terminate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (t *Terminator) Terminate(ctx context.Context) (retErr error) {
}

func (t *Terminator) terminateTerraform(ctx context.Context, cl terraformClient) error {
if err := cl.DestroyCluster(ctx); err != nil {
if err := cl.Destroy(ctx); err != nil {
return err
}
return cl.CleanUpWorkspace()
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/cloudcmd/terminate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestTerminator(t *testing.T) {
wantErr: true,
},
"destroy cluster error": {
tfClient: &stubTerraformClient{destroyClusterErr: someErr},
tfClient: &stubTerraformClient{destroyErr: someErr},
libvirt: &stubLibvirtRunner{},
wantErr: true,
},
Expand Down Expand Up @@ -70,7 +70,7 @@ func TestTerminator(t *testing.T) {
}
assert.NoError(err)
cl := tc.tfClient.(*stubTerraformClient)
assert.True(cl.destroyClusterCalled)
assert.True(cl.destroyCalled)
assert.True(cl.removeInstallerCalled)
assert.True(tc.libvirt.stopCalled)
})
Expand Down
15 changes: 15 additions & 0 deletions cli/internal/cmd/iam.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ func (c *iamCreator) create(ctx context.Context) error {
return err
}

if err := c.checkWorkingDir(); err != nil {
return err
}

if !flags.yesFlag {
c.cmd.Printf("The following IAM configuration will be created:\n\n")
c.providerCreator.printConfirmValues(c.cmd, flags)
Expand Down Expand Up @@ -260,6 +264,17 @@ func (c *iamCreator) parseFlagsAndSetupConfig() (iamFlags, error) {
return flags, nil
}

// checkWorkingDir checks if the current working directory already contains a Terraform dir or a Constellation config file.
func (c *iamCreator) checkWorkingDir() error {
if _, err := c.fileHandler.Stat(constants.TerraformIAMWorkingDir); err == nil {
return fmt.Errorf("the current working directory already contains the %s directory. Please run the command in a different directory", constants.TerraformIAMWorkingDir)
}
if _, err := c.fileHandler.Stat(constants.ConfigFilename); err == nil {
return fmt.Errorf("the current working directory already contains the %s file. Please run the command in a different directory", constants.ConfigFilename)
}
return nil
}

// iamFlags contains the parsed flags of the iam create command, including the parsed flags of the selected cloud provider.
type iamFlags struct {
aws awsFlags
Expand Down
69 changes: 57 additions & 12 deletions cli/internal/cmd/iam_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,18 @@ func TestParseIDFile(t *testing.T) {
}

func TestIAMCreateAWS(t *testing.T) {
defaultFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs {
defaultFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewMemMapFs()
fileHandler := file.NewHandler(fs)
for _, f := range existingFiles {
require.NoError(fileHandler.Write(f, []byte{1, 2, 3}, file.OptNone))
}
for _, d := range existingDirs {
require.NoError(fs.MkdirAll(d, 0o755))
}
return fs
}
readOnlyFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs {
readOnlyFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewReadOnlyFs(afero.NewMemMapFs())
return fs
}
Expand All @@ -87,7 +90,7 @@ func TestIAMCreateAWS(t *testing.T) {
}

testCases := map[string]struct {
setupFs func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs
setupFs func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs
creator *stubIAMCreator
provider cloudprovider.Provider
zoneFlag string
Expand All @@ -96,6 +99,7 @@ func TestIAMCreateAWS(t *testing.T) {
generateConfigFlag bool
configFlag string
existingFiles []string
existingDirs []string
stdin string
wantAbort bool
wantErr bool
Expand Down Expand Up @@ -152,6 +156,16 @@ func TestIAMCreateAWS(t *testing.T) {
configFlag: "custom-config.yaml",
existingFiles: []string{"custom-config.yaml"},
},
"iam create aws existing terraform dir": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
provider: cloudprovider.AWS,
zoneFlag: "us-east-2a",
prefixFlag: "test",
yesFlag: true,
wantErr: true,
existingDirs: []string{constants.TerraformIAMWorkingDir},
},
"interactive": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
Expand Down Expand Up @@ -242,7 +256,7 @@ func TestIAMCreateAWS(t *testing.T) {
require.NoError(cmd.Flags().Set("config", tc.configFlag))
}

fileHandler := file.NewHandler(tc.setupFs(require, tc.provider, tc.existingFiles))
fileHandler := file.NewHandler(tc.setupFs(require, tc.provider, tc.existingFiles, tc.existingDirs))

iamCreator := &iamCreator{
cmd: cmd,
Expand Down Expand Up @@ -282,15 +296,18 @@ func TestIAMCreateAWS(t *testing.T) {
}

func TestIAMCreateAzure(t *testing.T) {
defaultFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs {
defaultFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewMemMapFs()
fileHandler := file.NewHandler(fs)
for _, f := range existingFiles {
require.NoError(fileHandler.Write(f, []byte{1, 2, 3}, file.OptNone))
}
for _, d := range existingDirs {
require.NoError(fs.MkdirAll(d, 0o755))
}
return fs
}
readOnlyFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs {
readOnlyFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewReadOnlyFs(afero.NewMemMapFs())
return fs
}
Expand All @@ -306,7 +323,7 @@ func TestIAMCreateAzure(t *testing.T) {
}

testCases := map[string]struct {
setupFs func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs
setupFs func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs
creator *stubIAMCreator
provider cloudprovider.Provider
regionFlag string
Expand All @@ -316,6 +333,7 @@ func TestIAMCreateAzure(t *testing.T) {
generateConfigFlag bool
configFlag string
existingFiles []string
existingDirs []string
stdin string
wantAbort bool
wantErr bool
Expand Down Expand Up @@ -377,6 +395,17 @@ func TestIAMCreateAzure(t *testing.T) {
yesFlag: true,
wantErr: true,
},
"iam create azure existing terraform dir": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
provider: cloudprovider.Azure,
regionFlag: "westus",
servicePrincipalFlag: "constell-test-sp",
resourceGroupFlag: "constell-test-rg",
yesFlag: true,
wantErr: true,
existingDirs: []string{constants.TerraformIAMWorkingDir},
},
"interactive": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
Expand Down Expand Up @@ -465,7 +494,7 @@ func TestIAMCreateAzure(t *testing.T) {
require.NoError(cmd.Flags().Set("config", tc.configFlag))
}

fileHandler := file.NewHandler(tc.setupFs(require, tc.provider, tc.existingFiles))
fileHandler := file.NewHandler(tc.setupFs(require, tc.provider, tc.existingFiles, tc.existingDirs))

iamCreator := &iamCreator{
cmd: cmd,
Expand Down Expand Up @@ -508,15 +537,18 @@ func TestIAMCreateAzure(t *testing.T) {
}

func TestIAMCreateGCP(t *testing.T) {
defaultFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs {
defaultFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewMemMapFs()
fileHandler := file.NewHandler(fs)
for _, f := range existingFiles {
require.NoError(fileHandler.Write(f, []byte{1, 2, 3}, file.OptNone))
}
for _, d := range existingDirs {
require.NoError(fs.MkdirAll(d, 0o755))
}
return fs
}
readOnlyFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs {
readOnlyFs := func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs {
fs := afero.NewReadOnlyFs(afero.NewMemMapFs())
return fs
}
Expand All @@ -534,7 +566,7 @@ func TestIAMCreateGCP(t *testing.T) {
}

testCases := map[string]struct {
setupFs func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string) afero.Fs
setupFs func(require *require.Assertions, provider cloudprovider.Provider, existingFiles []string, existingDirs []string) afero.Fs
creator *stubIAMCreator
provider cloudprovider.Provider
zoneFlag string
Expand All @@ -544,6 +576,7 @@ func TestIAMCreateGCP(t *testing.T) {
generateConfigFlag bool
configFlag string
existingFiles []string
existingDirs []string
stdin string
wantAbort bool
wantErr bool
Expand Down Expand Up @@ -605,6 +638,18 @@ func TestIAMCreateGCP(t *testing.T) {
yesFlag: true,
wantErr: true,
},
"iam create gcp existing terraform dir": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
provider: cloudprovider.GCP,
zoneFlag: "europe-west1-a",
serviceAccountIDFlag: "constell-test",
projectIDFlag: "constell-1234",

existingDirs: []string{constants.TerraformIAMWorkingDir},
yesFlag: true,
wantErr: true,
},
"iam create gcp invalid flags": {
setupFs: defaultFs,
creator: &stubIAMCreator{id: validIAMIDFile},
Expand Down Expand Up @@ -712,7 +757,7 @@ func TestIAMCreateGCP(t *testing.T) {
require.NoError(cmd.Flags().Set("config", tc.configFlag))
}

fileHandler := file.NewHandler(tc.setupFs(require, tc.provider, tc.existingFiles))
fileHandler := file.NewHandler(tc.setupFs(require, tc.provider, tc.existingFiles, tc.existingDirs))

iamCreator := &iamCreator{
cmd: cmd,
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/terraform/terraform.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ func (c *Client) CreateIAMConfig(ctx context.Context, provider cloudprovider.Pro
}
}

// DestroyCluster destroys a Constellation cluster using Terraform.
func (c *Client) DestroyCluster(ctx context.Context) error {
// Destroy destroys Terraform-created cloud resources.
func (c *Client) Destroy(ctx context.Context) error {
if err := c.tf.Init(ctx); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/terraform/terraform_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ func TestDestroyInstances(t *testing.T) {
tf: tc.tf,
}

err := c.DestroyCluster(context.Background())
err := c.Destroy(context.Background())
if tc.wantErr {
assert.Error(err)
return
Expand Down

0 comments on commit baf03bd

Please sign in to comment.