Skip to content

[release-4.16] OCPBUGS-54165: aws: fix NLB creation in secret regions #9595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: release-4.16
Choose a base branch
from
Open
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
80 changes: 29 additions & 51 deletions pkg/asset/installconfig/aws/route53.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -156,67 +155,45 @@ 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 {
// 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
}

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"),
Expand All @@ -225,12 +202,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.
Expand Down Expand Up @@ -277,6 +254,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"),
Expand Down
80 changes: 61 additions & 19 deletions pkg/infrastructure/aws/clusterapi/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,46 +115,88 @@ 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")

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)
}
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
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 {
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
}
Expand Down