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

Fixes restore script AWS configuration #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rjz
Copy link

@rjz rjz commented Mar 6, 2017

This change removes environmental inference from the restore script, restoring the usual credential precedence established in aws-sdk. The default region (Sydney) is preserved for back-compatibility.

Thanks for your consideration!

@rjz rjz changed the title Fixes AWS configuration on restore Fixes restore script AWS configuration Mar 6, 2017
@dylanlingelbach dylanlingelbach self-assigned this Mar 6, 2017
@rjz rjz force-pushed the fix/aws-configuration branch from 97b7abe to 5beb92b Compare March 6, 2017 20:47
This change removes environmental inference from the restore script,
restoring the usual credential precedence established in `aws-sdk`.

The default region (Sydney) is preserved for back-compatibility.
@rjz rjz force-pushed the fix/aws-configuration branch from 5beb92b to 5cbd4c1 Compare March 6, 2017 20:59
@rjz
Copy link
Author

rjz commented Apr 11, 2017

@dylanlingelbach, any thoughts on this? Happy to address any suggestions (or concerns) stemming from the change.

@dylanlingelbach
Copy link
Collaborator

@rjz sorry for the delay - this looks good to me.

@dylanlingelbach dylanlingelbach removed their assignment Apr 12, 2017
@dylanlingelbach
Copy link
Collaborator

Although I seem to have lost my write access to this repo. I'll figure out what is going on and get this merged.

@rjz
Copy link
Author

rjz commented Apr 12, 2017

Thanks, @dylanlingelbach! Much appreciated.

@johncmckim
Copy link

Bump on this PR. Restore won't work on Lambda without this fix.

@sdesalas
Copy link
Contributor

LGTM

@rjz
Copy link
Author

rjz commented May 11, 2017

@dylanlingelbach, any word on this? Or any leads on who in markitx we might reach out to?

@dylanlingelbach
Copy link
Collaborator

@rjz sorry, no updates yet. I've been slammed at work the past few weeks. Will follow up and let you know. Worst case I will fork to my personal repo and move the npm package there

@rafaelmagu
Copy link

@dylanlingelbach any updates on this or other PRs being merged?

@dylanlingelbach
Copy link
Collaborator

@rafaelmagu still working on regaining write access. Sorry for the delay

@cg-cnu
Copy link

cg-cnu commented Sep 1, 2017

Hey guys,
Am set out to work on this issue #52
I saw this pr and wondering why the support for env variables is removed
@rjz mentioned that it is to

restoring the usual credential precedence established in aws-sdk.

Can some one guide me to what is the the credential precedence for node sdk
I searched the documentation, but could only find the one for java (look for Using the Default Credential Provider Chain)
Thanks 🙂

@dylanlingelbach
Copy link
Collaborator

@cg-cnu I think this link is what you are looking for

@cg-cnu
Copy link

cg-cnu commented Sep 1, 2017

@dylanlingelbach I guess its mostly talking about the ways in which you can load the credentials but not the order in which node SDK will work / prefer to work.
Is providing secrets using the environment variables considered insecure ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants