Add aws_organizations_delegated_administrator and aws_organizations_delegated_services_for_account tables (#2421)#2477
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two new Steampipe tables to query AWS Organizations delegated administrators and delegated services for an account.
- Added documentation markdown files for both tables.
- Implemented Go code files to list and format results from AWS Organizations.
- Updated the plugin registration to include the new tables.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/tables/aws_organizations_delegated_services_for_account.md | Added documentation for querying delegated services for an account |
| docs/tables/aws_organizations_delegated_administrator.md | Added documentation for querying delegated administrator accounts |
| aws/table_aws_organizations_delegated_services_for_account.go | Implemented table and list function for delegated services |
| aws/table_aws_organizations_delegated_administrator.go | Implemented table and list function for delegated administrators |
| aws/plugin.go | Registered the new tables in the plugin |
Comments suppressed due to low confidence (1)
aws/table_aws_organizations_delegated_services_for_account.go:41
- [nitpick] Clarify the purpose of mapping the 'title' and 'akas' columns from 'ServicePrincipal'. Once confirmed, consider removing the TODO comment to avoid confusion in production code.
// TODO
// I am unsure whether the title and akas below should correspond to 'ServicePrincipal'.
ParthaI
left a comment
There was a problem hiding this comment.
Hi @FuadAbdullah, I have left a few review comments. Could you please take a look? Thanks!
| Name: "aws_organizations_delegated_services_for_account", | ||
| Description: "AWS Organizations Delegated Services For Account", | ||
| List: &plugin.ListConfig{ | ||
| KeyColumns: plugin.SingleColumn("delegated_account_id"), |
There was a problem hiding this comment.
Instead of making delegated_account_id a required key qualifier, can we consider using the aws_organizations_delegated_administrator table as the parent?
Proposed Design:
- Use
listOrganizationsDelegatedAdminsas theParentHydratefunction. - Make
delegated_account_idan optional qualifier rather than required. - In the list function of this table, add a check to skip the API call if
delegated_account_idis explicitly provided in the query parameters.
You can refer to the aws_cloudwatch_log_stream table for a similar implementation pattern.
There was a problem hiding this comment.
Referenced aws_cloudwatch_log_stream and implemented the parent hydrate and direct query methods into aws_organizations_delegated_services_for_account table
| func listDelegatedServices(ctx context.Context, d *plugin.QueryData, _ *plugin.HydrateData) (interface{}, error) { | ||
|
|
||
| // Retrieve the `delegated_account_id` from the user's `WHERE` statement | ||
| delegatedAccountId := d.EqualsQuals["delegated_account_id"].GetStringValue() |
There was a problem hiding this comment.
Please take a look at the earlier suggestion (Instead of making delegated_account_id a required key qualifier, can we consider using the aws_organizations_delegated_administrator table as the parent?)
There was a problem hiding this comment.
I have made delegatedAccountId optional in the latest PR for table aws_organizations_delegated_services_for_account
|
Hello @FuadAbdullah, just checking in — did you get a chance to review the comments above? |
|
Hi @ParthaI, I am currently reviewing the comments. I will review and apply the changes based on your comments. Thanks for the heads up! |
…e two delegated administrators .go files
…legated_services_for_account.md
…ted_administrator.md
|
Hi @ParthaI, I have amended the four files related to Delegated Administrator per your comments and recommendations. Please have a look and let me know if there are any other amendments to be made. Thanks! |
ParthaI
left a comment
There was a problem hiding this comment.
Hello @FuadAbdullah,
The changes look great to me. However, I’ve added a couple of additional review comments—could you please take a look when you get a chance?
Additionally, if the log statements are not necessary, please consider cleaning them up. As a general practice, we try to avoid including log statements unless they are capturing API errors.
Thanks again!
…elegated_services_for_account and removed custom Transform functions from the two tables
|
Good day, @ParthaI, I have performed changes according to your latest feedback. Please have a look and let me know if there's anything to be changed. Also, I believe there's only four error loggers in both tables but I might have overlooked so do let me know so I can check the loggers out and see if I should remove them. |
Integration test logs
N/A
Example query results
NOTE: I have compared the output with AWS CLI-equivalent command
aws organizations list-delegated-administratorsResults
QUERY:
select * from aws_organizations_delegated_administratorQUERY:
select * from aws_organizations_delegated_services_for_account where delegated_account_id='\<redacted\>'