Use the correct private and public zone clients.

Set a Destroy User Agent.
Cleanup pointer references to use the aws sdk.

Author: barbacbd

pkg/destroy/aws/aws.go

@@ -8,7 +8,7 @@ import (
 
 	awsv2 "github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/aws/arn"
-	"github.com/aws/aws-sdk-go-v2/aws/middleware"
+	awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
 	configv2 "github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/credentials/stscreds"
 	ec2v2 "github.com/aws/aws-sdk-go-v2/service/ec2"
@@ -23,6 +23,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	s3types "github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
+	"github.com/aws/smithy-go/middleware"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -87,6 +88,9 @@ const (
 	endpointISOBEast1    = "us-isob-east-1"
 	endpointUSGovEast1   = "us-gov-east-1"
 	endpointUSGovWest1   = "us-gov-west-1"
+
+	// OpenShiftInstallerDestroyerUserAgent is the User Agent key to add to the AWS Destroy API request header.
+	OpenShiftInstallerDestroyerUserAgent = "OpenShift/4.x Destroyer"
 )
 
 // New returns an AWS destroyer from ClusterMetadata.
@@ -101,49 +105,58 @@ func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers.
 	ec2Client, err := awssession.NewEC2Client(ctx, awssession.EndpointOptions{
 		Region:    region,
 		Endpoints: metadata.AWS.ServiceEndpoints,
-	})
+	}, ec2v2.WithAPIOptions(awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw)))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create EC2 client: %w", err)
 	}
 
 	iamClient, err := awssession.NewIAMClient(ctx, awssession.EndpointOptions{
 		Region:    region,
 		Endpoints: metadata.AWS.ServiceEndpoints,
-	})
+	}, iamv2.WithAPIOptions(awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw)))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create IAM client: %w", err)
 	}
 
 	// FIXME: remove this code when the elb and elbv2 clients are "fixed" or figured out
-	elbCfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region))
+	elbCfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region),
+		configv2.WithAPIOptions([]func(*middleware.Stack) error{
+			awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw),
+		}))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create AWS config for elb client: %w", err)
 	}
 	elbclient := elb.NewFromConfig(elbCfg, func(options *elb.Options) {
 		options.Region = region
 		for _, endpoint := range metadata.AWS.ServiceEndpoints {
-			if strings.EqualFold(endpoint.Name, "elb") {
+			if strings.EqualFold(endpoint.Name, "elb") || strings.EqualFold(endpoint.Name, "elasticloadbalancing") {
 				options.BaseEndpoint = awsv2.String(endpoint.URL)
 			}
 		}
 	})
 
 	// FIXME: remove this code when the elb and elbv2 clients are "fixed" or figured out
-	elbv2Cfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region))
+	elbv2Cfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region),
+		configv2.WithAPIOptions([]func(*middleware.Stack) error{
+			awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw),
+		}))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create AWS config for elbv2 client: %w", err)
 	}
 	elbv2client := elbv2.NewFromConfig(elbv2Cfg, func(options *elbv2.Options) {
 		options.Region = region
 		for _, endpoint := range metadata.AWS.ServiceEndpoints {
-			if strings.EqualFold(endpoint.Name, "elbv2") {
+			if strings.EqualFold(endpoint.Name, "elbv2") || strings.EqualFold(endpoint.Name, "elasticloadbalancingv2") {
 				options.BaseEndpoint = awsv2.String(endpoint.URL)
 			}
 		}
 	})
 
 	// FIXME: remove this code when the s3client is made
-	s3Cfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region))
+	s3Cfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region),
+		configv2.WithAPIOptions([]func(*middleware.Stack) error{
+			awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw),
+		}))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create AWS config for S3 client: %w", err)
 	}
@@ -157,14 +170,17 @@ func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers.
 	})
 
 	// FIXME: remove this code when the EFS client is made
-	efsCfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region))
+	efsCfg, err := awssession.GetConfigWithOptions(ctx, configv2.WithRegion(region),
+		configv2.WithAPIOptions([]func(*middleware.Stack) error{
+			awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw),
+		}))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create AWS config for EFS client: %w", err)
 	}
 	efsClient := efs.NewFromConfig(efsCfg, func(options *efs.Options) {
 		options.Region = region
 		for _, endpoint := range metadata.AWS.ServiceEndpoints {
-			if strings.EqualFold(endpoint.Name, "efs") {
+			if strings.EqualFold(endpoint.Name, "elasticfilesystem") {
 				options.BaseEndpoint = awsv2.String(endpoint.URL)
 			}
 		}
@@ -173,7 +189,7 @@ func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers.
 	route53Client, err := awssession.NewRoute53Client(ctx, awssession.EndpointOptions{
 		Region:    region,
 		Endpoints: metadata.AWS.ServiceEndpoints,
-	}, "") // FIXME: Do we need an ARN here?
+	}, "", route53.WithAPIOptions(awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw)))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create Route53 client: %w", err)
 	}
@@ -213,15 +229,18 @@ func createResourceTaggingClientWithConfig(cfg awsv2.Config, region string, endp
 	return resourcegroupstaggingapi.NewFromConfig(cfg, func(options *resourcegroupstaggingapi.Options) {
 		options.Region = region
 		for _, endpoint := range endpoints {
-			if strings.EqualFold(endpoint.Name, "resourcegroupstaggingapi") {
+			if strings.EqualFold(endpoint.Name, "tagging") {
 				options.BaseEndpoint = awsv2.String(endpoint.URL)
 			}
 		}
 	})
 }
 
 func createResourceTaggingClient(region string, endpoints []awstypes.ServiceEndpoint) (*resourcegroupstaggingapi.Client, error) {
-	cfg, err := awssession.GetConfigWithOptions(context.Background(), configv2.WithRegion(region))
+	cfg, err := awssession.GetConfigWithOptions(context.Background(), configv2.WithRegion(region),
+		configv2.WithAPIOptions([]func(*middleware.Stack) error{
+			awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw),
+		}))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create AWS config for resource tagging client: %w", err)
 	}
@@ -250,7 +269,7 @@ func (o *ClusterUninstaller) RunWithContext(ctx context.Context) ([]string, erro
 		stsSvc, err := awssession.NewSTSClient(ctx, awssession.EndpointOptions{
 			Region:    endpointUSEast1,
 			Endpoints: o.endpoints,
-		}, sts.WithAPIOptions(middleware.AddUserAgentKeyValue("OpenShift/4.x Destroyer", version.Raw)))
+		}, sts.WithAPIOptions(awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw)))
 		if err != nil {
 			return nil, fmt.Errorf("failed to create STS client: %w", err)
 		}
@@ -691,7 +710,7 @@ func deleteRoute53(ctx context.Context, client *route53.Client, arn arn.ARN, log
 		for paginator.HasMorePages() {
 			page, err := paginator.NextPage(ctx)
 			if err != nil {
-				return fmt.Errorf("listing record sets for public zone: %w", err)
+				return fmt.Errorf("listing record sets for public zone %s: %w", publicZoneID, err)
 			}
 			for _, recordSet := range page.ResourceRecordSets {
 				key := recordSetKey(recordSet)
@@ -707,7 +726,7 @@ func deleteRoute53(ctx context.Context, client *route53.Client, arn arn.ARN, log
 	for paginator.HasMorePages() {
 		page, err := paginator.NextPage(ctx)
 		if err != nil {
-			return fmt.Errorf("listing record sets: %w", err)
+			return fmt.Errorf("listing record sets for zone %s: %w", id, err)
 		}
 
 		for _, recordSet := range page.ResourceRecordSets {

pkg/destroy/aws/elbhelpers.go

@@ -65,7 +65,7 @@ func deleteElasticLoadBalancerClassicByVPC(ctx context.Context, client *elbapi.C
 	for paginator.HasMorePages() {
 		page, err := paginator.NextPage(ctx)
 		if err != nil {
-			return fmt.Errorf("describing load balacers by vpc %s: %w", vpc, err)
+			return fmt.Errorf("describing load balancers by vpc %s: %w", vpc, err)
 		}
 
 		logger.Debugf("iterating over a page of %d v1 load balancers", len(page.LoadBalancerDescriptions))

pkg/destroy/aws/iamhelpers.go

@@ -63,7 +63,7 @@ func (search *IamRoleSearch) find(ctx context.Context) (arns []string, names []s
 					lastError = fmt.Errorf("get tags for %s: %w", *role.Arn, err)
 				}
 			} else {
-				tags := make(map[string]string, len(role.Tags))
+				tags := make(map[string]string, len(response.Tags))
 				for _, tag := range response.Tags {
 					tags[*tag.Key] = *tag.Value
 				}
@@ -111,7 +111,7 @@ func (search *IamUserSearch) arns(ctx context.Context) ([]string, error) {
 			}
 
 			// Unfortunately user.Tags is empty from ListUsers, so we need to query each one
-			response, err := search.client.ListUserTags(ctx, &iamv2.ListUserTagsInput{UserName: aws.String(*user.UserName)})
+			response, err := search.client.ListUserTags(ctx, &iamv2.ListUserTagsInput{UserName: user.UserName})
 			if err != nil {
 				switch {
 				case strings.Contains(HandleErrorCode(err), "NoSuchEntity"):

pkg/destroy/aws/shared.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/aws/arn"
+	awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
 	"github.com/aws/aws-sdk-go-v2/service/iam"
 	"github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi"
 	tagtypes "github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi/types"
@@ -16,6 +17,9 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/util/wait"
+
+	awssession "github.com/openshift/installer/pkg/asset/installconfig/aws"
+	"github.com/openshift/installer/pkg/version"
 )
 
 func (o *ClusterUninstaller) removeSharedTags(
@@ -76,7 +80,7 @@ func (o *ClusterUninstaller) removeSharedTag(ctx context.Context, tagClients []*
 				}
 
 				for _, resource := range page.ResourceTagMappingList {
-					arnString := *resource.ResourceARN
+					arnString := aws.ToString(resource.ResourceARN)
 					logger := o.Logger.WithField("arn", arnString)
 					parsedARN, err := arn.Parse(arnString)
 					if err != nil {
@@ -98,7 +102,7 @@ func (o *ClusterUninstaller) removeSharedTag(ctx context.Context, tagClients []*
 
 			if lastErr != nil {
 				o.Logger.Infof("failed to get tagged resources: %v", lastErr)
-				var invalidParameter tagtypes.InvalidParameterException
+				var invalidParameter *tagtypes.InvalidParameterException
 				if errors.As(lastErr, &invalidParameter) {
 					continue
 				}
@@ -107,7 +111,7 @@ func (o *ClusterUninstaller) removeSharedTag(ctx context.Context, tagClients []*
 			}
 
 			if len(arns) == 0 {
-				o.Logger.Debugf("No matches in %s for %s: shared, removing client", o.Region, key)
+				o.Logger.Debugf("No matches in %s for %s: shared, removing client", tagClient.Options().Region, key)
 				continue
 			}
 			// appending the tag client here but it needs to be removed if there is a InvalidParameterException when trying to
@@ -204,6 +208,17 @@ func (o *ClusterUninstaller) cleanSharedRoute53(ctx context.Context, arn arn.ARN
 }
 
 func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, id string, logger logrus.FieldLogger) error {
+	// The private hosted zone (phz) may belong to a different account,
+	// in which case we need a separate client.
+	// Note: the ClusterUninstaller has a basic Route53 client used for public zone resources.
+	privateZoneClient, err := awssession.NewRoute53Client(ctx, awssession.EndpointOptions{
+		Region:    o.Region,
+		Endpoints: o.endpoints,
+	}, o.HostedZoneRole, route53.WithAPIOptions(awsmiddleware.AddUserAgentKeyValue(OpenShiftInstallerDestroyerUserAgent, version.Raw)))
+	if err != nil {
+		return fmt.Errorf("failed to create Route53 private zone client: %w", err)
+	}
+
 	if o.ClusterDomain == "" {
 		logger.Debug("No cluster domain specified in metadata; cannot clean the shared hosted zone")
 		return nil
@@ -216,7 +231,7 @@ func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, id strin
 	}
 
 	var lastError error
-	paginator := route53.NewListResourceRecordSetsPaginator(o.Route53Client, &route53.ListResourceRecordSetsInput{HostedZoneId: aws.String(id)})
+	paginator := route53.NewListResourceRecordSetsPaginator(privateZoneClient, &route53.ListResourceRecordSetsInput{HostedZoneId: aws.String(id)})
 	for paginator.HasMorePages() {
 		page, err := paginator.NextPage(ctx)
 		if err != nil {
@@ -225,14 +240,14 @@ func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, id strin
 
 		for _, recordSet := range page.ResourceRecordSets {
 			// skip record sets that are not part of the cluster
-			name := *recordSet.Name
+			name := aws.ToString(recordSet.Name)
 			if !strings.HasSuffix(name, dottedClusterDomain) {
 				continue
 			}
 			if len(name) == len(dottedClusterDomain) {
 				continue
 			}
-			recordsetFields := logrus.Fields{"recordset": fmt.Sprintf("%s (%s)", *recordSet.Name, recordSet.Type)}
+			recordsetFields := logrus.Fields{"recordset": fmt.Sprintf("%s (%s)", aws.ToString(recordSet.Name), recordSet.Type)}
 			// delete any matching record sets in the public hosted zone
 			if publicZoneID != "" {
 				publicZoneLogger := logger.WithField("id", publicZoneID)
@@ -248,7 +263,7 @@ func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, id strin
 				publicZoneLogger.WithFields(recordsetFields).Debug("Deleted from public zone")
 			}
 			// delete the record set
-			if err := deleteRoute53RecordSet(ctx, o.Route53Client, id, &recordSet, logger); err != nil {
+			if err := deleteRoute53RecordSet(ctx, privateZoneClient, id, &recordSet, logger); err != nil {
 				if lastError != nil {
 					logger.Debug(lastError)
 				}
@@ -280,7 +295,7 @@ func deleteMatchingRecordSetInPublicZone(ctx context.Context, client *route53.Cl
 		return nil
 	}
 	matchingRecordSet := out.ResourceRecordSets[0]
-	if *matchingRecordSet.Name != *recordSet.Name || matchingRecordSet.Type != recordSet.Type {
+	if aws.ToString(matchingRecordSet.Name) != aws.ToString(recordSet.Name) || matchingRecordSet.Type != recordSet.Type {
 		return nil
 	}
 	return deleteRoute53RecordSet(ctx, client, zoneID, &matchingRecordSet, logger)