CORS-2613: AWS: Cross-account Shared VPC Support by patrickdillon · Pull Request #7161 · openshift/installer

patrickdillon · 2023-05-03T21:12:27Z

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

Enhancement: openshift/enhancements#1397

tag private hosted zone in another account
create records in private hosted zone in another account
destroy records and tags in/on phz in another account
regression testing
API updates
gate on TechPreviewNoUpgrade

Permissions checks will be moved to a separate PR.

openshift-ci-robot · 2023-05-03T21:12:30Z

@patrickdillon: This pull request references CORS-2613 which is a valid jira issue.

Details

In response to this:

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

/wip

tag private hosted zone in another account

create records in private hosted zone in another account

destroy records and tags in/on phz in another account (WIP but currently broken)

permissions checking

regression testing

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci · 2023-05-03T21:13:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from patrickdillon. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-05-04T13:05:34Z

@patrickdillon: This pull request references CORS-2613 which is a valid jira issue.

Details

In response to this:

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

/wip

tag private hosted zone in another account

create records in private hosted zone in another account

destroy records and tags in/on phz in another account (WIP but currently broken)

permissions checking

regression testing

API updates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

patrickdillon · 2023-05-16T13:29:23Z

I have some serious doubts about the destroy code in 06c8282cc1159ae0e5fdf8eaae4fb9a22ee2611f, so I might need to pull those into a separate PR.

I believe we use the private hosted zone to determine the public records to destroy. That logic probably is not supportable when the zones are cross-account.

openshift-ci-robot · 2023-05-16T13:30:16Z

@patrickdillon: This pull request references CORS-2613 which is a valid jira issue.

Details

In response to this:

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

Enhancement: openshift/enhancements#1397

/wip

tag private hosted zone in another account

create records in private hosted zone in another account

destroy records and tags in/on phz in another account (WIP but currently broken)

permissions checking

regression testing

API updates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

pkg/types/aws/validation/platform.go

barbacbd · 2023-05-23T16:12:25Z

@patrickdillon looks like go-genmock needs to be run and some of the installconfig/aws/ tests need to be adjusted:

TestGetSubDomainDNSRecords()
TestValidateForProvisioning()

openshift-ci-robot · 2023-05-31T21:00:48Z

@patrickdillon: This pull request references CORS-2613 which is a valid jira issue.

Details

In response to this:

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

Enhancement: openshift/enhancements#1397

/wip

tag private hosted zone in another account

create records in private hosted zone in another account

destroy records and tags in/on phz in another account (WIP but currently broken)

permissions checking

regression testing

API updates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-06-01T14:29:30Z

@patrickdillon: This pull request references CORS-2613 which is a valid jira issue.

Details

In response to this:

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

Enhancement: openshift/enhancements#1397

/wip

tag private hosted zone in another account

create records in private hosted zone in another account

destroy records and tags in/on phz in another account

regression testing

API updates

gate on TechPreviewNoUpgrade

Permissions checks will be moved to a separate PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-06-01T14:29:39Z

@patrickdillon: This pull request references CORS-2613 which is a valid jira issue.

Details

In response to this:

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

Enhancement: openshift/enhancements#1397

tag private hosted zone in another account

create records in private hosted zone in another account

destroy records and tags in/on phz in another account

regression testing

API updates

gate on TechPreviewNoUpgrade

Permissions checks will be moved to a separate PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-06-01T14:46:59Z

@patrickdillon: This pull request references CORS-2613 which is a valid jira issue.

Details

In response to this:

Adds the ability for the installer to perform operations on a pre-existing private hosted zone which belongs to another account by allowing the ARN for a role to be passed in the install config. When the role is provided, the installer will assume that role when performing operations on the hosted zone.

Enhancement: openshift/enhancements#1397

tag private hosted zone in another account

create records in private hosted zone in another account

destroy records and tags in/on phz in another account

regression testing

API updates

gate on TechPreviewNoUpgrade

Permissions checks will be moved to a separate PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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.

If a hosted zone role is specified in the install config, users must also supply a hosted zone.

The hostedZoneRole will need to be used when destroying the cluster.

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.

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.

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.

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.

Display to users any errors when retrieving a hosted zone specified in the install config.

The most recent k8s api bump requires a context is passed to the wait.ExponentialBackoffWithContext function.

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.

metal3-io/baremetal-operator@6c1abd0 moved the hardware profile package to the api. This commit points to the new package locations.

Update tests and mocks due to changes for adding cross-account private hosted-zone support.

$ go mod edit \ -replace=github.com/metal3-io/baremetal-operator=github.com/openshift/baremetal-operator@master \ -replace=github.com/metal3-io/baremetal-operator/apis=github.com/openshift/baremetal-operator/apis@master \ -replace=github.com/metal3-io/baremetal-operator/pkg/hardwareutils=github.com/openshift/baremetal-operator/pkg/hardwareutils@master $ go mod edit -require=github.com/openshift/api@master $ go mod edit -replace=sigs.k8s.io/controller-runtime=sigs.k8s.io/controller-runtime@v0.15.0 $ go mod edit -replace k8s.io/client-go=k8s.io/client-go@v0.27.2 (with $ go mod tidy run between each command)

Bumps core Installer to Go v1.20. Terraform and providers will be updated separately.

$ go mod vendor

Update generated CRD for explain after API vendoring.

Fixes deprecation issue: pkg/destroy/vsphere/vsphere.go:243:9: SA1019: wait.PollImmediateInfiniteWithContext is deprecated: This method does not return errors from context, use PollWithContextCancel. Note that the new method will no longer return ErrWaitTimeout and instead return errors defined by the context package. Will be removed in a future release. (staticcheck)

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

openshift-ci · 2023-06-01T20:37:36Z

@patrickdillon: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-e2e-aws-ovn-upgrade	`4666d72`	link	false	`/test okd-e2e-aws-ovn-upgrade`
ci/prow/e2e-vsphere-ovn	`4666d72`	link	true	`/test e2e-vsphere-ovn`
ci/prow/e2e-libvirt	`4666d72`	link	false	`/test e2e-libvirt`
ci/prow/e2e-aws-ovn-upi	`4666d72`	link	true	`/test e2e-aws-ovn-upi`
ci/prow/e2e-azure-ovn-upi	`4666d72`	link	true	`/test e2e-azure-ovn-upi`
ci/prow/e2e-vsphere-upi-zones	`4666d72`	link	false	`/test e2e-vsphere-upi-zones`
ci/prow/e2e-vsphere-upi	`4666d72`	link	true	`/test e2e-vsphere-upi`
ci/prow/okd-scos-e2e-aws-ovn	`4666d72`	link	false	`/test okd-scos-e2e-aws-ovn`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

mtulio

@patrickdillon aws changes look good to me. 💯
I am adding some suggestions to improve the UX, feel free to skip it.
I will take a look again tomorrow at the whole change.

mtulio · 2023-06-01T18:08:52Z

data/data/install.openshift.io_installconfigs.yaml

                      create the hosted zone on your behalf.
                    type: string
+                  hostedZoneRole:
+                    description: HostedZoneRole is the ARN of a role to be assumed


I would be specific of the name of the AWS service

Suggested change

description: HostedZoneRole is the ARN of a role to be assumed

description: HostedZoneRole is the ARN of an IAM role to be assumed

mtulio · 2023-06-01T18:11:34Z

pkg/explain/printer_test.go

      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 <string>
+      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.


If tou accept the suggestion above, need to change here too:

Suggested change

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.

HostedZoneRole is the ARN of an IAM 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.

pkg/types/aws/validation/platform.go

patrickdillon · 2023-06-02T13:35:33Z

@r4f4 @barbacbd @mtulio #7225 is the new, aws-only version of this PR after #7220 merged

openshift-merge-robot · 2023-06-02T13:35:43Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

patrickdillon · 2023-06-03T00:20:45Z

/close
In favor of #7225

openshift-ci · 2023-06-03T00:20:56Z

@patrickdillon: Closed this PR.

Details

In response to this:

/close
In favor of #7225

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 3, 2023

openshift-ci bot requested review from barbacbd and mtulio May 3, 2023 21:13

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 4, 2023

patrickdillon mentioned this pull request May 9, 2023

Installer: AWS Shared VPC openshift/enhancements#1397

Merged

patrickdillon force-pushed the aws-phz-role-ic branch from a1d247a to d73f198 Compare May 16, 2023 13:37

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 16, 2023

barbacbd reviewed May 23, 2023

View reviewed changes

pkg/types/aws/validation/platform.go Outdated Show resolved Hide resolved

patrickdillon force-pushed the aws-phz-role-ic branch from d73f198 to 649eaf8 Compare May 31, 2023 21:00

patrickdillon force-pushed the aws-phz-role-ic branch 2 times, most recently from 1d834c0 to 1fdcec8 Compare June 1, 2023 13:59

patrickdillon added 9 commits June 1, 2023 11:59

aws: add HostedZoneRole to platform

413be17

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.

aws: validate hosted zone role

26818f0

If a hosted zone role is specified in the install config, users must also supply a hosted zone.

aws: add hostedZoneRole to metadata

fb97aa5

The hostedZoneRole will need to be used when destroying the cluster.

aws: update route53 clients to allow assume role

c195c3a

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.

aws: destroy phzs in other accounts

ace31cf

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.

aws tfvars: pass hosted zone role

0291f66

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.

aws tf: private hosted zone assume role support

5a0fc54

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.

aws: surface error when validating hosted zone

abd6e17

Display to users any errors when retrieving a hosted zone specified in the install config.

PowerVS destroy: add context

c450ebe

The most recent k8s api bump requires a context is passed to the wait.ExponentialBackoffWithContext function.

patrickdillon added 9 commits June 1, 2023 11:59

aws: write hostedZoneRole to DNS config

0a6d3a2

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.

baremetal: update hardware profile packages

52bae61

metal3-io/baremetal-operator@6c1abd0 moved the hardware profile package to the api. This commit points to the new package locations.

aws: update tests for route53 client update

25f95db

Update tests and mocks due to changes for adding cross-account private hosted-zone support.

Bump to Go 1.20

79ef8db

Bumps core Installer to Go v1.20. Terraform and providers will be updated separately.

Vendor Dependencies

ce25f00

$ go mod vendor

data/data/installconfig: generate after vendoring

5bf3bc9

Update generated CRD for explain after API vendoring.

installconfig/aws: feature gate hostedZoneRole

4666d72

Specifying installconfig.platform.aws.hostedZoneRole and using a cross-account hosted zone in a shared vpc requires a TechPreviewNoUpgrade feature gate.

patrickdillon force-pushed the aws-phz-role-ic branch from a01d3e0 to 4666d72 Compare June 1, 2023 15:59

barbacbd reviewed Jun 1, 2023

View reviewed changes

pkg/types/validation/installconfig.go Show resolved Hide resolved

mtulio reviewed Jun 1, 2023

View reviewed changes

patrickdillon mentioned this pull request Jun 2, 2023

CORS-2613: AWS: Cross-account Shared VPC Support #7225

Merged

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2023

openshift-ci bot closed this Jun 3, 2023

	description: HostedZoneRole is the ARN of a role to be assumed
	description: HostedZoneRole is the ARN of an IAM role to be assumed

	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.
	HostedZoneRole is the ARN of an IAM 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.

Comments

Conversation

patrickdillon commented May 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented May 3, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented May 3, 2023

Uh oh!

openshift-ci-robot commented May 4, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon commented May 16, 2023

Uh oh!

openshift-ci-robot commented May 16, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

barbacbd commented May 23, 2023

Uh oh!

openshift-ci-robot commented May 31, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jun 1, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jun 1, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jun 1, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

openshift-ci bot commented Jun 1, 2023

Uh oh!

mtulio left a comment

Choose a reason for hiding this comment

Uh oh!

mtulio Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

mtulio Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patrickdillon commented Jun 2, 2023

Uh oh!

openshift-merge-robot commented Jun 2, 2023

Uh oh!

patrickdillon commented Jun 3, 2023

Uh oh!

openshift-ci bot commented Jun 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

patrickdillon commented May 3, 2023 •

edited

Loading

openshift-ci-robot commented May 3, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 4, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 16, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 31, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jun 1, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jun 1, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jun 1, 2023 •

edited by openshift-ci bot

Loading