Skip to content

feat(collector): add alicloud standalone ENI resource#109

Open
Center-Sun wants to merge 1 commit into
antgroup:mainfrom
Center-Sun:feat/alicloud-eni
Open

feat(collector): add alicloud standalone ENI resource#109
Center-Sun wants to merge 1 commit into
antgroup:mainfrom
Center-Sun:feat/alicloud-eni

Conversation

@Center-Sun
Copy link
Copy Markdown
Contributor

Adds a region-scoped ENI collector that calls DescribeNetworkInterfaces without an InstanceId filter, capturing all ENIs in each region — including unattached ENIs and ENIs attached to non-ECS resources (NAT gateway, RDS, SLB, ...). The existing per-instance ENI subquery in ecs.go is unchanged and continues to embed attached-to-ECS ENIs in the ECS Detail.

The Detail is a type alias of ecs.NetworkInterfaceSet so the persisted instance JSON keeps the API's top-level field names ($.NetworkInterfaceId, $.PrivateIpSets, $.AssociatedPublicIp, ...), matching the JSONPath shape downstream consumers already expect.

Registered in platform_config and wired into InitServices so the new ENI resource type reuses the ECS client.

Thank you for your contribution to CloudRec!

What About:

  • Server (java)
  • Collector (go)
  • Rule (opa)

Description:

Explain the purpose of the PR.

Adds a region-scoped ENI collector that calls DescribeNetworkInterfaces
without an InstanceId filter, capturing all ENIs in each region —
including unattached ENIs and ENIs attached to non-ECS resources
(NAT gateway, RDS, SLB, ...). The existing per-instance ENI subquery
in ecs.go is unchanged and continues to embed attached-to-ECS ENIs
in the ECS Detail.

The Detail is a type alias of ecs.NetworkInterfaceSet so the persisted
instance JSON keeps the API's top-level field names ($.NetworkInterfaceId,
$.PrivateIpSets, $.AssociatedPublicIp, ...), matching the JSONPath shape
downstream consumers already expect.

Registered in platform_config and wired into InitServices so the new
ENI resource type reuses the ECS client.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for collecting Elastic Network Interface (ENI) resources from AliCloud. Key changes include adding the ENI constant, updating the ECS client initialization, and implementing the collection logic in a new network_interface.go file. Feedback highlights the presence of an unused type alias, a duplicated and potentially incorrect regions list, and inconsistencies in the API request configuration compared to other collectors in the package.

// This matches the shape produced by the legacy n8n pipeline, so JSONPath
// expressions like $.NetworkInterfaceId and $.PrivateIpSets.PrivateIpSet[*]
// keep working against records written by this collector.
type NetworkInterfaceDetail = ecs.NetworkInterfaceSet
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.

medium

The type alias NetworkInterfaceDetail is defined but not used in the code. Furthermore, as a type alias (=), it does not change the JSON serialization of the underlying ecs.NetworkInterfaceSet. If this was intended for documentation or future use, it should be applied to the data sent to the res channel; otherwise, it should be removed to avoid dead code.

Comment on lines +49 to +86
Regions: []string{
"cn-qingdao",
"cn-beijing",
"cn-zhangjiakou",
"cn-huhehaote",
"cn-wulanchabu",
"cn-hangzhou",
"cn-shanghai",
"cn-nanjing",
"cn-fuzhou",
"cn-shenzhen",
"cn-heyuan",
"cn-guangzhou",
"cn-wuhan-lr",
"ap-southeast-6",
"ap-northeast-2",
"ap-southeast-3",
"ap-northeast-1",
"ap-southeast-7",
"cn-chengdu",
"ap-southeast-1",
"ap-southeast-5",
"cn-zhengzhou-jva",
"cn-hongkong",
"eu-central-1",
"us-east-1",
"us-west-1",
"us-southeast-1",
"na-south-1",
"eu-west-1",
"me-east-1",
"me-central-1",
"cn-beijing-finance-1",
"cn-hangzhou-finance",
"cn-shanghai-finance-1",
"cn-shenzhen-finance-1",
"cn-heyuan-acdr-1",
},
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.

medium

The Regions list is duplicated from ecs.go and includes regions that are explicitly commented out as "not enabled" in collector/alicloud/platform/platform_config.go (e.g., cn-zhengzhou-jva, cn-heyuan-acdr-1, cn-wuhan-lr). This duplication makes maintenance difficult and will cause the collector to attempt scanning regions that are intended to be disabled. Consider moving the shared region list to a central location or aligning this list with the platform's enabled regions.

Comment on lines +98 to +100
req.Scheme = "HTTPS"
req.PageSize = requests.NewInteger(constant.DefaultPageSize)
req.PageNumber = requests.NewInteger(constant.DefaultPage)
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.

medium

This request configuration is inconsistent with other ECS-related collectors in this package (see ecs.go). Specifically, it is missing req.QueryParams["product"] = "Ecs" and req.SetHTTPSInsecure(true). The product parameter is often required for correct API routing, and the insecure setting ensures consistency across the collector when running in environments with SSL-inspecting proxies.

Suggested change
req.Scheme = "HTTPS"
req.PageSize = requests.NewInteger(constant.DefaultPageSize)
req.PageNumber = requests.NewInteger(constant.DefaultPage)
req.Scheme = "HTTPS"
req.QueryParams["product"] = "Ecs"
req.SetHTTPSInsecure(true)
req.PageSize = requests.NewInteger(constant.DefaultPageSize)
req.PageNumber = requests.NewInteger(constant.DefaultPage)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant