dnsimple / terraform-provider-dnsimple

Terraform DNSimple provider.
https://www.terraform.io/docs/providers/dnsimple/
Mozilla Public License 2.0
22 stars 20 forks source link

Ignore DNSIMPLE_EMAIL if DNSIMPLE_ACCOUNT and DNSIMPLE_TOKEN are passed #10

Closed Nowaker closed 6 years ago

Nowaker commented 6 years ago

Although DNSIMPLE_ACCOUNT and DNSIMPLE_TOKEN contain v2 credentials, Terraform still thinks I'm providing it with v1 credentials merely by the fact I also supplied DNSIMPLE_EMAIL.

Error: Error running plan: 1 error(s) occurred:

* provider.dnsimple: DNSimple API v2 requires an account identifier and the new OAuth token. Please upgrade your configuration.

The code that validates environment variables looks like this pseudocode:

token = ENV['DNSIMPLE_TOKEN']
account = ENV['DNSIMPLE_ACCOUNT']

if ENV['DNSIMPLE_EMAIL']
  raise 'DNSimple API v2 requires an account identifier and the new OAuth token. Please upgrade your configuration.' 
end

This is wrong - it should only complain about v1 credential if no v2 credentials are provided:

token = ENV['DNSIMPLE_TOKEN']
account = ENV['DNSIMPLE_ACCOUNT']

if not token or not account
  if ENV['DNSIMPLE_EMAIL']
    raise 'DNSimple API v2 requires an account identifier and the new OAuth token. Please upgrade your configuration.' 
  end
end

Technically, this error message DNSimple API v2 requires an account identifier and the new OAuth token. Please upgrade your configuration. is wrong because I did pass API v2 credentials. A fact that I also passed v1 credentials via DNSIMPLE_EMAIL variable should not bother Terraform. I provided the two required variables as per the reference, right? Any other variables should be ignored because it's none of provider's business what I have in my environment, is it DNSIMPLE_EMAIL or HELLO_WORLD. :smile_cat:

Terraform Version

Terraform v0.11.7
+ provider.dnsimple v0.1.0
weppos commented 6 years ago

@Nowaker because there is an overlap in the var name between API v1 and v2, it's hard to guess which values you passed. In the best interest of avoiding auth issues, we assume that if you have any v1 auth info you need to update the setup.

After all, there is no reason you should keep the DNSIMPLE_EMAIL var around.

I am closing as #wontfix. If you think this is wrong, please provide a valid reason why you should be able to have DNSIMPLE_EMAIL. Eventually, the whole v1 vs v2 code will be removed and only v2 credentials will be checked.

Nowaker commented 6 years ago

After all, there is no reason you should keep the DNSIMPLE_EMAIL var around.

There is no reason why Dnsimple Provider would look up DNSIMPLE_EMAIL environment variable if DNSIMPLE_TOKEN and DNSIMPLE_ACCOUNT are passed.

This is a very valid reason.