Skip to content

[Asset Inventory][AWS] Update EC2, ELB and RDS fetchers for InfoSec#7277

Open
kubasobon wants to merge 2 commits into
mainfrom
infosec/update-awsfetcher
Open

[Asset Inventory][AWS] Update EC2, ELB and RDS fetchers for InfoSec#7277
kubasobon wants to merge 2 commits into
mainfrom
infosec/update-awsfetcher

Conversation

@kubasobon

Copy link
Copy Markdown
Member

Summary of your changes

Resource Field Change
EC2 entity.attributes.Role Added LookupTag(tags, "role") in buildAttributes
RDS entity.attributes.DBInstanceStatus Added Status field to wrapper struct; mapped from DBInstanceStatus in provider; emitted in fetcher
ELB v2 (ALB/NLB) entity.attributes.IPAddresses GetIPAddresses() now also collects PrivateIPv4Address and IPv6Address per AZ address (previously only IpAddress)
ELB v1 (Classic) entity.attributes.State GetState() returns "active" (hardcoded as classic API exposes no state field)
ELB v1 (Classic) entity.attributes.IPAddresses DNS-resolves the ELB DNSName at fetch time via injectable hostResolver; soft-fails to empty on error

Related Issues

Closes https://github.com/elastic/security-team/issues/18294

Checklist

  • I have added tests that prove my fix is effective or that my feature works


// newMockHostResolver creates a new instance of mockHostResolver. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
// The first argument is typically a *testing.T value.
func newMockHostResolver(t interface {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think that these are not being utilized. Is there a reason to implement them?
newMockHostResolver, mockHostResolver, mockHostResolver_Expecter, mockHostResolver_LookupHost_Call

"PubliclyAccessible": false, // scheme is "internal"
"AccountID": "123",
"LoadBalancerType": "classic",
"State": "active",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does it makes sense to add IPAddresses as well? Because we don't run the DNS resolver (like we do in the actual code to resolve the IPs).
Reference:
https://github.com/elastic/cloudbeat/pull/7277/changes#diff-45e6ce26ce27502d2d082db4502e0999a52d546b187833699c645fbfa297c146R88-R96

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants