Features/issue24 turning on iam role cannot be disabled #28
Conversation
| @@ -72,4 +72,7 @@ public AmazonS3Client s3Client(FetchConfig config) { | |||
| return client; | |||
| } | |||
|
|
|||
| public FetchConfig getFetchConfig(TaskConfig config, TaskExecutionContext context) { | |||
There was a problem hiding this comment.
Why do we have getFetchConfig which doesn't seem to be doing anything as a separate function?
There was a problem hiding this comment.
It is used in one place - FetchExecutor.java:84. The reason that was needed is to mock out system environment variables in GoEnvironment (otherwise environment variable on the build server end up in your test and one may get inconsistent results from test to test).
|
@manojlds Can you please help review the build.bat? Rest LGTM, except for minor comments here and there. |
| public FetchConfig(TaskConfig config, TaskExecutionContext context) { | ||
| this.env = new GoEnvironment(); | ||
| public FetchConfig(TaskConfig config, TaskExecutionContext context) | ||
| { |
There was a problem hiding this comment.
Nitpick - Can we please push this { to previous line? 😄
Remove redundant tests in GoEnvironmentTest Fix formatting of brace in FetchConfig.java
…leCannotBeDisabled
|
@manojlds @ashwanthkumar Thanks |
|
@haimizrael I changed our CI's configuration for this PR. It's all green now :) |
|
@ashwanthkumar Thank you! |
|
@ashwanthkumar do you guys need me to do anything else on this PR? |
|
LGTM 👍 |
1 similar comment
|
LGTM 👍 |
…leCannotBeDisabled
|
Merging this. |
…MRoleCannotBeDisabled Features/issue24 turning on iam role cannot be disabled
|
Thanks @haimizrael. |
This PR is for Issue #24. Note this is a breaking change for the use of environment variable AWS_USE_IAM_ROLE to turn on the use of IAM role. The value of the variable now has to be a valid Boolean truthful value ('True', 'On', or 'Yes') for the use of IAM role to be enabled. A value of 'False','Off', or 'No' will disable the use of IAM role, which can be a requirement for certain agents that are either not in AWS or for whatever reason do not have the appropriate role and you have to use access keys instead.
We've also added unit tests to cover this usage (and the use of AWS_USE_IAM_ROLE variable in general). To enable executing of such tests in our environment, we've also mocked out the system environment variables in tests by parameterizing the environment hashmap in GoEnvironment.java (originally it was hard-coded to System.getenv(), so system environment variables could be polluting the tests).
Finally, the build.bat file was added to support CI in a Windows environment.