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

providers/digitalocean: force fqdn in dns rr value #863

Closed
wants to merge 2 commits into from

Conversation

antonta0
Copy link
Contributor

Digital Ocean requires FQDN value (ending with dot) when creating some records (CNAME, MX, NS, etc.), while GET requests return a value without trailing dot. This forces the resource to be recreated:

paystee@ws1 tmp/do > cat no-dot.json
{
    "type": "CNAME",
    "name": "test1",
    "data": "value1.example.com",
    "priority": null,
    "port": null,
    "weight": null
}
paystee@ws1 tmp/do > curl -X POST -H 'Content-Type: application/json' -H 'Authorization: Bearer secret' -d @no-dot.json 'https://api.digitalocean.com/v2/domains/example.com/records'
{"id":"unprocessable_entity","message":"Data needs to end with a dot (.)"}
paystee@ws1 tmp/do > cat with-dot.json
{
    "type": "CNAME",
    "name": "test2",
    "data": "value2.example.com.",
    "priority": null,
    "port": null,
    "weight": null
}
paystee@ws1 tmp/do > curl -X POST -H 'Content-Type: application/json' -H 'Authorization: Bearer secret' -d @with-dot.json 'https://api.digitalocean.com/v2/domains/example.com/records'
{"domain_record":{"id":3986126,"type":"CNAME","name":"test2","data":"value2.example.com.","priority":null,"port":null,"weight":null}}
paystee@ws1 tmp/do > curl -X GET -H 'Content-Type: application/json' -H 'Authorization: Bearer secret' 'https://api.digitalocean.com/v2/domains/example.com/records/3986126'
{"domain_record":{"id":3986126,"type":"CNAME","name":"test2","data":"value2.example.com","priority":null,"port":null,"weight":null}}

This PR fixes the issue. The user must specify absolute value for hostname-targeted records without trailing dot to make it work.

The one downside I see is that it will affect records that were created using relative values (e.g. a instead of a.example.com).

Fixes a bug that forces DNS record to be recreated when dealing with
records that have domain values (CNAME, MX, NS, etc.)
@mitchellh
Copy link
Contributor

This LGTM, but let's talk through backwards compatibility. Did this work before? What happens to old resources using this?

@mitchellh mitchellh added the waiting-response An issue/pull request is waiting for a response from the community label Feb 17, 2015
@antonta0
Copy link
Contributor Author

It breaks some ways of using relative domains. Actually, DigitalOcean API has pretty weird behavior.

If you create a record with value = a, you will still create a relative record to the current domain. So, this will result in a.example.com..
But for relative values with more than one label, the value will be considered absolute. Thus, value = a.v will result in a.v. (it would be a.v.example.com. before the change).

The weird part. A record with value = a.com will create a.com., but would result in "Data needs to end with a dot (.)" error before the change.

I think it's better to suggest users to use only absolute values with hostname-targeted records.

@pearkes
Copy link
Contributor

pearkes commented Feb 26, 2015

@paystee Alternatively, can we just tack on a dot if it's not there before setting from the GET? That would allow us to not alter the behavior and still fix the bug.

@antonta0
Copy link
Contributor Author

@pearkes Yes, we can. But, if I understand this correctly, we still alter the behavior of these resources. This will force new resources for all relative records and absolute records without trailing dot. GET response has the same value in data as we specified in POST request.

Actually, if we append a dot to a GET response only if we have a trailing dot in resource value... at least it looks like it should work :)

@pearkes
Copy link
Contributor

pearkes commented Feb 27, 2015

@paystee Yea, I think so! I would try that.

Added tests for relative and external CNAME values.
@antonta0
Copy link
Contributor Author

It looks like Digital Ocean has changed their API behavior (or may be I did not pay attention to it?). Creating a record with value within a domain always returns relative data to that domain. So, this is what I came up with:

  • If relative value specified (without dot), just leave it as it is
  • If value ends with a dot, then append a dot to response
  • If value ends with a dot and is within current domain, convert response to absolute

Third case is added to avoid creating new resources when specifying relative values in FQDN form, e.g a.bla.example.com. for example.com domain.

Also, I have added acceptance tests for all these cases.

@mitchellh
Copy link
Contributor

This looks fantastic. Tested locally and tests pass, too. Merging.

@mitchellh
Copy link
Contributor

Merged!

@mitchellh mitchellh closed this Mar 26, 2015
@ghost
Copy link

ghost commented May 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants