From d6c677552f3feb2909f695fba1e8e466359ad756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Mon, 25 Mar 2024 11:45:29 +0000 Subject: [PATCH] Fix bug dns record name inconsistent (#307) * add fqdn to model, map fqdn in mapFields * add testing * update examples, generate docs, fix linting * addressed comments in PR * add comment to acc tests, explaining ignore * update docs --- docs/data-sources/dns_record_set.md | 1 + docs/resources/dns_record_set.md | 3 +- docs/resources/dns_zone.md | 2 +- .../stackit_dns_record_set/resource.tf | 2 +- .../resources/stackit_dns_zone/resource.tf | 2 +- stackit/internal/services/dns/dns_acc_test.go | 20 +++++++---- .../services/dns/recordset/datasource.go | 4 +++ .../services/dns/recordset/resource.go | 10 +++++- .../services/dns/recordset/resource_test.go | 35 +++++++++++++++---- 9 files changed, 61 insertions(+), 18 deletions(-) diff --git a/docs/data-sources/dns_record_set.md b/docs/data-sources/dns_record_set.md index 49b98368..6566491f 100644 --- a/docs/data-sources/dns_record_set.md +++ b/docs/data-sources/dns_record_set.md @@ -34,6 +34,7 @@ data "stackit_dns_record_set" "example" { - `active` (Boolean) Specifies if the record set is active or not. - `comment` (String) Comment. - `error` (String) Error shows error in case create/update/delete failed. +- `fqdn` (String) Fully qualified domain name (FQDN) of the record set. - `id` (String) Terraform's internal data source. ID. It is structured as "`project_id`,`zone_id`,`record_set_id`". - `name` (String) Name of the record which should be a valid domain according to rfc1035 Section 2.3.4. E.g. `example.com` - `records` (List of String) Records. diff --git a/docs/resources/dns_record_set.md b/docs/resources/dns_record_set.md index cbe6feef..de3b3954 100644 --- a/docs/resources/dns_record_set.md +++ b/docs/resources/dns_record_set.md @@ -16,7 +16,7 @@ DNS Record Set Resource schema. resource "stackit_dns_record_set" "example" { project_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" zone_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" - name = "example-record-set.www.example-zone.com" + name = "example-record-set" type = "A" comment = "Example comment" records = ["1.2.3.4"] @@ -43,6 +43,7 @@ resource "stackit_dns_record_set" "example" { ### Read-Only - `error` (String) Error shows error in case create/update/delete failed. +- `fqdn` (String) Fully qualified domain name (FQDN) of the record set. - `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`zone_id`,`record_set_id`". - `record_set_id` (String) The rr set id. - `state` (String) Record set state. diff --git a/docs/resources/dns_zone.md b/docs/resources/dns_zone.md index 02bf738c..8781859a 100644 --- a/docs/resources/dns_zone.md +++ b/docs/resources/dns_zone.md @@ -16,7 +16,7 @@ DNS Zone resource schema. resource "stackit_dns_zone" "example" { project_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" name = "Example zone" - dns_name = "www.example-zone.com" + dns_name = "example-zone.com" contact_email = "aa@bb.ccc" type = "primary" acl = "192.168.0.0/24" diff --git a/examples/resources/stackit_dns_record_set/resource.tf b/examples/resources/stackit_dns_record_set/resource.tf index b9581a22..9a910fe5 100644 --- a/examples/resources/stackit_dns_record_set/resource.tf +++ b/examples/resources/stackit_dns_record_set/resource.tf @@ -1,7 +1,7 @@ resource "stackit_dns_record_set" "example" { project_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" zone_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" - name = "example-record-set.www.example-zone.com" + name = "example-record-set" type = "A" comment = "Example comment" records = ["1.2.3.4"] diff --git a/examples/resources/stackit_dns_zone/resource.tf b/examples/resources/stackit_dns_zone/resource.tf index cfdd4db4..7aa30348 100644 --- a/examples/resources/stackit_dns_zone/resource.tf +++ b/examples/resources/stackit_dns_zone/resource.tf @@ -1,7 +1,7 @@ resource "stackit_dns_zone" "example" { project_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" name = "Example zone" - dns_name = "www.example-zone.com" + dns_name = "example-zone.com" contact_email = "aa@bb.ccc" type = "primary" acl = "192.168.0.0/24" diff --git a/stackit/internal/services/dns/dns_acc_test.go b/stackit/internal/services/dns/dns_acc_test.go index bc44eec2..e6148d01 100644 --- a/stackit/internal/services/dns/dns_acc_test.go +++ b/stackit/internal/services/dns/dns_acc_test.go @@ -41,8 +41,8 @@ var zoneResource = map[string]string{ // Record set resource data var recordSetResource = map[string]string{ - "name": fmt.Sprintf("tf-acc-%s.%s.", acctest.RandStringFromCharSet(5, acctest.CharSetAlpha), zoneResource["dns_name"]), - "name_min": fmt.Sprintf("tf-acc-%s.%s.", acctest.RandStringFromCharSet(5, acctest.CharSetAlpha), zoneResource["dns_name_min"]), + "name": fmt.Sprintf("tf-acc-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlpha)), + "name_min": fmt.Sprintf("tf-acc-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlpha)), "records": `"1.2.3.4"`, "records_updated": `"5.6.7.8", "9.10.11.12"`, "ttl": "3700", @@ -153,6 +153,7 @@ func TestAccDnsResource(t *testing.T) { ), resource.TestCheckResourceAttrSet("stackit_dns_record_set.record_set", "record_set_id"), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "name", recordSetResource["name"]), + resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "fqdn", recordSetResource["name"]+"."+zoneResource["dns_name"]+"."), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "records.#", "1"), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "records.0", strings.ReplaceAll(recordSetResource["records"], "\"", "")), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "type", recordSetResource["type"]), @@ -221,7 +222,8 @@ func TestAccDnsResource(t *testing.T) { // Record set data resource.TestCheckResourceAttrSet("data.stackit_dns_record_set.record_set", "record_set_id"), - resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set", "name", recordSetResource["name"]), + resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set", "name", recordSetResource["name"]+"."+zoneResource["dns_name"]+"."), + resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set", "fqdn", recordSetResource["name"]+"."+zoneResource["dns_name"]+"."), resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set", "records.#", "1"), resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set", "type", recordSetResource["type"]), resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set", "ttl", recordSetResource["ttl"]), @@ -267,6 +269,8 @@ func TestAccDnsResource(t *testing.T) { }, ImportState: true, ImportStateVerify: true, + // Will be different because of the name vs fqdn problem, but the value is already tested in the datasource acc test + ImportStateVerifyIgnore: []string{"name"}, }, // Update. The zone ttl should not be updated according to the DNS API. { @@ -306,6 +310,7 @@ func TestAccDnsResource(t *testing.T) { ), resource.TestCheckResourceAttrSet("stackit_dns_record_set.record_set", "record_set_id"), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "name", recordSetResource["name"]), + resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "fqdn", recordSetResource["name"]+"."+zoneResource["dns_name"]+"."), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "records.#", "2"), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "type", recordSetResource["type"]), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set", "ttl", recordSetResource["ttl"]), @@ -393,6 +398,7 @@ func TestAccDnsMinimalResource(t *testing.T) { ), resource.TestCheckResourceAttrSet("stackit_dns_record_set.record_set_min", "record_set_id"), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set_min", "name", recordSetResource["name_min"]), + resource.TestCheckResourceAttr("stackit_dns_record_set.record_set_min", "fqdn", recordSetResource["name_min"]+"."+zoneResource["dns_name_min"]+"."), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set_min", "records.#", "1"), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set_min", "records.0", strings.ReplaceAll(recordSetResource["records"], "\"", "")), resource.TestCheckResourceAttr("stackit_dns_record_set.record_set_min", "type", recordSetResource["type"]), @@ -458,7 +464,8 @@ func TestAccDnsMinimalResource(t *testing.T) { // Record set data resource.TestCheckResourceAttrSet("data.stackit_dns_record_set.record_set_min", "record_set_id"), - resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set_min", "name", recordSetResource["name_min"]), + resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set_min", "name", recordSetResource["name_min"]+"."+zoneResource["dns_name_min"]+"."), + resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set_min", "fqdn", recordSetResource["name_min"]+"."+zoneResource["dns_name_min"]+"."), resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set_min", "records.#", "1"), resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set_min", "records.0", strings.ReplaceAll(recordSetResource["records"], "\"", "")), resource.TestCheckResourceAttr("data.stackit_dns_record_set.record_set_min", "type", recordSetResource["type"]), @@ -503,8 +510,9 @@ func TestAccDnsMinimalResource(t *testing.T) { return fmt.Sprintf("%s,%s,%s", testutil.ProjectId, zoneId, recordSetId), nil }, - ImportState: true, - ImportStateVerify: true, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name"}, }, // Deletion is done by the framework implicitly }, diff --git a/stackit/internal/services/dns/recordset/datasource.go b/stackit/internal/services/dns/recordset/datasource.go index 1971747f..9d7057b7 100644 --- a/stackit/internal/services/dns/recordset/datasource.go +++ b/stackit/internal/services/dns/recordset/datasource.go @@ -107,6 +107,10 @@ func (d *recordSetDataSource) Schema(_ context.Context, _ datasource.SchemaReque Description: "Name of the record which should be a valid domain according to rfc1035 Section 2.3.4. E.g. `example.com`", Computed: true, }, + "fqdn": schema.StringAttribute{ + Description: "Fully qualified domain name (FQDN) of the record set.", + Computed: true, + }, "records": schema.ListAttribute{ Description: "Records.", Computed: true, diff --git a/stackit/internal/services/dns/recordset/resource.go b/stackit/internal/services/dns/recordset/resource.go index 9a143b68..257e67bf 100644 --- a/stackit/internal/services/dns/recordset/resource.go +++ b/stackit/internal/services/dns/recordset/resource.go @@ -46,6 +46,7 @@ type Model struct { Type types.String `tfsdk:"type"` Error types.String `tfsdk:"error"` State types.String `tfsdk:"state"` + FQDN types.String `tfsdk:"fqdn"` } // NewRecordSetResource is a helper function to simplify the provider implementation. @@ -151,6 +152,10 @@ func (r *recordSetResource) Schema(_ context.Context, _ resource.SchemaRequest, stringvalidator.LengthAtMost(63), }, }, + "fqdn": schema.StringAttribute{ + Description: "Fully qualified domain name (FQDN) of the record set.", + Computed: true, + }, "records": schema.ListAttribute{ Description: "Records.", ElementType: types.StringType, @@ -433,7 +438,10 @@ func mapFields(recordSetResp *dns.RecordSetResponse, model *Model) error { model.Active = types.BoolPointerValue(recordSet.Active) model.Comment = types.StringPointerValue(recordSet.Comment) model.Error = types.StringPointerValue(recordSet.Error) - model.Name = types.StringPointerValue(recordSet.Name) + if model.Name.IsNull() || model.Name.IsUnknown() { + model.Name = types.StringPointerValue(recordSet.Name) + } + model.FQDN = types.StringPointerValue(recordSet.Name) model.State = types.StringPointerValue(recordSet.State) model.TTL = types.Int64PointerValue(recordSet.Ttl) model.Type = types.StringPointerValue(recordSet.Type) diff --git a/stackit/internal/services/dns/recordset/resource_test.go b/stackit/internal/services/dns/recordset/resource_test.go index 4ad1eecf..8064ddfa 100644 --- a/stackit/internal/services/dns/recordset/resource_test.go +++ b/stackit/internal/services/dns/recordset/resource_test.go @@ -13,12 +13,17 @@ import ( func TestMapFields(t *testing.T) { tests := []struct { description string + state Model input *dns.RecordSetResponse expected Model isValid bool }{ { "default_values", + Model{ + ProjectId: types.StringValue("pid"), + ZoneId: types.StringValue("zid"), + }, &dns.RecordSetResponse{ Rrset: &dns.RecordSet{ Id: utils.Ptr("rid"), @@ -33,6 +38,7 @@ func TestMapFields(t *testing.T) { Comment: types.StringNull(), Error: types.StringNull(), Name: types.StringNull(), + FQDN: types.StringNull(), Records: types.ListNull(types.StringType), State: types.StringNull(), TTL: types.Int64Null(), @@ -42,6 +48,10 @@ func TestMapFields(t *testing.T) { }, { "simple_values", + Model{ + ProjectId: types.StringValue("pid"), + ZoneId: types.StringValue("zid"), + }, &dns.RecordSetResponse{ Rrset: &dns.RecordSet{ Id: utils.Ptr("rid"), @@ -67,6 +77,7 @@ func TestMapFields(t *testing.T) { Comment: types.StringValue("comment"), Error: types.StringValue("error"), Name: types.StringValue("name"), + FQDN: types.StringValue("name"), Records: types.ListValueMust(types.StringType, []attr.Value{ types.StringValue("record_1"), types.StringValue("record_2"), @@ -79,6 +90,11 @@ func TestMapFields(t *testing.T) { }, { "null_fields_and_int_conversions", + Model{ + ProjectId: types.StringValue("pid"), + ZoneId: types.StringValue("zid"), + Name: types.StringValue("other-name"), + }, &dns.RecordSetResponse{ Rrset: &dns.RecordSet{ Id: utils.Ptr("rid"), @@ -100,7 +116,8 @@ func TestMapFields(t *testing.T) { Active: types.BoolNull(), Comment: types.StringNull(), Error: types.StringNull(), - Name: types.StringValue("name"), + Name: types.StringValue("other-name"), + FQDN: types.StringValue("name"), Records: types.ListNull(types.StringType), State: types.StringValue("state"), TTL: types.Int64Value(2123456789), @@ -110,12 +127,20 @@ func TestMapFields(t *testing.T) { }, { "nil_response", + Model{ + ProjectId: types.StringValue("pid"), + ZoneId: types.StringValue("zid"), + }, nil, Model{}, false, }, { "no_resource_id", + Model{ + ProjectId: types.StringValue("pid"), + ZoneId: types.StringValue("zid"), + }, &dns.RecordSetResponse{}, Model{}, false, @@ -123,11 +148,7 @@ func TestMapFields(t *testing.T) { } for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - state := &Model{ - ProjectId: tt.expected.ProjectId, - ZoneId: tt.expected.ZoneId, - } - err := mapFields(tt.input, state) + err := mapFields(tt.input, &tt.state) if !tt.isValid && err == nil { t.Fatalf("Should have failed") } @@ -135,7 +156,7 @@ func TestMapFields(t *testing.T) { t.Fatalf("Should not have failed: %v", err) } if tt.isValid { - diff := cmp.Diff(state, &tt.expected) + diff := cmp.Diff(tt.state, tt.expected) if diff != "" { t.Fatalf("Data does not match: %s", diff) }