Update AWS config #23
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughDocumentation update to AWS IAM configuration guidance, removing a cross-account limitation statement and restructuring the Trust Policy JSON from a single statement to multiple granular permission statements covering minimum requirements, optional organizations support, and account alias features. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @baton/aws-v2.mdx:
- Around line 70-113: The JSON shown is invalid because it contains
JavaScript-style comments and is the wrong type of policy: it lists permissions
(iam:List*, iam:Get*, etc.) which belong in an inline permissions policy, not a
Trust Policy; replace the code block with a valid Trust Policy that includes a
Principal and sts:AssumeRole (similar to the example using "Principal": {"AWS":
"arn:aws:iam::...:role/ConductorOneService"} and a Condition for
"sts:ExternalId"), move the iam:* actions out of the Trust Policy and instead
show them as a separate inline permissions policy in the next step, and remove
all // comments from any JSON blocks (put explanatory notes as markdown text
before/after the JSON instead).
- Line 93: Fix the typo in the documentation string that currently reads
"ConductorOn e UI" by removing the stray space and changing it to "ConductorOne
UI" wherever that exact phrase appears (e.g., the comment line containing "//
Optional: Include this statement if you enable "Enable support for AWS
Organizations" checkbox in the ConductorOn e UI"); update the text to read "//
Optional: Include this statement if you enable "Enable support for AWS
Organizations" checkbox in the ConductorOne UI".
Comment on lines
+70
to
113
| Select **Custom Trust Policy** and paste the following into the Trust Policy JSON editor: | ||
|
|
||
| ```json | ||
| { | ||
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
| { | ||
| // The minimum permissions required for the connector to sync. This will sync IAM Users, Groups, and Roles | ||
| "Sid": "MinimumRequiredPermissionsSyncIAMUsersGroupsRoles", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "iam:ListUsers", | ||
| "iam:ListGroups", | ||
| "iam:ListRoles", | ||
| "iam:GetGroup", | ||
| "iam:GetRole", | ||
| // The following two permissions are only needed if you want ConductorOne to sync access key secret data | ||
| "iam:ListAccessKeys", | ||
| "iam:GetAccessKeyLastUsed" | ||
| ], | ||
| "Resource": "*" | ||
| }, | ||
| { | ||
| "Sid": "Statement1", | ||
| "Effect": "Allow", | ||
| "Principal": { | ||
| "AWS": "arn:aws:iam::{ROOT_ID}:role/ConductorOneService" | ||
| // Optional: Include this statement if you enable "Enable support for AWS Organizations" checkbox in the ConductorOn e UI | ||
| "Sid": "OrganizationsSupport", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "organizations:ListAccounts", | ||
| "organizations:DescribeOrganization" | ||
| ], | ||
| "Resource": "*" | ||
| }, | ||
| "Action": "sts:AssumeRole" | ||
| { | ||
| // Optional: Include this statement if you want to use account aliases instead of numeric IDs | ||
| "Sid": "UseMoreDescriptiveAccountAliases", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "iam:ListAccountAliases" | ||
| ], | ||
| "Resource": "*" | ||
| } | ||
| ] | ||
| ] | ||
| } | ||
| ``` |
Contributor
There was a problem hiding this comment.
Invalid JSON: Comments are not allowed in standard JSON.
The JSON block contains JavaScript-style comments (//), which are not valid JSON syntax. When users paste this directly into the AWS IAM Trust Policy JSON editor, it will fail to parse and be rejected.
Additionally, this appears to be structured as a Permissions Policy (with iam:List*, iam:Get* actions) rather than a Trust Policy. A Trust Policy should define who can assume the role using a Principal element and sts:AssumeRole action, similar to the SSO setup example at lines 246-264.
Consider:
- Moving the comments outside the JSON block as markdown explanations (like the notes section at lines 167-172)
- Providing the correct Trust Policy structure with Principal for cross-account access, then adding these permissions as an inline policy in the subsequent step
Example of valid Trust Policy structure (from SSO section)
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {
"AWS": "arn:aws:iam::765656841499:role/ConductorOneService"
},
"Action": "sts:AssumeRole",
"Condition": {
"StringEquals": {
"sts:ExternalId": "EXTERNAL_ID_FROM_C1_INTEGRATIONS_PAGE"
}
}
}
]
}🤖 Prompt for AI Agents
In @baton/aws-v2.mdx around lines 70 - 113, The JSON shown is invalid because it
contains JavaScript-style comments and is the wrong type of policy: it lists
permissions (iam:List*, iam:Get*, etc.) which belong in an inline permissions
policy, not a Trust Policy; replace the code block with a valid Trust Policy
that includes a Principal and sts:AssumeRole (similar to the example using
"Principal": {"AWS": "arn:aws:iam::...:role/ConductorOneService"} and a
Condition for "sts:ExternalId"), move the iam:* actions out of the Trust Policy
and instead show them as a separate inline permissions policy in the next step,
and remove all // comments from any JSON blocks (put explanatory notes as
markdown text before/after the JSON instead).
| "Effect": "Allow", | ||
| "Principal": { | ||
| "AWS": "arn:aws:iam::{ROOT_ID}:role/ConductorOneService" | ||
| // Optional: Include this statement if you enable "Enable support for AWS Organizations" checkbox in the ConductorOn e UI |
Contributor
There was a problem hiding this comment.
Typo: "ConductorOn e UI" should be "ConductorOne UI".
There's an errant space in the middle of "ConductorOne".
Suggested fix
- // Optional: Include this statement if you enable "Enable support for AWS Organizations" checkbox in the ConductorOn e UI
+ // Optional: Include this statement if you enable "Enable support for AWS Organizations" checkbox in the ConductorOne UI📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Optional: Include this statement if you enable "Enable support for AWS Organizations" checkbox in the ConductorOn e UI | |
| // Optional: Include this statement if you enable "Enable support for AWS Organizations" checkbox in the ConductorOne UI |
🤖 Prompt for AI Agents
In @baton/aws-v2.mdx at line 93, Fix the typo in the documentation string that
currently reads "ConductorOn e UI" by removing the stray space and changing it
to "ConductorOne UI" wherever that exact phrase appears (e.g., the comment line
containing "// Optional: Include this statement if you enable "Enable support
for AWS Organizations" checkbox in the ConductorOn e UI"); update the text to
read "// Optional: Include this statement if you enable "Enable support for AWS
Organizations" checkbox in the ConductorOne UI".
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Per https://conductorone.atlassian.net/browse/DOCS-436
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.