From 49f3497f5e511d704d3d18498875b31e1191d4b1 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Tue, 2 May 2023 15:23:14 -0400 Subject: [PATCH 01/11] aws: add HostedZoneRole to platform Adds a hosted zone role field. If provided, this role will be assumed whenever operations are performed on the provided hosted zone. This enables the private hosted zone to belong to a different account than the rest of the cluster. --- data/data/install.openshift.io_installconfigs.yaml | 8 ++++++++ pkg/explain/printer_test.go | 3 +++ pkg/types/aws/platform.go | 9 +++++++++ 3 files changed, 20 insertions(+) diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 87b54e808c4..5a1c94c1f34 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -2267,6 +2267,14 @@ spec: the subnets. Leave the hosted zone unset to have the installer create the hosted zone on your behalf. type: string + hostedZoneRole: + description: HostedZoneRole is the ARN of a role to be assumed + when performing operations on the provided HostedZone. HostedZoneRole + can be used in a shared VPC scenario when the private hosted + zone belongs to a different account than the rest of the cluster + resources. If HostedZoneRole is set, HostedZone must also be + set. + type: string lbType: description: "LBType is an optional field to specify a load balancer type. \n When this field is specified, the default ingresscontroller diff --git a/pkg/explain/printer_test.go b/pkg/explain/printer_test.go index bd9b06b55b1..35df45867cc 100644 --- a/pkg/explain/printer_test.go +++ b/pkg/explain/printer_test.go @@ -156,6 +156,9 @@ func Test_PrintFields(t *testing.T) { hostedZone 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. + hostedZoneRole + HostedZoneRole is the ARN of a role to be assumed when performing operations on the provided HostedZone. HostedZoneRole can be used in a shared VPC scenario when the private hosted zone belongs to a different account than the rest of the cluster resources. If HostedZoneRole is set, HostedZone must also be set. + lbType LBType is an optional field to specify a load balancer type. When this field is specified, the default ingresscontroller will be created using the specified load-balancer type. diff --git a/pkg/types/aws/platform.go b/pkg/types/aws/platform.go index 44f932cfcc6..dfdc28319eb 100644 --- a/pkg/types/aws/platform.go +++ b/pkg/types/aws/platform.go @@ -41,6 +41,15 @@ type Platform struct { // +optional HostedZone string `json:"hostedZone,omitempty"` + // HostedZoneRole is the ARN of a role to be assumed when performing + // operations on the provided HostedZone. HostedZoneRole can be used + // in a shared VPC scenario when the private hosted zone belongs to a + // different account than the rest of the cluster resources. + // If HostedZoneRole is set, HostedZone must also be set. + // + // +optional + HostedZoneRole string `json:"hostedZoneRole,omitempty"` + // UserTags additional keys and values that the installer will add // as tags to all resources that it creates. Resources created by the // cluster itself may not include these tags. From 63fa17d3d6e410874628e8503825dae3fb5c8be0 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Tue, 2 May 2023 15:31:34 -0400 Subject: [PATCH 02/11] aws: validate hosted zone role If a hosted zone role is specified in the install config, users must also supply a hosted zone. --- pkg/types/aws/validation/platform.go | 4 ++++ pkg/types/aws/validation/platform_test.go | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/pkg/types/aws/validation/platform.go b/pkg/types/aws/validation/platform.go index 07f729e89b3..72d518dcd7c 100644 --- a/pkg/types/aws/validation/platform.go +++ b/pkg/types/aws/validation/platform.go @@ -39,6 +39,10 @@ func ValidatePlatform(p *aws.Platform, fldPath *field.Path) field.ErrorList { } } + if p.HostedZoneRole != "" && p.HostedZone == "" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("hostedZoneRole"), p.HostedZoneRole, "may not specify a role to assume for hosted zone operations without also specifying a hosted zone")) + } + allErrs = append(allErrs, validateServiceEndpoints(p.ServiceEndpoints, fldPath.Child("serviceEndpoints"))...) allErrs = append(allErrs, validateUserTags(p.UserTags, p.PropagateUserTag, fldPath.Child("userTags"))...) diff --git a/pkg/types/aws/validation/platform_test.go b/pkg/types/aws/validation/platform_test.go index a48f9283b8f..6d2f9cf899c 100644 --- a/pkg/types/aws/validation/platform_test.go +++ b/pkg/types/aws/validation/platform_test.go @@ -175,6 +175,23 @@ func TestValidatePlatform(t *testing.T) { }, expected: fmt.Sprintf(`^\Qtest-path.userTags: Too many: %d: must have at most %d items`, userTagLimit+1, userTagLimit), }, + { + name: "hosted zone role without hosted zone", + platform: &aws.Platform{ + Region: "us-east-1", + HostedZoneRole: "test-hosted-zone-role", + }, + expected: `^test-path\.hostedZoneRole: Invalid value: "test-hosted-zone-role": may not specify a role to assume for hosted zone operations without also specifying a hosted zone$`, + }, + { + name: "hosted zone & role", + platform: &aws.Platform{ + Region: "us-east-1", + Subnets: []string{"test-subnet"}, + HostedZone: "test-hosted-zone", + HostedZoneRole: "test-hosted-zone-role", + }, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { From 0b546a8a0a9325f3d344b02de3f0661b5fdd3ea3 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 3 May 2023 13:39:13 -0400 Subject: [PATCH 03/11] aws: add hostedZoneRole to metadata The hostedZoneRole will need to be used when destroying the cluster. --- pkg/types/aws/metadata.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/types/aws/metadata.go b/pkg/types/aws/metadata.go index 609c48baaee..343f736489d 100644 --- a/pkg/types/aws/metadata.go +++ b/pkg/types/aws/metadata.go @@ -18,4 +18,8 @@ type Metadata struct { // ClusterDomain is the domain for the cluster. ClusterDomain string `json:"clusterDomain"` + + // HostedZoneRole is the role to assume when performing operations + // on a hosted zone owned by another account. + HostedZoneRole string `json:"hostedZoneRole,omitempty"` } From d74cf24827bc1b88a04ea478ab0dd6f804803054 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 3 May 2023 14:55:26 -0400 Subject: [PATCH 04/11] aws: update route53 clients to allow assume role Updates the route53 clients to allow passing config with assume role credentials. This allows the function caller to determine whether the service should authenticate with the default credentials or creds for another account. --- pkg/asset/cluster/aws/aws.go | 5 +++- pkg/asset/installconfig/aws/route53.go | 30 ++++++++++++++++------- pkg/asset/installconfig/aws/validation.go | 5 ++-- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/pkg/asset/cluster/aws/aws.go b/pkg/asset/cluster/aws/aws.go index 164c94a1846..80bfe1f04d5 100644 --- a/pkg/asset/cluster/aws/aws.go +++ b/pkg/asset/cluster/aws/aws.go @@ -11,6 +11,7 @@ import ( "github.com/pkg/errors" "github.com/openshift/installer/pkg/asset/installconfig" + awsic "github.com/openshift/installer/pkg/asset/installconfig/aws" "github.com/openshift/installer/pkg/types" awstypes "github.com/openshift/installer/pkg/types/aws" ) @@ -26,6 +27,7 @@ func Metadata(clusterID, infraID string, config *types.InstallConfig) *awstypes. }}, ServiceEndpoints: config.AWS.ServiceEndpoints, ClusterDomain: config.ClusterDomain(), + HostedZoneRole: config.AWS.HostedZoneRole, } } @@ -79,7 +81,8 @@ func tagSharedVPCResources(ctx context.Context, clusterID string, installConfig } if zone := installConfig.Config.AWS.HostedZone; zone != "" { - route53Client := route53.New(session) + r53cfg := awsic.GetR53ClientCfg(session, installConfig.Config.AWS.HostedZoneRole) + route53Client := route53.New(session, r53cfg) if _, err := route53Client.ChangeTagsForResourceWithContext(ctx, &route53.ChangeTagsForResourceInput{ ResourceType: aws.String("hostedzone"), ResourceId: aws.String(zone), diff --git a/pkg/asset/installconfig/aws/route53.go b/pkg/asset/installconfig/aws/route53.go index 99f1abe47a8..e40ce07fa16 100644 --- a/pkg/asset/installconfig/aws/route53.go +++ b/pkg/asset/installconfig/aws/route53.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/credentials/stscreds" awss "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/route53" "github.com/pkg/errors" @@ -17,10 +18,10 @@ import ( // API represents the calls made to the API. type API interface { - GetHostedZone(hostedZone string) (*route53.GetHostedZoneOutput, error) - ValidateZoneRecords(zone *route53.HostedZone, zoneName string, zonePath *field.Path, ic *types.InstallConfig) field.ErrorList + GetHostedZone(hostedZone string, cfg *aws.Config) (*route53.GetHostedZoneOutput, error) + ValidateZoneRecords(zone *route53.HostedZone, zoneName string, zonePath *field.Path, ic *types.InstallConfig, cfg *aws.Config) field.ErrorList GetBaseDomain(baseDomainName string) (*route53.HostedZone, error) - GetSubDomainDNSRecords(hostedZone *route53.HostedZone, ic *types.InstallConfig) ([]string, error) + GetSubDomainDNSRecords(hostedZone *route53.HostedZone, ic *types.InstallConfig, cfg *aws.Config) ([]string, error) } // Client makes calls to the AWS Route53 API. @@ -37,9 +38,9 @@ func NewClient(ssn *awss.Session) *Client { } // GetHostedZone attempts to get the hosted zone from the AWS Route53 instance -func (c *Client) GetHostedZone(hostedZone string) (*route53.GetHostedZoneOutput, error) { +func (c *Client) GetHostedZone(hostedZone string, cfg *aws.Config) (*route53.GetHostedZoneOutput, error) { // build a new Route53 instance from the same session that made it here - r53 := route53.New(c.ssn) + r53 := route53.New(c.ssn, cfg) // validate that the hosted zone exists hostedZoneOutput, err := r53.GetHostedZone(&route53.GetHostedZoneInput{Id: aws.String(hostedZone)}) @@ -50,10 +51,10 @@ func (c *Client) GetHostedZone(hostedZone string) (*route53.GetHostedZoneOutput, } // ValidateZoneRecords Attempts to validate each of the candidate HostedZones against the Config -func (c *Client) ValidateZoneRecords(zone *route53.HostedZone, zoneName string, zonePath *field.Path, ic *types.InstallConfig) field.ErrorList { +func (c *Client) ValidateZoneRecords(zone *route53.HostedZone, zoneName string, zonePath *field.Path, ic *types.InstallConfig, cfg *aws.Config) field.ErrorList { allErrs := field.ErrorList{} - problematicRecords, err := c.GetSubDomainDNSRecords(zone, ic) + problematicRecords, err := c.GetSubDomainDNSRecords(zone, ic, cfg) if err != nil { allErrs = append(allErrs, field.InternalError(zonePath, errors.Wrapf(err, "could not list record sets for domain %q", zoneName))) @@ -72,7 +73,7 @@ func (c *Client) ValidateZoneRecords(zone *route53.HostedZone, zoneName string, // GetSubDomainDNSRecords Validates the hostedZone against the cluster domain, and ensures that the // cluster domain does not have a current record set for the hostedZone -func (c *Client) GetSubDomainDNSRecords(hostedZone *route53.HostedZone, ic *types.InstallConfig) ([]string, error) { +func (c *Client) GetSubDomainDNSRecords(hostedZone *route53.HostedZone, ic *types.InstallConfig, cfg *aws.Config) ([]string, error) { dottedClusterDomain := ic.ClusterDomain() + "." // validate that the domain of the hosted zone is the cluster domain or a parent of the cluster domain @@ -80,7 +81,7 @@ func (c *Client) GetSubDomainDNSRecords(hostedZone *route53.HostedZone, ic *type return nil, errors.Errorf("hosted zone domain %q is not a parent of the cluster domain %q", *hostedZone.Name, dottedClusterDomain) } - r53 := route53.New(c.ssn) + r53 := route53.New(c.ssn, cfg) var problematicRecords []string // validate that the hosted zone does not already have any record sets for the cluster domain @@ -134,3 +135,14 @@ func (c *Client) GetBaseDomain(baseDomainName string) (*route53.HostedZone, erro } return baseDomainZone, nil } + +// GetR53ClientCfg creates a config for the route53 client by determining +// whether it is needed to obtain STS assume role credentials. +func GetR53ClientCfg(sess *awss.Session, roleARN string) *aws.Config { + if roleARN == "" { + return nil + } + + creds := stscreds.NewCredentials(sess, roleARN) + return &aws.Config{Credentials: creds} +} diff --git a/pkg/asset/installconfig/aws/validation.go b/pkg/asset/installconfig/aws/validation.go index 43191c8bcd3..0002ddff39d 100644 --- a/pkg/asset/installconfig/aws/validation.go +++ b/pkg/asset/installconfig/aws/validation.go @@ -387,11 +387,12 @@ func ValidateForProvisioning(client API, ic *types.InstallConfig, metadata *Meta errors := field.ErrorList{} allErrs := field.ErrorList{} + r53cfg := GetR53ClientCfg(metadata.session, ic.AWS.HostedZoneRole) if ic.AWS.HostedZone != "" { zoneName = ic.AWS.HostedZone zonePath = field.NewPath("aws", "hostedZone") - zoneOutput, err := client.GetHostedZone(zoneName) + zoneOutput, err := client.GetHostedZone(zoneName, r53cfg) if err != nil { return field.ErrorList{ field.Invalid(zonePath, zoneName, "cannot find hosted zone"), @@ -416,7 +417,7 @@ func ValidateForProvisioning(client API, ic *types.InstallConfig, metadata *Meta zone = baseDomainOutput } - if errors = client.ValidateZoneRecords(zone, zoneName, zonePath, ic); len(errors) > 0 { + if errors = client.ValidateZoneRecords(zone, zoneName, zonePath, ic, r53cfg); len(errors) > 0 { allErrs = append(allErrs, errors...) } From 62c27f21c878925a19f196d0d1d587599e4f0da3 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 3 May 2023 15:48:06 -0400 Subject: [PATCH 05/11] aws: destroy phzs in other accounts Add the ability to assume a role in a different account when destroying records in a private hosted zone. When the hostedZoneRole is passed in the metadata, that role will be used when running destroy. --- pkg/destroy/aws/aws.go | 36 +++++++++++++++++++++++++----------- pkg/destroy/aws/shared.go | 26 ++++++++++++++++++-------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/pkg/destroy/aws/aws.go b/pkg/destroy/aws/aws.go index a5bc3f559ed..9f0f34e8651 100644 --- a/pkg/destroy/aws/aws.go +++ b/pkg/destroy/aws/aws.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/credentials/stscreds" "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/aws/session" @@ -56,11 +57,12 @@ type ClusterUninstaller struct { // } // // will match resources with (a:b and c:d) or d:e. - Filters []Filter // filter(s) we will be searching for - Logger logrus.FieldLogger - Region string - ClusterID string - ClusterDomain string + Filters []Filter // filter(s) we will be searching for + Logger logrus.FieldLogger + Region string + ClusterID string + ClusterDomain string + HostedZoneRole string // Session is the AWS session to be used for deletion. If nil, a // new session will be created based on the usual credential @@ -84,12 +86,13 @@ func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers. } return &ClusterUninstaller{ - Filters: filters, - Region: region, - Logger: logger, - ClusterID: metadata.InfraID, - ClusterDomain: metadata.AWS.ClusterDomain, - Session: session, + Filters: filters, + Region: region, + Logger: logger, + ClusterID: metadata.InfraID, + ClusterDomain: metadata.AWS.ClusterDomain, + Session: session, + HostedZoneRole: metadata.AWS.HostedZoneRole, }, nil } @@ -134,6 +137,17 @@ func (o *ClusterUninstaller) RunWithContext(ctx context.Context) ([]string, erro resourcegroupstaggingapi.New(awsSession), } + if o.HostedZoneRole != "" { + creds := stscreds.NewCredentials(awsSession, o.HostedZoneRole) + // This client is specifically for finding route53 zones, + // so it needs to use the "global" us-east-1 region. + cfg := &aws.Config{ + Credentials: creds, + Region: aws.String(endpoints.UsEast1RegionID), + } + tagClients = append(tagClients, resourcegroupstaggingapi.New(awsSession, cfg)) + } + switch o.Region { case endpoints.CnNorth1RegionID, endpoints.CnNorthwest1RegionID: break diff --git a/pkg/destroy/aws/shared.go b/pkg/destroy/aws/shared.go index df5299ded42..b9da24dd48f 100644 --- a/pkg/destroy/aws/shared.go +++ b/pkg/destroy/aws/shared.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/credentials/stscreds" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" @@ -189,8 +190,6 @@ func (o *ClusterUninstaller) cleanSharedARN(ctx context.Context, session *sessio } func (o *ClusterUninstaller) cleanSharedRoute53(ctx context.Context, session *session.Session, arn arn.ARN, logger logrus.FieldLogger) error { - client := route53.New(session) - resourceType, id, err := splitSlash("resource", arn.Resource) if err != nil { return err @@ -199,27 +198,38 @@ func (o *ClusterUninstaller) cleanSharedRoute53(ctx context.Context, session *se switch resourceType { case "hostedzone": - return o.cleanSharedHostedZone(ctx, client, id, logger) + return o.cleanSharedHostedZone(ctx, session, id, logger) default: logger.Debugf("Nothing to clean for shared %s resource", resourceType) return nil } } -func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, client *route53.Route53, id string, logger logrus.FieldLogger) error { +func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, session *session.Session, id string, logger logrus.FieldLogger) error { + publicClient := route53.New(session) + + // The private hosted zone (phz) may belong to a different account, + // in which case we need a separate client. + phzClient := publicClient + if o.HostedZoneRole != "" { + creds := stscreds.NewCredentials(session, o.HostedZoneRole) + phzClient = route53.New(session, &aws.Config{Credentials: creds}) + logger.Infof("Assuming role %s to destroy records in private hosted zone", o.HostedZoneRole) + } + if o.ClusterDomain == "" { logger.Debug("No cluster domain specified in metadata; cannot clean the shared hosted zone") return nil } dottedClusterDomain := o.ClusterDomain + "." - publicZoneID, err := findAncestorPublicRoute53(ctx, client, dottedClusterDomain, logger) + publicZoneID, err := findAncestorPublicRoute53(ctx, publicClient, dottedClusterDomain, logger) if err != nil { return err } var lastError error - err = client.ListResourceRecordSetsPagesWithContext( + err = phzClient.ListResourceRecordSetsPagesWithContext( ctx, &route53.ListResourceRecordSetsInput{HostedZoneId: aws.String(id)}, func(results *route53.ListResourceRecordSetsOutput, lastPage bool) bool { @@ -236,7 +246,7 @@ func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, client * // delete any matching record sets in the public hosted zone if publicZoneID != "" { publicZoneLogger := logger.WithField("id", publicZoneID) - if err := deleteMatchingRecordSetInPublicZone(ctx, client, publicZoneID, recordSet, publicZoneLogger); err != nil { + if err := deleteMatchingRecordSetInPublicZone(ctx, publicClient, publicZoneID, recordSet, publicZoneLogger); err != nil { if lastError != nil { publicZoneLogger.Debug(lastError) } @@ -248,7 +258,7 @@ func (o *ClusterUninstaller) cleanSharedHostedZone(ctx context.Context, client * publicZoneLogger.WithFields(recordsetFields).Debug("Deleted from public zone") } // delete the record set - if err := deleteRoute53RecordSet(ctx, client, id, recordSet, logger); err != nil { + if err := deleteRoute53RecordSet(ctx, phzClient, id, recordSet, logger); err != nil { if lastError != nil { logger.Debug(lastError) } From 152639959e884f0440b945b397c5431893d02dba Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 3 May 2023 16:59:05 -0400 Subject: [PATCH 06/11] aws tfvars: pass hosted zone role Plumb the hosted zone role through to terraform, so that it can be used to create records when the hosted zone belongs to another account. --- pkg/asset/cluster/tfvars.go | 1 + pkg/tfvars/aws/aws.go | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 6215559a380..6e307df8699 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -289,6 +289,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { PrivateSubnets: privateSubnets, PublicSubnets: publicSubnets, InternalZone: installConfig.Config.AWS.HostedZone, + InternalZoneRole: installConfig.Config.AWS.HostedZoneRole, Services: installConfig.Config.AWS.ServiceEndpoints, Publish: installConfig.Config.Publish, MasterConfigs: masterConfigs, diff --git a/pkg/tfvars/aws/aws.go b/pkg/tfvars/aws/aws.go index 59859c677ac..d4af26ad412 100644 --- a/pkg/tfvars/aws/aws.go +++ b/pkg/tfvars/aws/aws.go @@ -33,6 +33,7 @@ type config struct { PrivateSubnets []string `json:"aws_private_subnets,omitempty"` PublicSubnets *[]string `json:"aws_public_subnets,omitempty"` InternalZone string `json:"aws_internal_zone,omitempty"` + InternalZoneRole string `json:"aws_internal_zone_role,omitempty"` PublishStrategy string `json:"aws_publish_strategy,omitempty"` IgnitionBucket string `json:"aws_ignition_bucket"` BootstrapIgnitionStub string `json:"aws_bootstrap_stub_ignition"` @@ -44,10 +45,10 @@ type config struct { // TFVarsSources contains the parameters to be converted into Terraform variables type TFVarsSources struct { - VPC string - PrivateSubnets, PublicSubnets []string - InternalZone string - Services []typesaws.ServiceEndpoint + VPC string + PrivateSubnets, PublicSubnets []string + InternalZone, InternalZoneRole string + Services []typesaws.ServiceEndpoint Publish types.PublishingStrategy @@ -132,6 +133,7 @@ func TFVars(sources TFVarsSources) ([]byte, error) { VPC: sources.VPC, PrivateSubnets: sources.PrivateSubnets, InternalZone: sources.InternalZone, + InternalZoneRole: sources.InternalZoneRole, PublishStrategy: string(sources.Publish), IgnitionBucket: sources.IgnitionBucket, MasterIAMRoleName: sources.MasterIAMRoleName, From 55519115743886d33cd45e4bec7c5a14f100377b Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 3 May 2023 17:03:10 -0400 Subject: [PATCH 07/11] aws tf: private hosted zone assume role support Adds the ability in terraform to assume a role when performing operations on a private hosted zone which belongs to a different account than the rest of the cluster. --- data/data/aws/cluster/main.tf | 2 ++ data/data/aws/cluster/route53/base.tf | 35 +++++++++++++++++++--- data/data/aws/cluster/route53/variables.tf | 19 ++++++++++++ data/data/aws/variables-aws.tf | 7 +++++ 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/data/data/aws/cluster/main.tf b/data/data/aws/cluster/main.tf index 89cc41cc141..26257025649 100644 --- a/data/data/aws/cluster/main.tf +++ b/data/data/aws/cluster/main.tf @@ -70,9 +70,11 @@ module "dns" { cluster_id = var.cluster_id tags = local.tags internal_zone = var.aws_internal_zone + internal_zone_role = var.aws_internal_zone_role vpc_id = module.vpc.vpc_id region = var.aws_region publish_strategy = var.aws_publish_strategy + custom_endpoints = var.custom_endpoints } module "vpc" { diff --git a/data/data/aws/cluster/route53/base.tf b/data/data/aws/cluster/route53/base.tf index 967b863cced..f7760ea69e5 100644 --- a/data/data/aws/cluster/route53/base.tf +++ b/data/data/aws/cluster/route53/base.tf @@ -9,6 +9,27 @@ locals { use_alias = ! local.use_cname } +provider "aws" { + alias = "private_hosted_zone" + + assume_role { + role_arn = var.internal_zone_role + } + + region = var.region + + skip_region_validation = true + + endpoints { + ec2 = lookup(var.custom_endpoints, "ec2", null) + elb = lookup(var.custom_endpoints, "elasticloadbalancing", null) + iam = lookup(var.custom_endpoints, "iam", null) + route53 = lookup(var.custom_endpoints, "route53", null) + s3 = lookup(var.custom_endpoints, "s3", null) + sts = lookup(var.custom_endpoints, "sts", null) + } +} + data "aws_route53_zone" "public" { count = local.public_endpoints ? 1 : 0 @@ -18,6 +39,8 @@ data "aws_route53_zone" "public" { } data "aws_route53_zone" "int" { + provider = aws.private_hosted_zone + zone_id = var.internal_zone == null ? aws_route53_zone.new_int[0].id : var.internal_zone } @@ -54,7 +77,8 @@ resource "aws_route53_record" "api_external_alias" { } resource "aws_route53_record" "api_internal_alias" { - count = local.use_alias ? 1 : 0 + provider = aws.private_hosted_zone + count = local.use_alias ? 1 : 0 zone_id = data.aws_route53_zone.int.zone_id name = "api-int.${var.cluster_domain}" @@ -68,7 +92,8 @@ resource "aws_route53_record" "api_internal_alias" { } resource "aws_route53_record" "api_external_internal_zone_alias" { - count = local.use_alias ? 1 : 0 + provider = aws.private_hosted_zone + count = local.use_alias ? 1 : 0 zone_id = data.aws_route53_zone.int.zone_id name = "api.${var.cluster_domain}" @@ -93,7 +118,8 @@ resource "aws_route53_record" "api_external_cname" { } resource "aws_route53_record" "api_internal_cname" { - count = local.use_cname ? 1 : 0 + provider = aws.private_hosted_zone + count = local.use_cname ? 1 : 0 zone_id = data.aws_route53_zone.int.zone_id name = "api-int.${var.cluster_domain}" @@ -104,7 +130,8 @@ resource "aws_route53_record" "api_internal_cname" { } resource "aws_route53_record" "api_external_internal_zone_cname" { - count = local.use_cname ? 1 : 0 + provider = aws.private_hosted_zone + count = local.use_cname ? 1 : 0 zone_id = data.aws_route53_zone.int.zone_id name = "api.${var.cluster_domain}" diff --git a/data/data/aws/cluster/route53/variables.tf b/data/data/aws/cluster/route53/variables.tf index 67f938ed2c9..47c983f8729 100644 --- a/data/data/aws/cluster/route53/variables.tf +++ b/data/data/aws/cluster/route53/variables.tf @@ -28,6 +28,12 @@ variable "internal_zone" { description = "An existing hosted zone (zone ID) to use for the internal API." } +variable "internal_zone_role" { + type = string + default = null + description = "(optional) A role to assume when using an existing hosted zone from another account." +} + variable "api_external_lb_dns_name" { description = "External API's LB DNS name" type = string @@ -63,3 +69,16 @@ variable "region" { type = string description = "The target AWS region for the cluster." } + +variable "custom_endpoints" { + type = map(string) + + description = < Date: Thu, 11 May 2023 09:12:37 -0400 Subject: [PATCH 08/11] aws: surface error when validating hosted zone Display to users any errors when retrieving a hosted zone specified in the install config. --- pkg/asset/installconfig/aws/validation.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/asset/installconfig/aws/validation.go b/pkg/asset/installconfig/aws/validation.go index 0002ddff39d..63005b64834 100644 --- a/pkg/asset/installconfig/aws/validation.go +++ b/pkg/asset/installconfig/aws/validation.go @@ -385,7 +385,6 @@ func ValidateForProvisioning(client API, ic *types.InstallConfig, metadata *Meta var zonePath *field.Path var zone *route53.HostedZone - errors := field.ErrorList{} allErrs := field.ErrorList{} r53cfg := GetR53ClientCfg(metadata.session, ic.AWS.HostedZoneRole) @@ -394,13 +393,14 @@ func ValidateForProvisioning(client API, ic *types.InstallConfig, metadata *Meta zonePath = field.NewPath("aws", "hostedZone") zoneOutput, err := client.GetHostedZone(zoneName, r53cfg) if err != nil { + errMsg := errors.Wrapf(err, "unable to retrieve hosted zone").Error() return field.ErrorList{ - field.Invalid(zonePath, zoneName, "cannot find hosted zone"), + field.Invalid(zonePath, zoneName, errMsg), }.ToAggregate() } - if errors = validateHostedZone(zoneOutput, zonePath, zoneName, metadata); len(errors) > 0 { - allErrs = append(allErrs, errors...) + if errs := validateHostedZone(zoneOutput, zonePath, zoneName, metadata); len(errs) > 0 { + allErrs = append(allErrs, errs...) } zone = zoneOutput.HostedZone @@ -417,8 +417,8 @@ func ValidateForProvisioning(client API, ic *types.InstallConfig, metadata *Meta zone = baseDomainOutput } - if errors = client.ValidateZoneRecords(zone, zoneName, zonePath, ic, r53cfg); len(errors) > 0 { - allErrs = append(allErrs, errors...) + if errs := client.ValidateZoneRecords(zone, zoneName, zonePath, ic, r53cfg); len(errs) > 0 { + allErrs = append(allErrs, errs...) } return allErrs.ToAggregate() From 5846b92a5724418cf2fa3168bfcb257212c22890 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 31 May 2023 10:10:03 -0400 Subject: [PATCH 09/11] aws: write hostedZoneRole to DNS config When a hostedZoneRole is specified for an AWS shared VPC install, write it to the DNS config so it can be used by the cluster ingress operator. --- pkg/asset/manifests/dns.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/asset/manifests/dns.go b/pkg/asset/manifests/dns.go index 49664fcfbbf..4c71216ece9 100644 --- a/pkg/asset/manifests/dns.go +++ b/pkg/asset/manifests/dns.go @@ -118,6 +118,15 @@ func (d *DNS) Generate(dependencies asset.Parents) error { }} } else { config.Spec.PrivateZone = &configv1.DNSZone{ID: hostedZone} + + if r := installConfig.Config.AWS.HostedZoneRole; r != "" { + config.Spec.Platform = configv1.DNSPlatformSpec{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSDNSSpec{ + PrivateZoneIAMRole: r, + }, + } + } } case azuretypes.Name: dnsConfig, err := installConfig.Azure.DNSConfig() From 91259361be8ce78af1f81b2beb131ca101ce3392 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 31 May 2023 16:42:56 -0400 Subject: [PATCH 10/11] aws: update tests for route53 client update Update tests and mocks due to changes for adding cross-account private hosted-zone support. --- .../aws/mock/awsroute53_generated.go | 25 ++-- .../installconfig/aws/validation_test.go | 20 +-- .../vsphere/mock/tagmanager_generated.go | 127 ++++++++++++++++++ 3 files changed, 150 insertions(+), 22 deletions(-) create mode 100644 pkg/asset/installconfig/vsphere/mock/tagmanager_generated.go diff --git a/pkg/asset/installconfig/aws/mock/awsroute53_generated.go b/pkg/asset/installconfig/aws/mock/awsroute53_generated.go index 0dc7a0c1821..422a12877cb 100644 --- a/pkg/asset/installconfig/aws/mock/awsroute53_generated.go +++ b/pkg/asset/installconfig/aws/mock/awsroute53_generated.go @@ -7,6 +7,7 @@ package mock import ( reflect "reflect" + aws "github.com/aws/aws-sdk-go/aws" route53 "github.com/aws/aws-sdk-go/service/route53" gomock "github.com/golang/mock/gomock" types "github.com/openshift/installer/pkg/types" @@ -52,45 +53,45 @@ func (mr *MockAPIMockRecorder) GetBaseDomain(baseDomainName interface{}) *gomock } // GetHostedZone mocks base method. -func (m *MockAPI) GetHostedZone(hostedZone string) (*route53.GetHostedZoneOutput, error) { +func (m *MockAPI) GetHostedZone(hostedZone string, cfg *aws.Config) (*route53.GetHostedZoneOutput, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetHostedZone", hostedZone) + ret := m.ctrl.Call(m, "GetHostedZone", hostedZone, cfg) ret0, _ := ret[0].(*route53.GetHostedZoneOutput) ret1, _ := ret[1].(error) return ret0, ret1 } // GetHostedZone indicates an expected call of GetHostedZone. -func (mr *MockAPIMockRecorder) GetHostedZone(hostedZone interface{}) *gomock.Call { +func (mr *MockAPIMockRecorder) GetHostedZone(hostedZone, cfg interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHostedZone", reflect.TypeOf((*MockAPI)(nil).GetHostedZone), hostedZone) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHostedZone", reflect.TypeOf((*MockAPI)(nil).GetHostedZone), hostedZone, cfg) } // GetSubDomainDNSRecords mocks base method. -func (m *MockAPI) GetSubDomainDNSRecords(hostedZone *route53.HostedZone, ic *types.InstallConfig) ([]string, error) { +func (m *MockAPI) GetSubDomainDNSRecords(hostedZone *route53.HostedZone, ic *types.InstallConfig, cfg *aws.Config) ([]string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSubDomainDNSRecords", hostedZone, ic) + ret := m.ctrl.Call(m, "GetSubDomainDNSRecords", hostedZone, ic, cfg) ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) return ret0, ret1 } // GetSubDomainDNSRecords indicates an expected call of GetSubDomainDNSRecords. -func (mr *MockAPIMockRecorder) GetSubDomainDNSRecords(hostedZone, ic interface{}) *gomock.Call { +func (mr *MockAPIMockRecorder) GetSubDomainDNSRecords(hostedZone, ic, cfg interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubDomainDNSRecords", reflect.TypeOf((*MockAPI)(nil).GetSubDomainDNSRecords), hostedZone, ic) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubDomainDNSRecords", reflect.TypeOf((*MockAPI)(nil).GetSubDomainDNSRecords), hostedZone, ic, cfg) } // ValidateZoneRecords mocks base method. -func (m *MockAPI) ValidateZoneRecords(zone *route53.HostedZone, zoneName string, zonePath *field.Path, ic *types.InstallConfig) field.ErrorList { +func (m *MockAPI) ValidateZoneRecords(zone *route53.HostedZone, zoneName string, zonePath *field.Path, ic *types.InstallConfig, cfg *aws.Config) field.ErrorList { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ValidateZoneRecords", zone, zoneName, zonePath, ic) + ret := m.ctrl.Call(m, "ValidateZoneRecords", zone, zoneName, zonePath, ic, cfg) ret0, _ := ret[0].(field.ErrorList) return ret0 } // ValidateZoneRecords indicates an expected call of ValidateZoneRecords. -func (mr *MockAPIMockRecorder) ValidateZoneRecords(zone, zoneName, zonePath, ic interface{}) *gomock.Call { +func (mr *MockAPIMockRecorder) ValidateZoneRecords(zone, zoneName, zonePath, ic, cfg interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateZoneRecords", reflect.TypeOf((*MockAPI)(nil).ValidateZoneRecords), zone, zoneName, zonePath, ic) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateZoneRecords", reflect.TypeOf((*MockAPI)(nil).ValidateZoneRecords), zone, zoneName, zonePath, ic, cfg) } diff --git a/pkg/asset/installconfig/aws/validation_test.go b/pkg/asset/installconfig/aws/validation_test.go index f26a4645538..28c91ed452c 100644 --- a/pkg/asset/installconfig/aws/validation_test.go +++ b/pkg/asset/installconfig/aws/validation_test.go @@ -840,13 +840,13 @@ func TestValidateForProvisioning(t *testing.T) { }, { name: "internal publish strategy invalid hosted zone", edits: editFunctions{publishInternal, invalidateHostedZone}, - expectedErr: "aws.hostedZone: Invalid value: \"invalid-hosted-zone\": cannot find hosted zone", + expectedErr: "aws.hostedZone: Invalid value: \"invalid-hosted-zone\": unable to retrieve hosted zone", }, { name: "external publish strategy valid hosted zone", }, { name: "external publish strategy invalid hosted zone", edits: editFunctions{invalidateHostedZone}, - expectedErr: "aws.hostedZone: Invalid value: \"invalid-hosted-zone\": cannot find hosted zone", + expectedErr: "aws.hostedZone: Invalid value: \"invalid-hosted-zone\": unable to retrieve hosted zone", }} mockCtrl := gomock.NewController(t) @@ -861,12 +861,12 @@ func TestValidateForProvisioning(t *testing.T) { route53Client.EXPECT().GetBaseDomain("").Return(nil, fmt.Errorf("invalid value: \"\": cannot find base domain")).AnyTimes() route53Client.EXPECT().GetBaseDomain(invalidBaseDomain).Return(nil, fmt.Errorf("invalid value: \"%s\": cannot find base domain", invalidBaseDomain)).AnyTimes() - route53Client.EXPECT().ValidateZoneRecords(&validDomainOutput, gomock.Any(), gomock.Any(), gomock.Any()).Return(field.ErrorList{}).AnyTimes() - route53Client.EXPECT().ValidateZoneRecords(gomock.Any(), validHostedZoneName, gomock.Any(), gomock.Any()).Return(field.ErrorList{}).AnyTimes() + route53Client.EXPECT().ValidateZoneRecords(&validDomainOutput, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(field.ErrorList{}).AnyTimes() + route53Client.EXPECT().ValidateZoneRecords(gomock.Any(), validHostedZoneName, gomock.Any(), gomock.Any(), gomock.Any()).Return(field.ErrorList{}).AnyTimes() // An invalid hosted zone should provide an error - route53Client.EXPECT().GetHostedZone(validHostedZoneName).Return(&validHostedZoneOutput, nil).AnyTimes() - route53Client.EXPECT().GetHostedZone(gomock.Not(validHostedZoneName)).Return(nil, fmt.Errorf("invalid value: \"invalid-hosted-zone\": cannot find hosted zone")).AnyTimes() + route53Client.EXPECT().GetHostedZone(validHostedZoneName, gomock.Any()).Return(&validHostedZoneOutput, nil).AnyTimes() + route53Client.EXPECT().GetHostedZone(gomock.Not(validHostedZoneName), gomock.Any()).Return(nil, fmt.Errorf("invalid value: \"invalid-hosted-zone\": cannot find hosted zone")).AnyTimes() for _, test := range cases { t.Run(test.name, func(t *testing.T) { @@ -939,7 +939,7 @@ func TestGetSubDomainDNSRecords(t *testing.T) { if test.expectedErr != "" { if test.problematicRecords == nil { - route53Client.EXPECT().GetSubDomainDNSRecords(&validDomainOutput, ic).Return(nil, errors.Errorf(test.expectedErr)).AnyTimes() + route53Client.EXPECT().GetSubDomainDNSRecords(&validDomainOutput, ic, gomock.Any()).Return(nil, errors.Errorf(test.expectedErr)).AnyTimes() } else { // mimic the results of what should happen in the internal function passed to // ListResourceRecordSetsPages by GetSubDomainDNSRecords. Skip certain problematicRecords @@ -950,13 +950,13 @@ func TestGetSubDomainDNSRecords(t *testing.T) { returnedProblems = append(returnedProblems, pr) } } - route53Client.EXPECT().GetSubDomainDNSRecords(&validDomainOutput, ic).Return(returnedProblems, errors.Errorf(test.expectedErr)).AnyTimes() + route53Client.EXPECT().GetSubDomainDNSRecords(&validDomainOutput, ic, gomock.Any()).Return(returnedProblems, errors.Errorf(test.expectedErr)).AnyTimes() } } else { - route53Client.EXPECT().GetSubDomainDNSRecords(&validDomainOutput, ic).Return(nil, nil).AnyTimes() + route53Client.EXPECT().GetSubDomainDNSRecords(&validDomainOutput, ic, gomock.Any()).Return(nil, nil).AnyTimes() } - _, err := route53Client.GetSubDomainDNSRecords(&validDomainOutput, ic) + _, err := route53Client.GetSubDomainDNSRecords(&validDomainOutput, ic, nil) if test.expectedErr == "" { assert.NoError(t, err) } else { diff --git a/pkg/asset/installconfig/vsphere/mock/tagmanager_generated.go b/pkg/asset/installconfig/vsphere/mock/tagmanager_generated.go new file mode 100644 index 00000000000..c324b56789b --- /dev/null +++ b/pkg/asset/installconfig/vsphere/mock/tagmanager_generated.go @@ -0,0 +1,127 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: ./validation.go + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + tags "github.com/vmware/govmomi/vapi/tags" + mo "github.com/vmware/govmomi/vim25/mo" +) + +// MockTagManager is a mock of TagManager interface. +type MockTagManager struct { + ctrl *gomock.Controller + recorder *MockTagManagerMockRecorder +} + +// MockTagManagerMockRecorder is the mock recorder for MockTagManager. +type MockTagManagerMockRecorder struct { + mock *MockTagManager +} + +// NewMockTagManager creates a new mock instance. +func NewMockTagManager(ctrl *gomock.Controller) *MockTagManager { + mock := &MockTagManager{ctrl: ctrl} + mock.recorder = &MockTagManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockTagManager) EXPECT() *MockTagManagerMockRecorder { + return m.recorder +} + +// GetAttachedTags mocks base method. +func (m *MockTagManager) GetAttachedTags(ctx context.Context, ref mo.Reference) ([]tags.Tag, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAttachedTags", ctx, ref) + ret0, _ := ret[0].([]tags.Tag) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetAttachedTags indicates an expected call of GetAttachedTags. +func (mr *MockTagManagerMockRecorder) GetAttachedTags(ctx, ref interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAttachedTags", reflect.TypeOf((*MockTagManager)(nil).GetAttachedTags), ctx, ref) +} + +// GetAttachedTagsOnObjects mocks base method. +func (m *MockTagManager) GetAttachedTagsOnObjects(ctx context.Context, objectID []mo.Reference) ([]tags.AttachedTags, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAttachedTagsOnObjects", ctx, objectID) + ret0, _ := ret[0].([]tags.AttachedTags) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetAttachedTagsOnObjects indicates an expected call of GetAttachedTagsOnObjects. +func (mr *MockTagManagerMockRecorder) GetAttachedTagsOnObjects(ctx, objectID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAttachedTagsOnObjects", reflect.TypeOf((*MockTagManager)(nil).GetAttachedTagsOnObjects), ctx, objectID) +} + +// GetCategories mocks base method. +func (m *MockTagManager) GetCategories(ctx context.Context) ([]tags.Category, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCategories", ctx) + ret0, _ := ret[0].([]tags.Category) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCategories indicates an expected call of GetCategories. +func (mr *MockTagManagerMockRecorder) GetCategories(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCategories", reflect.TypeOf((*MockTagManager)(nil).GetCategories), ctx) +} + +// GetCategory mocks base method. +func (m *MockTagManager) GetCategory(ctx context.Context, id string) (*tags.Category, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCategory", ctx, id) + ret0, _ := ret[0].(*tags.Category) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCategory indicates an expected call of GetCategory. +func (mr *MockTagManagerMockRecorder) GetCategory(ctx, id interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCategory", reflect.TypeOf((*MockTagManager)(nil).GetCategory), ctx, id) +} + +// GetTagsForCategory mocks base method. +func (m *MockTagManager) GetTagsForCategory(ctx context.Context, id string) ([]tags.Tag, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTagsForCategory", ctx, id) + ret0, _ := ret[0].([]tags.Tag) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTagsForCategory indicates an expected call of GetTagsForCategory. +func (mr *MockTagManagerMockRecorder) GetTagsForCategory(ctx, id interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTagsForCategory", reflect.TypeOf((*MockTagManager)(nil).GetTagsForCategory), ctx, id) +} + +// ListCategories mocks base method. +func (m *MockTagManager) ListCategories(ctx context.Context) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListCategories", ctx) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListCategories indicates an expected call of ListCategories. +func (mr *MockTagManagerMockRecorder) ListCategories(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCategories", reflect.TypeOf((*MockTagManager)(nil).ListCategories), ctx) +} From f3c2b06f940ff4c0c4fdcaded189090d8e664be8 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Thu, 1 Jun 2023 10:43:21 -0400 Subject: [PATCH 11/11] installconfig/aws: feature gate hostedZoneRole Specifying installconfig.platform.aws.hostedZoneRole and using a cross-account hosted zone in a shared vpc requires a TechPreviewNoUpgrade feature gate. --- pkg/types/validation/installconfig.go | 6 ++++++ pkg/types/validation/installconfig_test.go | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index d3b3089ab67..7a24c392be4 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -1050,6 +1050,12 @@ func validateFeatureSet(c *types.InstallConfig) field.ErrorList { allErrs = append(allErrs, field.Forbidden(field.NewPath("platform", "azure", "userTags"), errMsg)) } + if c.AWS != nil { + if len(c.AWS.HostedZoneRole) > 0 { + allErrs = append(allErrs, field.Forbidden(field.NewPath("platform", "aws", "hostedZoneRole"), errMsg)) + } + } + if c.OpenStack != nil { for _, f := range openstackvalidation.FilledInTechPreviewFields(c) { allErrs = append(allErrs, field.Forbidden(f, errMsg)) diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index b95e670306c..3d2bae6e6e1 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -2135,6 +2135,18 @@ func TestValidateInstallConfig(t *testing.T) { }(), expectedError: "platform.vsphere.apiVIPs: Required value: must specify VIP for API, when VIP for ingress is set", }, + { + name: "platform.aws.hostedZoneRole should return error without TechPreviewNoUpgrade", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + AWS: validAWSPlatform(), + } + c.Platform.AWS.HostedZoneRole = "test-zone" + return c + }(), + expectedError: `platform.aws.hostedZoneRole: Forbidden: the TechPreviewNoUpgrade feature set must be enabled to use this field`, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) {