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

Feature: Custom AWS client configuration #43

Merged
merged 1 commit into from
May 19, 2022
Merged

Conversation

chriskilding
Copy link
Contributor

@chriskilding chriskilding commented Oct 29, 2020

Add a custom AWS client configuration feature.

It should have these settings:

  • credentials provider (default, profile, assumeRole)
  • endpoint configuration
  • region

@chriskilding chriskilding self-assigned this Oct 29, 2020
@chriskilding chriskilding reopened this Oct 5, 2021
@chriskilding chriskilding changed the title Remove multi-client beta feature, add custom client feature Add custom client feature Oct 5, 2021
@chriskilding chriskilding changed the title Add custom client feature Add client configuration feature Oct 5, 2021
@chriskilding chriskilding changed the title Add client configuration feature Feature: Custom AWS client configuration Oct 5, 2021
@chriskilding
Copy link
Contributor Author

To do: Change the "Test Endpoint Configuration" button to "Test Client" - want to let users test all the client options they've set together.

@chriskilding chriskilding force-pushed the remove-clients branch 2 times, most recently from e8da694 to 9a92faa Compare November 2, 2021 14:27
@chriskilding
Copy link
Contributor Author

Confirmed working in a real installation.

The AWS SecretSource implementation does not use this configuration feature, which could cause confusion if we merge this feature now. So ideally we would remove the SecretSource dependency before merging this feature.

@parmou
Copy link

parmou commented Apr 13, 2022

@chriskilding can you implement this now as the SecretSource dependency is now removed. I think this will help because right now the plugin makes use of the node role instead of pod role. #172 . I want to use a custom role. If there is a way of doing it without this, I sure can use some help.

@chriskilding
Copy link
Contributor Author

Yep - that was the plan for removing the SecretSource, to unblock features like this.

The first feature I reworked, and want to get in to the plugin, is credential renaming (#40). This feature was asked for some time ago, however I've not yet found someone to beta test it. Would you be willing to do that?

Ideally, after that's merged then I can rework this PR and add it.

@chriskilding
Copy link
Contributor Author

I'm reworking this PR in the meantime to catch it up with all the recent changes. I get 3 tests that fail in (WebCredentialsProviderIT) but for different reasons depending on whether they're run locally or on CI...


Sometimes they fail claiming that the ListSecrets config class has no descriptor. This is quite weird because (a) ListSecrets has not been modified in this pull request and (b) it definitely does have a descriptor.


Sometimes they fail due to:

Caused by: java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
	at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248)
	at java.base/java.util.Objects.checkIndex(Objects.java:372)
	at java.base/java.util.ArrayList.get(ArrayList.java:459)
	at io.jenkins.plugins.credentials.secretsmanager.config.client.credentialsProvider.WebCredentialsProviderIT$EnhancedForm.setClientCredentialsProviderSelect(WebCredentialsProviderIT.java:66)
	at io.jenkins.plugins.credentials.secretsmanager.config.client.credentialsProvider.WebCredentialsProviderIT$EnhancedForm.setClientWithSTSAssumeRoleSessionCredentialsProvider(WebCredentialsProviderIT.java:60)
	at io.jenkins.plugins.credentials.secretsmanager.config.client.credentialsProvider.WebCredentialsProviderIT.lambda$setCredentialsProvider$1(WebCredentialsProviderIT.java:29)

My guess here is that it tries to find the HTML <select> element where it specifies Default, Profile, or STS AssumeRole, and fails to do so for some reason.

@chriskilding chriskilding marked this pull request as ready for review April 21, 2022 12:07
@chriskilding
Copy link
Contributor Author

chriskilding commented Apr 22, 2022

@parveshmourya I've now reworked the feature and got the build back to green. I need someone to test the feature in a real (non-production) Jenkins installation to ensure it works. Would you be able to test it? If so I'll send you a link to a beta build of the plugin .hpi. Alternatively you can build it from source on this branch.

@parmou
Copy link

parmou commented Apr 22, 2022

@chriskilding yeah, please share the link. I will try to test this, during the weekend or maybe in the coming week.

@chriskilding
Copy link
Contributor Author

@parveshmourya any update on this?

@chriskilding chriskilding merged commit 5510bd7 into master May 19, 2022
@chriskilding chriskilding deleted the remove-clients branch May 19, 2022 14:24
@parmou
Copy link

parmou commented Jul 5, 2022

@chriskilding apologies for the super-delay. We decided to hold on to our jenkins changes some time ago and I did not get the time to test it.

@chriskilding
Copy link
Contributor Author

Thanks for coming back to me, I got some feedback that the feature worked as intended so I went ahead and merged it. The feature is available in recent releases of the plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants