aws: refactor CreateOrUpdateRecord function

This function was doing way more than its name says: it was creating
records in both private and public zones. The argument names were also
not very descriptive and very hard to decipher at a glance.

This change moves the logic out of the function and into the aws
`InfraReady` hook. This not only makes the logic more readable, but it
also paves the way for the use of Classic Load Balancer types.

pkg/asset/installconfig/aws/route53.go

@@ -11,7 +11,6 @@ import (
 	"github.com/aws/aws-sdk-go/aws/endpoints"
 	awss "github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/route53"
-	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -156,67 +155,39 @@ func GetR53ClientCfg(sess *awss.Session, roleARN string) *aws.Config {
 	return &aws.Config{Credentials: creds}
 }
 
-// CreateOrUpdateRecord Creates or Updates the Route53 Record for the cluster endpoint.
-func (c *Client) CreateOrUpdateRecord(ctx context.Context, ic *types.InstallConfig, target string, intTarget string, phzID string, aliasZoneID string) error {
-	useCNAME := cnameRegions.Has(ic.AWS.Region)
-
-	apiName := fmt.Sprintf("api.%s.", ic.ClusterDomain())
-	apiIntName := fmt.Sprintf("api-int.%s.", ic.ClusterDomain())
-
-	// Create api record in public zone
-	if ic.Publish == types.ExternalPublishingStrategy {
-		zone, err := c.GetBaseDomain(ic.BaseDomain)
-		if err != nil {
-			return err
-		}
-
-		svc := route53.New(c.ssn) // we dont want to assume role here
-		if _, err := createRecord(ctx, svc, aws.StringValue(zone.Id), apiName, target, aliasZoneID, useCNAME); err != nil {
-			return fmt.Errorf("failed to create records for api: %w", err)
-		}
-		logrus.Debugln("Created public API record in public zone")
-	}
-
-	// Create service with assumed role for PHZ
-	svc := route53.New(c.ssn, GetR53ClientCfg(c.ssn, ic.AWS.HostedZoneRole))
-
-	// Create api record in private zone
-	if _, err := createRecord(ctx, svc, phzID, apiName, intTarget, aliasZoneID, useCNAME); err != nil {
-		return fmt.Errorf("failed to create records for api: %w", err)
-	}
-	logrus.Debugln("Created public API record in private zone")
-
-	// Create api-int record in private zone
-	if _, err := createRecord(ctx, svc, phzID, apiIntName, intTarget, aliasZoneID, useCNAME); err != nil {
-		return fmt.Errorf("failed to create records for api-int: %w", err)
-	}
-	logrus.Debugln("Created private API record in private zone")
-
-	return nil
+// CreateRecordInput collects information for creating a record.
+type CreateRecordInput struct {
+	Name           string
+	Region         string
+	DNSTarget      string
+	ZoneID         string
+	AliasZoneID    string
+	HostedZoneRole string
 }
 
-func createRecord(ctx context.Context, client *route53.Route53, zoneID, name, dnsName, aliasZoneID string, useCNAME bool) (*route53.ChangeInfo, error) {
+// CreateOrUpdateRecord Creates or Updates the Route53 Record for the cluster endpoint.
+func (c *Client) CreateOrUpdateRecord(ctx context.Context, in *CreateRecordInput) error {
 	recordSet := &route53.ResourceRecordSet{
-		Name: aws.String(name),
+		Name: aws.String(in.Name),
 	}
-	if useCNAME {
+	if cnameRegions.Has(in.Region) {
 		recordSet.SetType("CNAME")
 		recordSet.SetTTL(10)
 		recordSet.SetResourceRecords([]*route53.ResourceRecord{
-			{Value: aws.String(dnsName)},
+			{Value: aws.String(in.DNSTarget)},
 		})
 	} else {
 		recordSet.SetType("A")
 		recordSet.SetAliasTarget(&route53.AliasTarget{
-			DNSName:              aws.String(dnsName),
-			HostedZoneId:         aws.String(aliasZoneID),
+			DNSName:              aws.String(in.DNSTarget),
+			HostedZoneId:         aws.String(in.AliasZoneID),
 			EvaluateTargetHealth: aws.Bool(false),
 		})
 	}
 	input := &route53.ChangeResourceRecordSetsInput{
-		HostedZoneId: aws.String(zoneID),
+		HostedZoneId: aws.String(in.ZoneID),
 		ChangeBatch: &route53.ChangeBatch{
-			Comment: aws.String(fmt.Sprintf("Creating record %s", name)),
+			Comment: aws.String(fmt.Sprintf("Creating record %s", in.Name)),
 			Changes: []*route53.Change{
 				{
 					Action:            aws.String("UPSERT"),
@@ -225,12 +196,12 @@ func createRecord(ctx context.Context, client *route53.Route53, zoneID, name, dn
 			},
 		},
 	}
-	res, err := client.ChangeResourceRecordSetsWithContext(ctx, input)
-	if err != nil {
-		return nil, err
-	}
 
-	return res.ChangeInfo, nil
+	// Create service with assumed role, if set
+	svc := route53.New(c.ssn, GetR53ClientCfg(c.ssn, in.HostedZoneRole))
+
+	_, err := svc.ChangeResourceRecordSetsWithContext(ctx, input)
+	return err
 }
 
 // HostedZoneInput defines the input parameters for hosted zone creation.

pkg/infrastructure/aws/clusterapi/aws.go

@@ -114,18 +114,21 @@ func (*Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput)
 		}
 	}
 
-	tags := map[string]string{
-		fmt.Sprintf("kubernetes.io/cluster/%s", in.InfraID): "owned",
-	}
-	for k, v := range awsCluster.Spec.AdditionalTags {
-		tags[k] = v
-	}
-
 	client := awsconfig.NewClient(awsSession)
 
+	logrus.Infoln("Creating Route53 records for control plane load balancer")
+
 	phzID := in.InstallConfig.Config.AWS.HostedZone
 	if len(phzID) == 0 {
-		logrus.Infoln("Creating private Hosted Zone")
+		logrus.Debugln("Creating private Hosted Zone")
+
+		tags := map[string]string{
+			fmt.Sprintf("kubernetes.io/cluster/%s", in.InfraID): "owned",
+		}
+		for k, v := range awsCluster.Spec.AdditionalTags {
+			tags[k] = v
+		}
+
 		res, err := client.CreateHostedZone(ctx, &awsconfig.HostedZoneInput{
 			InfraID:  in.InfraID,
 			VpcID:    vpcID,
@@ -138,19 +141,68 @@ func (*Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput)
 			return fmt.Errorf("failed to create private hosted zone: %w", err)
 		}
 		phzID = aws.StringValue(res.Id)
+		logrus.Infoln("Created private Hosted Zone")
+	}
+
+	apiName := fmt.Sprintf("api.%s.", in.InstallConfig.Config.ClusterDomain())
+	apiIntName := fmt.Sprintf("api-int.%s.", in.InstallConfig.Config.ClusterDomain())
+
+	// Create api record in public zone
+	if in.InstallConfig.Config.PublicAPI() {
+		zone, err := client.GetBaseDomain(in.InstallConfig.Config.BaseDomain)
+		if err != nil {
+			return err
+		}
+
+		pubLB := awsCluster.Status.Network.SecondaryAPIServerELB
+		aliasZoneID, err := getHostedZoneIDForNLB(ctx, awsSession, awsCluster.Spec.Region, pubLB.DNSName)
+		if err != nil {
+			return fmt.Errorf("failed to find HostedZone ID for NLB: %w", err)
+		}
+
+		if err := client.CreateOrUpdateRecord(ctx, &awsconfig.CreateRecordInput{
+			Name:           apiName,
+			Region:         awsCluster.Spec.Region,
+			DNSTarget:      pubLB.DNSName,
+			ZoneID:         aws.StringValue(zone.Id),
+			AliasZoneID:    aliasZoneID,
+			HostedZoneRole: "", // we dont want to assume role here
+		}); err != nil {
+			return fmt.Errorf("failed to create records for api in public zone: %w", err)
+		}
+		logrus.Debugln("Created public API record in public zone")
 	}
 
-	logrus.Infoln("Creating Route53 records for control plane load balancer")
 	aliasZoneID, err := getHostedZoneIDForNLB(ctx, awsSession, awsCluster.Spec.Region, awsCluster.Status.Network.APIServerELB.Name)
 	if err != nil {
 		return fmt.Errorf("failed to find HostedZone ID for NLB: %w", err)
 	}
-	apiHost := awsCluster.Status.Network.SecondaryAPIServerELB.DNSName
-	apiIntHost := awsCluster.Spec.ControlPlaneEndpoint.Host
-	err = client.CreateOrUpdateRecord(ctx, in.InstallConfig.Config, apiHost, apiIntHost, phzID, aliasZoneID)
-	if err != nil {
-		return fmt.Errorf("failed to create route53 records: %w", err)
-	}
+
+	// Create api record in private zone
+	if err := client.CreateOrUpdateRecord(ctx, &awsconfig.CreateRecordInput{
+		Name:           apiName,
+		Region:         awsCluster.Spec.Region,
+		DNSTarget:      awsCluster.Spec.ControlPlaneEndpoint.Host,
+		ZoneID:         phzID,
+		AliasZoneID:    aliasZoneID,
+		HostedZoneRole: in.InstallConfig.Config.AWS.HostedZoneRole,
+	}); err != nil {
+		return fmt.Errorf("failed to create records for api in private zone: %w", err)
+	}
+	logrus.Debugln("Created public API record in private zone")
+
+	// Create api-int record in private zone
+	if err := client.CreateOrUpdateRecord(ctx, &awsconfig.CreateRecordInput{
+		Name:           apiIntName,
+		Region:         awsCluster.Spec.Region,
+		DNSTarget:      awsCluster.Spec.ControlPlaneEndpoint.Host,
+		ZoneID:         phzID,
+		AliasZoneID:    aliasZoneID,
+		HostedZoneRole: in.InstallConfig.Config.AWS.HostedZoneRole,
+	}); err != nil {
+		return fmt.Errorf("failed to create records for api-int in private zone: %w", err)
+	}
+	logrus.Debugln("Created private API record in private zone")
 
 	return nil
 }