Merge pull request #88 from zalando-incubator/bugfix/safe-resource-re…

…cords Safely handle records without ResourceRecords
linki · Feb 28, 2017 · b130c56 · b130c56
2 parents c1eda17 + dae32df
commit b130c56
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 5 deletions.
diff --git a/consumers/aws.go b/consumers/aws.go
@@ -260,20 +260,30 @@ func (a *awsConsumer) getAssignedTXTRecordObject(aliasRecord *route53.ResourceRe
 	}
 }
 
-//recordInfo returns the map of record assigned dns to its target and groupID (can be empty)
-func (a *awsConsumer) recordInfo(records []*route53.ResourceRecordSet) map[string]*pkg.RecordInfo {
+//groupIDInfo builds a map from dns name to its group ID
+func (a *awsConsumer) groupIDInfo(records []*route53.ResourceRecordSet) map[string]string {
 	groupIDMap := map[string]string{} //maps dns to group ID
 
 	for _, record := range records {
-		if (aws.StringValue(record.Type)) == "TXT" {
-			groupIDMap[aws.StringValue(record.Name)] = aws.StringValue(record.ResourceRecords[0].Value)
+		if aws.StringValue(record.Type) == "TXT" {
+			if len(record.ResourceRecords) > 0 {
+				groupIDMap[aws.StringValue(record.Name)] = aws.StringValue(record.ResourceRecords[0].Value)
+			} else {
+				log.Errorf("Unexpected response from AWS API, got TXT record with empty resources: %s. Record is excluded from syncing", aws.StringValue(record.Name))
+				groupIDMap[aws.StringValue(record.Name)] = ""
+			}
 		} else {
 			if _, exist := groupIDMap[aws.StringValue(record.Name)]; !exist {
 				groupIDMap[aws.StringValue(record.Name)] = ""
 			}
 		}
 	}
+	return groupIDMap
+}
 
+//recordInfo returns the map of record assigned dns to its target and groupID (can be empty string)
+func (a *awsConsumer) recordInfo(records []*route53.ResourceRecordSet) map[string]*pkg.RecordInfo {
+	groupIDMap := a.groupIDInfo(records)
 	infoMap := map[string]*pkg.RecordInfo{} //maps record DNS to its GroupID (if exists) and Target (LB)
 	for _, record := range records {
 		groupID := groupIDMap[aws.StringValue(record.Name)]

diff --git a/consumers/aws_test.go b/consumers/aws_test.go
@@ -127,6 +127,84 @@ func sameTargets(lb1, lb2 string) bool {
 	return lb1 == lb2
 }
 
+func TestGroupIDInfo(t *testing.T) {
+	groupID := "test"
+	client := &awsConsumer{
+		groupID: groupID,
+	}
+	records := []*route53.ResourceRecordSet{
+		&route53.ResourceRecordSet{
+			Type: aws.String("A"),
+			Name: aws.String("test.example.com."),
+			AliasTarget: &route53.AliasTarget{
+				DNSName:      aws.String("abc.def.ghi."),
+				HostedZoneId: aws.String("123"),
+			},
+		},
+		&route53.ResourceRecordSet{
+			Type: aws.String("TXT"),
+			Name: aws.String("test.example.com."),
+			ResourceRecords: []*route53.ResourceRecord{
+				&route53.ResourceRecord{
+					Value: aws.String(client.getGroupID()),
+				},
+			},
+		},
+		&route53.ResourceRecordSet{
+			Type:            aws.String("TXT"),
+			Name:            aws.String("unusual-1.example.com."),
+			ResourceRecords: nil,
+		},
+		&route53.ResourceRecordSet{
+			Type:            aws.String("TXT"),
+			Name:            aws.String("unusual-2.example.com."),
+			ResourceRecords: []*route53.ResourceRecord{},
+		},
+	}
+	groupIDInfoMap := client.groupIDInfo(records)
+	if len(groupIDInfoMap) != 3 {
+		t.Errorf("Incorrect record info for %v", records)
+	}
+	if val, exist := groupIDInfoMap["test.example.com."]; !exist {
+		t.Errorf("Incorrect record info for %v", records)
+	} else {
+		if val != client.getGroupID() {
+			t.Errorf("Incorrect record info for %v", records)
+		}
+	}
+	records = []*route53.ResourceRecordSet{
+		&route53.ResourceRecordSet{
+			Type: aws.String("A"),
+			Name: aws.String("test.example.com."),
+			ResourceRecords: []*route53.ResourceRecord{
+				&route53.ResourceRecord{
+					Value: aws.String("54.32.12.32"),
+				},
+			},
+		},
+		&route53.ResourceRecordSet{
+			Type: aws.String("TXT"),
+			Name: aws.String("test.example.com."),
+			ResourceRecords: []*route53.ResourceRecord{
+				&route53.ResourceRecord{
+					Value: aws.String(client.getGroupID()),
+				},
+			},
+		},
+	}
+	groupIDInfoMap = client.groupIDInfo(records)
+	if len(groupIDInfoMap) != 1 {
+		t.Errorf("Incorrect record info for %v", records)
+	}
+	if val, exist := groupIDInfoMap["test.example.com."]; !exist {
+		t.Errorf("Incorrect record info for %v", records)
+	} else {
+		if val != client.getGroupID() {
+			t.Errorf("Incorrect record info for %v", records)
+		}
+	}
+}
+
 func TestRecordInfo(t *testing.T) {
 	groupID := "test"
 	client := &awsConsumer{
@@ -150,9 +228,19 @@ func TestRecordInfo(t *testing.T) {
 				},
 			},
 		},
+		&route53.ResourceRecordSet{
+			Type:            aws.String("TXT"),
+			Name:            aws.String("unusual-1.example.com."),
+			ResourceRecords: nil,
+		},
+		&route53.ResourceRecordSet{
+			Type:            aws.String("TXT"),
+			Name:            aws.String("unusual-2.example.com."),
+			ResourceRecords: []*route53.ResourceRecord{},
+		},
 	}
 	recordInfoMap := client.recordInfo(records)
-	if len(recordInfoMap) != 1 {
+	if len(recordInfoMap) != 3 {
 		t.Errorf("Incorrect record info for %v", records)
 	}
 	if val, exist := recordInfoMap["test.example.com."]; !exist {