diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go index cc7523f20..dfccd6e9c 100644 --- a/integrationTest/integration_test.go +++ b/integrationTest/integration_test.go @@ -692,25 +692,51 @@ func makeTests(t *testing.T) []*TestGroup { // These are tested on "@" and "www". // When these tests pass, you've implemented the basics correctly. - testgroup("Protocol-Plain", - tc("Create an A record", a("@", "1.1.1.1")), - tc("Change it", a("@", "1.2.3.4")), - tc("Add another", a("@", "1.2.3.4"), a("www", "1.2.3.4")), - tc("Add another(same name)", a("@", "1.2.3.4"), a("www", "1.2.3.4"), a("www", "5.6.7.8")), + testgroup("A", + tc("Create A", a("testa", "1.1.1.1")), + tc("Change A target", a("testa", "1.2.3.4")), ), - testgroup("Protocol-TTL", + testgroup("MX", + tc("Create MX", mx("testmx", 5, "foo.com.")), + tc("Change MX target", mx("testmx", 5, "bar.com.")), + tc("Change MX p", mx("testmx", 100, "bar.com.")), + ), + + testgroup("CNAME", + tc("Create a CNAME", cname("testcname", "www.google.com.")), + tc("Change CNAME target", cname("testcname", "www.yahoo.com.")), + ), + + testgroup("ManyAtOne", + tc("CreateManyAtLabel", a("www", "1.1.1.1"), a("www", "2.2.2.2"), a("www", "3.3.3.3")), + clear(), + tc("Create an A record", a("www", "1.1.1.1")), + tc("Add at label1", a("www", "1.1.1.1"), a("www", "2.2.2.2")), + tc("Add at label2", a("www", "1.1.1.1"), a("www", "2.2.2.2"), a("www", "3.3.3.3")), + ), + + testgroup("manyAtOneTypes", + tc("CreateManyTypesAtLabel", a("www", "1.1.1.1"), mx("testmx", 5, "foo.com."), mx("testmx", 100, "bar.com.")), + clear(), + tc("Create an A record", a("www", "1.1.1.1")), + tc("Add Type At Label", a("www", "1.1.1.1"), mx("testmx", 5, "foo.com.")), + tc("Add Type At Label", a("www", "1.1.1.1"), mx("testmx", 5, "foo.com."), mx("testmx", 100, "bar.com.")), + ), + + // Make sure changes at the apex (the bare domain) work. + testgroup("Apex", + tc("Create A", a("@", "1.1.1.1")), + tc("Change A target", a("@", "1.2.3.4")), + ), + + // Exercise TTL operations. + testgroup("TTL", not("NETCUP"), // NETCUP does not support TTLs. tc("Start", a("@", "1.2.3.4"), a("www", "1.2.3.4"), a("www", "5.6.7.8")), tc("Change a ttl", ttl(a("@", "1.2.3.4"), 1000), a("www", "1.2.3.4"), a("www", "5.6.7.8")), tc("Change single target from set", ttl(a("@", "1.2.3.4"), 1000), a("www", "2.2.2.2"), a("www", "5.6.7.8")), tc("Change all ttls", ttl(a("@", "1.2.3.4"), 500), ttl(a("www", "2.2.2.2"), 400), ttl(a("www", "5.6.7.8"), 400)), - tc("Delete one", ttl(a("@", "1.2.3.4"), 500), ttl(a("www", "5.6.7.8"), 400)), - ), - - testgroup("add to existing label", - tc("Setup", ttl(a("www", "5.6.7.8"), 400)), - tc("Add at same label", ttl(a("www", "5.6.7.8"), 400), ttl(a("www", "1.2.3.4"), 400)), ), // This is a strange one. It adds a new record to an existing @@ -884,6 +910,7 @@ func makeTests(t *testing.T) []*TestGroup { "MSDNS", // No paging done. No need to test. "NAMEDOTCOM", // Their API is so damn slow. We'll add it back as needed. "NS1", // Free acct only allows 50 records, therefore we skip + //"ROUTE53", // Batches up changes in pages. ), tc("99 records", manyA("rec%04d", "1.2.3.4", 99)...), tc("100 records", manyA("rec%04d", "1.2.3.4", 100)...), @@ -899,7 +926,7 @@ func makeTests(t *testing.T) []*TestGroup { "GCLOUD", "HEXONET", //"MSDNS", // No paging done. No need to test. - "ROUTE53", + "ROUTE53", // Batches up changes in pages. ), tc("601 records", manyA("rec%04d", "1.2.3.4", 600)...), tc("Update 601 records", manyA("rec%04d", "1.2.3.5", 600)...), @@ -915,7 +942,7 @@ func makeTests(t *testing.T) []*TestGroup { "HEXONET", "HOSTINGDE", //"MSDNS", // No paging done. No need to test. - "ROUTE53", + "ROUTE53", // Batches up changes in pages. ), tc("1200 records", manyA("rec%04d", "1.2.3.4", 1200)...), tc("Update 1200 records", manyA("rec%04d", "1.2.3.5", 1200)...), @@ -1190,12 +1217,10 @@ func makeTests(t *testing.T) []*TestGroup { tc("remove cnames", r53alias("dev-system", "CNAME", "dev-system19.**current-domain**"), ), - clear(), - tc("create cname+alias in one step", - cname("dev-system18", "ec2-54-91-33-155.compute-1.amazonaws.com."), - r53alias("dev-system", "CNAME", "dev-system18.**current-domain**"), - ), - clear(), + ), + + testgroup("R53_ALIAS_CNAME", + requires(providers.CanUseRoute53Alias), tc("create alias+cname in one step", r53alias("dev-system", "CNAME", "dev-system18.**current-domain**"), cname("dev-system18", "ec2-54-91-33-155.compute-1.amazonaws.com."), diff --git a/providers/route53/route53Provider.go b/providers/route53/route53Provider.go index 2f9cb4754..e302cc51f 100644 --- a/providers/route53/route53Provider.go +++ b/providers/route53/route53Provider.go @@ -272,8 +272,8 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode return nil, err } + // update zone_id to current zone.id if not specified by the user for _, want := range dc.Records { - // update zone_id to current zone.id if not specified by the user if want.Type == "R53_ALIAS" && want.R53Alias["zone_id"] == "" { want.R53Alias["zone_id"] = getZoneID(zone, want) } @@ -284,7 +284,7 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records var corrections []*models.Correction - if !diff2.EnableDiff2 || true { // Remove "|| true" when diff2 version arrives + if !diff2.EnableDiff2 { // diff differ := diff.New(dc, getAliasMap) @@ -471,9 +471,123 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode } - // Insert Future diff2 version here. + changes := []r53Types.Change{} + changeDesc := []string{} + + // Amazon Route53 is a "ByRecordSet" API. + // At each label:rtype pair, we either delete all records or UPSERT the desired records. + instructions, err := diff2.ByRecordSet(existingRecords, dc, nil) + if err != nil { + return nil, err + } + instructions = reorderInstructions(instructions) + for _, inst := range instructions { + instNameFQDN := inst.Key.NameFQDN + instType := inst.Key.Type + var chg r53Types.Change + + switch inst.Type { + + case diff2.CREATE: + fallthrough + case diff2.CHANGE: + // To CREATE/CHANGE, build a new record set from the desired state and UPSERT it. + + // Make the rrset to be UPSERTed: + var rrset *r53Types.ResourceRecordSet + if instType == "R53_ALIAS" { + // A R53_ALIAS_* requires ResourceRecordSet to a a single item, not a list. + if len(inst.New) != 1 { + log.Fatal("Only one R53_ALIAS_ permitted on a label") + } + rrset = aliasToRRSet(zone, inst.New[0]) + rrset.Name = aws.String(instNameFQDN) + } else { + // Make a list of all the records to be installed at label:rtype + rrset = &r53Types.ResourceRecordSet{ + Name: aws.String(instNameFQDN), + Type: r53Types.RRType(instType), + } + + for _, r := range inst.New { + rr := r53Types.ResourceRecord{ + Value: aws.String(r.GetTargetCombined()), + } + rrset.ResourceRecords = append(rrset.ResourceRecords, rr) + i := int64(r.TTL) + rrset.TTL = &i + } + } + chg = r53Types.Change{ + Action: r53Types.ChangeActionUpsert, + ResourceRecordSet: rrset, + } + + case diff2.DELETE: + rrset := inst.Old[0].Original.(r53Types.ResourceRecordSet) // The native record as downloaded via the API + chg = r53Types.Change{ + Action: r53Types.ChangeActionDelete, + ResourceRecordSet: &rrset, + } + } + + changes = append(changes, chg) + changeDesc = append(changeDesc, inst.Msgs...) + } + + addCorrection := func(msg string, req *r53.ChangeResourceRecordSetsInput) { + corrections = append(corrections, + &models.Correction{ + Msg: msg, + F: func() error { + var err error + req.HostedZoneId = zone.Id + withRetry(func() error { + _, err = r.client.ChangeResourceRecordSets(context.Background(), req) + return err + }) + return err + }, + }) + + } + + // Send the changes in as few API calls as possible. + batcher := newChangeBatcher(changes) + for batcher.Next() { + start, end := batcher.Batch() + batch := changes[start:end] + descBatchStr := strings.Join(changeDesc[start:end], "\n") + req := &r53.ChangeResourceRecordSetsInput{ + ChangeBatch: &r53Types.ChangeBatch{Changes: batch}, + } + addCorrection(descBatchStr, req) + } + if err := batcher.Err(); err != nil { + return nil, err + } return corrections, nil + +} + +// reorderInstructions returns changes reordered to comply with AWS's requirements: +// - The R43_ALIAS updates must come after records they refer to. To handle +// this, we simply move all R53_ALIAS instructions to the end of the list, thus +// guaranteeing they will happen after the records they refer to have been +// reated. +func reorderInstructions(changes diff2.ChangeList) diff2.ChangeList { + var main, tail diff2.ChangeList + for _, change := range changes { + if change.Key.Type == "R53_ALIAS" { + tail = append(tail, change) + } else { + main = append(main, change) + } + } + return append(main, tail...) + // NB(tlim): This algorithm is O(n*2) but it is simple and usually only + // operates on very small lists. } func nativeToRecords(set r53Types.ResourceRecordSet, origin string) ([]*models.RecordConfig, error) { @@ -489,6 +603,10 @@ func nativeToRecords(set r53Types.ResourceRecordSet, origin string) ([]*models.R } rc.SetLabelFromFQDN(unescape(set.Name), origin) rc.SetTarget(aws.ToString(set.AliasTarget.DNSName)) + // rc.Original stores a pointer to the original set for use by + // r53Types.ChangeActionDelete and anything else that needs the + // native record verbatim. + rc.Original = set results = append(results, rc) } else if set.TrafficPolicyInstanceId != nil { // skip traffic policy records @@ -535,6 +653,7 @@ func nativeToRecords(set r53Types.ResourceRecordSet, origin string) ([]*models.R if err := rc.PopulateFromString(string(rtype), val, origin); err != nil { return nil, fmt.Errorf("unparsable record received from R53: %w", err) } + rc.Original = set results = append(results, rc) } }