Skip to content
Merged
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
50 changes: 36 additions & 14 deletions pkg/dns/aws/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,30 +135,52 @@ func (m *Manager) discoverZones() error {
// Find the private zone, which is the private zone whose openshiftClusterID tag
// value matches the cluster ID.
if len(m.privateZoneID) == 0 {
zones, err := m.tags.GetResources(&resourcegroupstaggingapi.GetResourcesInput{
// Even though we use filters when getting resources, the
// resources are still paginated as though no filter were
// applied. If the desired resource is not on the first page,
// then GetResources will not return it. We need to use
// GetResourcesPages and possibly go through one or more empty
// pages of resources till we find a resource that gets through
// the filters.
var innerError error
f := func(resp *resourcegroupstaggingapi.GetResourcesOutput, lastPage bool) (shouldContinue bool) {
for _, zone := range resp.ResourceTagMappingList {
zoneARN, err := arn.Parse(aws.StringValue(zone.ResourceARN))
if err != nil {
innerError = fmt.Errorf("failed to parse hostedzone ARN %q: %v", aws.StringValue(zone.ResourceARN), err)

return false
}

elems := strings.Split(zoneARN.Resource, "/")
if len(elems) != 2 || elems[0] != "hostedzone" {
innerError = fmt.Errorf("got unexpected resource ARN: %v", zoneARN)

return false
}

m.privateZoneID = elems[1]

return false
}

return true
}
err := m.tags.GetResourcesPages(&resourcegroupstaggingapi.GetResourcesInput{
ResourceTypeFilters: []*string{aws.String("route53:hostedzone")},
TagFilters: []*resourcegroupstaggingapi.TagFilter{
{
Key: aws.String("openshiftClusterID"),
Values: []*string{aws.String(m.config.ClusterID)},
},
},
})
}, f)
if innerError != nil {
err = innerError
Copy link
Contributor

@ironcladlou ironcladlou Jan 11, 2019

Choose a reason for hiding this comment

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

If both err and innerError are not nil, err is lost. I don't want to block the PR on it. How about a followup to use an aggregate (either from k8s utils if available, or https://github.com/pkg/errors?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured innerError was more important, but yeah, aggregating the error values would be best. I'll make a follow-up.

}
if err != nil {
return fmt.Errorf("failed to get tagged resources: %v", err)
}
for _, zone := range zones.ResourceTagMappingList {
zoneARN, err := arn.Parse(aws.StringValue(zone.ResourceARN))
if err != nil {
return fmt.Errorf("failed to parse hostedzone ARN %q: %v", aws.StringValue(zone.ResourceARN), err)
}
elems := strings.Split(zoneARN.Resource, "/")
if len(elems) != 2 || elems[0] != "hostedzone" {
return fmt.Errorf("got unexpected resource ARN: %v", zoneARN)
}
m.privateZoneID = elems[1]
break
}
if len(m.privateZoneID) == 0 {
return fmt.Errorf("couldn't find private hosted zone for cluster id %q", m.config.ClusterID)
}
Expand Down