Skip to content

fix: support for AWS LegacyMd5Plugin #36408

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
Merged
merged 2 commits into from
Aug 5, 2025
Merged

fix: support for AWS LegacyMd5Plugin #36408

merged 2 commits into from
Aug 5, 2025

Conversation

panagiotisgts
Copy link
Contributor

@panagiotisgts panagiotisgts commented Aug 5, 2025 •
edited
Loading

Description

Conditionally enable the AWS provided LegacyMd5Plugin to extend compatibility support for our backups with S3-compatible object storages. Ref. https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/s3-checksums.html

Checklist

Related issues

closes #35953

@panagiotisgts panagiotisgts requested a review from a team as a code owner August 5, 2025 06:04
@panagiotisgts panagiotisgts requested review from lenaschoenburg and removed request for a team August 5, 2025 06:04
@github-actions github-actions bot added the component/zeebe Related to the Zeebe component/team label Aug 5, 2025
@panagiotisgts panagiotisgts force-pushed the pg/35953-s3-legacy-md5 branch from 125c2e7 to 0bc6035 Compare August 5, 2025 06:20
@panagiotisgts panagiotisgts added backport stable/8.6 Backport a pull request to stable/8.6 backport stable/8.7 Backport a pull request to stable/8.7 labels Aug 5, 2025
lenaschoenburg
lenaschoenburg requested changes Aug 5, 2025
View reviewed changes
Copy link
Member

@lenaschoenburg lenaschoenburg left a comment

Choose a reason for hiding this comment

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

❌ The new config flag is not set when the broker starts. Compare how S3BackupConfig is built from broker config.

🔧 The new tests are good but I'd prefer a small amount of code duplication instead of having two classes (for legacy and latest) inherit from the parent. The test class structure is already a bit confusing, throwing in a new abstract class in the middle complicates this further.

@panagiotisgts
Copy link
Contributor Author

@lenaschoenburg Isn't this how it's being parsed? I've run the StandaloneCamunda with this flag enabled (ZEEBE_BROKER_DATA_BACKUP_S3_SUPPORTLEGACYMD5=true) and I could see it being registered at runtime, visible also on the /actuator/configprops.

The tests themselves are 99.9% the same that's why I've extracted them as a common base. I can have a second look on that.

@lenaschoenburg
Copy link
Member

@panagiotisgts No you are right, I somehow missed that part during the review 👍

lenaschoenburg
lenaschoenburg approved these changes Aug 5, 2025
View reviewed changes
Copy link
Member

@lenaschoenburg lenaschoenburg left a comment

Choose a reason for hiding this comment

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

Change itself looks good, please still consider adjusting the test as suggested.

@panagiotisgts panagiotisgts force-pushed the pg/35953-s3-legacy-md5 branch from 0bc6035 to 2e3e8db Compare August 5, 2025 12:26
@panagiotisgts panagiotisgts changed the title pg/35953-s3-legacy-md5 fix: support for AWS LegacyMd5Plugin Aug 5, 2025
@panagiotisgts panagiotisgts added this pull request to the merge queue Aug 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 5, 2025
@panagiotisgts panagiotisgts added this pull request to the merge queue Aug 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 5, 2025
@panagiotisgts panagiotisgts added this pull request to the merge queue Aug 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 5, 2025
@panagiotisgts panagiotisgts added this pull request to the merge queue Aug 5, 2025
Merged via the queue into main with commit 30c8b7a Aug 5, 2025
164 of 166 checks passed
@panagiotisgts panagiotisgts deleted the pg/35953-s3-legacy-md5 branch August 5, 2025 17:58
@backport-action backport-action mentioned this pull request Aug 5, 2025
Merged
@backport-action
Copy link
Collaborator

Successfully created backport PR for stable/8.6:

@backport-action
Copy link
Collaborator

Successfully created backport PR for stable/8.7:

@backport-action backport-action mentioned this pull request Aug 5, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Aug 5, 2025
@camundait
[Backport stable/8.6] fix: support for AWS LegacyMd5Plugin (#36453)
5decd52 
# Description
Backport of #36408 to `stable/8.6`.

relates to #35953
github-merge-queue bot pushed a commit that referenced this pull request Aug 5, 2025
@camundait
[Backport stable/8.7] fix: support for AWS LegacyMd5Plugin (#36454)
b0d7d0d 
# Description
Backport of #36408 to `stable/8.7`.

relates to #35953
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lenaschoenburg lenaschoenburg lenaschoenburg approved these changes

Assignees
No one assigned
Labels
backport stable/8.6 Backport a pull request to stable/8.6 backport stable/8.7 Backport a pull request to stable/8.7 component/zeebe Related to the Zeebe component/team version:8.6.25 version:8.7.11
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dell ECS Backups Delete investigation
3 participants
@panagiotisgts @lenaschoenburg @backport-action