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

fix(dataplane): deserialize secret depending on content #480

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

ronjaquensel
Copy link
Contributor

What this PR changes/adds

Deserializes the secret obtained from the vault to either an AwsSecretToken or an AwsTemporarySecretToken depending on whether it contains a sessionToken field or not.

Why it does that

To allow both the usage of static and temporary credentials with the S3 data plane. Previously, the secret was always deserialized directly to an AwsTemporarySecretToken, which caused the transfer to fail if static credentials were used.

Further notes

The code for deserializing the secret has been duplicated for S3DataSourceFactory and S3DataSinkFactory for now, as also the code for creating the S3 client request is duplicated. Refactoring to remove these duplications should be done in another PR.

Linked Issue(s)

Closes #457

@ronjaquensel ronjaquensel added the bug Something isn't working label Oct 29, 2024
@ronjaquensel ronjaquensel self-assigned this Oct 29, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

We are always happy to welcome new contributors ❤️ To make things easier for everyone, please make sure to follow our contribution guidelines, check if you have already signed the ECA, and relate this pull request to an existing issue or discussion.

@ronjaquensel ronjaquensel changed the title fix(s3-data-plane): deserialize secret depending on content fix(dataplane): deserialize secret depending on content Oct 29, 2024
@ronjaquensel ronjaquensel force-pushed the fix/457-secret-parsing branch from 49319df to 60d22aa Compare October 29, 2024 15:12
Copy link
Member

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

just a proposal about the deserialization process

@ronjaquensel ronjaquensel requested a review from ndr-brt October 30, 2024 15:17
@ndr-brt ndr-brt merged commit 9fa6886 into eclipse-edc:main Oct 31, 2024
8 checks passed
@ronjaquensel ronjaquensel deleted the fix/457-secret-parsing branch October 31, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

S3 data plane fails on non-temporary credentials in the vault
2 participants