From 747d88ce663593e110d4abbb6392d6b967c0265c Mon Sep 17 00:00:00 2001 From: Bartosz Slawianowski Date: Fri, 10 May 2024 11:05:14 +0200 Subject: [PATCH] Rewrite to new Azure SDK Signed-off-by: Bartosz Slawianowski --- pkg/issuer/acme/dns/azuredns/azuredns.go | 201 +++++++++--------- pkg/issuer/acme/dns/azuredns/azuredns_test.go | 8 +- 2 files changed, 99 insertions(+), 110 deletions(-) diff --git a/pkg/issuer/acme/dns/azuredns/azuredns.go b/pkg/issuer/acme/dns/azuredns/azuredns.go index aca2c92da..f7a1b3a94 100644 --- a/pkg/issuer/acme/dns/azuredns/azuredns.go +++ b/pkg/issuer/acme/dns/azuredns/azuredns.go @@ -27,6 +27,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" dns "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/dns/armdns" + "github.com/aws/smithy-go/ptr" "github.com/go-logr/logr" cmacme "github.com/cert-manager/cert-manager/pkg/apis/acme/v1" @@ -138,57 +139,35 @@ func getAuthorization(clientOpt policy.ClientOptions, clientID, clientSecret, te // Present creates a TXT record using the specified parameters func (c *DNSProvider) Present(ctx context.Context, domain, fqdn, value string) error { - return c.createRecord(ctx, fqdn, value, 60) + return c.updateTXTRecord(ctx, fqdn, func(set *dns.RecordSet) { + var found bool + for _, r := range set.Properties.TxtRecords { + if len(r.Value) > 0 && *r.Value[0] == value { + found = true + break + } + } + + if !found { + set.Properties.TxtRecords = append(set.Properties.TxtRecords, &dns.TxtRecord{ + Value: []*string{ptr.String(value)}, + }) + } + }) } // CleanUp removes the TXT record matching the specified parameters func (c *DNSProvider) CleanUp(ctx context.Context, domain, fqdn, value string) error { - z, err := c.getHostedZoneName(ctx, fqdn) - if err != nil { - c.log.Error(err, "Error getting hosted zone name for fqdn", "fqdn", fqdn) - return err - } + return c.updateTXTRecord(ctx, fqdn, func(set *dns.RecordSet) { + var records []*dns.TxtRecord + for _, r := range set.Properties.TxtRecords { + if len(r.Value) > 0 && *r.Value[0] != value { + records = append(records, r) + } + } - _, err = c.recordClient.Delete( - ctx, - c.resourceGroupName, - z, - c.trimFqdn(fqdn, z), - dns.RecordTypeTXT, nil) - if err != nil { - c.log.Error(err, "Error deleting TXT", "zone", z, "domain", fqdn, "resource group", c.resourceGroupName) - return stabilizeError(err) - } - return nil -} - -func (c *DNSProvider) createRecord(ctx context.Context, fqdn, value string, ttl int) error { - rparams := &dns.RecordSet{ - Properties: &dns.RecordSetProperties{ - TTL: to.Ptr(int64(ttl)), - TxtRecords: []*dns.TxtRecord{ - {Value: []*string{&value}}, - }, - }, - } - - z, err := c.getHostedZoneName(ctx, fqdn) - if err != nil { - return err - } - - _, err = c.recordClient.CreateOrUpdate( - ctx, - c.resourceGroupName, - z, - c.trimFqdn(fqdn, z), - dns.RecordTypeTXT, - *rparams, nil) - if err != nil { - c.log.Error(err, "Error creating TXT", "zone", z, "domain", fqdn, "resource group", c.resourceGroupName) - return stabilizeError(err) - } - return nil + set.Properties.TxtRecords = records + }) } func (c *DNSProvider) getHostedZoneName(ctx context.Context, fqdn string) (string, error) { @@ -220,6 +199,76 @@ func (c *DNSProvider) trimFqdn(fqdn string, zone string) string { return strings.TrimSuffix(strings.TrimSuffix(fqdn, "."), "."+z) } +// Updates or removes DNS TXT record while respecting optimistic concurrency control +func (c *DNSProvider) updateTXTRecord(ctx context.Context, fqdn string, updater func(*dns.RecordSet)) error { + zone, err := c.getHostedZoneName(ctx, fqdn) + if err != nil { + return err + } + + name := c.trimFqdn(fqdn, zone) + + var set *dns.RecordSet + + resp, err := c.recordClient.Get(ctx, c.resourceGroupName, zone, name, dns.RecordTypeTXT, nil) + if err != nil { + var respErr *azcore.ResponseError + if errors.As(err, &respErr); respErr.StatusCode == 404 { + set = &dns.RecordSet{ + Properties: &dns.RecordSetProperties{ + TTL: to.Ptr(int64(60)), + TxtRecords: []*dns.TxtRecord{}, + }, + Etag: to.Ptr(""), + } + } else { + c.log.Error(err, "Error reading TXT", "zone", zone, "domain", fqdn, "resource group", c.resourceGroupName) + return stabilizeError(err) + } + } else { + set = &resp.RecordSet + } + + updater(set) + + if len(set.Properties.TxtRecords) == 0 { + if *set.Etag != "" { + // Etag will cause the deletion to fail if any updates happen concurrently + _, err = c.recordClient.Delete(ctx, c.resourceGroupName, zone, name, dns.RecordTypeTXT, &dns.RecordSetsClientDeleteOptions{IfMatch: set.Etag}) + if err != nil { + c.log.Error(err, "Error deleting TXT", "zone", zone, "domain", fqdn, "resource group", c.resourceGroupName) + return stabilizeError(err) + } + } + + return nil + } + + opts := &dns.RecordSetsClientCreateOrUpdateOptions{} + if *set.Etag == "" { + // This is used to indicate that we want the API call to fail if a conflicting record was created concurrently + // Only relevant when this is a new record, for updates conflicts are solved with Etag + opts.IfNoneMatch = to.Ptr("*") + } else { + opts.IfMatch = set.Etag + } + + _, err = c.recordClient.CreateOrUpdate( + ctx, + c.resourceGroupName, + zone, + name, + dns.RecordTypeTXT, + *set, + opts) + if err != nil { + c.log.Error(err, "Error upserting TXT", "zone", zone, "domain", fqdn, "resource group", c.resourceGroupName) + return stabilizeError(err) + } + + return nil +} + // The azure-sdk library returns the contents of the HTTP requests in its // error messages. We want our error messages to be the same when the cause // is the same to avoid spurious challenge updates. @@ -255,63 +304,3 @@ func stabilizeError(err error) error { return err } - -// Updates or removes DNS TXT record while respecting optimistic concurrency control -func (c *DNSProvider) updateTXTRecord(zone, name string, updater func(*dns.RecordSet)) error { - var set *dns.RecordSet - - resp, err := c.recordClient.Get(context.TODO(), c.resourceGroupName, zone, name, dns.RecordTypeTXT, nil) - if err != nil { - var respErr *azcore.ResponseError - if errors.As(err, &respErr); respErr.StatusCode == 404 { - set = &dns.RecordSet{ - Properties: &dns.RecordSetProperties{ - TTL: to.Ptr(int64(60)), - TxtRecords: []*dns.TxtRecord{}, - }, - Etag: to.Ptr(""), - } - } else { - return fmt.Errorf("cannot get DNS record set: %w", err) - } - } else { - set = &resp.RecordSet - } - - updater(set) - - if len(set.Properties.TxtRecords) == 0 { - if *set.Etag != "" { - // Etag will cause the deletion to fail if any updates happen concurrently - _, err = c.recordClient.Delete(context.TODO(), c.resourceGroupName, zone, name, dns.RecordTypeTXT, &dns.RecordSetsClientDeleteOptions{IfMatch: set.Etag}) - if err != nil { - return fmt.Errorf("cannot delete DNS record set: %w", err) - } - } - - return nil - } - - opts := &dns.RecordSetsClientCreateOrUpdateOptions{} - if *set.Etag == "" { - // This is used to indicate that we want the API call to fail if a conflicting record was created concurrently - // Only relevant when this is a new record, for updates conflicts are solved with Etag - opts.IfNoneMatch = to.Ptr("*") - } else { - opts.IfMatch = set.Etag - } - - _, err = c.recordClient.CreateOrUpdate( - context.TODO(), - c.resourceGroupName, - zone, - name, - dns.RecordTypeTXT, - *set, - opts) - if err != nil { - return fmt.Errorf("cannot upsert DNS record set: %w", err) - } - - return nil -} diff --git a/pkg/issuer/acme/dns/azuredns/azuredns_test.go b/pkg/issuer/acme/dns/azuredns/azuredns_test.go index c8d9f85cb..0cd7c0c31 100644 --- a/pkg/issuer/acme/dns/azuredns/azuredns_test.go +++ b/pkg/issuer/acme/dns/azuredns/azuredns_test.go @@ -76,9 +76,9 @@ func TestLiveAzureDnsPresentMultiple(t *testing.T) { provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, &v1.AzureManagedIdentity{}) assert.NoError(t, err) - err = provider.Present(azureDomain, "_acme-challenge."+azureDomain+".", "123d==") + err = provider.Present(context.TODO(), azureDomain, "_acme-challenge."+azureDomain+".", "123d==") assert.NoError(t, err) - err = provider.Present(azureDomain, "_acme-challenge."+azureDomain+".", "1123d==") + err = provider.Present(context.TODO(), azureDomain, "_acme-challenge."+azureDomain+".", "1123d==") assert.NoError(t, err) } @@ -106,9 +106,9 @@ func TestLiveAzureDnsCleanUpMultiple(t *testing.T) { provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, &v1.AzureManagedIdentity{}) assert.NoError(t, err) - err = provider.CleanUp(azureDomain, "_acme-challenge."+azureDomain+".", "123d==") + err = provider.CleanUp(context.TODO(), azureDomain, "_acme-challenge."+azureDomain+".", "123d==") assert.NoError(t, err) - err = provider.CleanUp(azureDomain, "_acme-challenge."+azureDomain+".", "1123d==") + err = provider.CleanUp(context.TODO(), azureDomain, "_acme-challenge."+azureDomain+".", "1123d==") assert.NoError(t, err) }