Make session token duration configurable in AmazonWebServicesCredentialsBinding #52

nathandines · 2019-04-06T03:19:59Z

AmazonWebServicesCredentialsBinding is often used as the entrypoint to using this plugin in Jenkins pipelines. This change allows developers to override the session token duration in their pipelines.

This can be convenient if it's preferable to have a lower default value for session durations, but need a longer duration for a specific operation such as baking a large AMI.

Example usage:

stage('build') {
    steps {
        withCredentials([[
            $class: 'AmazonWebServicesCredentialsBinding',
            credentialsId: 'aws-dev-creds',
            stsTokenDuration: 7200
        ]]) {
            sh 'packer packer.json'
        }
    }
}

nathandines · 2019-05-13T08:11:04Z

Hey @andresrc, could I please get a review on this?

jglick · 2019-06-10T17:41:30Z

...n/java/com/cloudbees/jenkins/plugins/awscredentials/AmazonWebServicesCredentialsBinding.java

-        AWSCredentials credentials = getCredentials(build).getCredentials();
+        AmazonWebServicesCredentials credentialRetriever = getCredentials(build);
+        if (stsTokenDuration != null) {
+            credentialRetriever.setStsTokenDuration(stsTokenDuration);


This will potentially change the configuration in the credentials store, and thus change the behavior at least for all builds in the current session which do not specify this parameter.

ah, I didn't realise this. Thanks for letting me know.

jglick · 2019-06-10T17:46:25Z

Perhaps you could have an AWSCredentialsImpl.getCredentials() overload taking an alternate int stsTokenDuration.

jglick · 2019-06-12T23:00:53Z

...n/java/com/cloudbees/jenkins/plugins/awscredentials/AmazonWebServicesCredentialsBinding.java

@@ -87,14 +88,25 @@ public String getSecretKeyVariable() {
        return secretKeyVariable;
    }

+    @DataBoundSetter


You also need a matching public getter; and an update to config.jelly.

Cheers. Just for my benefit, could you please point me to the docs explaining the need for both the setter and the getter together? Or just briefly explain why? I believe that it's necessary, but would appreciate a better understanding of it.

Docs are not really written I am afraid. The getter is needed so that the equivalent of ${instance.stsTokenDuration} would work when reconfiguring an object from Jelly. Something like this would be evaluated by having a databound control inside an f:entry. You can get a test to fail against your code as written by using JenkinsRule.configRoundtrip with a SecretBuildWrapper (or StepConfigTester with a BindingStep, a bit more work), though it would be overkill if you are not doing anything out of the ordinary.

Ah, okay. That makes sense. Thanks for the explanation :)

jglick · 2019-06-12T23:02:13Z

src/main/java/com/cloudbees/jenkins/plugins/awscredentials/AWSCredentialsImpl.java

+        return getCredentials(this.getStsTokenDuration());
+    }
+
+    public AWSCredentials getCredentials(Integer stsTokenDuration) {


Probably int not Integer, or can this be null?

I don't think it should generally be null. Probably better to prevent this.

I had used Integer rather than int, as it seems to be used pretty consistently across the project. For the moment, I've added the @NonNull annotation on it. Do you think this would be sufficient, or do you think I should still change it to int?

Should be int unless you have a clear use case for a null value.

Okay. Just changed it

By enabling runtime configuration of the STS token duration, this enables a shorter token duration to be defined by default on a Jenkins master, but enables users to explicilty request a longer session token for longer running jobs. This helps to ensure that the credential lifetime is only as long as necessary.

nathandines · 2019-06-18T21:17:11Z

Hey @jglick, I made the requested changes last week. Are you able to re-review, please?

jglick · 2019-06-21T18:08:14Z

No time, sorry, will leave it to the plugin maintainer (whoever that may be).

andresrc · 2019-06-24T17:25:28Z

src/main/java/com/cloudbees/jenkins/plugins/awscredentials/AmazonWebServicesCredentials.java

@@ -45,6 +45,7 @@
    String getDisplayName();

    AWSCredentials getCredentials(String mfaToken);
+    AWSCredentials getCredentials(int stsTokenDuration);


Adding a new method is a binary incompatible change. Can you check if there are any implementations (at least in the jenkinsci organization) that could break? Anyway, some kind of notice would be needed.

Hey @andresrc, I've just been thinking about this lately, what do you think if this change was removed, and the dependent class (AmazonWebServicesCredentialsBinding) was adjusted to reference our class rather than this interface?

As its usage is within the same package, I think this should be okay; could you please validate this? I'm not 100% sure on what's coming in through the build parameter, however, and not sure if this would influence whether the resulting var will always be of AWSCredentialsImpl type.

I haven't actually tested this theory, so it might not even functionally work, but just your thoughts on the approach before trying it.

Location: found here

AmazonWebServicesCredentials credentialRetriever = getCredentials(build);

would become

AWSCredentialsImpl credentialRetriever = getCredentials(build);

jglick suggested changes Jun 10, 2019

View reviewed changes

nathandines force-pushed the feature/request-session-duration branch from 61bef5c to 5838c1f Compare June 12, 2019 22:47

jglick suggested changes Jun 12, 2019

View reviewed changes

nathandines force-pushed the feature/request-session-duration branch 2 times, most recently from fdbd780 to f48977f Compare June 13, 2019 22:24

nathandines force-pushed the feature/request-session-duration branch from f48977f to 7e201fb Compare June 13, 2019 22:43

andresrc reviewed Jun 24, 2019

View reviewed changes

Vlatombe added the enhancement New feature or request label Oct 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make session token duration configurable in AmazonWebServicesCredentialsBinding #52

Make session token duration configurable in AmazonWebServicesCredentialsBinding #52

nathandines commented Apr 6, 2019

nathandines commented May 13, 2019

jglick Jun 10, 2019

nathandines Jun 12, 2019

jglick commented Jun 10, 2019

jglick Jun 12, 2019

nathandines Jun 13, 2019

jglick Jun 13, 2019

nathandines Jun 13, 2019

jglick Jun 12, 2019

nathandines Jun 13, 2019

jglick Jun 13, 2019

nathandines Jun 13, 2019 •

edited

Loading

nathandines commented Jun 18, 2019

jglick commented Jun 21, 2019

andresrc Jun 24, 2019

nathandines Sep 17, 2019

Make session token duration configurable in AmazonWebServicesCredentialsBinding #52

Are you sure you want to change the base?

Make session token duration configurable in AmazonWebServicesCredentialsBinding #52

Conversation

nathandines commented Apr 6, 2019

nathandines commented May 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jglick commented Jun 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathandines Jun 13, 2019 • edited Loading

Choose a reason for hiding this comment

nathandines commented Jun 18, 2019

jglick commented Jun 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathandines Jun 13, 2019 •

edited

Loading