Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NE-226: Support both DNSZone and PrivateDNSZone resources #300

Merged
merged 2 commits into from
Oct 7, 2019
Merged

NE-226: Support both DNSZone and PrivateDNSZone resources #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68878a7
vendor: refresh to include privatedns service
abhinavdahiya Sep 20, 2019
5265fad
dns/azure/client: support DNSZone type Private and PrivateDNSZone for…
abhinavdahiya Sep 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU=
github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
github.com/onsi/gomega v1.5.0 h1:izbySO9zDPmjJ8rDjLvkA2zJHIo+HkYXHnf7eN7SSyo=
github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
github.com/openshift/api v3.9.1-0.20190713024856-f15120709e0a+incompatible h1:Y/UXROcoe8yS41+3Q9kgm2GgIv/1Z10eRh4XAiP3w74=
github.com/openshift/api v3.9.1-0.20190713024856-f15120709e0a+incompatible/go.mod h1:dh9o4Fs58gpFXGSYfnVxGR9PnV53I8TW84pQaJDdGiY=
github.com/openshift/api v3.9.1-0.20190905013149-e6c3eeae444f+incompatible h1:CARWXyP03A6YNIk3nC6HdJ1ths/0fUtZDPJqYD5UDi4=
github.com/openshift/api v3.9.1-0.20190905013149-e6c3eeae444f+incompatible/go.mod h1:dh9o4Fs58gpFXGSYfnVxGR9PnV53I8TW84pQaJDdGiY=
github.com/openshift/library-go v0.0.0-20190613200606-e617f832835d h1:a/+OnGnX5qMHSSuKcSSLMPZghOgD48NNZgzDklj9cIc=
Expand Down
106 changes: 94 additions & 12 deletions pkg/dns/azure/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/Azure/azure-sdk-for-go/services/dns/mgmt/2017-10-01/dns"
privatedns "github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -33,27 +34,63 @@ type ARecord struct {
}

type dnsClient struct {
zones dns.ZonesClient
recordSets dns.RecordSetsClient
config Config
recordSetClient, privateRecordSetClient DNSClient
}

// New returns an authenticated DNSClient
func New(config Config, userAgentExtension string) (DNSClient, error) {
rsc, err := newRecordSetClient(config, userAgentExtension)
if err != nil {
return nil, errors.Wrap(err, "failed to create recordSetClient")
}

prsc, err := newPrivateRecordSetClient(config, userAgentExtension)
if err != nil {
return nil, errors.Wrap(err, "failed to create privateRecordSetClient")
}

return &dnsClient{recordSetClient: rsc, privateRecordSetClient: prsc}, nil
}

func (c *dnsClient) Put(ctx context.Context, zone Zone, arec ARecord) error {
switch zone.Provider {
case "Microsoft.Network/privateDnsZones":
return c.privateRecordSetClient.Put(ctx, zone, arec)
case "Microsoft.Network/dnszones":
return c.recordSetClient.Put(ctx, zone, arec)
default:
return errors.Errorf("unsupported Zone provider %s", zone.Provider)
}
}

func (c *dnsClient) Delete(ctx context.Context, zone Zone, arec ARecord) error {
switch zone.Provider {
case "Microsoft.Network/privateDnsZones":
return c.privateRecordSetClient.Delete(ctx, zone, arec)
case "Microsoft.Network/dnszones":
return c.recordSetClient.Delete(ctx, zone, arec)
default:
return errors.Errorf("unsupported Zone provider %s", zone.Provider)
}
}

type recordSetClient struct {
client dns.RecordSetsClient
}

func newRecordSetClient(config Config, userAgentExtension string) (*recordSetClient, error) {
authorizer, err := getAuthorizerForResource(config)
if err != nil {
return nil, err
}
zc := dns.NewZonesClient(config.SubscriptionID)
zc.AddToUserAgent(userAgentExtension)
zc.Authorizer = authorizer

rc := dns.NewRecordSetsClient(config.SubscriptionID)
rc.AddToUserAgent(userAgentExtension)
rc.Authorizer = authorizer
return &dnsClient{zones: zc, recordSets: rc, config: config}, nil
return &recordSetClient{client: rc}, nil
}

func (c *dnsClient) Put(ctx context.Context, zone Zone, arec ARecord) error {
func (c *recordSetClient) Put(ctx context.Context, zone Zone, arec ARecord) error {
rs := dns.RecordSet{
RecordSetProperties: &dns.RecordSetProperties{
TTL: &arec.TTL,
Expand All @@ -62,20 +99,65 @@ func (c *dnsClient) Put(ctx context.Context, zone Zone, arec ARecord) error {
},
},
}
_, err := c.recordSets.CreateOrUpdate(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A, rs, "", "")
_, err := c.client.CreateOrUpdate(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A, rs, "", "")
if err != nil {
return errors.Wrapf(err, "failed to update dns a record: %s.%s", arec.Name, zone.Name)
}
return nil
}

func (c *dnsClient) Delete(ctx context.Context, zone Zone, arec ARecord) error {
_, err := c.recordSets.Get(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A)
func (c *recordSetClient) Delete(ctx context.Context, zone Zone, arec ARecord) error {
_, err := c.client.Get(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A)
if err != nil {
// TODO: How do we interpret this as a notfound error?
return nil
}
_, err = c.client.Delete(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A, "")
if err != nil {
return errors.Wrapf(err, "failed to delete dns a record: %s.%s", arec.Name, zone.Name)
}
return nil
}

type privateRecordSetClient struct {
client privatedns.RecordSetsClient
}

func newPrivateRecordSetClient(config Config, userAgentExtension string) (*privateRecordSetClient, error) {
authorizer, err := getAuthorizerForResource(config)
if err != nil {
return nil, err
}

prc := privatedns.NewRecordSetsClient(config.SubscriptionID)
prc.AddToUserAgent(userAgentExtension)
prc.Authorizer = authorizer
return &privateRecordSetClient{client: prc}, nil
}

func (c *privateRecordSetClient) Put(ctx context.Context, zone Zone, arec ARecord) error {
rs := privatedns.RecordSet{
RecordSetProperties: &privatedns.RecordSetProperties{
TTL: &arec.TTL,
ARecords: &[]privatedns.ARecord{
{Ipv4Address: &arec.Address},
},
},
}
_, err := c.client.CreateOrUpdate(ctx, zone.ResourceGroup, zone.Name, privatedns.A, arec.Name, rs, "", "")
if err != nil {
return errors.Wrapf(err, "failed to update dns a record: %s.%s", arec.Name, zone.Name)
}
return nil
}

func (c *privateRecordSetClient) Delete(ctx context.Context, zone Zone, arec ARecord) error {
_, err := c.client.Get(ctx, zone.ResourceGroup, zone.Name, privatedns.A, arec.Name)
if err != nil {
// TODO: How do we interpret this as a notfound error?
return nil
}
_, err = c.recordSets.Delete(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A, "")
_, err = c.client.Delete(ctx, zone.ResourceGroup, zone.Name, privatedns.A, arec.Name, "")
if err != nil {
return errors.Wrapf(err, "failed to delete dns a record: %s.%s", arec.Name, zone.Name)
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/dns/azure/client/zone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package client

import (
"errors"
"fmt"
"strings"
)

type Zone struct {
SubscriptionID string
ResourceGroup string
Provider string
Name string
}

Expand All @@ -16,5 +18,5 @@ func ParseZone(id string) (*Zone, error) {
if len(s) < 9 {
return nil, errors.New("invalid azure dns zone id")
}
return &Zone{SubscriptionID: s[2], ResourceGroup: s[4], Name: s[8]}, nil
return &Zone{SubscriptionID: s[2], ResourceGroup: s[4], Provider: fmt.Sprintf("%s/%s", s[6], s[7]), Name: s[8]}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the structure of the string id documented?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well i couldn't find any documentations around it, but https://docs.microsoft.com/en-us/rest/api/resources/resources/getbyid

has information on the resource id structure.

secondly, i think i have errors for unknown cases, which should be clear to identify when things are out of place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe I'm misunderstanding, but maybe it has to be okay because we're using IDs instead of tags to address zones generally in Azure. So if that ID format isn't stable, some day we're going to lose track of all our zones. I'm not sure how to assess the stability of the documented ID format.

This PR certainly isn't making the situation worse and there's literally nothing else right now for it to go by.

A regex with named capture groups might at least be more readable at this point, but that's a nit.

If the reliability of ID's format is in question, should we open a parallel investigation into our strategy for addressing Azure zones?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err, wait a sec, if you have the opaque resource ID, can you not do some versioned API call to get back a resource containing the info we're currently extracting from the ID itself (presumed to be opaque)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can take the ID and request a get using https://docs.microsoft.com/en-us/rest/api/resources/resources/getbyid to get a response..
https://docs.microsoft.com/en-us/rest/api/resources/resources/getbyid#genericresource

but the response doesn't give us any more information that we would need, compared to the already parsing one.

I think for now the resource ID is standard enough like https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/helpers/azure/resourceid.go

that it should be good enough IMO.

}
24 changes: 15 additions & 9 deletions pkg/dns/azure/client/zone_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,22 @@ import (

func TestFunction(t *testing.T) {
var zoneTests = []struct {
desc string
id string
err error
subID string
rg string
name string
desc string
id string
err error
subID string
rg string
provider string
name string
}{
{"TestValidZoneID", "/subscriptions/E540B02D-5CCE-4D47-A13B-EB05A19D696E/resourceGroups/test-rg/providers/Microsoft.Network/dnszones/test-rg.dnszone.io",
nil, "E540B02D-5CCE-4D47-A13B-EB05A19D696E", "test-rg", "test-rg.dnszone.io"},
nil, "E540B02D-5CCE-4D47-A13B-EB05A19D696E", "test-rg", "Microsoft.Network/dnszones", "test-rg.dnszone.io"},
{"TestValidZoneID", "/subscriptions/E540B02D-5CCE-4D47-A13B-EB05A19D696E/resourceGroups/test-rg/providers/Microsoft.Network/privateDnsZones/test-rg.dnszone.io",
nil, "E540B02D-5CCE-4D47-A13B-EB05A19D696E", "test-rg", "Microsoft.Network/privateDnsZones", "test-rg.dnszone.io"},
{"TestInvalidZoneID", "/subscriptions/E540B02D-5CCE-4D47-A13B-EB05A19D696E/resourceGroups/test-rg/providers/Microsoft.Network/dnszones",
errors.New("invalid azure dns zone id"), "E540B02D-5CCE-4D47-A13B-EB05A19D696E", "test-rg", "test-rg.dnszone.io"},
errors.New("invalid azure dns zone id"), "E540B02D-5CCE-4D47-A13B-EB05A19D696E", "test-rg", "Microsoft.Network/dnszones", "test-rg.dnszone.io"},
{"TestEmptyZoneID", "", errors.New("invalid azure dns zone id"),
"E540B02D-5CCE-4D47-A13B-EB05A19D696E", "test-rg", "test-rg.dnszone.io"},
"E540B02D-5CCE-4D47-A13B-EB05A19D696E", "test-rg", "Microsoft.Network/dnszones", "test-rg.dnszone.io"},
}
for _, tt := range zoneTests {
t.Run(tt.desc, func(t *testing.T) {
Expand All @@ -44,6 +47,9 @@ func TestFunction(t *testing.T) {
if zone.ResourceGroup != tt.rg {
t.Errorf("expected [%s] actual [%s]", tt.rg, zone.ResourceGroup)
}
if zone.Provider != tt.provider {
t.Errorf("expected [%s] actual [%s]", tt.provider, zone.Provider)
}
if zone.Name != tt.name {
t.Errorf("expected [%s] actual [%s]", tt.name, zone.Name)
}
Expand Down
51 changes: 51 additions & 0 deletions ...ithub.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/client.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading