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

Fixes #444 - Add duration parameter to WithAwsPlugin.withRole() #445

Merged
merged 5 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- [Issue #432](https://github.com/manheim/terraform-pipeline/issues/432) pass TagPlugin through `-var-file={env}-tags.tfvars`
- [Issue #417](https://github.com/manheim/terraform-pipeline/issues/417) DestroyPlugin & PassPlanFilePlugin - Terraform Destroy can't be called with a plan file
- [Issue #436](https://github.com/manheim/terraform-pipeline/issues/436) Bug Fix: Omit variables and variable files from apply command if a plan file is specified
- [Issue #444](https://github.com/manheim/terraform-pipeline/issues/444) Expose optional duration parameter on WithAwsPlugin's `withRole()`

# v5.19

Expand Down
12 changes: 12 additions & 0 deletions docs/WithAwsPlugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,15 @@ validate.then(deployQa)
.then(deployProd)
.build()
```

If you want to specify a role session duration other than the default of 1 hour (3600 seconds), you can do so by providing an integer duration to `withDuration()`:

```
WithAwsPlugin.withDuration(43200).init()
```

or, with a specific role ARN

```
WithAwsPlugin.withRole('MY_ROLE_ARN').withDuration(43200).init()
```
Copy link
Collaborator

@kmanning kmanning Nov 9, 2022

Choose a reason for hiding this comment

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

This fix looks good overall, but one minor tweak.

This feature is an example of trying to configure a plugin that has multiple different parameters, which can be set independently of one another.

Rather than trying to overload the .withRole() method to optionally accept a role/role+duration/duration, can we simply add a .withDuration() method? That way, if you want to set duration, you can do so explicitly. If you want to set the role, you can do so explicitly. If you want to set them both, then you chain and call them both explicitly.

By using the suggestion above, we can avoid the awkardness and lack-of-context in your example above: WithAwsPlugin.withRole(null, 43200). "What is 43200? What does that have to do with a null role?" vs WithAwsPlugin.withDuration(43200) which gives context to 43200. "43200 is a duration".

This is a convention that's used heavily across all the plugins, so I'd like to keep that consistent. https://en.wikipedia.org/wiki/Fluent_interface

21 changes: 20 additions & 1 deletion src/WithAwsPlugin.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import static TerraformEnvironmentStage.ALL

class WithAwsPlugin implements TerraformEnvironmentStagePlugin, Resettable {
private static role
private static duration

public static void init() {
WithAwsPlugin plugin = new WithAwsPlugin()
Expand All @@ -19,9 +20,10 @@ class WithAwsPlugin implements TerraformEnvironmentStagePlugin, Resettable {
public Closure addWithAwsRole(String environment) {
return { closure ->
String iamRole = getRole(environment)
Integer sessionDuration = getDuration()

if (iamRole != null) {
withAWS(role: iamRole) {
withAWS(role: iamRole, duration: sessionDuration) {
sh "echo Running AWS commands under the role: ${iamRole}"
closure()
}
Expand All @@ -38,6 +40,12 @@ class WithAwsPlugin implements TerraformEnvironmentStagePlugin, Resettable {
return this
}

public static withDuration(Integer duration = 3600) {
this.duration = duration

return this
}

public String getRole(String environment) {
def tempRole = this.role

Expand All @@ -56,7 +64,18 @@ class WithAwsPlugin implements TerraformEnvironmentStagePlugin, Resettable {
return tempRole
}

public Integer getDuration() {
def tempDuration = this.@duration

if (tempDuration == null) {
tempDuration = 3600
}

return tempDuration
}

public static void reset() {
this.role = null
this.duration = 3600
}
}
42 changes: 14 additions & 28 deletions test/WithAwsPluginTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -85,44 +85,30 @@ class WithAwsPluginTest {
}

@Nested
public class WithExplicitRole {
public class WithDefaultDuration {
@Test
void returnsProvidedRole() {
def expectedRole = "myRole"
void returnsDefaultDuration() {
def expectedDuration = 3600
def plugin = new WithAwsPlugin()
MockJenkinsfile.withEnv(AWS_ROLE_ARN: 'foo')

plugin.withRole(expectedRole)

def actualRole = plugin.getRole()

assertThat(actualRole, is(expectedRole))
}

@Test
void prefersProvidedRoleOverGenericRole() {
def expectedRole = "correctRole"
def plugin = new WithAwsPlugin()
MockJenkinsfile.withEnv(AWS_ROLE_ARN: 'incorrectRole')

plugin.withRole(expectedRole)

def actualRole = plugin.getRole()

assertThat(actualRole, is(expectedRole))
def actualDuration = plugin.getDuration()
assertThat(actualDuration, is(expectedDuration))
}
}

@Nested
public class WithExplicitDuration {
@Test
void prefersProvidedRoleOverEnvironmntSpecificRole() {
def expectedRole = "correctRole"
void returnsExplicitDuration() {
def expectedDuration = 43200
def plugin = new WithAwsPlugin()
MockJenkinsfile.withEnv(QA_AWS_ROLE_ARN: 'incorrectRole')

plugin.withRole(expectedRole)
plugin.withDuration(expectedDuration)

def actualRole = plugin.getRole('qa')
def actualDuration = plugin.getDuration()

assertThat(actualRole, is(expectedRole))
assertThat(actualDuration, is(expectedDuration))
}
}
}