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

DNS record name validation improvements #1740

Merged
merged 4 commits into from
Jul 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/1740.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/cloudflare_record: Validate that DNS record names are non-empty
```
12 changes: 8 additions & 4 deletions internal/provider/resource_cloudflare_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,13 @@ func resourceCloudflareRecordCreate(ctx context.Context, d *schema.ResourceData,
newRecord.TTL = ttl.(int)
}

if newRecord.Name == "" {
return diag.FromErr(fmt.Errorf("record on zone %s must not have an empty name (use @ for the zone apex)", newRecord.ZoneID))
}

// Validate value based on type
if err := validateRecordName(newRecord.Type, newRecord.Content); err != nil {
return diag.FromErr(fmt.Errorf("error validating record name %q: %w", newRecord.Name, err))
if err := validateRecordContent(newRecord.Type, newRecord.Content); err != nil {
return diag.FromErr(fmt.Errorf("error validating record content of %q: %w", newRecord.Name, err))
}

var proxiedVal *bool
Expand Down Expand Up @@ -185,8 +189,8 @@ func resourceCloudflareRecordRead(ctx context.Context, d *schema.ResourceData, m

record, err := client.DNSRecord(ctx, zoneID, d.Id())
if err != nil {
if strings.Contains(err.Error(), "Invalid dns record identifier") ||
strings.Contains(err.Error(), "HTTP status 404") {
var notFoundError *cloudflare.NotFoundError
if errors.As(err, &notFoundError) {
tflog.Warn(ctx, fmt.Sprintf("Removing record from state because it's not found in API"))
d.SetId("")
return nil
Expand Down
4 changes: 2 additions & 2 deletions internal/provider/resource_cloudflare_record_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ func TestAccCloudflareRecord_MXWithPriorityZero(t *testing.T) {
Config: testAccCheckCloudflareRecordConfigMXWithPriorityZero(zoneID, rnd, zoneName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "priority", "0"),
resource.TestCheckResourceAttr(resourceName, "value", "."),
resource.TestCheckResourceAttr(resourceName, "value", "mail.terraform.cfapi.net"),
),
},
},
Expand Down Expand Up @@ -772,7 +772,7 @@ func testAccCheckCloudflareRecordConfigMXWithPriorityZero(zoneID, name, zoneName
resource "cloudflare_record" "%[2]s" {
zone_id = "%[1]s"
name = "%[2]s"
value = "."
value = "mail.terraform.cfapi.net"
type = "MX"
priority = 0
proxied = false
Expand Down
6 changes: 3 additions & 3 deletions internal/provider/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func validateRecordType(t string, proxied bool) error {
return fmt.Errorf("type %q cannot be proxied", t)
}

// validateRecordName ensures that based on supplied record type, the name content matches
// Currently only validates A and AAAA types.
func validateRecordName(t string, value string) error {
// validateRecordContent ensures that the record's content is valid for the
// supplied record type. Currently only validates A and AAAA types.
func validateRecordContent(t string, value string) error {
switch t {
case "A":
// Must be ipv4 addr
Expand Down
8 changes: 4 additions & 4 deletions internal/provider/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func TestValidateRecordName(t *testing.T) {
}

for k, v := range validNames {
if err := validateRecordName(k, v); err != nil {
t.Fatalf("%q should be a valid name for type %q: %v", v, k, err)
if err := validateRecordContent(k, v); err != nil {
t.Fatalf("%q should be valid content for type %q: %v", v, k, err)
}
}

Expand All @@ -62,8 +62,8 @@ func TestValidateRecordName(t *testing.T) {
"TXT": "\n",
}
for k, v := range invalidNames {
if err := validateRecordName(k, v); err == nil {
t.Fatalf("%q should be an invalid name for type %q", v, k)
if err := validateRecordContent(k, v); err == nil {
t.Fatalf("%q should be invalid content for type %q", v, k)
}
}
}