Skip to content
This repository was archived by the owner on Jun 23, 2020. It is now read-only.

Fixes #12 don't require region w/ instance principals - #135

Open
jhorwit2 wants to merge 2 commits into
masterfrom
jah/fixes-12
Open

Fixes #12 don't require region w/ instance principals#135
jhorwit2 wants to merge 2 commits into
masterfrom
jah/fixes-12

Conversation

@jhorwit2

@jhorwit2 jhorwit2 commented Jul 9, 2018

Copy link
Copy Markdown
Member

No description provided.

@jhorwit2
jhorwit2 requested a review from harveylowndes July 9, 2018 17:20
@jhorwit2

jhorwit2 commented Jul 9, 2018

Copy link
Copy Markdown
Member Author

@harveylowndes the default logic sets the region information aka the region_key. IMO, the validation logic is a little too strict but i don't want to fix that here. There's no reason to fail hard if non-instance principal configuration is present; warnings suffice IMO.

@harveylowndes

harveylowndes commented Jul 10, 2018

Copy link
Copy Markdown
Contributor

@jhorwit2 I did open a PR which contained a commit to skip setting defaults if instance principals were used e7e5ad4. I wonder if it's worth skipping validation completely if instance principals are used and not allowing the user to override the auth config credentials?

I agree however that warning over hard fail should be preferred.

@owainlewis

Copy link
Copy Markdown
Contributor

Needs rebase. Since we're throwing away the auth credentials when instance principals are used, validating them doesn't serve any purpose so we could just skip.

@owainlewis owainlewis self-assigned this Sep 26, 2018
@owainlewis
owainlewis removed the request for review from harveylowndes September 26, 2018 11:21
@owainlewis

Copy link
Copy Markdown
Contributor

Needs rebase or closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants