plugin/azure: clean up readme (#3102)

document the environment option and some cleanups. Go over the code and
fix/tweak random bits here and there.

Condense a few lines here and there.

Signed-off-by: Miek Gieben <miek@miek.nl>

plugin/azure/README.md

@@ -6,9 +6,9 @@
 
 ## Description
 
-The azure plugin is useful for serving zones from Microsoft Azure DNS.
+The azure plugin is useful for serving zones from Microsoft Azure DNS. The *azure* plugin supports
-Thi *azure* plugin supports all the DNS records supported by Azure, viz. A, AAAA, CAA, CNAME, MX, NS, PTR, SOA, SRV, and TXT record types. For a non-existing resource record, zone's SOA response will returned.
+all the DNS records supported by Azure, viz. A, AAAA, CNAME, MX, NS, PTR, SOA, SRV, and TXT
-
+record types.
 
 ## Syntax
 
@@ -18,35 +18,39 @@ azure RESOURCE_GROUP:ZONE... {
     client CLIENT_ID
     secret CLIENT_SECRET
     subscription SUBSCRIPTION_ID
+    environment ENVIRONMENT
+    fallthrough [ZONES...]
 }
 ~~~
 
-*   **`RESOURCE_GROUP`** The resource group to which the dns hosted zones belong on Azure
+*   **RESOURCE_GROUP:ZONE** is the resource group to which the hosted zones belongs on Azure,
+    and  **ZONE** the zone that contains data.
 
-*   **`ZONE`** the zone that contains the resource record sets to be
+*   **CLIENT_ID** and **CLIENT_SECRET** are the credentials for Azure, and `tenant` specifies the
-    accessed.
+    **TENANT_ID** to be used. **SUBSCRIPTION_ID** is the subscription ID. All of these are needed
+    to access the data in Azure.
+
+*  `environment` specifies the Azure **ENVIRONMENT**.
 
 *   `fallthrough` If zone matches and no record can be generated, pass request to the next plugin.
     If **ZONES** is omitted, then fallthrough happens for all zones for which the plugin is
-    authoritative. If specific zones are listed (for example `in-addr.arpa` and `ip6.arpa`), then
+    authoritative.
-    only queries for those zones will be subject to fallthrough.
-
-*   `environment` the azure environment to use. Defaults to `AzurePublicCloud`. Possible values: `AzureChinaCloud`, `AzureGermanCloud`, `AzurePublicCloud`, `AzureUSGovernmentCloud`.
 
 ## Examples
 
-Enable the *azure* plugin with Azure credentials:
+Enable the *azure* plugin with Azure credentials for the zone `example.org`:
 
 ~~~ txt
-. {
+example.org {
-    azure resource_group_foo:foo.com {
+    azure resource_group_foo:example.org {
       tenant 123abc-123abc-123abc-123abc
-      client 123abc-123abc-123abc-123abc
+      client 123abc-123abc-123abc-234xyz
-      secret 123abc-123abc-123abc-123abc
+      subscription 123abc-123abc-123abc-563abc
-      subscription 123abc-123abc-123abc-123abc
+      secret mysecret
     }
 }
 ~~~
 
 ## Also See
-- [Azure DNS Overview](https://docs.microsoft.com/en-us/azure/dns/dns-overview)
+
+The [Azure DNS Overview](https://docs.microsoft.com/en-us/azure/dns/dns-overview).

plugin/azure/azure.go

@@ -27,38 +27,39 @@ type zones map[string][]*zone
 
 // Azure is the core struct of the azure plugin.
 type Azure struct {
-	Next      plugin.Handler
-	Fall      fall.F
 	zoneNames []string
 	client    azuredns.RecordSetsClient
 	upstream  *upstream.Upstream
 	zMu       sync.RWMutex
 	zones     zones
+
+	Next plugin.Handler
+	Fall fall.F
 }
 
 // New validates the input DNS zones and initializes the Azure struct.
-func New(ctx context.Context, dnsClient azuredns.RecordSetsClient, keys map[string][]string, up *upstream.Upstream) (*Azure, error) {
+func New(ctx context.Context, dnsClient azuredns.RecordSetsClient, keys map[string][]string) (*Azure, error) {
 	zones := make(map[string][]*zone, len(keys))
-	zoneNames := make([]string, 0, len(keys))
+	names := make([]string, len(keys))
-	for resourceGroup, inputZoneNames := range keys {
+
-		for _, zoneName := range inputZoneNames {
+	for resourceGroup, znames := range keys {
-			_, err := dnsClient.ListAllByDNSZone(context.Background(), resourceGroup, zoneName, nil, "")
+		for _, name := range znames {
-			if err != nil {
+			if _, err := dnsClient.ListAllByDNSZone(context.Background(), resourceGroup, name, nil, ""); err != nil {
 				return nil, err
 			}
-			// Normalizing zoneName to make it fqdn if required.
+
-			zoneNameFQDN := dns.Fqdn(zoneName)
+			fqdn := dns.Fqdn(name)
-			if _, ok := zones[zoneNameFQDN]; !ok {
+			if _, ok := zones[fqdn]; !ok {
-				zoneNames = append(zoneNames, zoneNameFQDN)
+				names = append(names, fqdn)
 			}
-			zones[zoneNameFQDN] = append(zones[zoneNameFQDN], &zone{id: resourceGroup, zone: zoneName, z: file.NewZone(zoneName, "")})
+			zones[fqdn] = append(zones[fqdn], &zone{id: resourceGroup, zone: fqdn, z: file.NewZone(fqdn, "")})
 		}
 	}
 	return &Azure{
 		client:    dnsClient,
 		zones:     zones,
-		zoneNames: zoneNames,
+		zoneNames: names,
-		upstream:  up,
+		upstream:  upstream.New(),
 	}, nil
 }
 
@@ -114,107 +115,67 @@ func updateZoneFromResourceSet(recordSet azuredns.RecordSetListResultPage, zName
 		resultTTL := uint32(*(result.RecordSetProperties.TTL))
 		if result.RecordSetProperties.ARecords != nil {
 			for _, A := range *(result.RecordSetProperties.ARecords) {
-				a := dns.A{
+				a := &dns.A{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: resultTTL},
-					Hdr: dns.RR_Header{
-						Name:   resultFqdn,
-						Rrtype: dns.TypeA,
-						Class:  dns.ClassINET,
-						Ttl:    resultTTL},
 					A: net.ParseIP(*(A.Ipv4Address))}
-				newZ.Insert(&a)
+				newZ.Insert(a)
 			}
 		}
 
 		if result.RecordSetProperties.AaaaRecords != nil {
 			for _, AAAA := range *(result.RecordSetProperties.AaaaRecords) {
-				aaaa := dns.AAAA{
+				aaaa := &dns.AAAA{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeAAAA, Class: dns.ClassINET, Ttl: resultTTL},
-					Hdr: dns.RR_Header{
-						Name:   resultFqdn,
-						Rrtype: dns.TypeAAAA,
-						Class:  dns.ClassINET,
-						Ttl:    resultTTL},
 					AAAA: net.ParseIP(*(AAAA.Ipv6Address))}
-				newZ.Insert(&aaaa)
+				newZ.Insert(aaaa)
 			}
 		}
 
 		if result.RecordSetProperties.MxRecords != nil {
 			for _, MX := range *(result.RecordSetProperties.MxRecords) {
-				mx := dns.MX{
+				mx := &dns.MX{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeMX, Class: dns.ClassINET, Ttl: resultTTL},
-					Hdr: dns.RR_Header{
-						Name:   resultFqdn,
-						Rrtype: dns.TypeMX,
-						Class:  dns.ClassINET,
-						Ttl:    resultTTL},
 					Preference: uint16(*(MX.Preference)),
 					Mx:         dns.Fqdn(*(MX.Exchange))}
-				newZ.Insert(&mx)
+				newZ.Insert(mx)
 			}
 		}
 
 		if result.RecordSetProperties.PtrRecords != nil {
 			for _, PTR := range *(result.RecordSetProperties.PtrRecords) {
-				ptr := dns.PTR{
+				ptr := &dns.PTR{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypePTR, Class: dns.ClassINET, Ttl: resultTTL},
-					Hdr: dns.RR_Header{
-						Name:   resultFqdn,
-						Rrtype: dns.TypePTR,
-						Class:  dns.ClassINET,
-						Ttl:    resultTTL},
 					Ptr: dns.Fqdn(*(PTR.Ptrdname))}
-				newZ.Insert(&ptr)
+				newZ.Insert(ptr)
 			}
 		}
 
 		if result.RecordSetProperties.SrvRecords != nil {
 			for _, SRV := range *(result.RecordSetProperties.SrvRecords) {
-				srv := dns.SRV{
+				srv := &dns.SRV{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeSRV, Class: dns.ClassINET, Ttl: resultTTL},
-					Hdr: dns.RR_Header{
-						Name:   resultFqdn,
-						Rrtype: dns.TypeSRV,
-						Class:  dns.ClassINET,
-						Ttl:    resultTTL},
 					Priority: uint16(*(SRV.Priority)),
 					Weight:   uint16(*(SRV.Weight)),
 					Port:     uint16(*(SRV.Port)),
 					Target:   dns.Fqdn(*(SRV.Target))}
-				newZ.Insert(&srv)
+				newZ.Insert(srv)
 			}
 		}
 
 		if result.RecordSetProperties.TxtRecords != nil {
 			for _, TXT := range *(result.RecordSetProperties.TxtRecords) {
-				txt := dns.TXT{
+				txt := &dns.TXT{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeTXT, Class: dns.ClassINET, Ttl: resultTTL},
-					Hdr: dns.RR_Header{
-						Name:   resultFqdn,
-						Rrtype: dns.TypeTXT,
-						Class:  dns.ClassINET,
-						Ttl:    resultTTL},
 					Txt: *(TXT.Value)}
-				newZ.Insert(&txt)
+				newZ.Insert(txt)
 			}
 		}
 
 		if result.RecordSetProperties.NsRecords != nil {
 			for _, NS := range *(result.RecordSetProperties.NsRecords) {
-				ns := dns.NS{
+				ns := &dns.NS{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeNS, Class: dns.ClassINET, Ttl: resultTTL},
-					Hdr: dns.RR_Header{
-						Name:   resultFqdn,
-						Rrtype: dns.TypeNS,
-						Class:  dns.ClassINET,
-						Ttl:    resultTTL},
 					Ns: *(NS.Nsdname)}
-				newZ.Insert(&ns)
+				newZ.Insert(ns)
 			}
 		}
 
 		if result.RecordSetProperties.SoaRecord != nil {
 			SOA := result.RecordSetProperties.SoaRecord
-			soa := dns.SOA{
+			soa := &dns.SOA{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: resultTTL},
-				Hdr: dns.RR_Header{
-					Name:   resultFqdn,
-					Rrtype: dns.TypeSOA,
-					Class:  dns.ClassINET,
-					Ttl:    resultTTL},
 				Minttl:  uint32(*(SOA.MinimumTTL)),
 				Expire:  uint32(*(SOA.ExpireTime)),
 				Retry:   uint32(*(SOA.RetryTime)),
@@ -222,19 +183,14 @@ func updateZoneFromResourceSet(recordSet azuredns.RecordSetListResultPage, zName
 				Serial:  uint32(*(SOA.SerialNumber)),
 				Mbox:    dns.Fqdn(*(SOA.Email)),
 				Ns:      *(SOA.Host)}
-			newZ.Insert(&soa)
+			newZ.Insert(soa)
 		}
 
 		if result.RecordSetProperties.CnameRecord != nil {
 			CNAME := result.RecordSetProperties.CnameRecord.Cname
-			cname := dns.CNAME{
+			cname := &dns.CNAME{Hdr: dns.RR_Header{Name: resultFqdn, Rrtype: dns.TypeCNAME, Class: dns.ClassINET, Ttl: resultTTL},
-				Hdr: dns.RR_Header{
-					Name:   resultFqdn,
-					Rrtype: dns.TypeCNAME,
-					Class:  dns.ClassINET,
-					Ttl:    resultTTL},
 				Target: dns.Fqdn(*CNAME)}
-			newZ.Insert(&cname)
+			newZ.Insert(cname)
 		}
 	}
 	return newZ
@@ -245,13 +201,13 @@ func (h *Azure) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	state := request.Request{W: w, Req: r}
 	qname := state.Name()
 
-	zName := plugin.Zones(h.zoneNames).Matches(qname)
+	zone := plugin.Zones(h.zoneNames).Matches(qname)
-	if zName == "" {
+	if zone == "" {
 		return plugin.NextOrFailure(h.Name(), h.Next, ctx, w, r)
 	}
 
-	z, ok := h.zones[zName] // ok true if we are authoritive for the zone.
+	zones, ok := h.zones[zone] // ok true if we are authoritive for the zone.
-	if !ok || z == nil {
+	if !ok || zones == nil {
 		return dns.RcodeServerFailure, nil
 	}
 
@@ -259,9 +215,9 @@ func (h *Azure) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	m.SetReply(r)
 	m.Authoritative = true
 	var result file.Result
-	for _, hostedZone := range z {
+	for _, z := range zones {
 		h.zMu.RLock()
-		m.Answer, m.Ns, m.Extra, result = hostedZone.z.Lookup(ctx, state, qname)
+		m.Answer, m.Ns, m.Extra, result = z.z.Lookup(ctx, state, qname)
 		h.zMu.RUnlock()
 
 		// record type exists for this name (NODATA).

plugin/azure/setup.go

@@ -8,7 +8,6 @@ import (
 	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/plugin/pkg/fall"
 	clog "github.com/coredns/coredns/plugin/pkg/log"
-	"github.com/coredns/coredns/plugin/pkg/upstream"
 
 	azuredns "github.com/Azure/azure-sdk-for-go/profiles/latest/dns/mgmt/dns"
 	azurerest "github.com/Azure/go-autorest/autorest/azure"
@@ -31,19 +30,21 @@ func setup(c *caddy.Controller) error {
 		return plugin.Error("azure", err)
 	}
 	ctx := context.Background()
+
 	dnsClient := azuredns.NewRecordSetsClient(env.Values[auth.SubscriptionID])
-	dnsClient.Authorizer, err = env.GetAuthorizer()
+	if dnsClient.Authorizer, err = env.GetAuthorizer(); err != nil {
-	if err != nil {
+		return plugin.Error("azure", err)
-		return c.Errf("failed to create azure plugin: %v", err)
 	}
-	h, err := New(ctx, dnsClient, keys, upstream.New())
+
+	h, err := New(ctx, dnsClient, keys)
 	if err != nil {
-		return c.Errf("failed to initialize azure plugin: %v", err)
+		return plugin.Error("azure", err)
 	}
 	h.Fall = fall
 	if err := h.Run(ctx); err != nil {
-		return c.Errf("failed to run azure plugin: %v", err)
+		return plugin.Error("azure", err)
 	}
+
 	dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler {
 		h.Next = next
 		return h
@@ -54,26 +55,25 @@ func setup(c *caddy.Controller) error {
 func parse(c *caddy.Controller) (auth.EnvironmentSettings, map[string][]string, fall.F, error) {
 	resourceGroupMapping := map[string][]string{}
 	resourceGroupSet := map[string]struct{}{}
-	var err error
-	var fall fall.F
-
 	azureEnv := azurerest.PublicCloud
 	env := auth.EnvironmentSettings{Values: map[string]string{}}
 
+	var fall fall.F
+
 	for c.Next() {
 		args := c.RemainingArgs()
 
 		for i := 0; i < len(args); i++ {
 			parts := strings.SplitN(args[i], ":", 2)
 			if len(parts) != 2 {
-				return env, resourceGroupMapping, fall, c.Errf("invalid resource group / zone '%s'", args[i])
+				return env, resourceGroupMapping, fall, c.Errf("invalid resource group/zone: %q", args[i])
 			}
 			resourceGroup, zoneName := parts[0], parts[1]
 			if resourceGroup == "" || zoneName == "" {
-				return env, resourceGroupMapping, fall, c.Errf("invalid resource group / zone '%s'", args[i])
+				return env, resourceGroupMapping, fall, c.Errf("invalid resource group/zone: %q", args[i])
 			}
 			if _, ok := resourceGroupSet[args[i]]; ok {
-				return env, resourceGroupMapping, fall, c.Errf("conflict zone '%s'", args[i])
+				return env, resourceGroupMapping, fall, c.Errf("conflicting zone: %q", args[i])
 			}
 
 			resourceGroupSet[args[i]] = struct{}{}
@@ -106,18 +106,20 @@ func parse(c *caddy.Controller) (auth.EnvironmentSettings, map[string][]string,
 					return env, resourceGroupMapping, fall, c.ArgErr()
 				}
 				env.Values[auth.ClientSecret] = c.Val()
-				azureEnv, err = azurerest.EnvironmentFromName(c.Val())
+				var err error
-				if err != nil {
+				if azureEnv, err = azurerest.EnvironmentFromName(c.Val()); err != nil {
-					return env, resourceGroupMapping, fall, c.Errf("cannot set azure environment: %s", err.Error())
+					return env, resourceGroupMapping, fall, c.Errf("cannot set azure environment: %q", err.Error())
 				}
 			case "fallthrough":
 				fall.SetZonesFromArgs(c.RemainingArgs())
 			default:
-				return env, resourceGroupMapping, fall, c.Errf("unknown property '%s'", c.Val())
+				return env, resourceGroupMapping, fall, c.Errf("unknown property: %q", c.Val())
 			}
 		}
 	}
+
 	env.Values[auth.Resource] = azureEnv.ResourceManagerEndpoint
 	env.Environment = azureEnv
+
 	return env, resourceGroupMapping, fall, nil
 }