CORS-2613: AWS: Cross-account Shared VPC Support#7225
CORS-2613: AWS: Cross-account Shared VPC Support#7225patrickdillon merged 11 commits intoopenshift:masterfrom
Conversation
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.
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.
Update tests and mocks due to changes for adding cross-account private hosted-zone support.
Specifying installconfig.platform.aws.hostedZoneRole and using a cross-account hosted zone in a shared vpc requires a TechPreviewNoUpgrade feature gate.
openshift-ci-robot
added
the
jira/valid-reference
|
@patrickdillon: This pull request references CORS-2613 which is a valid jira issue. DetailsIn response to this:
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. |
|
Supercedes #7161 |
| cfg := &aws.Config{ | ||
| Credentials: creds, | ||
| Region: aws.String(endpoints.UsEast1RegionID), | ||
| } |
There was a problem hiding this comment.
Any reason not to use the GetR53ClientCfg method?
|
|
||
| // The private hosted zone (phz) may belong to a different account, | ||
| // in which case we need a separate client. | ||
| phzClient := publicClient |
There was a problem hiding this comment.
Not gonna lie, this is a bit hard to read/validate with the back-and-forth of the clients.
There was a problem hiding this comment.
Maybe something like this would be easier to read?
// The private hosted zone (phz) may belong to a different account,
// in which case we need a separate client.
publicZoneClient := route53.New(session)
privateZoneClient := route53.New(session)There was a problem hiding this comment.
I think it does but no need to change it unless you also have to make other changes.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: r4f4 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Successfully tested this using: cluster bot build 4.14.0-0.nightly-2023-05-31-080250,openshift/cluster-ingress-operator#928,openshift/cluster-config-operator#314 |
|
I missed adding credentialmode validation. This only works in Passthrough or Manual mode. Created https://issues.redhat.com/browse/CORS-2673 to complete that work separately. |
|
/lgtm Patrick noted that another PR would be open to address the issues above. |
|
/test e2e-aws-ovn-single-node |
|
@patrickdillon: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/skip |
|
pre-merge tests get passed.
|
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
| var zonePath *field.Path | ||
| var zone *route53.HostedZone | ||
|
|
||
| errors := field.ErrorList{} |
There was a problem hiding this comment.
Is this change to replace errors with errs really necessary?
There was a problem hiding this comment.
Using the errors variable name conflicted with the errors package imported in this file. I should have documented this in the commit message.
|
/label jira/valid-bug |
openshift-ci
bot
added
the
jira/valid-bug
|
/refresh |
7 participants
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