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

adds validation to only allow valid DNS record types #610

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

joel-g
Copy link

@joel-g joel-g commented Mar 4, 2020

Current version allows user to submit any string to API as a Record's Type.

Anything not from this list will return an error from the API: A, AAAA, CNAME, TXT, SRV, LOC, MX, NS, SPF, CERT, DNSKEY, DS, NAPTR, SMIMEA, SSHFP, TLSA, URI

This validation will prompt the user before attempting to create resources.

@ghost ghost added the size/XS label Mar 4, 2020
Copy link
Member

@jacobbednarz jacobbednarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

@jacobbednarz jacobbednarz merged commit 5f67ce1 into cloudflare:master Mar 4, 2020
@rdeleon-zebra
Copy link

So, why not CAA?

@jacobbednarz
Copy link
Member

@rdeleon-zebra please see the latest commits on master - CAA was added within hours of identifying this oversight.

@rdeleon-zebra
Copy link

rdeleon-zebra commented Mar 10, 2020

@jacobbednarz : Yup, saw it here: hashicorp@2b1b4fc

Any ETA on this release fix? Curious... we down-graded away from 2.3.0 b/c of this - hmm, I see that 2.4.0 should absorb this fix. Thanks for the breadcrumbs!

@jacobbednarz
Copy link
Member

No ETA at this point. We aim for semi regular releases but will let 2.3.0 sit for a couple of days before releasing a patch release to address in case anything else pops up. The issue isn’t a major one that requires an immediate fix.

@joel-g joel-g deleted the record-type-validation branch March 10, 2020 22:26
@T4cC0re
Copy link

T4cC0re commented Mar 11, 2020

The issue isn’t a major one that requires an immediate fix.

@jacobbednarz It might not be for you, but it is for us. This MR caught us off-guard and caused all of our terraform to collapse since we use this provider's resources in a module, used across our code. Some parts of which want to create CAA records, which of course fail now.

This patch broke something and is thus IMO worth a dedicated patch release asap. To wait in case anything else pops up is not what I look for when there already is a fix for an issue, that was introduced as a regression.

We have since pinned this provider to an older version, but I don't think we should have to, for the reasons outlined above.

boekkooi-lengoo pushed a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this pull request Feb 28, 2022
…pport-tsig

Add support for secondary DNS TSIG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants