Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cli: fix iam rollback #1148

Merged
merged 3 commits into from
Feb 13, 2023
Merged

cli: fix iam rollback #1148

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
564022f
AB#2897 rename DestroyCluster
msanft Feb 6, 2023
6d72619
#AB2897 error if terraform dir exists
msanft Feb 6, 2023
d2b69cc
AB#2897 reword DestroyResources
msanft Feb 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/internal/cloudcmd/clients.go
View file
Open in desktop
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
DestroyResources(ctx context.Context) error
msanft marked this conversation as resolved.
Show resolved Hide resolved
CleanUpWorkspace() error
RemoveInstaller()
}
Expand Down
10 changes: 5 additions & 5 deletions cli/internal/cloudcmd/clients_test.go
View file
Open in desktop
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
destroyResourcesCalled bool
createClusterErr error
destroyClusterErr error
destroyResourcesErr 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) DestroyResources(ctx context.Context) error {
c.destroyResourcesCalled = true
return c.destroyResourcesErr
}

func (c *stubTerraformClient) CleanUpWorkspace() error {
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cloudcmd/create_test.go
View file
Open in desktop
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.destroyResourcesCalled)
}
assert.True(cl.cleanUpWorkspaceCalled)
if tc.provider == cloudprovider.QEMU {
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/cloudcmd/rollback.go
View file
Open in desktop
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.DestroyResources(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.DestroyResources(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
Open in desktop
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{destroyResourcesErr: 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.destroyResourcesCalled)
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{destroyResourcesErr: 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.destroyResourcesCalled)
} else {
assert.False(tc.tfClient.destroyClusterCalled)
assert.False(tc.tfClient.destroyResourcesCalled)
}
assert.True(tc.tfClient.cleanUpWorkspaceCalled)
})
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cloudcmd/terminate.go
View file
Open in desktop
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.DestroyResources(ctx); err != nil {
return err
}
return cl.CleanUpWorkspace()
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/cloudcmd/terminate_test.go
View file
Open in desktop
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{destroyResourcesErr: 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.destroyResourcesCalled)
assert.True(cl.removeInstallerCalled)
assert.True(tc.libvirt.stopCalled)
})
Expand Down
15 changes: 15 additions & 0 deletions cli/internal/cmd/iam.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 {
// DestroyResources destroys Terraform-created cloud resources.
func (c *Client) DestroyResources(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
Open in desktop
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.DestroyResources(context.Background())
if tc.wantErr {
assert.Error(err)
return
Expand Down