seporaitis / yum-s3-iam

Yum package manager plugin for private S3 repositories. Uses Amazon IAM & EC2 Roles.
Apache License 2.0
162 stars 99 forks source link

Add check for IAM accessibility #49

Closed seren closed 7 years ago

seren commented 8 years ago

This addresses issue #48 by checking that the IAM security-credentials url is accessible before trying to use IAM credentials to access the S3 repo.

ronaldtse commented 7 years ago

Thanks @seren, will we see this merged soon?

seren commented 7 years ago

I'm not sure. Probably a question for @mbrossard

mbrossard commented 7 years ago

Hi,

I had some questions and some improvements in mind but I haven't had the time to work on them. I'm worried about regressions and having unexpected behavior. I'm thinking about people trying to use the plugin outside EC2 and not getting an error that IAM role credentials we not accessible but an access denied S3 error (because we're not providing the access credentials). I'd prefer something more explicit. For Docker (the use case you mentioned in #48), a solution might be to use environment variables (instead of an explicit "iam_check" boolean option in the configuration to trigger your check):

def prereposetup_hook(conduit):
      if 'DISABLE_YUM_S3_IAM' in os.environ and os.environ['DISABLE_YUM_S3_IAM']:
          return

Sincerely, Mathias

seren commented 7 years ago

Agreed that this fix is probably not robust enough for all use cases and doesn't provide feedback about the S3 fallback path. Environment vars could work, though it would be nice if there were a solution that was solid enough to work automatically without environment-specific configuration (part of the reason we're playing with Docker is to try and reduce the environmental differences we have to manage).