apply review remarks

Author: kubasobon

internal/inventory/awsfetcher/fetcher_elb_test.go

@@ -37,8 +37,8 @@ import (
 )
 
 func TestELBv1Fetcher_Fetch(t *testing.T) {
-	asset := elb.ElasticLoadBalancerInfo{
-		LoadBalancer: types.LoadBalancerDescription{
+	asset := elb.NewElasticLoadBalancerInfo(
+		types.LoadBalancerDescription{
 			AvailabilityZones:         []string{"us-east-1a"},
 			CanonicalHostedZoneName:   pointers.Ref("HZ-NAME"),
 			CanonicalHostedZoneNameID: pointers.Ref("HZ-ID"),
@@ -64,7 +64,11 @@ func TestELBv1Fetcher_Fetch(t *testing.T) {
 			Subnets: []string{"subnet-123"},
 			VPCId:   pointers.Ref("vpc-id"),
 		},
-	}
+		"", // awsAccount
+		"", // region
+		nil,
+		[]string{"203.0.113.1", "203.0.113.2"}, // DNS-resolved by the provider in real code
+	)
 	in := []awslib.AwsResource{asset}
 
 	expected := []inventory.AssetEvent{
@@ -85,6 +89,7 @@ func TestELBv1Fetcher_Fetch(t *testing.T) {
 				"AccountID":          "123",
 				"LoadBalancerType":   "classic",
 				"State":              "active",
+				"IPAddresses":        []string{"203.0.113.1", "203.0.113.2"},
 			}),
 			inventory.WithCreatedAt(asset.GetCreatedAt()),
 		),

internal/resources/providers/awslib/elb/load_balancer.go

@@ -36,6 +36,18 @@ type ElasticLoadBalancerInfo struct {
 	ipAddresses  []string
 }
 
+// NewElasticLoadBalancerInfo constructs a classic load balancer wrapper. ipAddresses are the
+// addresses resolved from the load balancer's DNS name (classic ELBs expose no IPs via the API).
+func NewElasticLoadBalancerInfo(lb types.LoadBalancerDescription, awsAccount, region string, tags map[string]string, ipAddresses []string) *ElasticLoadBalancerInfo {
+	return &ElasticLoadBalancerInfo{
+		LoadBalancer: lb,
+		awsAccount:   awsAccount,
+		region:       region,
+		tags:         tags,
+		ipAddresses:  ipAddresses,
+	}
+}
+
 func (v ElasticLoadBalancerInfo) GetResourceArn() string {
 	id := pointers.Deref(v.LoadBalancer.LoadBalancerName)
 	if id == "" {

internal/resources/providers/awslib/elb/provider.go

@@ -79,21 +79,22 @@ func (p *Provider) DescribeAllLoadBalancers(ctx context.Context) ([]awslib.AwsRe
 
 		var result []awslib.AwsResource
 		for _, item := range all {
-			info := &ElasticLoadBalancerInfo{
-				LoadBalancer: item,
-				awsAccount:   p.awsAccountID,
-				region:       region,
-				tags:         tagsByName[pointers.Deref(item.LoadBalancerName)],
-			}
+			var ipAddresses []string
 			if dnsName := pointers.Deref(item.DNSName); dnsName != "" {
 				if ips, err := p.resolver.LookupHost(ctx, dnsName); err != nil {
 					p.log.Debugf("Could not resolve IPs for classic ELB %q: %v", dnsName, err)
 				} else {
 					sort.Strings(ips)
-					info.ipAddresses = ips
+					ipAddresses = ips
 				}
 			}
-			result = append(result, info)
+			result = append(result, NewElasticLoadBalancerInfo(
+				item,
+				p.awsAccountID,
+				region,
+				tagsByName[pointers.Deref(item.LoadBalancerName)],
+				ipAddresses,
+			))
 		}
 		return result, nil
 	})

internal/resources/providers/awslib/elb/provider_test.go

@@ -18,7 +18,6 @@
 package elb
 
 import (
-	"context"
 	"errors"
 	"testing"
 	"time"
@@ -34,16 +33,6 @@ import (
 	"github.com/elastic/cloudbeat/internal/resources/utils/testhelper"
 )
 
-// fakeResolver is a test double for hostResolver that never touches the network.
-type fakeResolver struct {
-	ips []string
-	err error
-}
-
-func (r *fakeResolver) LookupHost(_ context.Context, _ string) ([]string, error) {
-	return r.ips, r.err
-}
-
 var onlyDefaultRegion = []string{awslib.DefaultRegion}
 
 func TestProvider_DescribeLoadBalancers(t *testing.T) {
@@ -182,7 +171,7 @@ func TestProvider_DescribeAllLoadBalancers(t *testing.T) {
 	tests := []struct {
 		name            string
 		client          func() Client
-		resolver        hostResolver
+		resolver        func(t *testing.T) hostResolver
 		expectedResults int
 		wantErr         bool
 		regions         []string
@@ -195,14 +184,24 @@ func TestProvider_DescribeAllLoadBalancers(t *testing.T) {
 				m.On("DescribeLoadBalancers", mock.Anything, mock.Anything).Return(nil, errors.New("failed"))
 				return m
 			},
-			resolver: &fakeResolver{},
-			wantErr:  true,
-			regions:  onlyDefaultRegion,
+			resolver: func(t *testing.T) hostResolver {
+				t.Helper()
+				// LookupHost is never reached: DescribeLoadBalancers fails first.
+				return newMockHostResolver(t)
+			},
+			wantErr: true,
+			regions: onlyDefaultRegion,
 		},
 		{
-			name:            "with resources and DNS IPs",
-			client:          elbV1ClientWithResources,
-			resolver:        &fakeResolver{ips: []string{"10.0.0.2", "10.0.0.1"}}, // unsorted: expect sorted output
+			name:   "with resources and DNS IPs",
+			client: elbV1ClientWithResources,
+			resolver: func(t *testing.T) hostResolver {
+				t.Helper()
+				m := newMockHostResolver(t)
+				// unsorted on purpose: the provider is expected to sort the IPs
+				m.EXPECT().LookupHost(mock.Anything, mock.Anything).Return([]string{"10.0.0.2", "10.0.0.1"}, nil)
+				return m
+			},
 			regions:         onlyDefaultRegion,
 			expectedResults: 1,
 			checkResult: func(t *testing.T, got []awslib.AwsResource) {
@@ -214,9 +213,14 @@ func TestProvider_DescribeAllLoadBalancers(t *testing.T) {
 			},
 		},
 		{
-			name:            "with resolver error (soft-fail)",
-			client:          elbV1ClientWithResources,
-			resolver:        &fakeResolver{err: errors.New("dns timeout")},
+			name:   "with resolver error (soft-fail)",
+			client: elbV1ClientWithResources,
+			resolver: func(t *testing.T) hostResolver {
+				t.Helper()
+				m := newMockHostResolver(t)
+				m.EXPECT().LookupHost(mock.Anything, mock.Anything).Return(nil, errors.New("dns timeout"))
+				return m
+			},
 			regions:         onlyDefaultRegion,
 			expectedResults: 1,
 			checkResult: func(t *testing.T, got []awslib.AwsResource) {
@@ -240,7 +244,7 @@ func TestProvider_DescribeAllLoadBalancers(t *testing.T) {
 				log:      testhelper.NewLogger(t),
 				clients:  clients,
 				client:   client,
-				resolver: tt.resolver,
+				resolver: tt.resolver(t),
 			}
 			got, err := p.DescribeAllLoadBalancers(t.Context())
 			if tt.wantErr {