[release-4.12] NE-1372: Add support for AWS shared VPC in another account #966 #971

gcs278 · 2023-08-09T16:43:29Z

Add support for configuring DNS records in AWS Route 53 using a separate account for the private hosted zone.

This PR cherry-picks 7251aaa (from #928, the original implementation) and 047bd98 (from #951, a bug fix for https://issues.redhat.com/browse/OCPBUGS-14998).

How I generated this backport:

Created a new commit to vendor openshift/api backport [release-4.13] OCPSTRAT-659: DNS: Add awsPrivateHostedZoneRole api#1529
- Modified go.mod to use github.com/openshift/api@v0.0.0-20230817133225-564be9ddb58e (from OCPSTRAT-660: AWS Shared VPC [release-4.12] api#1551)
- go mod tidy
- go mod vendor
- make to regenerate bindata.go
git cherry-pick 7251aaa9a30f3759c2731d061207d2f63cd945de (7251aaa)
Resolved merge conflicts:
- pkg/manifests/bindata.go: Regenerated
- pkg/operator/controller/dns/controller.go: Removed PrivateHostedZoneAWSEnabled from config structure for the dns controller. Removed associated check for r.config.PrivateHostedZoneAWSEnabled
- pkg/operator/operator.go: 7251aaa had feature gate logic. We are not backporting the feature gate so I did not integrate any feature gate logic in the merge.
Completed cherry-pick with add, git cherry-pick --continue, etc...
git cherry-pick 047bd986587a923029586a2497ec99d0a5a242a0 (047bd98)
- No merge conflicts

openshift-ci-robot · 2023-08-10T19:09:47Z

@gcs278: This pull request references NE-1372 which is a valid jira issue.

Details

In response to this:

Add support for configuring DNS records in AWS Route 53 using a separate account for the private hosted zone.

This PR cherry-picks 7251aaa (from #928, the original implementation) and 047bd98 (from #951, a bug fix for https://issues.redhat.com/browse/OCPBUGS-14998).

How I generated this backport:

Created a new commit to vendor openshift/api backport [release-4.13] OCPSTRAT-659: DNS: Add awsPrivateHostedZoneRole api#1529

Modified go.mod to use github.com/openshift/api <TBD>

go mod tidy

go mod vendor

make to regenerate bindata.go

git cherry-pick 7251aaa9a30f3759c2731d061207d2f63cd945de (7251aaa)

Resolved merge conflicts:

pkg/manifests/bindata.go: Regenerated

pkg/operator/controller/dns/controller.go: Removed PrivateHostedZoneAWSEnabled from config structure for the dns controller. Removed associated check for r.config.PrivateHostedZoneAWSEnabled

pkg/operator/operator.go: 7251aaa had feature gate logic. We are not backporting the feature gate so I did not integrate any feature gate logic in the merge.

Completed cherry-pick with add, git cherry-pick --continue, etc...

git cherry-pick 047bd986587a923029586a2497ec99d0a5a242a0 (047bd98)

No merge conflicts

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.

melvinjoseph86 · 2023-08-14T05:52:32Z

/retest-required

melvinjoseph86 · 2023-08-14T05:56:21Z

/retest-required

melvinjoseph86 · 2023-08-15T10:15:07Z

/retest-required

manifests/00-ingress-credentials-request.yaml

openshift-ci-robot · 2023-08-17T15:40:19Z

@gcs278: This pull request references NE-1372 which is a valid jira issue.

Details

In response to this:

Add support for configuring DNS records in AWS Route 53 using a separate account for the private hosted zone.

This PR cherry-picks 7251aaa (from #928, the original implementation) and 047bd98 (from #951, a bug fix for https://issues.redhat.com/browse/OCPBUGS-14998).

How I generated this backport:

Created a new commit to vendor openshift/api backport [release-4.13] OCPSTRAT-659: DNS: Add awsPrivateHostedZoneRole api#1529

Modified go.mod to use github.com/openshift/api@v0.0.0-20230817133225-564be9ddb58e (from OCPSTRAT-660: AWS Shared VPC [release-4.12] api#1551)

go mod tidy

go mod vendor

make to regenerate bindata.go

git cherry-pick 7251aaa9a30f3759c2731d061207d2f63cd945de (7251aaa)

Resolved merge conflicts:

pkg/manifests/bindata.go: Regenerated

pkg/operator/controller/dns/controller.go: Removed PrivateHostedZoneAWSEnabled from config structure for the dns controller. Removed associated check for r.config.PrivateHostedZoneAWSEnabled

pkg/operator/operator.go: 7251aaa had feature gate logic. We are not backporting the feature gate so I did not integrate any feature gate logic in the merge.

Completed cherry-pick with add, git cherry-pick --continue, etc...

git cherry-pick 047bd986587a923029586a2497ec99d0a5a242a0 (047bd98)

No merge conflicts

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.

gcs278 · 2023-08-17T15:41:26Z

Since Miciah has been helping with these backports:
/assign @Miciah

gcs278 · 2023-08-17T15:43:06Z

/hold
Per @sdodson's suggestion. Need to understand what is happening with this week's 4.12.z release.

sdodson · 2023-08-17T18:15:00Z

/retest-required

Bump to github.com/openshift/api@v0.0.0-20230817133225-564be9ddb58e to get the new "PrivateHostedZoneAWS" feature gate and the DNS.spec.platform.aws.privateZoneIAMRole API field to allow configuring a private hosted zone in a shared VPC. * go.mod: Bump openshift/api. * go.sum: * vendor/*: Regenerate. * pkg/manifests/bindata.go: Regenerate. * manifests/00-custom-resource-definition.yaml: Regenerate.

Add support for configuring DNS records in AWS Route 53 using a separate account for the private hosted zone. This commit resolves NE-1294. https://issues.redhat.com/browse/NE-1294 Modified for 4.12 backport: Removed feature gate logic and credentials request `sts:AssumeRole` update. * pkg/manifests/bindata.go: Regenerate. * pkg/dns/aws/dns.go (Config): Add a RoleARN field. (NewProvider): If config.RoleARN is set, use it to configure the AWS client using the specified role. * pkg/dns/split/dns.go: New file. Define a DNS provider implementation that wraps two other DNS providers, using one of them to publish records to the public zone and the other to publish records to the private zone. (Provider): New type. Store the private and public DNS providers, as well as the private zone so that the Ensure, Delete, and Replace methods can use it to determine whether they are publishing to the public zone or to the private zone. (NewProvider): New function. Return a split DNS provider. (Ensure, Delete, Replace): New methods. Implement the dns.Provider interface by calling the respective methods on the wrapped private and public DNS providers. * pkg/dns/split/dns_test.go (TestSplitDNSProvider): Verify that the split DNS provider correctly dispatches to the private or public DNS provider as appropriate, using fakeProvider. (fakeProvider): New type. Define a fake named DNS provider that records its name when invoked. (Ensure, Delete, Replace): New methods for fakeProvider to record invocations and implement the dns.Provider interface. (newFakeProvider): New function. Return a fake provider. * pkg/operator/controller/dns/controller.go (createDNSProvider): Use the new split DNS provider and the AWS DNSprovider's new RoleARN configuration option to configure separate DNS providers for public and private zones when a role ARN for the private zoneis specified in the cluster infrastructure config. Modified-by: Grant Spence <gspence@redhat.com>

To support Shared VPC, we split the DNS client into public and private providers, the private using the RoleARN (Account A) and the public using the default (Account B). However, the RoleARN only provides API access for Account A's Route53 service, not the ability to describe Account B's ELBs. This fix isolates the RoleARN to only be used with Route53 API services. `pkg/dns/aws/dns.go`: Create a separate Route53 session object that uses the RoleARN when provided.

gcs278 · 2023-08-17T18:43:28Z

https://github.com/openshift/cluster-ingress-operator/compare/0a774bc0ce151b1b7a4052e0399cde62470aea1d..fb01b1991085a7e55c0f45998b723c353d03fc3a

Just commit message updates:

Added API version in d321d8f (was TBD)
Moved credentials request drop of sts:assume-role to f4c49cb, which added it, not fb01b19 (where I put the removal)
Updated the commit message of f4c49cb to reflect the modifications in this backport, something I forgot to do for 4.13. The commit message had details of the feature gate, and credentials requests updates, which I removed, since those chanages aren't being backported.

openshift-ci · 2023-08-17T21:08:32Z

@gcs278: all tests passed!

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.

Miciah · 2023-08-29T02:38:20Z

/approve
/lgtm

This change does not change the operator's behavior unless the new spec.platform.aws.privateZoneIAMRole option in the cluster DNS config is specified. Note that specifying spec.platform.aws.privateZoneIAMRole could cause the operator to fail to update DNS records if the operator does not have the sts:AssumeRole permission. However, adding the sts:AssumeRole permission to the credentials request in a z-stream update can cause other problems (see OCPBUGS-17733 and #972), so rather than modifying the credentials request, we will have a release note to warn against using the new API field without giving the operator the required permissions. (In addition, we might amend openshift/api#1551 and update openshift/cluster-config-operator#344 to add a warning to the godoc for spec.platform.aws.privateZoneIAMRole, but even if we do amend the godoc, that change only needs to be vendored in cluster-config-operator, not necessarily cluster-ingress-operator, so that change needn't block this PR.)
/label backport-risk-assessed

@gcs278, as far as I am concerned, the hold can be removed once you have the release note sorted out.

openshift-ci · 2023-08-29T02:38:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Miciah]

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

melvinjoseph86 · 2023-08-30T05:52:00Z

/label cherry-pick-approved

melvinjoseph86 · 2023-08-30T06:10:49Z

/jira refresh

openshift-ci-robot · 2023-08-30T06:10:51Z

@melvinjoseph86: This pull request references NE-1372 which is a valid jira issue.

Details

In response to this:

/jira refresh

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.

gcs278 · 2023-09-07T14:27:17Z

/unhold

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

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2023

openshift-ci bot requested review from alebedev87 and miheer August 9, 2023 16:48

gcs278 force-pushed the release-4.12-shared-vpc branch 2 times, most recently from 03940b3 to 4aad583 Compare August 9, 2023 20:22

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

gcs278 changed the title ~~[WIP] [release-4.12] NE-????: Add support for AWS shared VPC in another account #966~~ [WIP] [release-4.12] NE-1372: Add support for AWS shared VPC in another account #966 Aug 10, 2023

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

patrickdillon reviewed Aug 15, 2023

View reviewed changes

manifests/00-ingress-credentials-request.yaml Outdated Show resolved Hide resolved

gcs278 force-pushed the release-4.12-shared-vpc branch 2 times, most recently from c5717d1 to 0a774bc Compare August 17, 2023 15:39

gcs278 changed the title ~~[WIP] [release-4.12] NE-1372: Add support for AWS shared VPC in another account #966~~ [release-4.12] NE-1372: Add support for AWS shared VPC in another account #966 Aug 17, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 17, 2023

openshift-ci bot assigned Miciah Aug 17, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 17, 2023

gcs278 and others added 3 commits August 17, 2023 14:28

gcs278 force-pushed the release-4.12-shared-vpc branch from 0a774bc to fb01b19 Compare August 17, 2023 18:40

openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Aug 29, 2023

openshift-ci bot assigned lihongan, melvinjoseph86 and ShudiLi Aug 29, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 29, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 29, 2023

openshift-ci bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Aug 30, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 7, 2023

mrunalp added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Sep 7, 2023

openshift-merge-robot merged commit 0bb8ffc into openshift:release-4.12 Sep 7, 2023

yunjiang29 mentioned this pull request Sep 12, 2023

[4.12 backport] CI jobs for AWS Shared-VPC install openshift/release#43231

Merged

[release-4.12] NE-1372: Add support for AWS shared VPC in another account #966 #971

[release-4.12] NE-1372: Add support for AWS shared VPC in another account #966 #971

Uh oh!

Conversation

gcs278 commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

melvinjoseph86 commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melvinjoseph86 commented Aug 14, 2023

Uh oh!

melvinjoseph86 commented Aug 15, 2023

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

gcs278 commented Aug 17, 2023

Uh oh!

gcs278 commented Aug 17, 2023

Uh oh!

sdodson commented Aug 17, 2023

Uh oh!

gcs278 commented Aug 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Aug 17, 2023

Uh oh!

Miciah commented Aug 29, 2023

Uh oh!

openshift-ci bot commented Aug 29, 2023

Uh oh!

melvinjoseph86 commented Aug 30, 2023

Uh oh!

melvinjoseph86 commented Aug 30, 2023

Uh oh!

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

Uh oh!

Uh oh!

gcs278 commented Sep 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

gcs278 commented Aug 9, 2023 •

edited

Loading

openshift-ci-robot commented Aug 10, 2023 •

edited by openshift-ci bot

Loading

melvinjoseph86 commented Aug 14, 2023 •

edited

Loading

openshift-ci-robot commented Aug 17, 2023 •

edited by openshift-ci bot

Loading

gcs278 commented Aug 17, 2023 •

edited

Loading

openshift-ci-robot commented Aug 30, 2023 •

edited by openshift-ci bot

Loading