Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
130 changes: 89 additions & 41 deletions pkg/dns/aws/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ const (
ELBService = elb.ServiceName
// TaggingService is the name of the Resource Group Tagging service.
TaggingService = resourcegroupstaggingapi.ServiceName
// govCloudRoute53Region is the AWS GovCloud region for Route 53. See:
// https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/using-govcloud-endpoints.html
govCloudRoute53Region = "us-gov"
// govCloudRoute53Endpoint is the Route 53 service endpoint used for AWS GovCloud.
govCloudRoute53Endpoint = "https://route53.us-gov.amazonaws.com"
// chinaRoute53Endpoint is the Route 53 service endpoint used for AWS China regions.
chinaRoute53Endpoint = "https://route53.amazonaws.com.cn"
// standardRoute53Endpoint is the standard AWS Route 53 service endpoint.
standardRoute53Endpoint = "https://route53.amazonaws.com"
)

var (
Expand Down Expand Up @@ -126,54 +135,57 @@ func NewProvider(config Config, operatorReleaseVersion string) (*Provider, error
tagConfig := aws.NewConfig()

// If the region is in aws china, cn-north-1 or cn-northwest-1, we should:
// 1. hard code route53 api endpoint to https://route53.amazonaws.com.cn and region to "cn-northwest-1" as route53 is not GA in AWS China,
// and aws sdk didn't have the endpoint.
// 1. hard code route53 api endpoint to https://route53.amazonaws.com.cn and region to "cn-northwest-1"
// as route53 is not GA in AWS China and aws sdk didn't have the endpoint.
// 2. use the aws china region cn-northwest-1 to setup tagging api correctly instead of "us-east-1"
switch region {
case endpoints.CnNorth1RegionID, endpoints.CnNorthwest1RegionID:
tagConfig = tagConfig.WithRegion(endpoints.CnNorthwest1RegionID)
r53Config = r53Config.WithRegion(endpoints.CnNorthwest1RegionID)
r53Config = r53Config.WithEndpoint("https://route53.amazonaws.com.cn")
r53Config = r53Config.WithRegion(endpoints.CnNorthwest1RegionID).WithEndpoint(chinaRoute53Endpoint)
case endpoints.UsGovEast1RegionID, endpoints.UsGovWest1RegionID:
// Route53 for GovCloud uses the "us-gov" region id:
// https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/using-govcloud-endpoints.html
r53Config = r53Config.WithRegion(govCloudRoute53Region)
tagConfig = tagConfig.WithRegion(region)
Copy link
Contributor Author

@danehans danehans Aug 25, 2020

Choose a reason for hiding this comment

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

Note that I have not implemented the same tagging api region restrictions as standard aws since the govcloud service endpoints doc does not specify the tagging api. The PR needs to merge so @yunjiang29 can verify against aws govcloud.

default:
// Since Route 53 is not a regionalized service, the Tagging API will
// only return hosted zone resources when the region is "us-east-1".
tagConfig = tagConfig.WithRegion(endpoints.UsEast1RegionID)
// Route 53 in AWS Regions other than the AWS Beijing and
// Ningxia (China) Regions: specify us-east-1 as the Region.
// Use us-east-1 for Route 53 in AWS Regions other than China or GovCloud Regions.
// See https://docs.aws.amazon.com/general/latest/gr/r53.html for details.
r53Config = r53Config.WithRegion(endpoints.UsEast1RegionID)
if len(config.ServiceEndpoints) > 0 {
route53Found := false
elbFound := false
tagFound := false
for _, ep := range config.ServiceEndpoints {
switch {
case route53Found && elbFound && tagFound:
break
case ep.Name == Route53Service:
route53Found = true
if urlContainsRegion(endpoints.UsEast1RegionID, ep.URL) || ep.URL == "https://route53.amazonaws.com" {
r53Config = r53Config.WithEndpoint(ep.URL)
log.Info("using route53 custom endpoint", "url", ep.URL)
} else {
return nil, fmt.Errorf("%s does not include required %s region identifier", ep.URL, endpoints.UsEast1RegionID)
}
case ep.Name == TaggingService:
tagFound = true
if urlContainsRegion(endpoints.UsEast1RegionID, ep.URL) {
tagConfig = tagConfig.WithEndpoint(ep.URL)
log.Info("using group tagging custom endpoint", "url", ep.URL)
} else {
return nil, fmt.Errorf("%s does not include required %s region identifier", ep.URL, endpoints.UsEast1RegionID)
}
case ep.Name == ELBService:
elbFound = true
if urlContainsRegion(region, ep.URL) {
elbConfig = elbConfig.WithEndpoint(ep.URL)
log.Info("using elb custom endpoint", "url", ep.URL)
} else {
return nil, fmt.Errorf("%s does not include required %s region identifier", ep.URL, region)
}
}
if len(config.ServiceEndpoints) > 0 {
route53Found := false
elbFound := false
tagFound := false
for _, ep := range config.ServiceEndpoints {
switch {
case route53Found && elbFound && tagFound:
break
case ep.Name == Route53Service:
route53Found = true
if urlContainsValidRegion(region, ep.URL) {
r53Config = r53Config.WithEndpoint(ep.URL)
log.Info("using route53 custom endpoint", "url", ep.URL)
} else {
return nil, fmt.Errorf("invalid %s service url %s for region %s", Route53Service, ep.URL, region)
}
case ep.Name == TaggingService:
tagFound = true
if urlContainsValidRegion(region, ep.URL) {
tagConfig = tagConfig.WithEndpoint(ep.URL)
log.Info("using group tagging custom endpoint", "url", ep.URL)
} else {
return nil, fmt.Errorf("invalid %s service url %s for region %s", TaggingService, ep.URL, region)
}
case ep.Name == ELBService:
elbFound = true
if urlContainsValidRegion(region, ep.URL) {
elbConfig = elbConfig.WithEndpoint(ep.URL)
log.Info("using elb custom endpoint", "url", ep.URL)
} else {
return nil, fmt.Errorf("invalid %s service url %s for region %s", ELBService, ep.URL, region)
}
}
}
Expand All @@ -192,9 +204,45 @@ func NewProvider(config Config, operatorReleaseVersion string) (*Provider, error
}, nil
}

// urlContainsRegion checks whether uri contains region.
func urlContainsRegion(region, uri string) bool {
return strings.Contains(uri, region)
// urlContainsValidRegion checks whether uri is valid based on provided region.
func urlContainsValidRegion(region, uri string) bool {
switch {
case strings.Contains(uri, Route53Service):
switch region {
case endpoints.CnNorth1RegionID, endpoints.CnNorthwest1RegionID:
if uri == chinaRoute53Endpoint {
return true
}
case endpoints.UsGovEast1RegionID, endpoints.UsGovWest1RegionID:
if uri == govCloudRoute53Endpoint {
return true
}
default:
if uri == standardRoute53Endpoint || strings.Contains(uri, endpoints.UsEast1RegionID) {
return true
}
}
case strings.Contains(uri, TaggingService):
switch region {
case endpoints.CnNorth1RegionID, endpoints.CnNorthwest1RegionID:
if strings.Contains(uri, endpoints.CnNorthwest1RegionID) {
return true
}
case endpoints.UsGovEast1RegionID, endpoints.UsGovWest1RegionID:
if strings.Contains(uri, endpoints.UsGovWest1RegionID) || strings.Contains(uri, endpoints.UsGovEast1RegionID) {
return true
}
default:
if strings.Contains(uri, endpoints.UsEast1RegionID) {
return true
}
}
default:
Copy link
Contributor

Choose a reason for hiding this comment

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

When exactly is this default case hit? When case ep.Name == ELBService:?
Could you add a unit test to cover it, if you don't mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ELBService is correct. Yes, I'll add a unit test.

if strings.Contains(uri, region) {
return true
}
}
return false
}

// getZoneID finds the ID of given zoneConfig in Route53. If an ID is already
Expand Down
134 changes: 134 additions & 0 deletions pkg/dns/aws/dns_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package aws

import (
"github.com/aws/aws-sdk-go/aws/endpoints"
"testing"
)

// TestURLContainsValidRegion validates uri as being a valid for the provided region.
func TestURLContainsValidRegion(t *testing.T) {
testCases := []struct {
description string
uri, region string
expected bool
}{
{
description: "regionalized standard route53 uri for region us-east-1",
uri: "https://route53.us-east-1.amazonaws.com",
region: endpoints.UsEast1RegionID,
expected: true,
},
{
description: "non-regionalized standard route53 uri for region us-west-1",
uri: standardRoute53Endpoint,
region: endpoints.UsWest1RegionID,
expected: true,
},
{
description: "invalid non-regionalized standard route53 uri for region us-west-2",
uri: "https://route53.us-west-2.amazonaws.com",
region: endpoints.UsWest2RegionID,
expected: false,
},
{
description: "route53 China uri for region cn-north-1",
uri: chinaRoute53Endpoint,
region: endpoints.CnNorth1RegionID,
expected: true,
},
{
description: "route53 China uri for region cn-northwest-1",
uri: chinaRoute53Endpoint,
region: endpoints.CnNorthwest1RegionID,
expected: true,
},
{
description: "invalid route53 China uri for region cn-northwest-1",
uri: "https://route53.cn-northwest-1.amazonaws.com.cn",
region: endpoints.CnNorthwest1RegionID,
expected: false,
},
{
description: "route53 GovCloud uri for region us-gov-east-1",
uri: govCloudRoute53Endpoint,
region: endpoints.UsGovEast1RegionID,
expected: true,
},
{
description: "route53 GovCloud uri for region us-gov-west-1",
uri: govCloudRoute53Endpoint,
region: endpoints.UsGovWest1RegionID,
expected: true,
},
{
description: "route53 GovCloud uri for region us-west-2",
uri: govCloudRoute53Endpoint,
region: endpoints.UsWest2RegionID,
expected: false,
},
{
description: "tagging uri for region us-west-2",
uri: "https://tagging.us-east-1.amazonaws.com",
region: endpoints.UsWest2RegionID,
expected: true,
},
{
description: "invalid standard tagging uri for region us-west-2",
uri: "https://tagging.us-west-2.amazonaws.com",
region: endpoints.UsWest2RegionID,
expected: false,
},
{
description: "tagging GovCloud uri for region us-gov-west-1",
uri: "https://tagging.us-gov-west-1.amazonaws.com",
region: endpoints.UsGovWest1RegionID,
expected: true,
},
{
description: "tagging China uri for region cn-north-1",
uri: "https://tagging.cn-northwest-1.amazonaws.com.cn",
region: endpoints.CnNorth1RegionID,
expected: true,
},
{
description: "tagging China uri for region cn-northwest-1",
uri: "https://tagging.cn-northwest-1.amazonaws.com.cn",
region: endpoints.CnNorthwest1RegionID,
expected: true,
},
{
description: "invalid tagging China uri for region cn-north-1",
uri: "https://tagging.cn-north-1.amazonaws.com.cn",
region: endpoints.CnNorth1RegionID,
expected: false,
},
{
description: "elb China uri for region cn-north-1",
uri: "https://elasticloadbalancing.cn-north-1.amazonaws.com.cn",
region: endpoints.CnNorth1RegionID,
expected: true,
},
{
description: "elb GovCloud uri for region us-gov-west-1",
uri: "https://elasticloadbalancing.us-gov-west-1.amazonaws.com",
region: endpoints.UsGovWest1RegionID,
expected: true,
},
{
description: "elb standard uri for region us-west-2",
uri: "https://elasticloadbalancing.us-west-2.amazonaws.com",
region: endpoints.UsWest2RegionID,
expected: true,
},
}

for _, tc := range testCases {
valid := urlContainsValidRegion(tc.region, tc.uri)
switch {
case !valid && tc.expected:
t.Errorf("test %s failed, invalid url %s", tc.description, tc.uri)
case valid && !tc.expected:
t.Errorf("test %s expected to fail, but passed for url: %s", tc.description, tc.uri)
}
}
}