Skip to content

Commit 3649cd1

Browse files
vekshveksh
authored
Merge pull request #13 from veksh/12-multiple-As
allow multiple A records for the same name
2 parents 2cfac0a + 7c9f0d2 commit 3649cd1

File tree

6 files changed

+185
-17
lines changed

6 files changed

+185
-17
lines changed

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ require (
5454
github.com/mitchellh/reflectwalk v1.0.2 // indirect
5555
github.com/oklog/run v1.0.0 // indirect
5656
github.com/pkg/errors v0.9.1
57+
github.com/samber/lo v1.39.0
5758
github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
5859
github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
5960
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
137137
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
138138
github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
139139
github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
140+
github.com/samber/lo v1.39.0 h1:4gTz1wUhNYLhFSKl6O+8peW0v2F4BCY034GRpU9WnuA=
141+
github.com/samber/lo v1.39.0/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA=
140142
github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
141143
github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
142144
github.com/skeema/knownhosts v1.2.0 h1:h9r9cf0+u7wSE+M183ZtMGgOJKiL96brpaz5ekfJCpM=

‎internal/model/model.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ type DNSUpdateRecord struct {
5252
}
5353

5454
// compare key field to determine if two records refer to the same object
55-
// - for A and CNAME there could be only 1 RR with the same name, TTL is the only value
56-
// - for TXT and NS there could be several (so need to match by data),
55+
// - for CNAME there could be only 1 RR with the same name, TTL is the only value
56+
// - for A, TXT and NS there could be several (so need to match by data),
5757
// - MX matches the same way, value is ttl + prio (in theory, MX 0 and MX 10
5858
// could point to the same host in "data", but lets think that it is a perversion
5959
// and replace it with one record
@@ -62,18 +62,15 @@ func (r DNSRecord) SameKey(r1 DNSRecord) bool {
6262
if r.Type != r1.Type || r.Name != r1.Name {
6363
return false
6464
}
65-
if r.Type == REC_CNAME || r.Type == REC_A || r.Type == REC_AAAA {
65+
if r.Type == REC_CNAME {
6666
return true
6767
}
68-
if r.Type == REC_TXT || r.Type == REC_MX || r.Type == REC_NS {
69-
return r.Data == r1.Data
70-
}
7168
if r.Type == REC_SRV {
7269
return r.Protocol == r1.Protocol && r.Service == r1.Service &&
7370
r.Port == r1.Port && r.Data == r1.Data
7471
}
75-
// soa?
76-
return false
72+
// TXT, MX, NS, A, AAAA
73+
return r.Data == r1.Data
7774
}
7875

7976
// convert DNSRecord to update format (dropping 2 first fields)
@@ -89,10 +86,10 @@ func (r DNSRecord) ToUpdate() DNSUpdateRecord {
8986
}
9087
}
9188

92-
// true if there is only one possible value for dom+type+key combination
93-
// i.e record is CNAME, A or AAAA
89+
// true if there is only one possible value for domain+type+key combination
90+
// i.e record is CNAME
9491
func (t DNSRecordType) IsSingleValue() bool {
95-
return t == REC_CNAME || t == REC_A || t == REC_AAAA
92+
return t == REC_CNAME
9693
}
9794

9895
// client API interface

‎internal/provider/record.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func setLogCtx(ctx context.Context, tfRec tfDNSRecord, op string) context.Contex
4545
ctx = tflog.SetField(ctx, "domain", tfRec.Domain.ValueString())
4646
ctx = tflog.SetField(ctx, "type", tfRec.Type.ValueString())
4747
ctx = tflog.SetField(ctx, "name", tfRec.Name.ValueString())
48+
ctx = tflog.SetField(ctx, "data", tfRec.Data.ValueString())
4849
ctx = tflog.SetField(ctx, "operation", op)
4950
return ctx
5051
}
@@ -212,9 +213,9 @@ func (r *RecordResource) Read(ctx context.Context, req resource.ReadRequest, res
212213
tflog.Info(ctx, fmt.Sprintf(
213214
"Reading DNS record: got %d answers", numRecs))
214215
// meaning of "match" is different between types
215-
// - for A, AAAA, and CNAME (and SOA), there could be only 1 records
216-
// with a given name in domain
217-
// - for TXT, MX and NS there could be several, have to match by data
216+
// - for CNAME (and SOA), there could be only 1 records with a given name
217+
// in a (sub-)domain
218+
// - for A, TXT, MX and NS there could be several, have to match by data
218219
// - MXes could have different priorities; in theory, MX 0 and MX 10
219220
// could point to the same "data", but lets think that it is a
220221
// preversion and replace it with one :)
@@ -264,7 +265,7 @@ func (r *RecordResource) Update(ctx context.Context, req resource.UpdateRequest,
264265

265266
var err error
266267
if apiRecPlan.Type.IsSingleValue() {
267-
// for CNAME and A: just one record replacing another
268+
// for CNAME: just one record replacing another
268269
err = r.client.SetRecords(ctx,
269270
apiDomain, apiRecPlan.Type, apiRecPlan.Name,
270271
[]model.DNSUpdateRecord{{
@@ -292,6 +293,8 @@ func (r *RecordResource) Update(ctx context.Context, req resource.UpdateRequest,
292293
TTL: apiRecPlan.TTL,
293294
Priority: apiRecPlan.Priority,
294295
}
296+
// TODO: probably cannot be by construction :)
297+
// and comparing them this way is wrong anyway
295298
if slices.Index(apiUpdateRecs, ourRec) >= 0 {
296299
tflog.Info(ctx, "Record is already present, nothing to do: done")
297300
err = nil
@@ -318,7 +321,8 @@ func (r *RecordResource) Delete(ctx context.Context, req resource.DeleteRequest,
318321
return
319322
}
320323

321-
ctx = setLogCtx(ctx, stateData, "delete: start")
324+
ctx = setLogCtx(ctx, stateData, "delete")
325+
tflog.Info(ctx, "delete: start")
322326
defer tflog.Info(ctx, "delete: end")
323327

324328
apiDomain, apiRecState := tf2model(stateData)

‎internal/provider/record_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,71 @@ var (
3838
mDom = model.DNSDomain(TEST_DOMAIN)
3939
)
4040

41+
// two A resources + 1 pre-existing, all with the same hame
42+
func TestUnitALifecycle(t *testing.T) {
43+
// this will be found as pre-existing
44+
mRecsPre := makeTestRecSet([]model.DNSRecordData{"1.1.1.1"})
45+
// records to add
46+
mRecsToAdd := makeTestRecSet([]model.DNSRecordData{"2.2.2.2", "3.3.3.3"})
47+
// after adding 1/2 + kept pre
48+
mRecsTgt1 := makeTestRecSet([]model.DNSRecordData{"1.1.1.1", "2.2.2.2"})
49+
// mRecsTgt2 := makeTestRecSet(model.REC_A, []model.DNSRecordData{"1.1.1.1", "3.3.3.3"})
50+
// after adding 2/2 + kept pre
51+
mRecsTgt := makeTestRecSet([]model.DNSRecordData{"1.1.1.1", "2.2.2.2", "3.3.3.3"})
52+
53+
// 2nd step: update
54+
mRecsToUpd := makeTestRecSet([]model.DNSRecordData{"2.2.2.2", "4.4.4.4"})
55+
mRecsToUpdTgt := makeTestRecSet([]model.DNSRecordData{"1.1.1.1", "2.2.2.2", "4.4.4.4"})
56+
mRecsTgt4 := makeTestRecSet([]model.DNSRecordData{"1.1.1.1", "4.4.4.4"})
57+
58+
mType, mName := model.REC_A, mRecsPre.DNSRecName
59+
60+
// add records for .2 and .3, read it back (with pre .1)
61+
mockClientAdd := model.NewMockDNSApiClient(t)
62+
mockClientAdd.EXPECT().AddRecords(mCtx, mDom, mRecsToAdd.Records[0:1]).Return(nil).Once()
63+
mockClientAdd.EXPECT().AddRecords(mCtx, mDom, mRecsToAdd.Records[1:2]).Return(nil).Once()
64+
mockClientAdd.EXPECT().GetRecords(mCtx, mDom, mType, mName).Return(mRecsTgt.Records, nil).Twice()
65+
66+
// destroy after 1st step: cleanup right after 1st step; really not deterministic:
67+
// mockClientAdd.EXPECT().GetRecords(mCtx, mDom, mType, mName).Return(mRecsTgt.Records, nil).Once()
68+
// mockClientAdd.EXPECT().SetRecords(mCtx, mDom, mType, mName, mRecsTgt1.UpdRecords).Return(nil).Once()
69+
// mockClientAdd.EXPECT().GetRecords(mCtx, mDom, mType, mName).Return(mRecsTgt1.Records, nil).Once()
70+
// mockClientAdd.EXPECT().SetRecords(mCtx, mDom, mType, mName, mRecsPre.UpdRecords).Return(nil).Once()
71+
72+
// destroy after 1st step: a bit absurd, but at least stable :)
73+
// mockClientAdd.EXPECT().GetRecords(mCtx, mDom, mType, mName).Return(mRecsTgt.Records, nil).Twice()
74+
// mockClientAdd.EXPECT().SetRecords(mCtx, mDom, mType, mName, mRecsTgt1.UpdRecords).Return(nil).Once()
75+
// mockClientAdd.EXPECT().SetRecords(mCtx, mDom, mType, mName, mRecsTgt2.UpdRecords).Return(nil).Once()
76+
77+
// read, update, then delete for clean-up
78+
mockClientUpd := model.NewMockDNSApiClient(t)
79+
mockClientUpd.EXPECT().GetRecords(mCtx, mDom, mType, mName).Return(mRecsTgt.Records, nil).Times(3)
80+
mockClientUpd.EXPECT().SetRecords(mCtx, mDom, mType, mName, mRecsToUpdTgt.UpdRecords).Return(nil).Once()
81+
// again, clean-up order is not deterministic, so lets return same results to both and see them keeping pre
82+
// better way would be "3 recs -> del 3rd -> 2 recs -> del 2nd -> only pre-existing left"
83+
// the problem is that clean-up is going in parallel, so it is hard to return a proper results,
84+
// and there is no way to express these dependencies in mock
85+
mockClientUpd.EXPECT().GetRecords(mCtx, mDom, mType, mName).Return(mRecsToUpdTgt.Records, nil).Times(4)
86+
mockClientUpd.EXPECT().SetRecords(mCtx, mDom, mType, mName, mRecsTgt1.UpdRecords).Return(nil).Once()
87+
mockClientUpd.EXPECT().SetRecords(mCtx, mDom, mType, mName, mRecsTgt4.UpdRecords).Return(nil).Once()
88+
89+
resource.UnitTest(t, resource.TestCase{
90+
// ProtoV6ProviderFactories: testProviderFactory,
91+
Steps: []resource.TestStep{
92+
// create, read back
93+
{
94+
ProtoV6ProviderFactories: mockClientProviderFactory(mockClientAdd),
95+
Config: mRecsToAdd.TFConfig,
96+
},
97+
// read back, change, delete (must be noop because already gone)
98+
{
99+
ProtoV6ProviderFactories: mockClientProviderFactory(mockClientUpd),
100+
Config: mRecsToUpd.TFConfig,
101+
},
102+
},
103+
})
104+
}
105+
41106
// simple acceptance test for MX resource, with pre-existing one
42107
func TestAccMXLifecycle(t *testing.T) {
43108
mData := model.DNSRecordData("mx1.test.com")

‎internal/provider/record_test_utils.go

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@ package provider
33
import (
44
"context"
55
"fmt"
6-
"html/template"
76
"os"
87
"strings"
98
"testing"
9+
"text/template"
1010

1111
"github.com/hashicorp/terraform-plugin-framework/providerserver"
1212
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
1313
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
1414
"github.com/hashicorp/terraform-plugin-testing/terraform"
1515
"github.com/pkg/errors"
16+
"github.com/samber/lo"
1617
"github.com/veksh/terraform-provider-godaddy-dns/internal/client"
1718
"github.com/veksh/terraform-provider-godaddy-dns/internal/model"
1819
)
@@ -107,6 +108,104 @@ func simpleResourceConfig(rectype model.DNSRecordType, target model.DNSRecordDat
107108
return buff.String()
108109
}
109110

111+
// - terraform config with N records
112+
// - domain record name for it ("test-<type>._test")
113+
// - terraform resource name for record type ("godaddy-dns_record.test-<type>")
114+
// - []model.DNSRecord array with all attrs
115+
// - []model.DNSUpdateRecord w/o name and type
116+
type testRecSet struct {
117+
TFConfig string
118+
DNSRecName model.DNSRecordName
119+
TFResName string
120+
Records []model.DNSRecord
121+
UpdRecords []model.DNSUpdateRecord
122+
}
123+
124+
// create terraform config for N record with the same name but different values
125+
func makeTestRecSet(values []model.DNSRecordData) testRecSet {
126+
res := testRecSet{}
127+
rectype := model.REC_A // for now it is always A, lets make a linter happy :)
128+
129+
templateString := `
130+
provider "godaddy-dns" {}
131+
locals {
132+
dataValues = [{{ .RecValsJoined }}]
133+
}
134+
resource "godaddy-dns_record" "{{ .RecName }}" {
135+
domain = "{{ .Domain }}"
136+
type = "{{ .RecType | upper }}"
137+
name = "{{ .DNSRecName }}"
138+
139+
count = length(local.dataValues)
140+
data = local.dataValues[count.index]
141+
142+
{{ if gt .Priority -1 }}
143+
priority = {{ .Priority }}
144+
{{ end}}
145+
}`
146+
funcMap := template.FuncMap{
147+
"lower": strings.ToLower,
148+
"upper": strings.ToUpper,
149+
}
150+
tmpl, err := template.New("config").Funcs(funcMap).Parse(templateString)
151+
if err != nil {
152+
return res
153+
}
154+
155+
recName := "test-" + strings.ToLower(string(rectype))
156+
var recPrio model.DNSRecordPrio
157+
if rectype == model.REC_MX {
158+
recPrio = 10
159+
}
160+
var buff strings.Builder
161+
valStrings := lo.Map(values, func(x model.DNSRecordData, _ int) string {
162+
return fmt.Sprintf("\"%v\"", x)
163+
})
164+
valStringsJoined := strings.Join(valStrings, ", ")
165+
resConf := struct {
166+
Domain string
167+
RecType string
168+
RecValsJoined *string
169+
RecName string
170+
DNSRecName string
171+
Priority int
172+
}{
173+
TEST_DOMAIN,
174+
string(rectype),
175+
&valStringsJoined,
176+
recName,
177+
recName + "._test",
178+
-1,
179+
}
180+
if rectype == model.REC_MX {
181+
resConf.Priority = 10
182+
}
183+
err = tmpl.ExecuteTemplate(&buff, "config", resConf)
184+
if err != nil {
185+
return res
186+
}
187+
res.TFConfig = buff.String()
188+
res.DNSRecName = model.DNSRecordName(recName + "._test")
189+
res.TFResName = "godaddy-dns_record." + recName
190+
res.Records = lo.Map(values, func(data model.DNSRecordData, _ int) model.DNSRecord {
191+
return model.DNSRecord{
192+
Name: res.DNSRecName,
193+
Type: rectype,
194+
Data: data,
195+
TTL: 3600,
196+
Priority: recPrio,
197+
}
198+
})
199+
res.UpdRecords = lo.Map(values, func(data model.DNSRecordData, _ int) model.DNSUpdateRecord {
200+
return model.DNSUpdateRecord{
201+
Data: data,
202+
TTL: 3600,
203+
Priority: recPrio,
204+
}
205+
})
206+
return res
207+
}
208+
110209
// check that actual record (from API query) matches resource state
111210
func CheckApiRecordMach(resourceName string, apiClient *client.Client) resource.TestCheckFunc {
112211
return func(s *terraform.State) error {

0 commit comments

Comments
 (0)