Upgrade to AWS SDK v2 with IAM Instance profile & Assume Role support#477
Conversation
See: https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/migration-howto.html Used this init.gradle file: ============== initscript { repositories { maven { url "https://plugins.gradle.org/m2" } } dependencies { classpath("org.openrewrite:plugin:7.15.0") } } rootProject { plugins.apply(org.openrewrite.gradle.RewritePlugin) dependencies { rewrite("software.amazon.awssdk:v2-migration:latest.release") } afterEvaluate { if (repositories.isEmpty()) { repositories { mavenCentral() } } } } ============== Ran this: ./gradlew rewriteRun --init-script init.gradle \ -Drewrite.activeRecipes=software.amazon.awssdk.v2migration.AwsSdkJavaV1ToV2
This version supports AWS SDK v2.x
- SDKGlobalConfiguration has been removed. No replacement seems to be in place. So, the names of the environment variables (which have changed a bit, as mentioned in https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/migration-env-and-system-props.html) have been added to a new class called AwsSdkNames. - Remove unnecessary mocking in AWSClientFactoryTest, which had hidden the fact that region was not present in the setup.
The default does seem to be "sync" rather than async, but the code that was automatically migrated in commit da6c6b2 explicitly set it to use sync.
This comment was marked as resolved.
This comment was marked as resolved.
|
The second issue is the tiny matter that it ... doesn't work yet. Secret configPipeline configAWS secrets manager setupIt fails with this stack trace: I'm using this Java version: I haven't dug depeer into the issue yet. I'm thinking it's one of two problems:
I suspect it's the first one. The reason we wanted to pass through some packages is so that Felix / OSGi will not have those classes separately in the plugin classloader, which would make it hard to send messages back and forth. Even without that, the classes would have been available within the plugin. So, it's unlikely to be the second possibility mentioned above. |
|
I wrote a throw-away test which connects to the real AWS secrets manager and it works fine. So, I'm leaning more towards the OSGi related possibility. Needs more investigation. |
|
I recall now there is this config here which I understand restricts the classloading outside the plugin itself to the bootclassloader: IIRC Apache HttpClient folks started using jdk.net in a 5.x patch release for spurious reasons, IMHO a bad idea - yet they persist with being obstinate https://lists.apache.org/thread/5pmyy8xclqwht49gccy3xvnd2pmv2pz4 I think it will likely not work on any current GoCD container images because we use Temurin JREs and I don't think they include this module? I think If it's not solely an OSGI thing, unless they have reverted this in a subsequent release we can update to, perhaps should consider using a different client. https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/http-configuration.html Not that I like the options very much. Netty is also pretty big/complex and async first, httpurlconnection possibly too simple and maybe without pooling (while maybe not needed here, might be needed for the ECS elastic agent plugin). A few other plugins have ended up depending on vertx with netty though so maybe netty isn't so bad. 😬 Otherwise many are on various okio versions including ones needing to embed kotlin. It's really quite a mess 😅 |
|
The jar above looks like it is bringing in all possible http client options which is likely why it's so huge, and also doesn't make much sense :) If we do stick with HttpClient 5 (supposed to be the default?) perhaps we can avoid the jdk.net dependency by providing a |
It does seem to be there, at least on the |
Yes, that's the one I mentioned about the "pass-through packages" above. I tried adding When debugging in the GoCD server process, this works fine: But this doesn't: It's possible that the bundle's classloader's ancestor tree doesn't include the classloader which has I tried adding this to wrapper-properties.conf: I don't remember if that is what brought |
|
Oh, that's very surprising that it didn't help, unless you were doing interactively and it's some quirk of the debugger context - or it's something to do with the extension being loaded by the parent, but if the class is loaded via something inside the plugin it works differently because of the delegation? In any case, if jdk.net modules are within "standard" definitions of what is considered a JRE, we should probably change the Felix defaults. They are very stale, predating modularization and all the package moves that entailed - and with 26.1.0 im changing to Java 21 minimum so we can move things forward with limited need to sanity check older runtimes. e.g The org.w3c stuff should be long gone I thought, replaced with com.sun and not to be classloaded directly? I'll take a look at that bit. If it helps - i have no concern with bumping plugin major version and changing this plugin (and ECS plugin) to a GoCD 26.1.0 (and Java 21) minimum. IIRC this plugin and the ECS one is one of the less frequently used - I think https://gocd.github.io/gocd-plugin-releases-dashboard/ is relatively reliable an indicator as I've been maintaining it. (Results skewed for plugins used on build.gocd.org to be 30*24 more than they should be, and a bit skewed for some used in ruby-functional-tests) |
Ahh, after taking another look, is the reason this didn't work that If you try with I possibly don't fully understand quite what the goals of boot delegation are or how this all works with the GoCD Felix/OSGi plugin setup but I guess it's just to avoid managing manifests, and to ensure certain classes cannot be overridden from within the plugin itself; to give some predictable semantics. In the history, it looks like you were key to the design of this, which is cool! Seems it went through a few quick iterations in 2011/2012 to end up with using boot delegation off the Not looking to rock the boat too much anyway - only reason for digging a bit further than needed for this issue was to understand what level of (security?) isolation we are actually getting from plugins with Felix. Will come back to this in a bit, as I also need to migrate the ECS plugin so I can get rid of its need for use of access keys and use STS and assumed roles properly. |
|
Have resolved the first dependency bloat issue, but pending actual validation and deciding whether we want to stick to HttpClient 5 and thus require GoCD 26.2.0 - or use a different client. It least gets as far as the below from a quick test with bad creds: |
Bumps the github-actions group with 2 updates: [actions/checkout](https://github.com/actions/checkout) and [actions/setup-java](https://github.com/actions/setup-java). Updates `actions/checkout` from 6.0.3 to 7.0.0 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@df4cb1c...9c091bb) Updates `actions/setup-java` from 5.2.0 to 5.4.0 - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](actions/setup-java@be666c2...1bcf9fb) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions - dependency-name: actions/setup-java dependency-version: 5.4.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps the gradle-deps group with 2 updates in the / directory: [org.junit:junit-bom](https://github.com/junit-team/junit-framework) and [gradle-wrapper](https://github.com/gradle/gradle). Updates `org.junit:junit-bom` from 6.1.0 to 6.1.1 - [Release notes](https://github.com/junit-team/junit-framework/releases) - [Commits](junit-team/junit-framework@r6.1.0...r6.1.1) Updates `gradle-wrapper` from 9.5.1 to 9.6.1 - [Release notes](https://github.com/gradle/gradle/releases) - [Commits](gradle/gradle@v9.5.1...v9.6.1) --- updated-dependencies: - dependency-name: org.junit:junit-bom dependency-version: 6.1.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: gradle-deps - dependency-name: gradle-wrapper dependency-version: 9.6.1 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: gradle-deps ... Signed-off-by: dependabot[bot] <support@github.com>
- not using async interaction mode - not using post-quantum cryptography
The plugin right now depends on Apache HttpClient 5, which in turn depends on gocd/gocd#14493 which requires a newer GoCD release. We can drop this commit if we switch to an alternative client.
363d05e to
8770d9b
Compare
# Conflicts: # build.gradle
…hen discovering a "new" one
… the plugin Modelled after validated approach from the ECS Elastic Agent plugin; will try and use the IAM profile as an auto-detection source; and try to assume a role if necessary.
|
Let's go with this for now. If someone pipes up wanting to use this on older GoCD versions I'll take a look then about changing the client to work on older versions. Have also ported the IAM Instance profile and Assume Rule support over here from the ECS plugin and seems to be working OK for now. |
Upgrading to AWS SDK v2, since the older SDK v1 is deprecated and out of support. As it stands, the tests pass fine but there are two big issues with this pull request that I will mention in the comments.