Skip to content

OSDOCS-1871 Supplying predefined IAM roles to the cluster#31702

Merged
codyhoag merged 1 commit intoopenshift:masterfrom
codyhoag:existing-iam-roles
Apr 30, 2021
Merged

OSDOCS-1871 Supplying predefined IAM roles to the cluster#31702
codyhoag merged 1 commit intoopenshift:masterfrom
codyhoag:existing-iam-roles

Conversation

@codyhoag
Copy link
Copy Markdown
Contributor

@codyhoag codyhoag commented Apr 20, 2021

@codyhoag codyhoag added this to the Future Release milestone Apr 20, 2021
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 20, 2021
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 20, 2021

Deploy preview for osdocs ready!

Built with commit 7b3d618

https://deploy-preview-31702--osdocs.netlify.app

@codyhoag codyhoag force-pushed the existing-iam-roles branch 2 times, most recently from 229fcfb to 99e9dd5 Compare April 20, 2021 17:53
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 20, 2021
@codyhoag
Copy link
Copy Markdown
Contributor Author

@staebler PTAL. Thanks!

@yunjiang29
Copy link
Copy Markdown
Contributor

I got a question regarding sts:AssumeRole, the sts:AssumeRole is the default permission when creating a role via web console (without any attached policy):

aws iam get-role --role-name yunjiang-test2
{
    "Role": {
        "Path": "/",
        "RoleName": "yunjiang-test2",
        "RoleId": "AROAUMQAHCJOPRPODCRBR",
        "Arn": "arn:aws:iam::301721915996:role/yunjiang-test2",
        "CreateDate": "2021-04-21T06:59:17+00:00",
        "AssumeRolePolicyDocument": {
            "Version": "2012-10-17",
            "Statement": [
                {
                    "Effect": "Allow",
                    "Principal": {
                        "Service": "ec2.amazonaws.com"
                    },
                    "Action": "sts:AssumeRole"
                }
            ]
        },
        "Description": "Allows EC2 instances to call AWS services on your behalf.",
        "MaxSessionDuration": 3600,
        "RoleLastUsed": {}
    }
}

In the previous IPI way, installer will also create a similar role as above, and attach an external policy which does not contain sts:AssumeRole permission.

My question is, do we need to add sts:AssumeRole permission in the document explicitly?

@codyhoag
Copy link
Copy Markdown
Contributor Author

I'll defer to @staebler. Any thoughts on the previous comment, and any feedback on the PR in general? Thanks!

@staebler
Copy link
Copy Markdown

My question is, do we need to add sts:AssumeRole permission in the document explicitly?

If the permission is required, then we should document it explicitly, even if it is the default. Unless you cannot have an IAM role that does not have the sts:AssumeRole permission.

@codyhoag
Copy link
Copy Markdown
Contributor Author

@yunjiang29 Do you have any further comments on this PR? Thanks!

Comment thread modules/installation-configuration-parameters.adoc Outdated
Comment thread modules/installation-configuration-parameters.adoc Outdated
@codyhoag codyhoag force-pushed the existing-iam-roles branch from 99e9dd5 to 9dc6ebf Compare April 28, 2021 19:51
@yunjiang29
Copy link
Copy Markdown
Contributor

My question is, do we need to add sts:AssumeRole permission in the document explicitly?

If the permission is required, then we should document it explicitly, even if it is the default. Unless you cannot have an IAM role that does not have the sts:AssumeRole permission.

Get it, thanks @staebler

@yunjiang29
Copy link
Copy Markdown
Contributor

@codyhoag LGTM

@codyhoag codyhoag added the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 30, 2021
Comment thread modules/installation-aws-permissions-iam-roles.adoc Outdated
@adellape adellape added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Apr 30, 2021
@codyhoag codyhoag force-pushed the existing-iam-roles branch from 9dc6ebf to 7b3d618 Compare April 30, 2021 17:50
@codyhoag codyhoag merged commit 3211c2a into openshift:master Apr 30, 2021
@codyhoag
Copy link
Copy Markdown
Contributor Author

/cherrypick enterprise-4.8

@openshift-cherrypick-robot
Copy link
Copy Markdown

@codyhoag: new pull request created: #32093

Details

In response to this:

/cherrypick enterprise-4.8

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.8 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.

7 participants