Skip to content

OSDOCS-6652 AWS shared private hosted zones#64114

Merged
bscott-rh merged 1 commit intoopenshift:mainfrom
bscott-rh:OSDOCS-6652
Oct 20, 2023
Merged

OSDOCS-6652 AWS shared private hosted zones#64114
bscott-rh merged 1 commit intoopenshift:mainfrom
bscott-rh:OSDOCS-6652

Conversation

@bscott-rh
Copy link
Contributor

@bscott-rh bscott-rh commented Aug 30, 2023

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 30, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Aug 30, 2023

🤖 Updated build preview is available at:
https://64114--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/29320

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 28, 2023
@bscott-rh
Copy link
Contributor Author

@yunjiang29 PTAL at this PR modifying the AWS "existing VPC" documentation to account for the shared private hosted zone feature for AWS. Thank you

@yunjiang29
Copy link
Contributor

@bscott-rh Some notable info for your reference:

Restriction for Shared-VPC install:

  • Only Manual and Passthrough mode are supported

For Cluster owner’s account (Account-B)

  • sts:AssumeRole is required for Shared-VPC install OCPBUGS-17751

For VPC&PHZ owner's account (Account-A)

  • The PHZ is associated with VPC.
  • The subnets are shared with Account-B
  • The Principal of trust policy for shared-vpc role:
    • if Passthrough is used, cluster creator's ARN (e.g. arn:aws:iam::123456789012:user/clustercreator) is required.
    • if Manual is used, cluster creator's ARN and ingress operator role's ARN (e.g. arn:aws:iam::123456789012:role/<cluster-name>-openshift-ingress-operator-cloud-credentials) are required.
  • The policy for shared-vpc role
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "route53:ChangeResourceRecordSets",
                "route53:ListHostedZones",
                "route53:ListHostedZonesByName",
                "route53:ListResourceRecordSets",
                "route53:ChangeTagsForResource",
                "route53:GetAccountLimit",
                "route53:GetChange",
                "route53:GetHostedZone",
                "route53:ListTagsForResource",
                "route53:UpdateHostedZoneComment",
                "tag:GetResources",
                "tag:UntagResources"
            ],
            "Resource": "*"
        }
    ]
}

@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 16, 2023
@bscott-rh bscott-rh force-pushed the OSDOCS-6652 branch 2 times, most recently from 4c04694 to 01835d3 Compare October 16, 2023 19:04
@bscott-rh
Copy link
Contributor Author

@bscott-rh Some notable info for your reference:

Restriction for Shared-VPC install:

* Only `Manual` and `Passthrough` mode are supported

For Cluster owner’s account (Account-B)

* `sts:AssumeRole` is required for Shared-VPC install [OCPBUGS-17751](https://issues.redhat.com/browse/OCPBUGS-17751)

For VPC&PHZ owner's account (Account-A)

* The PHZ is associated with VPC.

* The subnets are shared with Account-B

* The `Principal` of trust policy for shared-vpc role:
  
  * if `Passthrough` is used, cluster creator's ARN (e.g. `arn:aws:iam::123456789012:user/clustercreator`) is required.
  * if `Manual` is used, cluster creator's ARN and ingress operator role's ARN (e.g. `arn:aws:iam::123456789012:role/<cluster-name>-openshift-ingress-operator-cloud-credentials`) are required.

* The policy for shared-vpc role
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "route53:ChangeResourceRecordSets",
                "route53:ListHostedZones",
                "route53:ListHostedZonesByName",
                "route53:ListResourceRecordSets",
                "route53:ChangeTagsForResource",
                "route53:GetAccountLimit",
                "route53:GetChange",
                "route53:GetHostedZone",
                "route53:ListTagsForResource",
                "route53:UpdateHostedZoneComment",
                "tag:GetResources",
                "tag:UntagResources"
            ],
            "Resource": "*"
        }
    ]
}

Thanks Yunfei, I have updated the PR to 1) Add the permissions information to the account page 2) add the extra information about the credentials modes and 3) add a new module to the account page describing the trust policy modifications. Please let me know if what I've written makes sense as I am not an AWS expert :)

Copy link
Contributor

@yunjiang29 yunjiang29 left a comment

Choose a reason for hiding this comment

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

@bscott-rh thank you for the updates, I added some comments, let me know if there is anything unclear.

|String, for example `Z3URY6TWQ91KVV`.

|`platform.aws.hostedZoneRole`
|An Amazon Resource Name (ARN) for an existing IAM Role in the account containing the specified hosted zone. The installation program and cluster operators will assume this role when performing operations on the hosted zone.
Copy link
Contributor

@yunjiang29 yunjiang29 Oct 19, 2023

Choose a reason for hiding this comment

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

  1. The platform.aws.hostedZoneRole is designed for Shared VPC scenario, and in the account configuration section, we used word shared VPC, I would suggest to mention shared VPC in this part, to correspond to the former.

  2. As my comment, I think we can move trust policy and permission content to here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I've made that addition.

@patrickdillon
Copy link
Contributor

LGTM except the one small comment

Copy link
Contributor

@yunjiang29 yunjiang29 left a comment

Choose a reason for hiding this comment

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

@bscott-rh looks good to me, just need a small update.

@bscott-rh bscott-rh added this to the Planned for 4.14 GA milestone Oct 20, 2023
@bscott-rh bscott-rh added the peer-review-needed Signifies that the peer review team needs to review this PR label Oct 20, 2023
@jldohmann jldohmann added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Oct 20, 2023
Copy link
Contributor

@jldohmann jldohmann left a comment

Choose a reason for hiding this comment

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

Nice job Ben! generally LGTM, some ISG nits below

@jldohmann jldohmann added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 20, 2023
@bscott-rh bscott-rh merged commit 5d4e28e into openshift:main Oct 20, 2023
@bscott-rh
Copy link
Contributor Author

/cherrypick enterprise-4.14

@openshift-cherrypick-robot

@bscott-rh: new pull request created: #66667

Details

In response to this:

/cherrypick enterprise-4.14

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.14 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants