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.

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

Allow to override env vars for terraform operations in acc tests #153

Merged
merged 1 commit into from
Jan 27, 2021

Conversation

eliecharra
Copy link
Contributor

Q A
πŸ› Bug fix? no
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #83
❓ Documentation yes

@eliecharra eliecharra added the kind/maintenance Refactoring or changes to the workspace label Jan 25, 2021
@eliecharra eliecharra requested a review from a team January 25, 2021 17:44
@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #153 (9fe8268) into main (297e0c1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #153   +/-   ##
=======================================
  Coverage   67.76%   67.76%           
=======================================
  Files         186      186           
  Lines        4197     4197           
=======================================
  Hits         2844     2844           
  Misses       1045     1045           
  Partials      308      308           

if environMap[envKeyValue[0]] == "" {
environMap[envKeyValue[0]] = envKeyValue[1]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is one case that it will not be overridden but I don't know if it's possible to be in that special case. Imagine this one in that exact order:

os.Setenv("ACC_TEST_VAR_5", "")
os.Setenv("TEST_VAR_5", "test2")

The output would be that TEST_VAR_5 at the end will be test2.

Copy link
Contributor Author

@eliecharra eliecharra Jan 27, 2021

Choose a reason for hiding this comment

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

Good catch, my test to check if key is present in map was weak, fixed using this form πŸ‘πŸ»

		if _, alreadyExist := environMap[envKeyValue[0]]; !alreadyExist {
			environMap[envKeyValue[0]] = envKeyValue[1]
		}

Test updated

@@ -124,6 +124,34 @@ Each acceptance test should be prefixed by `TestAcc_` and should be run using en
DRIFTCTL_ACC=true go test -run=TestAcc_ ./pkg/resource/aws/aws_instance_test.go
```

### Credentials

Acceptance test need credentials to perform real world action on cloud providers:
Copy link
Contributor

Choose a reason for hiding this comment

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

Acceptance tests or Acceptance test needs

### Credentials

Acceptance test need credentials to perform real world action on cloud providers:
- Read/write access are required to perform terraform action.
Copy link
Contributor

Choose a reason for hiding this comment

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

Either we put point at the end of the sentence or not, your choice.

- Read/write access are required to perform terraform action.
- Read only access is required for driftctl execution

Recommended way to run acc tests is to use two distinct credentials,
Copy link
Contributor

Choose a reason for hiding this comment

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

credentials: one for terraform related actions, and one for driftctl scan.

Recommended way to run acc tests is to use two distinct credentials,
one for terraform related actions, and one for driftctl scan.

You can override environment variables passed to terraform operations by using `ACC_` prefix on env variables.
Copy link
Contributor

Choose a reason for hiding this comment

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

by adding ...


#### AWS

You can use `ACC_AWS_PROFILE` to override profile used for terraform operations
Copy link
Contributor

Choose a reason for hiding this comment

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

to override AWS named profile used for terraform operations.

@eliecharra eliecharra force-pushed the fix_permissions_in_acc_tests branch 2 times, most recently from adb47eb to 9fe8268 Compare January 27, 2021 09:47
Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

Let's go !

@wbeuil wbeuil merged commit 4ac20fc into main Jan 27, 2021
@wbeuil wbeuil deleted the fix_permissions_in_acc_tests branch January 27, 2021 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/maintenance Refactoring or changes to the workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants