From f3b063d46de745f53111fc5d79656c1ace1d163f Mon Sep 17 00:00:00 2001 From: Rafael Fonseca Date: Wed, 19 Jun 2024 23:22:17 +0200 Subject: [PATCH 1/4] infra/aws: remove apiHost condition on Load Balancer This is leftover from before CAPA had support for a public LB as the secondary controlPlane load balancer. We had to configure the AWSCluster in such a way the primary load balancer would either be `InternetFacing` if publish was set to "External" or `Internal` otherwise. Now the primary LB is always `Internal` and the secondary LB only exists when publish is "External". --- pkg/infrastructure/aws/clusterapi/aws.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/infrastructure/aws/clusterapi/aws.go b/pkg/infrastructure/aws/clusterapi/aws.go index ca1dbbb4d5d..c3a81d7fa67 100644 --- a/pkg/infrastructure/aws/clusterapi/aws.go +++ b/pkg/infrastructure/aws/clusterapi/aws.go @@ -146,9 +146,6 @@ func (*Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput) return fmt.Errorf("failed to find HostedZone ID for NLB: %w", err) } apiHost := awsCluster.Status.Network.SecondaryAPIServerELB.DNSName - if awsCluster.Status.Network.APIServerELB.Scheme == capa.ELBSchemeInternetFacing { - apiHost = awsCluster.Status.Network.APIServerELB.DNSName - } apiIntHost := awsCluster.Spec.ControlPlaneEndpoint.Host err = client.CreateOrUpdateRecord(ctx, in.InstallConfig.Config, apiHost, apiIntHost, phzID, aliasZoneID) if err != nil { From 26338beebee546c1d8700a31bd3ce3abd318e4f4 Mon Sep 17 00:00:00 2001 From: Rafael Fonseca Date: Wed, 19 Jun 2024 23:57:33 +0200 Subject: [PATCH 2/4] 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 | 73 +++++++-------------- pkg/infrastructure/aws/clusterapi/aws.go | 82 +++++++++++++++++++----- 2 files changed, 89 insertions(+), 66 deletions(-) diff --git a/pkg/asset/installconfig/aws/route53.go b/pkg/asset/installconfig/aws/route53.go index acb664a9a42..6a8c7352d37 100644 --- a/pkg/asset/installconfig/aws/route53.go +++ b/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. diff --git a/pkg/infrastructure/aws/clusterapi/aws.go b/pkg/infrastructure/aws/clusterapi/aws.go index c3a81d7fa67..f05f7f55aee 100644 --- a/pkg/infrastructure/aws/clusterapi/aws.go +++ b/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 } From 15cdd65fa7b4f3c2eace27f01fd74d61f23e9153 Mon Sep 17 00:00:00 2001 From: Rafael Fonseca Date: Thu, 20 Jun 2024 09:22:13 +0200 Subject: [PATCH 3/4] aws: move owned tag creation to `CreateHostedZone` Tagging the resource as "owned" should be part of the creation. --- pkg/asset/installconfig/aws/route53.go | 1 + pkg/infrastructure/aws/clusterapi/aws.go | 9 +-------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/asset/installconfig/aws/route53.go b/pkg/asset/installconfig/aws/route53.go index 6a8c7352d37..aa0684d7f2f 100644 --- a/pkg/asset/installconfig/aws/route53.go +++ b/pkg/asset/installconfig/aws/route53.go @@ -248,6 +248,7 @@ func (c *Client) CreateHostedZone(ctx context.Context, input *HostedZoneInput) ( // Tag the hosted zone tags := mergeTags(input.UserTags, map[string]string{ "Name": fmt.Sprintf("%s-int", input.InfraID), + fmt.Sprintf("kubernetes.io/cluster/%s", input.InfraID): "owned", }) _, err = svc.ChangeTagsForResourceWithContext(ctx, &route53.ChangeTagsForResourceInput{ ResourceType: aws.String("hostedzone"), diff --git a/pkg/infrastructure/aws/clusterapi/aws.go b/pkg/infrastructure/aws/clusterapi/aws.go index f05f7f55aee..cc55fe265e3 100644 --- a/pkg/infrastructure/aws/clusterapi/aws.go +++ b/pkg/infrastructure/aws/clusterapi/aws.go @@ -122,20 +122,13 @@ func (*Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput) if len(phzID) == 0 { 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, Region: awsCluster.Spec.Region, Name: in.InstallConfig.Config.ClusterDomain(), Role: in.InstallConfig.Config.AWS.HostedZoneRole, - UserTags: tags, + UserTags: awsCluster.Spec.AdditionalTags, }) if err != nil { return fmt.Errorf("failed to create private hosted zone: %w", err) From 20cd86a9fe9674438b1422f6f348ae42057b661f Mon Sep 17 00:00:00 2001 From: Rafael Fonseca Date: Mon, 30 Sep 2024 17:01:42 +0200 Subject: [PATCH 4/4] ic: aws: document createRecord input fields. This should remove any ambiguity/misunderstanding when the field names are not clear enough. --- pkg/asset/installconfig/aws/route53.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/asset/installconfig/aws/route53.go b/pkg/asset/installconfig/aws/route53.go index aa0684d7f2f..e7ccf88a561 100644 --- a/pkg/asset/installconfig/aws/route53.go +++ b/pkg/asset/installconfig/aws/route53.go @@ -157,11 +157,17 @@ func GetR53ClientCfg(sess *awss.Session, roleARN string) *aws.Config { // CreateRecordInput collects information for creating a record. type CreateRecordInput struct { - Name string - Region string - DNSTarget string - ZoneID string - AliasZoneID string + // Fully qualified record domain name. + Name string + // Cluster Region. + Region string + // Where to route the DNS queries to. + DNSTarget string + // ID of the Hosted Zone. + ZoneID string + // ID of the Hosted Zone for Alias record. + AliasZoneID string + // Role to assume to create the record. Leave empty to not assume role. HostedZoneRole string }