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

Add check for IAM accessibility #49

Closed
wants to merge 1 commit into from
Closed

Conversation

seren
Copy link

@seren seren commented Sep 13, 2016

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
Copy link

Thanks @seren, will we see this merged soon?

@seren
Copy link
Author

seren commented May 2, 2017

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

@mbrossard
Copy link
Collaborator

mbrossard commented May 3, 2017

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
Copy link
Author

seren commented May 3, 2017

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).

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.

3 participants