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.

Sign up for GitHub

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

Jump to bottom

Fix AWS JSON protocol errors by harmonising relevant AWS libs #52

Merged
merged 1 commit into from
Jan 8, 2025
Merged

Fix AWS JSON protocol errors by harmonising relevant AWS libs #52

merged 1 commit into from
Jan 8, 2025

Conversation

rtyley
Copy link
Member

@rtyley rtyley commented Dec 5, 2024 •
edited
Loading

@rtyley rtyley force-pushed the reproduce-json-protocol-issue branch from 997f831 to 2400522 Compare December 5, 2024 11:24
@rtyley rtyley force-pushed the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch from 5bde237 to 197ffc2 Compare December 5, 2024 11:26
@rtyley rtyley mentioned this pull request Dec 5, 2024
Merged
rtyley
rtyley commented Dec 5, 2024
View reviewed changes
build.sbt
"org.json" % "json" % "20231013",
"org.xerial.snappy" % "snappy-java" % "1.1.10.4",
"org.apache.commons" % "commons-compress" % "1.26.0",
"com.amazon.ion" % "ion-java" % "1.10.5",//overriding until a version of amazon-kinesis-client is available that removes the ion-java vulnerability
Copy link
Member Author

Choose a reason for hiding this comment

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

ion-java had a dependency override added with #42 in March 2024. ion-java was a dependency of AWS SDK v1 (com.amazonaws:aws-java-sdk-core:1.12.151). However, release 3.0.0 of the Amazon Kinesis Client Library for Java removed the last transitory dependency of the KCL on AWS SDK v1, and so content-api-firehose-client no longer depends upon it.

rtyley
rtyley commented Dec 5, 2024
View reviewed changes
build.sbt

val jacksonVersion = "2.17.2"
dependencyOverrides ++= Seq(
"com.charleskorn.kaml" % "kaml" % "0.53.0",
"com.fasterxml.jackson.core" % "jackson-databind" % jacksonVersion,
"com.fasterxml.jackson.core" % "jackson-annotations" % jacksonVersion,
"com.fasterxml.jackson.core" % "jackson-core" % jacksonVersion,
"software.amazon.awssdk" % "netty-nio-client" % "2.26.25",
Copy link
Member Author

@rtyley rtyley Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

"software.amazon.awssdk" % "netty-nio-client" had a dependency override added first with #26, then to version 2.26.25 with #49 in October 2024.

However, as in this PR we're updating all relevant software.amazon.awssdk libraries to 2.29.23, past 2.26.25, the manual upgrade is no longer necessary.

@rtyley rtyley force-pushed the reproduce-json-protocol-issue branch 11 times, most recently from 585392d to d595ef9 Compare December 6, 2024 17:00
@rtyley rtyley force-pushed the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch from 197ffc2 to d32f084 Compare December 6, 2024 17:33
@rtyley rtyley changed the base branch from reproduce-json-protocol-issue to main December 11, 2024 11:06
@rtyley rtyley force-pushed the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch from d32f084 to 5169c82 Compare December 11, 2024 14:35
@gu-scala-library-release
Copy link
Contributor

@rtyley has published a preview version of this PR with release workflow run #27, based on commit 5169c82:

1.0.26-PREVIEW.upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts.2024-12-11T1530.5169c828

Want to make another preview release?

Click 'Run workflow' in the GitHub UI, specifying the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch, or use the GitHub CLI command:

gh workflow run release.yml --ref upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts

Want to make a full release after this PR is merged?

Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command:

gh workflow run release.yml

@rtyley rtyley force-pushed the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch from 5169c82 to b870d36 Compare January 8, 2025 11:57
@gu-scala-library-release
Copy link
Contributor

@rtyley has published a preview version of this PR with release workflow run #28, based on commit b870d36:

1.0.26-PREVIEW.upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts.2025-01-08T1159.b870d366

Want to make another preview release?

Click 'Run workflow' in the GitHub UI, specifying the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch, or use the GitHub CLI command:

gh workflow run release.yml --ref upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts

Want to make a full release after this PR is merged?

Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command:

gh workflow run release.yml

@rtyley
Fix AWS JSON protocol errors by harmonising relevant AWS libs
e0188d3 
Additional changes:

* `ion-java` had a dependency override added with #42
  in March 2024. `ion-java` was a dependency of AWS SDK v1 (`com.amazonaws:aws-java-sdk-core:1.12.151`).
  However, release 3.0.0 of the Amazon Kinesis Client Library for Java (https://github.com/awslabs/amazon-kinesis-client/releases/tag/v3.0.0)
  removed the last transitory dependency of the KCL on AWS SDK v1, and
  `content-api-firehose-client` no longer depends upon it.
* "software.amazon.awssdk" % "netty-nio-client" had a dependency override, first with #26
  and then to version 2.26.25 with #49 in October 2024. However, here we're updating all
  relevant `software.amazon.awssdk` libraries to 2.29.23,
  meaning that the manual upgrade to 2.26.25 is no longer necessary.
@rtyley rtyley force-pushed the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch from b870d36 to e0188d3 Compare January 8, 2025 12:16
rtyley
rtyley commented Jan 8, 2025
View reviewed changes
build.sbt
"com.gu" %% "content-api-models-scala" % "25.0.0",
"com.gu" %% "content-api-models-scala" % "26.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

I found in apple-news that this would need to be upgraded to fit in with the upgrade of the FAPI client.

[info] Resolved apple-news_2.13 dependencies
[info] Fetching artifacts of apple-news_2.13
[info] Fetched artifacts of apple-news_2.13
[error] stack trace is suppressed; run last update for the full output
[error] (update) found version conflict(s) in library dependencies; some are suspected to be binary incompatible:
[error]
[error] 	* com.gu:content-api-models-scala_2.13:26.0.0 (early-semver) is selected over 25.0.0
[error] 	    +- com.gu:content-api-client_2.13:33.0.0              (depends on 26.0.0)
[error] 	    +- com.gu:content-api-client-default_2.13:33.0.0      (depends on 26.0.0)
[error] 	    +- com.gu:content-api-firehose-client_2.13:1.0.26-PREVIEW.upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts.2025-01-08T1159.b870d366 (depends on 25.0.0)

@gu-scala-library-release
Copy link
Contributor

@rtyley has published a preview version of this PR with release workflow run #29, based on commit e0188d3:

1.0.26-PREVIEW.upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts.2025-01-08T1221.e0188d31

Want to make another preview release?

Click 'Run workflow' in the GitHub UI, specifying the upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts branch, or use the GitHub CLI command:

gh workflow run release.yml --ref upgrade-aws-sdk-version-to-avoid-json-protocol-conflicts

Want to make a full release after this PR is merged?

Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command:

gh workflow run release.yml

@rtyley rtyley marked this pull request as ready for review January 8, 2025 17:52
@rtyley rtyley requested a review from a team as a code owner January 8, 2025 17:52
fredex42
fredex42 approved these changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@fredex42 fredex42 left a comment

Choose a reason for hiding this comment

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

👍

@rtyley rtyley merged commit 90fe1fc into main Jan 8, 2025
10 checks passed
rtyley
rtyley commented Jan 16, 2025
View reviewed changes
build.sbt
"software.amazon.kinesis" % "amazon-kinesis-client" % "2.6.0",
"software.amazon.kinesis" % "amazon-kinesis-client" % "3.0.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

I did not read this document about upgrading from KCL v2 to v3:

https://docs.aws.amazon.com/streams/latest/dev/kcl-migration-from-2-3.html

...this is unfortunate as it probably means that the release that included this PR (v1.0.26) is probably missing at least one crucial setting, and so should be avoided.

At the very least, the increased IAM permissions required by KCL v3 means that https://github.com/guardian/apple-news/pull/398 failed:

@rtyley rtyley mentioned this pull request Jan 17, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fredex42 fredex42 fredex42 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rtyley @fredex42