feat: enable reusable credentials for S3 integration #248

yurimssilva · 2024-03-01T11:19:55Z

What this PR changes/adds

Introduces the objectName property to store the S3 object name
Refactors keyName to exclusively handle secret aliases
Renames keyPrefix to objectPrefix
Destination object(s) naming decision based on Part list size.
Updates documentation
Updates tests

Why it does that

To align with the existing behavior of TransferProcessManagerImpl, which defaults to using keyName for fetching secret values, rather than introducing a new property configuration specifically for holding the secret alias, a more effective approach would be to introduce an objectName property to store the S3 object name. This modification aims to enhance consistency and also alignment between the AWS and Azure platforms.

Linked Issue(s)

Closes #211

codecov-commenter · 2024-03-01T11:36:29Z

Codecov Report

Attention: Patch coverage is 80.72289% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 65.12%. Comparing base (d177a98) to head (3010144).
Report is 24 commits behind head on main.

Files	Patch %	Lines
...e/edc/connector/dataplane/aws/s3/S3DataSource.java	63.63%	6 Missing and 2 partials ⚠️
...ws/s3/validation/S3SourceDataAddressValidator.java	86.66%	0 Missing and 2 partials ⚠️
...pse/edc/connector/dataplane/aws/s3/S3DataSink.java	66.66%	1 Missing and 1 partial ⚠️
.../connector/dataplane/aws/s3/S3DataSinkFactory.java	88.88%	0 Missing and 2 partials ⚠️
...onnector/dataplane/aws/s3/S3DataSourceFactory.java	87.50%	0 Missing and 2 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
+ Coverage   63.82%   65.12%   +1.30%     
==========================================
  Files          26       28       +2     
  Lines         633      671      +38     
  Branches       30       32       +2     
==========================================
+ Hits          404      437      +33     
+ Misses        222      220       -2     
- Partials        7       14       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Introduces the "objectName" property to store the S3 object name - Refactors "keyName" to exclusively handle secret aliases - Renames "keyPrefix" to "objectPrefix" - Utilizes Part list size for naming of destination objects

github-actions · 2024-03-12T01:47:55Z

This pull request is stale because it has been open for 7 days with no activity.

…into feat/reusable_credentials_new_proposal # Conflicts: # DEPENDENCIES

gerbigf · 2024-03-19T13:17:41Z

extensions/data-plane/data-plane-aws-s3/README.md

@yurimssilva generally lgtm. I would be a bit more specific on the examples, providing an example for the use of keys via the vault and the use of the not-recommended clear-text credentials. Probably both examples right below the respective method of using the credentials.

Thanks, @gerbigf . New examples have been added to the documentation.

…the not-recommended clear-text credentials

...va/org/eclipse/edc/connector/provision/aws/s3/S3ConsumerResourceDefinitionGeneratorTest.java

extensions/data-plane/data-plane-aws-s3/README.md

...ane-aws-s3/src/main/java/org/eclipse/edc/connector/dataplane/aws/s3/S3DataSourceFactory.java

extensions/common/aws/aws-s3-core/src/main/java/org/eclipse/edc/aws/s3/S3BucketSchema.java

- variable renaming - doc adjustments - property deprecation - validation for new attributes

…into feat/reusable_credentials_new_proposal # Conflicts: # extensions/control-plane/provision/provision-aws-s3/src/test/java/org/eclipse/edc/connector/provision/aws/s3/S3ConsumerResourceDefinitionGeneratorTest.java

ndr-brt · 2024-03-26T09:45:44Z

...ws-s3-core/src/main/java/org/eclipse/edc/aws/s3/validation/S3SourceDataAddressValidator.java

+            }
+        });
+
+        if (Stream.of(OBJECT_NAME, KEY_NAME, OBJECT_PREFIX, KEY_PREFIX)


given that KEY_NAME and KEY_PREFIX have been deprecated, I would avoid checking them, given that this validator is triggered on asset create/update

The idea was to include them to be compatible with assets already stored in previous versions, as the validation is also run at DataSource creation, and then remove them after the deprecation period.

...e/data-plane-aws-s3/src/main/java/org/eclipse/edc/connector/dataplane/aws/s3/S3DataSink.java

...plane-aws-s3/src/main/java/org/eclipse/edc/connector/dataplane/aws/s3/S3DataSinkFactory.java

...e/data-plane-aws-s3/src/main/java/org/eclipse/edc/connector/dataplane/aws/s3/S3DataSink.java

- tests adjustments - unnecessary code removed - source creation methods simplified

yurimssilva self-assigned this Mar 1, 2024

yurimssilva force-pushed the feat/reusable_credentials_new_proposal branch 2 times, most recently from 2a16e90 to 08140ca Compare March 1, 2024 11:32

yurimssilva added the enhancement New feature or request label Mar 1, 2024

yurimssilva force-pushed the feat/reusable_credentials_new_proposal branch from 08140ca to 4b0a7d1 Compare March 1, 2024 11:36

yurimssilva force-pushed the feat/reusable_credentials_new_proposal branch 3 times, most recently from a53dec0 to 03612d4 Compare March 4, 2024 15:57

yurimssilva force-pushed the feat/reusable_credentials_new_proposal branch from 03612d4 to bbb2b1c Compare March 4, 2024 16:00

github-actions bot added the stale label Mar 12, 2024

yurimssilva added 2 commits March 18, 2024 09:59

docs: adjust README.md

539d976

Merge branch 'main' of https://github.com/yurimssilva/Technology-Aws …

7910ff0

…into feat/reusable_credentials_new_proposal # Conflicts: # DEPENDENCIES

yurimssilva force-pushed the feat/reusable_credentials_new_proposal branch from 61d8d5b to 7910ff0 Compare March 18, 2024 10:00

github-actions bot removed the stale label Mar 19, 2024

gerbigf reviewed Mar 19, 2024

View reviewed changes

yurimssilva added 4 commits March 20, 2024 20:55

test: adds new cases to test objects naming logic into destination

44c08ab

docs: adds examples for the use of keys via the vault and the use of …

70914ef

…the not-recommended clear-text credentials

fix: DEPENDENCIES and checkstyle

51cc3e5

refactor: adjust TestFunctions

eea207d

yurimssilva requested review from gerbigf, wolf4ood and ndr-brt and removed request for gerbigf March 20, 2024 21:50

yurimssilva marked this pull request as ready for review March 20, 2024 21:51

fix: adapt to EDC upstream

ce744ab

ndr-brt requested changes Mar 21, 2024

View reviewed changes

yurimssilva requested a review from ndr-brt March 21, 2024 16:34

yurimssilva force-pushed the feat/reusable_credentials_new_proposal branch 2 times, most recently from 5ad02aa to 8619651 Compare March 22, 2024 01:03

yurimssilva added 2 commits March 22, 2024 02:11

refactor:

629408a

- variable renaming - doc adjustments - property deprecation - validation for new attributes

yurimssilva force-pushed the feat/reusable_credentials_new_proposal branch from 8619651 to 7de30dd Compare March 22, 2024 02:12

ndr-brt requested changes Mar 26, 2024

View reviewed changes

refactor:

3010144

- tests adjustments - unnecessary code removed - source creation methods simplified

yurimssilva requested a review from ndr-brt March 26, 2024 18:04

ndr-brt approved these changes Mar 27, 2024

View reviewed changes

ndr-brt merged commit 86bdec8 into eclipse-edc:main Mar 27, 2024
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable reusable credentials for S3 integration #248

feat: enable reusable credentials for S3 integration #248

yurimssilva commented Mar 1, 2024 •

edited

Loading

codecov-commenter commented Mar 1, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024

gerbigf Mar 19, 2024

yurimssilva Mar 20, 2024

ndr-brt Mar 26, 2024

yurimssilva Mar 26, 2024 •

edited

Loading

feat: enable reusable credentials for S3 integration #248

feat: enable reusable credentials for S3 integration #248

Conversation

yurimssilva commented Mar 1, 2024 • edited Loading

What this PR changes/adds

Why it does that

Linked Issue(s)

codecov-commenter commented Mar 1, 2024 • edited Loading

Codecov Report

github-actions bot commented Mar 12, 2024

gerbigf Mar 19, 2024

Choose a reason for hiding this comment

yurimssilva Mar 20, 2024

Choose a reason for hiding this comment

ndr-brt Mar 26, 2024

Choose a reason for hiding this comment

yurimssilva Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

yurimssilva commented Mar 1, 2024 •

edited

Loading

codecov-commenter commented Mar 1, 2024 •

edited

Loading

yurimssilva Mar 26, 2024 •

edited

Loading