[Cloudflare] Custom record TTL #572
Conversation
Signed-off-by: Nick Jüttner <[email protected]>
k8s-ci-robot
added
cncf-cla: yes
njuettner
changed the title
provider/cloudflare.go
Outdated
| if proxied && (cloudFlareTypeNotSupported[endpoint.RecordType] || strings.Contains(endpoint.DNSName, "*")) { | ||
| proxied = false | ||
| } | ||
| //// min value:120 | ||
| if endpoint.RecordTTL.IsConfigured() && endpoint.RecordTTL >= 120 { |
There was a problem hiding this comment.
I am not sure if this logic belongs in External-DNS. I don't think we should try to do validation on behalf of DNS provider. This value might change and we don't want to keep track of it. If a user specifies a value under 120, then we should provide a reasonable error log message and ask user to fix it.
There was a problem hiding this comment.
Yes I agree, usually I would expect this to be handled by the DNS provider. However if you would not implement it and you would set the TTL below 120 it will fail:
ERRO[0302] failed to create record: error from makeRequest: HTTP status 400: content "{\"success\":false,\"errors\":[{\"code\":1004,\"message\":\"DNS Validation Error\",\"error_chain\":[{\"code\":9021,\"message\":\"Invalid TTL. Must be between 120 and 2,147,483,647 seconds, or 1 for automatic\"}]}],\"messages\":[],\"result\":null}"
I can remove the && endpoint.RecordTTL >= 120 part and document it in docs/tutorials/cloudflare.md to make people aware of it.
WDYT?
There was a problem hiding this comment.
Yeah i think having a documentation plus handling the error with proper log message would be enough in my opinion.
Signed-off-by: Nick Jüttner <[email protected]>
|
@ideahitme I removed the part and added some context in the Cloudflare tutorial. I checked external-dns again and it seems like the error is already logged and I think it's pretty understandable that you have to change the TTL at least to 120 or higher. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
* refactor AssertPluginFromIndex * no need to pass t * mark as helper * swap parameter order to match method name + more intuitive * refactor existing "krew install" integ tests to use this Signed-off-by: Ahmet Alp Balkan <[email protected]>
Custom record TTL annotation should be noted by Cloudflare provider
TTL min value is 120 everything below will take the default TTL which is 1 (automatic which means 300)