-
Notifications
You must be signed in to change notification settings - Fork 110
Tables for Roles Anywhere Profiles and Trust Anchors #2475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@2XXE-SRA thank you so much for the new tables 🎉 !! Could you please fix the lint failure? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for AWS Roles Anywhere by introducing two new Steampipe tables: one for Trust Anchors and one for Profiles. Key changes include the addition of documentation, table definition implementations, and integration into the plugin service.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
docs/tables/aws_rolesanywhere_trust_anchor.md | Added documentation outlining the trust anchor table usage. |
docs/tables/aws_rolesanywhere_profile.md | Added documentation outlining the profile table usage. |
aws/table_aws_rolesanywhere_trust_anchor.go | Implemented table definition and query functions for trust anchors. |
aws/table_aws_rolesanywhere_profile.go | Implemented table definition and query functions for profiles. |
aws/service.go | Added RolesAnywhereClient to support new Roles Anywhere APIs. |
aws/plugin.go | Registered the new Roles Anywhere tables with the plugin. |
Files not reviewed (1)
- go.mod: Language not supported
Hello @2XXE-SRA, great to see the PR with the new table addition! Just a quick note:
Thanks! |
Yeah, no problem. I think it was just an auto-upgrade when I grabbed the service package. I will test on the lower version and then update the PR. |
Looks like the did the trick! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces two new Steampipe table implementations for AWS Roles Anywhere by adding support for querying Trust Anchors and Profiles. Key changes include the addition of documentation files for both tables, implementation of the corresponding table definitions and list/get functions in Go, and registration of these new tables via AWS clients in the plugin.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
docs/tables/aws_rolesanywhere_trust_anchor.md | Adds documentation for the aws_rolesanywhere_trust_anchor table |
docs/tables/aws_rolesanywhere_profile.md | Adds documentation for the aws_rolesanywhere_profile table |
aws/table_aws_rolesanywhere_trust_anchor.go | Implements table definition and list/get functions for Trust Anchors |
aws/table_aws_rolesanywhere_profile.go | Implements table definition and list/get functions for Profiles |
aws/service.go | Adds a new AWS client creation function (RolesAnywhereClient) |
aws/plugin.go | Registers the new Roles Anywhere tables in the plugin |
Files not reviewed (1)
- go.mod: Language not supported
Comments suppressed due to low confidence (2)
aws/table_aws_rolesanywhere_profile.go:121
- [nitpick] Consider using Go naming conventions (e.g., profileID) instead of snake_case for variable names.
profile_id := d.EqualsQuals["id"].GetStringValue()
aws/table_aws_rolesanywhere_profile.go:114
- Ensure that the transform functions 'unescape' and 'policyToCanonical' are properly defined and imported, as their absence may lead to runtime errors.
Transform: transform.FromField("SessionPolicy").Transform(unescape).Transform(policyToCanonical),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @2XXE-SRA,
I’ve left a few review comments for you to take a look at.
Additionally, here are some suggestions:
- Name the table columns to match the API response, with the exception of the
arn
column. - Include the standard Steampipe columns.
- Update the column descriptions based on the AWS documentation for the API.
- Organize the function ordering as follows:
- List Hydrate function
- Get Hydrate function
- Any other Hydrate functions
- Transform functions
- It would be great to include more example queries in the table documentation.
Please note: I haven't added any comments for the table aws_rolesanywhere_trust_anchor
, but the suggestions above is also applicable for this table.
Thanks!
Hydrate: listProfiles, | ||
Tags: map[string]string{"service": "rolesanywhere", "action": "ListProfiles"}, | ||
}, | ||
GetMatrixItemFunc: SupportedRegionMatrix(rolesanywherev1.EndpointsID), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetMatrixItemFunc: SupportedRegionMatrix(rolesanywherev1.EndpointsID), | |
GetMatrixItemFunc: SupportedRegionMatrix(AWS_ROLESANYWHERE_SERVICE_ID), |
We are halfway through removing support for AWS SDK V1 from the plugin, as the V1 SDK has reached its End of Life.
We have listed all the AWS-supported service IDs in the following file: endpoint_service_ids_gen.go.
GetMatrixItemFunc: SupportedRegionMatrix(rolesanywherev1.EndpointsID), | ||
Columns: awsRegionalColumns([]*plugin.Column{ | ||
{ | ||
Name: "id", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please rename the column to profile_id
? The table schema should exactly match the API response, except for the arn
column.
}, | ||
{ | ||
Name: "accept_role_session_name", | ||
Description: "Accept custom role session names.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please update the column descriptions to align with the API response descriptions? You can refer to the details here: API_ProfileDetail.
Kindly update the descriptions for the remaining columns as well.
Type: proto.ColumnType_STRING, | ||
}, | ||
{ | ||
Name: "duration", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: "duration", | |
Name: "duration_seconds", |
Description: "Contains the session policy in a canonical form for easier searching.", | ||
Type: proto.ColumnType_JSON, | ||
Transform: transform.FromField("SessionPolicy").Transform(unescape).Transform(policyToCanonical), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include the steampipe standard column here. For reference: aws_acm_certificate
if d.RowsRemaining(ctx) == 0 { | ||
return nil, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if d.RowsRemaining(ctx) == 0 { | |
return nil, nil | |
} | |
// Context may get cancelled due to manual cancellation or if the limit has been reached | |
if d.RowsRemaining(ctx) == 0 { | |
return nil, nil | |
} |
Thanks for the review! I will address these in the next day or two and get back to you. |
I have pushed changes to both tables per your above suggestions. Please let me know if there are any other required changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @2XXE-SRA,
The changes look great! However, I left a few minor review comments—could you please take another look?
I noticed that a couple of columns were missed based on the API response. For example, the name
column is missing from the aws_rolesanywhere_trust_anchor
table.
Could you please cross-verify if any columns are missing according to the API response?
Note: Please validate both the GET and List API calls response and add the missing columns.
Thanks!
Type: proto.ColumnType_JSON, | ||
}, | ||
{ | ||
Name: "require_instance_properties", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move the column upwards as it is a non-JSON column.
Name: "title", | ||
Description: resourceInterfaceDescription("title"), | ||
Type: proto.ColumnType_STRING, | ||
Transform: transform.FromField("ProfileId"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transform: transform.FromField("ProfileId"), | |
Transform: transform.FromField("Name"), |
Name: "akas", | ||
Description: resourceInterfaceDescription("akas"), | ||
Type: proto.ColumnType_JSON, | ||
Transform: transform.FromField("ProfileArn").Transform(arnToAkas), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transform: transform.FromField("ProfileArn").Transform(arnToAkas), | |
Transform: transform.FromField("ProfileArn").Transform(transform.EnsureStringArray), |
Name: "akas", | ||
Description: resourceInterfaceDescription("akas"), | ||
Type: proto.ColumnType_JSON, | ||
Transform: transform.FromField("TrustAnchorArn").Transform(arnToAkas), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transform: transform.FromField("TrustAnchorArn").Transform(arnToAkas), | |
Transform: transform.FromField("TrustAnchorArn").Transform(transform.EnsureStringArray), |
Name: "title", | ||
Description: resourceInterfaceDescription("title"), | ||
Type: proto.ColumnType_STRING, | ||
Transform: transform.FromField("TrustAnchorId"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transform: transform.FromField("TrustAnchorId"), | |
Transform: transform.FromField("Name"), |
Above items addressed!
Looks like it was just the |
This PR adds two new tables for the Roles Anywhere service. Specifically, it adds support for enumerating Trust Anchors (
aws_rolesanywhere_trust_anchor
) and Profiles (aws_rolesanywhere_profile
).Example query results
Results