[S3 Sync Backend] Support for Shared Credential File

related: S3 bucket as backend for sync

In order to make use of the S3 sync target with non-permanent credentials I need support for shared credential files stored on disk. I went ahead and wrote a patch that implements the feature. I'd appreciate if somebody took a look!

(edit: let's actually post the link)
Commit: SYNC: S3: Add SharedIniFileCredentials support · ocelotsloth/joplin@c36dca1 · GitHub
Branch: GitHub - ocelotsloth/joplin at s3-shared-credential-file

I can't tell if I'm supposed to hold off on actually filing a PR on GitHub or not. Let me know if I should do that now.

@alexc I think you wrote the original patch (the commit history is broken post-lerna)--can you give this a spin and confirm whether this works for you or not?

The patch adds two new configuration options:

  • AWS Shared Credential File Path
  • AWS Shared Credential File Profile

Some context on shared credential ini files: Loading credentials in Node.js from the shared credentials file - AWS SDK for JavaScript

If a user specifies both a credential file and an explicit access and secret key then this patch will use the explicit keys and ignore the configured file.

There are a couple test failures that appear--but I don't believe they have anything to do with this patch since they also fail on the current dev branch head:

@joplin/lib: A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks.
@joplin/lib: Summary of all failing tests
@joplin/lib: FAIL services/searchengine/SearchFilter.test.js (50.157 s)
@joplin/lib:   ● services_SearchFilter › should support filtering by created date
@joplin/lib:     expect(received).toBe(expected) // Object.is equality
@joplin/lib:     Expected: 1
@joplin/lib:     Received: 0
@joplin/lib:       353 | 
@joplin/lib:       354 | 		rows = await engine.search('created:20200520');
@joplin/lib:     > 355 | 		expect(rows.length).toBe(1);
@joplin/lib:           | 		                    ^
@joplin/lib:       356 | 		expect(ids(rows)).toContain(n1.id);
@joplin/lib:       357 | 
@joplin/lib:       358 | 		rows = await engine.search('created:20200519');
@joplin/lib:       at Object.<anonymous> (services/searchengine/SearchFilter.test.js:355:23)
@joplin/lib:   ● services_SearchFilter › should support filtering by between two dates
@joplin/lib:     expect(received).toBe(expected) // Object.is equality
@joplin/lib:     Expected: 2
@joplin/lib:     Received: 1
@joplin/lib:       377 | 
@joplin/lib:       378 | 		rows = await engine.search('created:20200101 -created:20200220');
@joplin/lib:     > 379 | 		expect(rows.length).toBe(2);
@joplin/lib:           | 		                    ^
@joplin/lib:       380 | 		expect(ids(rows)).toContain(n1.id);
@joplin/lib:       381 | 		expect(ids(rows)).toContain(n2.id);
@joplin/lib:       382 | 
@joplin/lib:       at Object.<anonymous> (services/searchengine/SearchFilter.test.js:379:23)
@joplin/lib:   ● services_SearchFilter › should support filtering by updated date
@joplin/lib:     expect(received).toBe(expected) // Object.is equality
@joplin/lib:     Expected: 1
@joplin/lib:     Received: 0
@joplin/lib:       495 | 
@joplin/lib:       496 | 		rows = await engine.search('updated:20200520');
@joplin/lib:     > 497 | 		expect(rows.length).toBe(1);
@joplin/lib:           | 		                    ^
@joplin/lib:       498 | 		expect(ids(rows)).toContain(n1.id);
@joplin/lib:       499 | 
@joplin/lib:       500 | 		rows = await engine.search('updated:20200519');
@joplin/lib:       at Object.<anonymous> (services/searchengine/SearchFilter.test.js:497:23)
@joplin/lib:           at runMicrotasks (<anonymous>)
@joplin/lib: Test Suites: 1 failed, 56 passed, 57 total
@joplin/lib: Tests:       3 failed, 457 passed, 460 total
@joplin/lib: Snapshots:   0 total
@joplin/lib: Time:        59.299 s
@joplin/lib: Ran all test suites.

@laurent I don't want to jump ahead of the contributing guidelines which recommend posting here first.

Could you take a look at this and let me know if it's something you'd be willing to move forward with? If so I'll post a pull request where any needed changes to the PR can be worked though.

I'm not really involved with that part of the code actually. It was implemented by someone else so you might want to check previous pull requests and get in touch with him to see if he's interested in reviewing.

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.