Skip to content

Commit

Permalink
Rewrite to new Azure SDK
Browse files Browse the repository at this point in the history
  • Loading branch information
eplightning committed May 10, 2024
1 parent 53f73d5 commit 8bb7273
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 91 deletions.
163 changes: 76 additions & 87 deletions pkg/issuer/acme/dns/azuredns/azuredns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -220,47 +199,18 @@ func (c *DNSProvider) trimFqdn(fqdn string, zone string) string {
return strings.TrimSuffix(strings.TrimSuffix(fqdn, "."), "."+z)
}

// 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.
//
// The given error must not be nil. This function must be called everywhere
// we have a non-nil error coming from a azure-sdk func that makes API calls.
func stabilizeError(err error) error {
if err == nil {
return nil
}

redactResponse := func(resp *http.Response) *http.Response {
if resp == nil {
return nil
}

response := *resp
response.Body = io.NopCloser(bytes.NewReader([]byte("<REDACTED>")))
return &response
}

var authErr *azidentity.AuthenticationFailedError
if errors.As(err, &authErr) {
//nolint: bodyclose // False positive, this already a processed body, probably just pointing to a buffer.
authErr.RawResponse = redactResponse(authErr.RawResponse)
}

var respErr *azcore.ResponseError
if errors.As(err, &respErr) {
//nolint: bodyclose // False positive, this already a processed body, probably just pointing to a buffer.
respErr.RawResponse = redactResponse(respErr.RawResponse)
// 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
}

return err
}
name := c.trimFqdn(fqdn, zone)

// 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)
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 {
Expand All @@ -272,7 +222,8 @@ func (c *DNSProvider) updateTXTRecord(zone, name string, updater func(*dns.Recor
Etag: to.Ptr(""),
}
} else {
return fmt.Errorf("cannot get DNS record set: %w", err)
c.log.Error(err, "Error reading TXT", "zone", zone, "domain", fqdn, "resource group", c.resourceGroupName)
return stabilizeError(err)
}
} else {
set = &resp.RecordSet
Expand All @@ -283,9 +234,10 @@ func (c *DNSProvider) updateTXTRecord(zone, name string, updater func(*dns.Recor
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})
_, err = c.recordClient.Delete(ctx, c.resourceGroupName, zone, name, dns.RecordTypeTXT, &dns.RecordSetsClientDeleteOptions{IfMatch: set.Etag})
if err != nil {
return fmt.Errorf("cannot delete DNS record set: %w", err)
c.log.Error(err, "Error deleting TXT", "zone", zone, "domain", fqdn, "resource group", c.resourceGroupName)
return stabilizeError(err)
}
}

Expand All @@ -302,16 +254,53 @@ func (c *DNSProvider) updateTXTRecord(zone, name string, updater func(*dns.Recor
}

_, err = c.recordClient.CreateOrUpdate(
context.TODO(),
ctx,
c.resourceGroupName,
zone,
name,
dns.RecordTypeTXT,
*set,
opts)
if err != nil {
return fmt.Errorf("cannot upsert DNS record set: %w", err)
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.
//
// The given error must not be nil. This function must be called everywhere
// we have a non-nil error coming from a azure-sdk func that makes API calls.
func stabilizeError(err error) error {
if err == nil {
return nil
}

redactResponse := func(resp *http.Response) *http.Response {
if resp == nil {
return nil
}

response := *resp
response.Body = io.NopCloser(bytes.NewReader([]byte("<REDACTED>")))
return &response
}

var authErr *azidentity.AuthenticationFailedError
if errors.As(err, &authErr) {
//nolint: bodyclose // False positive, this already a processed body, probably just pointing to a buffer.
authErr.RawResponse = redactResponse(authErr.RawResponse)
}

var respErr *azcore.ResponseError
if errors.As(err, &respErr) {
//nolint: bodyclose // False positive, this already a processed body, probably just pointing to a buffer.
respErr.RawResponse = redactResponse(respErr.RawResponse)
}

return err
}
8 changes: 4 additions & 4 deletions pkg/issuer/acme/dns/azuredns/azuredns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit 8bb7273

Please sign in to comment.