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

Add --backend-config cli option to tfmigrate plan/apply #94

Merged
merged 11 commits into from
Jul 7, 2022
Merged

Add --backend-config cli option to tfmigrate plan/apply #94

merged 11 commits into from
Jul 7, 2022

Conversation

uchida
Copy link
Contributor

@uchida uchida commented Jul 3, 2022

This PR will close #15, adding --backend-config options to tfmigate plan which adds -backend-config options to terraform init on switch back remote state.

uchida
uchida commented Jul 3, 2022
View reviewed changes
docker-compose.yml
@@ -17,7 +17,7 @@ services:
- localstack

localstack:
image: localstack/localstack:0.11.3
image: localstack/localstack:0.14.4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for development for arm64

uchida
uchida commented Jul 4, 2022
View reviewed changes
docker-compose.yml
@@ -17,7 +17,7 @@ services:
- localstack

localstack:
image: localstack/localstack:0.11.3
image: localstack/localstack:0.14.4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Apple Silicon, my development environment, docker image which has linux/arm64/v8 arch is preferable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

minamijoyo
minamijoyo reviewed Jul 4, 2022
View reviewed changes
command/plan.go Outdated
assignments (same format as terraform.tfvars) or a
'key=value' format, and can be specified multiple
times. The backend type must be in the configuration
itself.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is expected that tfmigrate users have some knowledge of terraform. Rather than simply copying the terraform init --backend-config help message, it would be clearer to explicitly explain that the flag is passed to terraform init --backend-config when switching backend to remote.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update help message in fc6422e

minamijoyo
minamijoyo reviewed Jul 4, 2022
View reviewed changes
command/plan.go
@@ -13,13 +13,15 @@ import (
// migration operations to a temporary state.
type PlanCommand struct {
Meta
out string
backendConfig []string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tfmigrate apply command also uses the same logic, so we need to add this flag to the tfmigrate apply command too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add --backend-config to tfmigrate apply in 6cb4eca

minamijoyo
minamijoyo reviewed Jul 4, 2022
View reviewed changes
tfexec/terraform.go Outdated
@@ -61,7 +61,7 @@ type TerraformCLI interface {
Version(ctx context.Context) (string, error)

// Init initializes the current work directory.
Init(ctx context.Context, opts ...string) error
Init(ctx context.Context, backendConfig []string, opts ...string) error
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the existing opts argument instead of adding the new argument for Init.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing backend config options as opts in 0f5711e

minamijoyo
minamijoyo reviewed Jul 4, 2022
View reviewed changes
tfexec/test_helper.go Outdated
@@ -233,19 +233,20 @@ func setupTestPluginCacheDir(e Executor) error {
// GetTestAccBackendS3Config returns mocked backend s3 config for testing.
// Its endpoint can be set via LOCALSTACK_ENDPOINT environment variable.
// default to "http://localhost:4566"
func GetTestAccBackendS3Config(dir string) string {
func GetTestAccBackendS3Config(dir string, skipS3Bucket bool) string {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many test codes are affected by this change. Don't add complexity for something that is only needed in part. Helper functions exist to make it easy to write tests for common cases. Special cases should be written directly in the test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert and directly write backend in 2bfdbe2

minamijoyo
minamijoyo reviewed Jul 4, 2022
View reviewed changes
tfexec/test_helper.go Outdated
return backendConfig
}

// SetupTestAccWithApply is an acceptance test helper for initializing a
// temporary work directory and applying a given source.
func SetupTestAccWithApply(t *testing.T, workspace string, source string) TerraformCLI {
func SetupTestAccWithApply(t *testing.T, workspace string, source string, backendConfig []string) TerraformCLI {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, this change should also be written directly in the specific test case without modifying the helper function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert and directly write backend in 232c8ce

@uchida uchida force-pushed the add-backend-config-cli-option branch from 1ab353b to 3f74134 Compare July 5, 2022 10:08
@uchida uchida force-pushed the add-backend-config-cli-option branch from 3f74134 to 232c8ce Compare July 5, 2022 10:08
@uchida uchida requested a review from minamijoyo July 5, 2022 10:09
@uchida
Copy link
Contributor Author

uchida commented Jul 5, 2022

Reflected review comments, thanks for giving your feedbacks.

minamijoyo
minamijoyo reviewed Jul 7, 2022
View reviewed changes
tfexec/terraform_test.go Outdated
`
workspace := "work1"
backendConfig := []string{"bucket=tfstate-test"}
e := SetupTestAcc(t, source)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is failing: https://github.com/minamijoyo/tfmigrate/runs/7225213659?check_suite_focus=true

2022/07/07 00:45:47 [DEBUG] [executor@/tmp/workDir3305983562] failed to run command: (*exec.ExitError)(0xc0002cc640)(exit status 1)
--- FAIL: TestAccTerraformCLIOverrideBackendToLocalWithBackendConfig (3.65s)
    terraform_test.go:242: failed to run terraform apply: failed to run command (exited 1): terraform apply -input=false -no-color -auto-approve
        stdout:
        stderr:
        Error: error configuring Terraform AWS Provider: no valid credential sources for Terraform AWS Provider found.
        Please see https://registry.terraform.io/providers/hashicorp/aws
        for more information about providing credentials.
        Error: failed to refresh cached credentials, no EC2 IMDS role found, operation error ec2imds: GetMetadata, failed to get API token, cannot get API token, operation error ec2imds: getToken, http response error StatusCode: 400, request to EC2 IMDS failed

I think the mocked test config should be backend+source instead of source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, passed source+backend in bcae4e1

minamijoyo
minamijoyo approved these changes Jul 7, 2022
View reviewed changes
Copy link
Owner

@minamijoyo minamijoyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uchida Thank you for your contribution!

@minamijoyo minamijoyo merged commit 7bc05db into minamijoyo:master Jul 7, 2022
@minamijoyo minamijoyo changed the title Add --backend-config cli option to tfmigrate plan Add --backend-config cli option to tfmigrate plan/apply Jul 7, 2022
@minamijoyo
Copy link
Owner

Released in v0.3.4 🚀

@minamijoyo minamijoyo mentioned this pull request Aug 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@minamijoyo minamijoyo minamijoyo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handle partial backend config
2 participants
@uchida @minamijoyo