Skip to content

OSDOCS#7657: Adding new AWS permissions#64238

Closed
mjpytlak wants to merge 1 commit intoopenshift:mainfrom
mjpytlak:osdocs-7657
Closed

OSDOCS#7657: Adding new AWS permissions#64238
mjpytlak wants to merge 1 commit intoopenshift:mainfrom
mjpytlak:osdocs-7657

Conversation

@mjpytlak
Copy link
Copy Markdown
Contributor

@mjpytlak mjpytlak commented Sep 1, 2023

Version(s):
4.14+

Issue:
This issue addresses osdocs-7657.

Link to docs preview:

QE review:

  • QE has approved this change.

@mjpytlak mjpytlak added this to the Planned for 4.14 GA milestone Sep 1, 2023
@openshift-ci openshift-ci Bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 1, 2023
@ocpdocs-previewbot
Copy link
Copy Markdown

@mjpytlak
Copy link
Copy Markdown
Contributor Author

mjpytlak commented Sep 1, 2023

Hi @yunjiang29 looks like we have to add a few permissions based on this conversation [1] with @barbacbd. PTAL. I see you were involved in the doc review when this topic was added [2]. Are you able to confirm if these permissions belong in the control plane list or compute list? Thanks in advance.

[1] https://redhat-internal.slack.com/archives/C68TNFWA2/p1693472847139919
[2] #31702

Copy link
Copy Markdown
Contributor

@barbacbd barbacbd left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2023
@barbacbd
Copy link
Copy Markdown
Contributor

barbacbd commented Sep 5, 2023

We should probably add @patrickdillon and QE

@mjpytlak
Copy link
Copy Markdown
Contributor Author

mjpytlak commented Sep 6, 2023

We should probably add @patrickdillon and QE

Thanks for the review @barbacbd. I had added @yunjiang29 to the original request.

Comment on lines 22 to +24
* `ec2:Describe*`
* `ec2:DescribeSecurityGroups`
* `ec2:DescribeSecurityGroupRules`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just noticed that the previous line is a wildcard that should technically already include these two, so we may not actually need this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just noticed that the previous line is a wildcard that should technically already include these two, so we may not actually need this.

+1

By default, "ec2:Describe*", already presents in the current control plane role policy.

@mjpytlak
Copy link
Copy Markdown
Contributor Author

mjpytlak commented Sep 7, 2023

So good to close this PR @yunjiang29 or still looking into it?

@yunjiang29
Copy link
Copy Markdown
Contributor

@mjpytlak I'm good to close this issue.

@mjpytlak
Copy link
Copy Markdown
Contributor Author

mjpytlak commented Sep 8, 2023

No docs update required.

@mjpytlak mjpytlak closed this Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.14 lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants