Skip to content
Merged
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
1 change: 1 addition & 0 deletions data/data/aws/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ module "dns" {
cluster_domain = var.cluster_domain
cluster_id = var.cluster_id
tags = local.tags
internal_zone = var.aws_internal_zone
vpc_id = module.vpc.vpc_id
region = var.aws_region
publish_strategy = var.aws_publish_strategy
Expand Down
16 changes: 11 additions & 5 deletions data/data/aws/route53/base.tf
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ data "aws_route53_zone" "public" {
name = var.base_domain
}

resource "aws_route53_zone" "int" {
data "aws_route53_zone" "int" {
zone_id = var.internal_zone == null ? aws_route53_zone.new_int[0].id : var.internal_zone
}

resource "aws_route53_zone" "new_int" {
count = var.internal_zone == null ? 1 : 0

name = var.cluster_domain
force_destroy = true

Expand Down Expand Up @@ -50,7 +56,7 @@ resource "aws_route53_record" "api_external_alias" {
resource "aws_route53_record" "api_internal_alias" {
count = local.use_alias ? 1 : 0

zone_id = aws_route53_zone.int.zone_id
zone_id = data.aws_route53_zone.int.zone_id
name = "api-int.${var.cluster_domain}"
type = "A"

Expand All @@ -64,7 +70,7 @@ resource "aws_route53_record" "api_internal_alias" {
resource "aws_route53_record" "api_external_internal_zone_alias" {
count = local.use_alias ? 1 : 0

zone_id = aws_route53_zone.int.zone_id
zone_id = data.aws_route53_zone.int.zone_id
name = "api.${var.cluster_domain}"
type = "A"

Expand All @@ -89,7 +95,7 @@ resource "aws_route53_record" "api_external_cname" {
resource "aws_route53_record" "api_internal_cname" {
count = local.use_cname ? 1 : 0

zone_id = aws_route53_zone.int.zone_id
zone_id = data.aws_route53_zone.int.zone_id
name = "api-int.${var.cluster_domain}"
type = "CNAME"
ttl = 10
Expand All @@ -100,7 +106,7 @@ resource "aws_route53_record" "api_internal_cname" {
resource "aws_route53_record" "api_external_internal_zone_cname" {
count = local.use_cname ? 1 : 0

zone_id = aws_route53_zone.int.zone_id
zone_id = data.aws_route53_zone.int.zone_id
name = "api.${var.cluster_domain}"
type = "CNAME"
ttl = 10
Expand Down
5 changes: 5 additions & 0 deletions data/data/aws/route53/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ variable "tags" {
description = "AWS tags to be applied to created resources."
}

variable "internal_zone" {
type = string
description = "An existing hosted zone (zone ID) to use for the internal API."
}

variable "api_external_lb_dns_name" {
description = "External API's LB DNS name"
type = string
Expand Down
6 changes: 6 additions & 0 deletions data/data/aws/variables-aws.tf
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ variable "aws_private_subnets" {
description = "(optional) Existing private subnets into which the cluster should be installed."
}

variable "aws_internal_zone" {
type = string
default = null
description = "(optional) An existing hosted zone (zone ID) to use for the internal API."
}

variable "aws_publish_strategy" {
type = string
description = "The cluster publishing strategy, either Internal or External"
Expand Down
8 changes: 8 additions & 0 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,14 @@ spec:
type: string
type: array
type: object
hostedZone:
description: HostedZone is the ID of an existing hosted zone into
which to add DNS records for the cluster's internal API. An
existing hosted zone can only be used when also using existing
subnets. The hosted zone must be associated with the VPC containing
the subnets. Leave the hosted zone unset to have the installer
create the hosted zone on your behalf.
type: string
region:
description: Region specifies the AWS region where the cluster
will be created.
Expand Down
34 changes: 23 additions & 11 deletions pkg/asset/cluster/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/aws/aws-sdk-go/service/route53"
"github.com/pkg/errors"

"github.com/openshift/installer/pkg/asset/installconfig"
Expand All @@ -25,25 +26,26 @@ func Metadata(clusterID, infraID string, config *types.InstallConfig) *awstypes.
"openshiftClusterID": clusterID,
}},
ServiceEndpoints: config.AWS.ServiceEndpoints,
ClusterDomain: config.ClusterDomain(),
}
}

// PreTerraform performs any infrastructure initialization which must
// happen before Terraform creates the remaining infrastructure.
func PreTerraform(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error {

if err := tagSubnetEC2Instances(ctx, clusterID, installConfig); err != nil {
if err := tagSharedVPCResources(ctx, clusterID, installConfig); err != nil {
return err
}

if err := tagIamRoles(ctx, clusterID, installConfig); err != nil {
if err := tagSharedIAMRoles(ctx, clusterID, installConfig); err != nil {
return err
}

return nil
}

func tagSubnetEC2Instances(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error {
func tagSharedVPCResources(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error {
if len(installConfig.Config.Platform.AWS.Subnets) == 0 {
return nil
}
Expand All @@ -58,7 +60,6 @@ func tagSubnetEC2Instances(ctx context.Context, clusterID string, installConfig
return err
}

//arns := make([]*string, 0, len(privateSubnets)+len(publicSubnets))
ids := make([]*string, 0, len(privateSubnets)+len(publicSubnets))
for id := range privateSubnets {
ids = append(ids, aws.String(id))
Expand All @@ -69,23 +70,34 @@ func tagSubnetEC2Instances(ctx context.Context, clusterID string, installConfig

session, err := installConfig.AWS.Session(ctx)
if err != nil {
return err
return errors.Wrap(err, "could not create AWS session")
}
key, value := sharedTag(clusterID)
ec2Tags := []*ec2.Tag{{Key: &key, Value: &value}}
ec2Client := ec2.New(session, aws.NewConfig().WithRegion(installConfig.Config.Platform.AWS.Region))

tagKey, tagValue := sharedTag(clusterID)

ec2Client := ec2.New(session, aws.NewConfig().WithRegion(installConfig.Config.Platform.AWS.Region))
if _, err = ec2Client.CreateTagsWithContext(ctx, &ec2.CreateTagsInput{
Resources: ids,
Tags: ec2Tags,
Tags: []*ec2.Tag{{Key: &tagKey, Value: &tagValue}},
}); err != nil {
return err
return errors.Wrap(err, "could not add tags to subnets")
}

if zone := installConfig.Config.AWS.HostedZone; zone != "" {
route53Client := route53.New(session)
if _, err := route53Client.ChangeTagsForResourceWithContext(ctx, &route53.ChangeTagsForResourceInput{
ResourceType: aws.String("hostedzone"),
ResourceId: aws.String(zone),
AddTags: []*route53.Tag{{Key: &tagKey, Value: &tagValue}},
}); err != nil {
return errors.Wrap(err, "could not add tags to hosted zone")
}
}

return nil
}

func tagIamRoles(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error {
func tagSharedIAMRoles(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error {
var iamRoleNames []*string
if mp := installConfig.Config.ControlPlane; mp != nil {
awsMP := &awstypes.MachinePool{}
Expand Down
1 change: 1 addition & 0 deletions pkg/asset/cluster/tfvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
VPC: vpc,
PrivateSubnets: privateSubnets,
PublicSubnets: publicSubnets,
InternalZone: installConfig.Config.AWS.HostedZone,
Services: installConfig.Config.AWS.ServiceEndpoints,
Publish: installConfig.Config.Publish,
MasterConfigs: masterConfigs,
Expand Down
88 changes: 88 additions & 0 deletions pkg/asset/installconfig/aws/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ import (
"net"
"net/url"
"sort"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/endpoints"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/route53"
"github.com/pkg/errors"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -262,3 +266,87 @@ var requiredServices = []string{
"sts",
"tagging",
}

// ValidateForProvisioning validates if the install config is valid for provisioning the cluster.
func ValidateForProvisioning(session *session.Session, ic *types.InstallConfig, metadata *Metadata) error {
allErrs := field.ErrorList{}
allErrs = append(allErrs, validateExistingHostedZone(session, ic, metadata)...)
return allErrs.ToAggregate()
}

func validateExistingHostedZone(session *session.Session, ic *types.InstallConfig, metadata *Metadata) field.ErrorList {
if ic.AWS.HostedZone == "" {
return nil
}

// validate that the hosted zone exists
hostedZonePath := field.NewPath("aws", "hostedZone")
client := route53.New(session)
zone, err := client.GetHostedZone(&route53.GetHostedZoneInput{Id: aws.String(ic.AWS.HostedZone)})
if err != nil {
return field.ErrorList{
field.Invalid(hostedZonePath, ic.AWS.HostedZone, "cannot find hosted zone"),
}
}

allErrs := field.ErrorList{}

// validate that the hosted zone is associated with the VPC containing the existing subnets for the cluster
vpcID, err := metadata.VPC(context.TODO())
if err == nil {
if !isHostedZoneAssociatedWithVPC(zone, vpcID) {
allErrs = append(allErrs, field.Invalid(hostedZonePath, ic.AWS.HostedZone, "hosted zone is not associated with the VPC"))
}
} else {
allErrs = append(allErrs, field.Invalid(hostedZonePath, ic.AWS.HostedZone, "no VPC found"))
}

// validate that the hosted zone does not already have any record sets for the cluster domain
dottedClusterDomain := ic.ClusterDomain() + "."
var problematicRecords []string
if err := client.ListResourceRecordSetsPages(
&route53.ListResourceRecordSetsInput{HostedZoneId: zone.HostedZone.Id},
func(out *route53.ListResourceRecordSetsOutput, lastPage bool) bool {
for _, recordSet := range out.ResourceRecordSets {
name := aws.StringValue(recordSet.Name)
// skip record sets that are not sub-domains of the cluster domain. Such record sets may exist for
// hosted zones that are used for other clusters or other purposes.
if !strings.HasSuffix(name, dottedClusterDomain) {
continue
}
// skip record sets that are the cluster domain. Record sets for the cluster domain are fine. If the
// hosted zone has the name of the cluster domain, then there will be NS and SOA record sets for the
// cluster domain.
if len(name) == len(dottedClusterDomain) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't get why we continue here. It seems like it is ok to have a record equal to the cluster domain. Is that the case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, every hosted zone has two records which are NS and SOA records with the same domain as the hosted zone. I will add a comment about this.

continue
}
problematicRecords = append(problematicRecords, fmt.Sprintf("%s (%s)", name, aws.StringValue(recordSet.Type)))
}
return !lastPage
},
); err != nil {
allErrs = append(allErrs, field.InternalError(hostedZonePath,
errors.Wrapf(err, "could not list record sets for hosted zone %q", ic.AWS.HostedZone)))
}
if len(problematicRecords) > 0 {
detail := fmt.Sprintf(
"hosted zone already has record sets for the domain of the cluster: [%s]",
strings.Join(problematicRecords, ", "),
)
allErrs = append(allErrs, field.Invalid(hostedZonePath, ic.AWS.HostedZone, detail))
}

return allErrs
}

func isHostedZoneAssociatedWithVPC(hostedZone *route53.GetHostedZoneOutput, vpcID string) bool {
if vpcID == "" {
return false
}
for _, vpc := range hostedZone.VPCs {
if aws.StringValue(vpc.VPCId) == vpcID {
return true
}
}
return false
}
9 changes: 8 additions & 1 deletion pkg/asset/installconfig/platformprovisioncheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

"github.com/openshift/installer/pkg/asset"
awsconfig "github.com/openshift/installer/pkg/asset/installconfig/aws"
azconfig "github.com/openshift/installer/pkg/asset/installconfig/azure"
bmconfig "github.com/openshift/installer/pkg/asset/installconfig/baremetal"
gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp"
Expand Down Expand Up @@ -45,6 +46,12 @@ func (a *PlatformProvisionCheck) Generate(dependencies asset.Parents) error {
var err error
platform := ic.Config.Platform.Name()
switch platform {
case aws.Name:
session, err := ic.AWS.Session(context.TODO())
if err != nil {
return err
}
return awsconfig.ValidateForProvisioning(session, ic.Config, ic.AWS)
case azure.Name:
dnsConfig, err := ic.Azure.DNSConfig()
if err != nil {
Expand Down Expand Up @@ -92,7 +99,7 @@ func (a *PlatformProvisionCheck) Generate(dependencies asset.Parents) error {
if err != nil {
return err
}
case aws.Name, libvirt.Name, none.Name, ovirt.Name:
case libvirt.Name, none.Name, ovirt.Name:
// no special provisioning requirements to check
default:
err = fmt.Errorf("unknown platform type %q", platform)
Expand Down
12 changes: 8 additions & 4 deletions pkg/asset/manifests/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,14 @@ func (d *DNS) Generate(dependencies asset.Parents) error {
}
config.Spec.PublicZone = &configv1.DNSZone{ID: strings.TrimPrefix(*zone.Id, "/hostedzone/")}
}
config.Spec.PrivateZone = &configv1.DNSZone{Tags: map[string]string{
fmt.Sprintf("kubernetes.io/cluster/%s", clusterID.InfraID): "owned",
"Name": fmt.Sprintf("%s-int", clusterID.InfraID),
}}
if hostedZone := installConfig.Config.AWS.HostedZone; hostedZone == "" {
config.Spec.PrivateZone = &configv1.DNSZone{Tags: map[string]string{
fmt.Sprintf("kubernetes.io/cluster/%s", clusterID.InfraID): "owned",
"Name": fmt.Sprintf("%s-int", clusterID.InfraID),
}}
} else {
config.Spec.PrivateZone = &configv1.DNSZone{ID: hostedZone}
}
case azuretypes.Name:
dnsConfig, err := installConfig.Azure.DNSConfig()
if err != nil {
Expand Down
Loading