TTL and value for CNAME record does not get updated while creating a DNS zone record set

[kirankumarbv]

Hi,

I referenced one of the DNS examples to create a CNAME record using SDK. It used to work perfectly in the pre 14 versions. But after the recent SDK change, I am hitting an error where the record created in a hosted zone does not have TTL or value information even though I send it through my CreateOrUpdate parameter.
Below is the snippet I use to create the record.

`cnameRecordParams := &dns.RecordSet{
ID: &name,
RecordSetProperties: &dns.RecordSetProperties{
TTL: &ttl,
CnameRecord: &dns.CnameRecord{
Cname: &value,
},
},
}

	_, err = DNS.DnsRrsClient.CreateOrUpdate(ctx, resGrp, zoneId, name, rrsType, *cnameRecordParams, "", "")

`

ttl and value are of string data type. I went inside the CreateOrUpdate method and all the arguments are being passed properly.

Please let me know if I am missing something or if this is a bug?

[jhendrixMSFT]

In models.go could you try deleting the JSON marshaler for RecordSetProperties and see if the problem goes away?
https://github.com/Azure/azure-sdk-for-go/blob/master/services/dns/mgmt/2017-09-01/dns/models.go#L328

[jhendrixMSFT]

Assigning to Vlad as I think this is an issue with generating spurious marshalers/unmarshalers.

[kirankumarbv]

Hi @jhendrixMSFT It worked as expected when I removed JSON marshaler for RecordSetProperties.

[vladbarosan]

@kirankumarbv I think i know whats the issue can you try keeping it as it was originally ( without the change Joel mentioned )and add this to (https://github.com/Azure/azure-sdk-for-go/blob/master/services/dns/mgmt/2017-09-01/dns/models.go#L328):

// MarshalJSON is the custom marshaler for RecordSet.
func (rs RecordSet) MarshalJSON() ([]byte, error) {
	objectMap := make(map[string]interface{})
	if rs.ID != nil {
		objectMap["id"] = rs.ID
	}
	if rs.Name != nil {
		objectMap["name"] = rs.Name
	}
	if rs.Type != nil {
		objectMap["type"] = rs.Type
	}
	if rs.Etag != nil {
		objectMap["etag"] = rs.Etag
	}
	if rs.RecordSetProperties != nil {
		objectMap["properties"] = rs.RecordSetProperties
	}
	return json.Marshal(objectMap)
}

[kirankumarbv]

I apologize for the late reply. Yes it works with the changes you suggested.

[vladbarosan]

@kirankumarbv we will be releasing a fix soon.

[kirankumarbv]

@vladbarosan Thanks

[tombuildsstuff]

@vladbarosan any idea when this fix will be available? Looking at the HTTP responses it appears this is due to the properties block being omited.

Here's the Go SDK code we're using:

parameters := dns.RecordSet{
	Name: &name,
	RecordSetProperties: &dns.RecordSetProperties{
		Metadata: expandTags(tags),
		TTL:      &ttl,
		ARecords: &records,
	},
}

eTag := ""
ifNoneMatch := "" // set to empty to allow updates to records after creation
resp, err := dnsClient.CreateOrUpdate(ctx, resGroup, zoneName, name, dns.A, parameters, eTag, ifNoneMatch)
if err != nil {
	return err
}

and the Request sent using v12.5.0-beta of the Azure SDK / v9.7.0 of go-autorest:

{
	"name": "internal",
	"properties": {
		"metadata": {
			"environment": "staging"
		},
		"TTL": 300,
		"ARecords": [{
			"ipv4Address": "1.2.4.5"
		}, {
			"ipv4Address": "1.2.3.4"
		}]
	}
}

and the Request sent via SDK v14.5.0 / go-autorest v10.2:

{
	"ARecords": [{
		"ipv4Address": "1.2.4.5"
	}, {
		"ipv4Address": "1.2.3.4"
	}],
	"TTL": 300,
	"metadata": {
		"environment": "staging"
	}
}

This behaviour seems to affect both the 2016-04-01 and the 2017-09-01 API endpoints, FWIW.

[vladbarosan]

Hi @tombuildsstuff in 14.5 we introduced a new API version for DNS which has the fix. ( https://github.com/Azure/azure-sdk-for-go/tree/master/services/preview/dns/mgmt/2018-03-01-preview/dns)

Does it work for you to use the new API version ?

[tombuildsstuff]

@vladbarosan cool, thanks for confirming that, it does - is there a timeline to roll this fix out to the stable API versions too? I'd rather we didn't roll out a Preview API unless we need something contained within the Preview version if possible.

Thanks!

[jhendrixMSFT]

@tombuildsstuff Unfortunately there were a few breaking changes in the DNS swaggers so this will have to wait for the v15 release which is currently planned for next week. Does that work for you?

[tombuildsstuff]

@jhendrixMSFT we're happy to wait for it, but given this SDK is now GA (and this functionality is currently broken) I think we could probably include a breaking change in a new version (e.g. v14.6.1) using the existing Swagger file?

[jhendrixMSFT]

@tombuildsstuff So this is something we've discussed internally, i.e. is introducing a breaking change that fixes a broken package really a breaking change. We've been leaning towards not counting it as a breaking change, the only problem is there is no way for us to automatically differentiate the two cases (we're working on tooling to automatically flow non-breaking changes from latest into master). Perhaps though since this case is rare it doesn't really matter and we can flow such changes by hand as required. If you folks are ok with introducing breaking changes in a patch revision to fix a broken package then we can probably go with that (I will probably run this by our partners in kubernates as well so we're all on the same page).

[tombuildsstuff]

@jhendrixMSFT so I'm fine with that in a minor version, but I can understand why others could not be :)

From our side, at this point we've got 4 blockers for upgrading to SDKv14: hashicorp/terraform-provider-azurerm#1006. As such I don't believe we'll get to upgrading until next week either way, if it helps?

[jhendrixMSFT]

Fixed in https://github.com/Azure/azure-sdk-for-go/releases/tag/v15.0.0