Check record type when upserting.

Previously this was not checked so an existing record (e.g. with type A) would be overwritten on upsert by a record with the same name but different type (e.g. TXT).

This commit also:

* publicizes the type variable appending the underscore affix (required to maintain compatibility with CloudFormation which sets type as the CF type),
* fixes a wrong assumption in tests that UPSERT applies a change to Type (it creates a distinct record instead),
* Updates ACM model to use serial_number instead of deprecated and remove serial causing Travis failures.

moto/acm/models.py

@@ -243,7 +243,7 @@ class CertBundle(BaseModel):
                 'KeyAlgorithm': key_algo,
                 'NotAfter': datetime_to_epoch(self._cert.not_valid_after),
                 'NotBefore': datetime_to_epoch(self._cert.not_valid_before),
-                'Serial': self._cert.serial,
+                'Serial': self._cert.serial_number,
                 'SignatureAlgorithm': self._cert.signature_algorithm_oid._name.upper().replace('ENCRYPTION', ''),
                 'Status': self.status,  # One of PENDING_VALIDATION, ISSUED, INACTIVE, EXPIRED, VALIDATION_TIMED_OUT, REVOKED, FAILED.
                 'Subject': 'CN={0}'.format(self.common_name),

moto/route53/models.py

@@ -24,7 +24,7 @@ class HealthCheck(BaseModel):
         self.id = health_check_id
         self.ip_address = health_check_args.get("ip_address")
         self.port = health_check_args.get("port", 80)
-        self._type = health_check_args.get("type")
+        self.type_ = health_check_args.get("type")
         self.resource_path = health_check_args.get("resource_path")
         self.fqdn = health_check_args.get("fqdn")
         self.search_string = health_check_args.get("search_string")
@@ -58,7 +58,7 @@ class HealthCheck(BaseModel):
             <HealthCheckConfig>
                 <IPAddress>{{ health_check.ip_address }}</IPAddress>
                 <Port>{{ health_check.port }}</Port>
-                <Type>{{ health_check._type }}</Type>
+                <Type>{{ health_check.type_ }}</Type>
                 <ResourcePath>{{ health_check.resource_path }}</ResourcePath>
                 <FullyQualifiedDomainName>{{ health_check.fqdn }}</FullyQualifiedDomainName>
                 <RequestInterval>{{ health_check.request_interval }}</RequestInterval>
@@ -76,7 +76,7 @@ class RecordSet(BaseModel):
 
     def __init__(self, kwargs):
         self.name = kwargs.get('Name')
-        self._type = kwargs.get('Type')
+        self.type_ = kwargs.get('Type')
         self.ttl = kwargs.get('TTL')
         self.records = kwargs.get('ResourceRecords', [])
         self.set_identifier = kwargs.get('SetIdentifier')
@@ -130,7 +130,7 @@ class RecordSet(BaseModel):
     def to_xml(self):
         template = Template("""<ResourceRecordSet>
                 <Name>{{ record_set.name }}</Name>
-                <Type>{{ record_set._type }}</Type>
+                <Type>{{ record_set.type_ }}</Type>
                 {% if record_set.set_identifier %}
                     <SetIdentifier>{{ record_set.set_identifier }}</SetIdentifier>
                 {% endif %}
@@ -183,7 +183,7 @@ class FakeZone(BaseModel):
     def upsert_rrset(self, record_set):
         new_rrset = RecordSet(record_set)
         for i, rrset in enumerate(self.rrsets):
-            if rrset.name == new_rrset.name:
+            if rrset.name == new_rrset.name and rrset.type_ == new_rrset.type_:
                 self.rrsets[i] = new_rrset
                 break
         else:
@@ -202,7 +202,7 @@ class FakeZone(BaseModel):
         record_sets = list(self.rrsets)  # Copy the list
         if start_type:
             record_sets = [
-                record_set for record_set in record_sets if record_set._type >= start_type]
+                record_set for record_set in record_sets if record_set.type_ >= start_type]
         if start_name:
             record_sets = [
                 record_set for record_set in record_sets if record_set.name >= start_name]

tests/test_route53/test_route53.py

@@ -98,6 +98,16 @@ def test_rrset():
     rrsets.should.have.length_of(1)
     rrsets[0].resource_records[0].should.equal('5.6.7.8')
 
+    changes = ResourceRecordSets(conn, zoneid)
+    change = changes.add_change("UPSERT", "foo.bar.testdns.aws.com", "TXT")
+    change.add_value("foo")
+    changes.commit()
+
+    rrsets = conn.get_all_rrsets(zoneid)
+    rrsets.should.have.length_of(2)
+    rrsets[0].resource_records[0].should.equal('5.6.7.8')
+    rrsets[1].resource_records[0].should.equal('foo')
+
     changes = ResourceRecordSets(conn, zoneid)
     changes.add_change("DELETE", "foo.bar.testdns.aws.com", "A")
     changes.commit()
@@ -520,7 +530,7 @@ def test_change_resource_record_sets_crud_valid():
 
     # Create A Record.
     a_record_endpoint_payload = {
-        'Comment': 'create A record prod.redis.db',
+        'Comment': 'Create A record prod.redis.db',
         'Changes': [
             {
                 'Action': 'CREATE',
@@ -545,15 +555,15 @@ def test_change_resource_record_sets_crud_valid():
     a_record_detail['TTL'].should.equal(10)
     a_record_detail['ResourceRecords'].should.equal([{'Value': '127.0.0.1'}])
 
-    # Update type to CNAME
+    # Update A Record.
     cname_record_endpoint_payload = {
-        'Comment': 'Update to CNAME prod.redis.db',
+        'Comment': 'Update A record prod.redis.db',
         'Changes': [
             {
                 'Action': 'UPSERT',
                 'ResourceRecordSet': {
                     'Name': 'prod.redis.db.',
-                    'Type': 'CNAME',
+                    'Type': 'A',
                     'TTL': 60,
                     'ResourceRecords': [{
                         'Value': '192.168.1.1'
@@ -568,7 +578,7 @@ def test_change_resource_record_sets_crud_valid():
     len(response['ResourceRecordSets']).should.equal(1)
     cname_record_detail = response['ResourceRecordSets'][0]
     cname_record_detail['Name'].should.equal('prod.redis.db.')
-    cname_record_detail['Type'].should.equal('CNAME')
+    cname_record_detail['Type'].should.equal('A')
     cname_record_detail['TTL'].should.equal(60)
     cname_record_detail['ResourceRecords'].should.equal([{'Value': '192.168.1.1'}])