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

Make S3 Key/Secret optional to allow support for "IAM role based access to s3 from EC2" #336

Closed
BhautikDoshi opened this issue Dec 23, 2016 · 3 comments

Comments

@BhautikDoshi
Copy link
Contributor

IAM role based access to s3 from EC2 (instead of storing s3 key/secret)
http://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2.html

BTW, thanks for oc - trying it out for a big UI project mashing new/old UI coming from different projects/groups with version matrix. OC seems to be a great fit.

So to fix the s3 key/secret validation error during registry startup, I just had to remove the validation code by changing line 66 of oc/src/registry/domain/validators/registry-configuration.js
from:

if(!conf.s3 || !conf.s3.bucket || !conf.s3.key || !conf.s3.region || !conf.s3.secret){

to:

if(!conf.s3 || !conf.s3.bucket || !conf.s3.region ){

After the change, the oc-registry on EC2 comes up fine with just bucket name, region.
(The Javascript AWS SDK will pull the Key/Secret from EC2 instance using http://169.254.169.254/latest/metadata/iam/security-credentials/role-name - these key/secret are automatically rotated and SDK will refresh it - more secure than having to handle s3 credentials ourselves)

s3: {
bucket: '<BUCKET_NAME>',
region: 'us-east-1',
componentsDir: 'components'
},

Still new to the OC code and not very familiar yet. But if this above code change seems fine, let me know if you like me to post a PR. Thanks.

@matteofigus
Copy link
Member

Hi @BhautikDoshi this is great, I actually wasn't aware of this.

If you could make a PR to make those properties optional as you recommend it would be awesome!

Also, thanks for your nice words about our work. Very glad to hear it is useful to you 👍

@BhautikDoshi
Copy link
Contributor Author

Hi Matteo, I have pushed the PR. Ran tests successfully.
Look forward to contributing more as and when need arises to enhance OC for our use case which could benefit others.

matteofigus pushed a commit that referenced this issue Dec 29, 2016
@matteofigus
Copy link
Member

Fixed with #337

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

No branches or pull requests

2 participants