Merged
Conversation
thank you, gemini Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…c into feat/support-aws-resources
…cs/cluster, ecs/service, ecs/task, ecs/task_definition, dynamodb/table, config/recorder, cloudwatch/log_group, cloudwatch/alarm, cloudtrail/types, cloudtrail/trail, cloudformation/stack, autoscaling/group. update vpc: flowlog, networkinterface, vpcendpointservice
Feat/support aws resources
Feat/support aws resources
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Reviewer's GuideThis PR implements support for ~30 new AWS resource collectors by parallelizing data fetching, refactoring existing IAM collectors to use AWS SDK v2 paginators and worker pools, and updating service initialization, constants, and platform configuration. Class diagram for new and updated AWS resource collectorsclassDiagram
class Services {
+EC2
+IAM
+S3
+Lambda
+KMS
+ECR
+ElastiCache
+ELB
+CLB
+FSx
+RDS
+Route53Domains
+Route53
+CloudFront
+WAFv2
+CloudTrail
+APIGatewayV2
+ACM
+SecretsManager
+AutoScaling
+ECS
+EKS
+DynamoDB
+CloudFormation
+GuardDuty
+EFS
+SNS
+SQS
+CloudWatch
+CloudWatchLogs
+Account
+Config
+AppStream
+AccessAnalyzer
+CognitoIdentityProvider
+CognitoIdentity
+FMS
+Inspector2
+SecurityHub
+Macie
+NetworkFirewall
+OpenSearch
}
class UserDetail {
+User
+AttachedPolicies
+InlinePolicies
+MFADevices
+AccessKeys
+LoginProfile
+Tags
}
class RoleDetail {
+Role
+AttachedPolicies
+InlinePolicies
+Tags
}
class VPCDetail {
+RouteTables
+InternetGateways
+VPCEndpoints
+VPCPeeringConnections
+VPNConnections
}
class UserPoolDetail {
+UserPool
+UserPoolClients
+Users
+Tags
}
class IdentityPoolDetail {
+IdentityPool
+Tags
}
class KeyDetail {
+Key
+RotationEnabled
+Policy
+Tags
}
class SNSTopicDetail {
+Topic
+Attributes
+Policy
+Subscriptions
+Tags
}
class SQSQueueDetail {
+Url
+Name
+Region
+Attributes
+Policy
+Tags
}
class PolicyDetail {
+Policy
+ComplianceStatus
+Tags
}
class ClusterDetail {
+Cluster
+Services
+Tasks
}
class DomainDetail {
+Domain
+Tags
}
class FunctionDetail {
+Function
+Policy
+URLConfigs
+Tags
}
class AnalyzerDetail {
+Analyzer
+Findings
+Tags
}
class StackDetail {
+Stack
}
Class diagram for IAM User and Role collector refactorclassDiagram
class UserDetail {
+User
+AttachedPolicies
+InlinePolicies
+MFADevices
+AccessKeys
+LoginProfile
+Tags
}
class RoleDetail {
+Role
+AttachedPolicies
+InlinePolicies
+Tags
}
Class diagram for Cognito UserPool and IdentityPool collectorsclassDiagram
class UserPoolDetail {
+UserPool
+UserPoolClients
+Users
+Tags
}
class IdentityPoolDetail {
+IdentityPool
+Tags
}
Class diagram for KMS Key collectorclassDiagram
class KeyDetail {
+Key
+RotationEnabled
+Policy
+Tags
}
Class diagram for SNS Topic and SQS Queue collectorsclassDiagram
class SNSTopicDetail {
+Topic
+Attributes
+Policy
+Subscriptions
+Tags
}
class SQSQueueDetail {
+Url
+Name
+Region
+Attributes
+Policy
+Tags
}
Class diagram for ECS Cluster collectorclassDiagram
class ClusterDetail {
+Cluster
+Services
+Tasks
}
Class diagram for OpenSearch Domain collectorclassDiagram
class DomainDetail {
+Domain
+Tags
}
Class diagram for Lambda Function collectorclassDiagram
class FunctionDetail {
+Function
+Policy
+URLConfigs
+Tags
}
Class diagram for Access Analyzer collectorclassDiagram
class AnalyzerDetail {
+Analyzer
+Findings
+Tags
}
Class diagram for CloudFormation Stack collectorclassDiagram
class StackDetail {
+Stack
}
Class diagram for FMS Policy collectorclassDiagram
class PolicyDetail {
+Policy
+ComplianceStatus
+Tags
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @j3ttt - I've reviewed your changes - here's some feedback:
- The worker‐pool concurrency pattern is duplicated across most collectors—consider extracting a reusable helper to reduce boilerplate and improve consistency.
- The giant switch in Services.InitServices makes adding new services cumbersome—refactor to use a map of resource types to client initializers for easier maintenance.
- Inside your worker goroutines errors are logged but not propagated—consider capturing and returning errors from workers so failed descriptors don’t go unnoticed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The worker‐pool concurrency pattern is duplicated across most collectors—consider extracting a reusable helper to reduce boilerplate and improve consistency.
- The giant switch in Services.InitServices makes adding new services cumbersome—refactor to use a map of resource types to client initializers for easier maintenance.
- Inside your worker goroutines errors are logged but not propagated—consider capturing and returning errors from workers so failed descriptors don’t go unnoticed.
## Individual Comments
### Comment 1
<location> `collector/aws/collector/opensearch/domain.go:129` </location>
<code_context>
+func extractDomainTags(domain *opensearch.DescribeDomainOutput) map[string]string {
</code_context>
<issue_to_address>
Tag extraction in extractDomainTags is not robust and may mislead consumers.
Use the ListTags API to retrieve actual resource tags. If you intend to provide metadata, separate it from tags to prevent confusion.
</issue_to_address>
### Comment 2
<location> `collector/aws/collector/iam/user.go:89` </location>
<code_context>
+func describeUserDetail(ctx context.Context, client *iam.Client, user types.User) UserDetail {
</code_context>
<issue_to_address>
Errors from concurrent fetches in describeUserDetail are ignored.
Please aggregate and handle or log errors from concurrent fetches to prevent incomplete or misleading results.
</issue_to_address>
### Comment 3
<location> `collector/aws/collector/iam/role.go:89` </location>
<code_context>
- if err != nil {
- return nil
+// describeRoleDetail fetches all details for a single role.
+func describeRoleDetail(ctx context.Context, client *iam.Client, role types.Role) RoleDetail {
+ var wg sync.WaitGroup
+ var attachedPolicies []types.AttachedPolicy
+ var inlinePolicies []string
+ var tags []types.Tag
+
+ wg.Add(3)
+
+ go func() {
+ defer wg.Done()
+ attachedPolicies, _ = listAttachedRolePolicies(ctx, client, role.RoleName)
+ }()
+
+ go func() {
+ defer wg.Done()
+ inlinePolicies, _ = listRolePolicies(ctx, client, role.RoleName)
+ }()
+
+ go func() {
+ defer wg.Done()
+ tags, _ = listRoleTags(ctx, client, role.RoleName)
+ }()
+
+ wg.Wait()
+
+ return RoleDetail{
+ Role: role,
+ AttachedPolicies: attachedPolicies,
</code_context>
<issue_to_address>
Errors from concurrent fetches in describeRoleDetail are ignored.
As with the user detail function, unhandled errors from these fetches may lead to incomplete data without any error reporting. Please handle or log these errors to improve reliability and observability.
</issue_to_address>
### Comment 4
<location> `collector/aws/collector/networkfirewall/rulegroup.go:155` </location>
<code_context>
+ tags["StatefulRulesCount"] = string(rune(len(ruleGroup.RuleGroup.RulesSource.StatefulRules)))
+ } else if ruleGroup.RuleGroup.RulesSource.StatelessRulesAndCustomActions != nil {
+ tags["RulesSourceType"] = "StatelessRulesAndCustomActions"
+ if ruleGroup.RuleGroup.RulesSource.StatelessRulesAndCustomActions.StatelessRules != nil {
+ tags["StatelessRulesCount"] = string(rune(len(ruleGroup.RuleGroup.RulesSource.StatelessRulesAndCustomActions.StatelessRules)))
+ }
</code_context>
<issue_to_address>
Incorrect integer-to-string conversion for StatelessRulesCount.
Use strconv.Itoa(len(...)) to convert the integer to a string, as string(rune(len(...))) will not yield the correct string representation.
</issue_to_address>
### Comment 5
<location> `collector/aws/collector/networkfirewall/rulegroup.go:161` </location>
<code_context>
+ }
+ }
+
+ // Add capacity information
+ if ruleGroup.RuleGroupResponse.Capacity != nil {
+ tags["Capacity"] = string(rune(*ruleGroup.RuleGroupResponse.Capacity))
</code_context>
<issue_to_address>
Capacity should be converted to string using strconv.Itoa.
The current conversion returns a character, not the numeric string. Use strconv.Itoa(int(*ruleGroup.RuleGroupResponse.Capacity)) instead.
</issue_to_address>
### Comment 6
<location> `collector/aws/collector/networkfirewall/firewall.go:126` </location>
<code_context>
+
+ var tags map[string]string
+
+ // Get tags - Network Firewall doesn't have a direct API to list tags
+ // but we can extract relevant information from the firewall itself
+ tags = extractFirewallTags(describeOutput)
</code_context>
<issue_to_address>
Consider using ListTagsForResource API if available for Network Firewall.
If Network Firewall supports ListTagsForResource, use it to retrieve tags directly rather than extracting them from the firewall response.
</issue_to_address>
### Comment 7
<location> `collector/aws/collector/inspector2/coverage.go:73` </location>
<code_context>
+ go func() {
+ defer wg.Done()
+ for coverage := range tasks {
+ coverage := coverage
+ detail := describeCoverageDetail(ctx, client, coverage)
+ if detail != nil {
</code_context>
<issue_to_address>
Redundant variable assignment inside goroutine.
The assignment 'coverage := coverage' can be removed, as the loop variable is already properly scoped within the goroutine.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
4 similar comments
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Contributor
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Thank you for your contribution to CloudRec!
What About:
java)go)opa)Description:
Implement about 30 new AWS resource collectors
Summary by Sourcery
Add comprehensive AWS support by implementing about 30 new resource collectors, updating service initialization and platform configuration, refactoring collector patterns for concurrency and pagination, and updating AWS SDK dependencies.
New Features:
Enhancements:
Build: