Skip to content

Commit c1203db

Browse files
committed
ec2 client credential init fix
current use of ec2.New() does not work with IRSA. (not tested but most like same problem with EKS Pod Identity as well) ``` (pkg/util/ec2/ec2_tags.go:104 in fetchEc2TagsFromAPI) | unable to get tags using default credentials (falling back to instance role): operation error EC2: DescribeTags, https response error StatusCode: 400, RequestID: 1234-1234-1234, api error MissingParameter: The request must contain the parameter AWSAccessKeyId ``` This change is to change to use aws config package to help initialise aws ec2 client with the built-in credentials such as AWS IRSA tokens.
1 parent 66190f7 commit c1203db

File tree

2 files changed

+127
-10
lines changed

2 files changed

+127
-10
lines changed

pkg/util/ec2/ec2_tags.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"time"
1616

1717
"github.com/aws/aws-sdk-go-v2/aws"
18+
"github.com/aws/aws-sdk-go-v2/config"
1819
"github.com/aws/aws-sdk-go-v2/credentials"
1920
"github.com/aws/aws-sdk-go-v2/service/ec2"
2021
"github.com/aws/aws-sdk-go-v2/service/ec2/types"
@@ -97,7 +98,7 @@ func fetchEc2TagsFromAPI(ctx context.Context) ([]string, error) {
9798
// except when a more specific role (e.g. task role in ECS) does not have
9899
// EC2:DescribeTags permission, but a more general role (e.g. instance role)
99100
// does have it.
100-
tags, err := getTagsWithCreds(ctx, instanceIdentity, nil)
101+
tags, err := getTagsWithCreds(ctx, instanceIdentity, ec2Cconnection(ctx, instanceIdentity.Region, nil))
101102
if err == nil {
102103
return tags, nil
103104
}
@@ -111,14 +112,25 @@ func fetchEc2TagsFromAPI(ctx context.Context) ([]string, error) {
111112
}
112113

113114
awsCreds := credentials.NewStaticCredentialsProvider(iamParams.AccessKeyID, iamParams.SecretAccessKey, iamParams.Token)
114-
return getTagsWithCreds(ctx, instanceIdentity, awsCreds)
115+
return getTagsWithCreds(ctx, instanceIdentity, ec2Cconnection(ctx, instanceIdentity.Region, awsCreds))
115116
}
116117

117-
func getTagsWithCreds(ctx context.Context, instanceIdentity *EC2Identity, awsCreds aws.CredentialsProvider) ([]string, error) {
118-
connection := ec2.New(ec2.Options{
119-
Region: instanceIdentity.Region,
120-
Credentials: awsCreds,
121-
})
118+
type ec2ClientInterface interface {
119+
DescribeTags(ctx context.Context, params *ec2.DescribeTagsInput, optFns ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error)
120+
}
121+
122+
// ec2Cconnection creates an ec2 client with the given region and credentials
123+
func ec2Cconnection(ctx context.Context, region string, awsCreds aws.CredentialsProvider) ec2ClientInterface {
124+
// using aws config to read the build in credentials to set up the ec2 client.
125+
cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion(region), config.WithCredentialsProvider(awsCreds))
126+
if err != nil {
127+
log.Warnf("unable to get aws configurations: %s", err)
128+
return nil
129+
}
130+
return ec2.NewFromConfig(cfg)
131+
}
132+
133+
func getTagsWithCreds(ctx context.Context, instanceIdentity *EC2Identity, connection ec2ClientInterface) ([]string, error) {
122134

123135
// We want to use 'ec2_metadata_timeout' here instead of current context. 'ctx' comes from the agent main and will
124136
// only be canceled if the agent is stopped. The default timeout for the AWS SDK is 1 minutes (20s timeout with

pkg/util/ec2/ec2_tags_test.go

Lines changed: 108 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,22 @@ package ec2
99

1010
import (
1111
"context"
12+
"errors"
1213
"fmt"
1314
"io"
1415
"net/http"
1516
"net/http/httptest"
1617
"os"
1718
"testing"
1819

19-
"github.com/stretchr/testify/assert"
20-
"github.com/stretchr/testify/require"
21-
2220
configmock "github.com/DataDog/datadog-agent/pkg/config/mock"
2321
pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup"
2422
"github.com/DataDog/datadog-agent/pkg/util/cache"
23+
"github.com/aws/aws-sdk-go-v2/aws"
24+
"github.com/aws/aws-sdk-go-v2/service/ec2"
25+
"github.com/aws/aws-sdk-go-v2/service/ec2/types"
26+
"github.com/stretchr/testify/assert"
27+
"github.com/stretchr/testify/require"
2528
)
2629

2730
func TestGetIAMRole(t *testing.T) {
@@ -208,3 +211,105 @@ func TestGetTagsFullWorkflow(t *testing.T) {
208211
assert.NoError(t, err)
209212
assert.Equal(t, []string{"tag1", "tag2"}, tags)
210213
}
214+
215+
// Mock implementation of ec2ClientInterface
216+
type mockEC2Client struct {
217+
DescribeTagsFunc func(ctx context.Context, params *ec2.DescribeTagsInput, optFns ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error)
218+
}
219+
220+
func (m *mockEC2Client) DescribeTags(ctx context.Context, params *ec2.DescribeTagsInput, optFns ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error) {
221+
return m.DescribeTagsFunc(ctx, params, optFns...)
222+
}
223+
224+
// Helper function to compare slices of strings
225+
func equalStringSlices(a, b []string) bool {
226+
if len(a) != len(b) {
227+
return false
228+
}
229+
aMap := make(map[string]struct{}, len(a))
230+
for _, v := range a {
231+
aMap[v] = struct{}{}
232+
}
233+
for _, v := range b {
234+
if _, ok := aMap[v]; !ok {
235+
return false
236+
}
237+
}
238+
return true
239+
}
240+
241+
func TestGetTagsWithCreds(t *testing.T) {
242+
tests := []struct {
243+
name string
244+
instanceIdentity *EC2Identity
245+
mockDescribeTags func(ctx context.Context, params *ec2.DescribeTagsInput, optFns ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error)
246+
expectedTags []string
247+
expectedError assert.ErrorAssertionFunc
248+
}{
249+
{
250+
name: "Successful retrieval of tags",
251+
instanceIdentity: &EC2Identity{
252+
InstanceID: "i-1234567890abcdef0",
253+
},
254+
mockDescribeTags: func(_ context.Context, _ *ec2.DescribeTagsInput, _ ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error) {
255+
return &ec2.DescribeTagsOutput{
256+
Tags: []types.TagDescription{
257+
{Key: aws.String("Name"), Value: aws.String("TestInstance")},
258+
{Key: aws.String("Env"), Value: aws.String("Production")},
259+
},
260+
}, nil
261+
},
262+
expectedTags: []string{"Name:TestInstance", "Env:Production"},
263+
expectedError: assert.NoError,
264+
},
265+
{
266+
name: "Excluded tags are filtered out",
267+
instanceIdentity: &EC2Identity{
268+
InstanceID: "i-1234567890abcdef0",
269+
},
270+
mockDescribeTags: func(_ context.Context, _ *ec2.DescribeTagsInput, _ ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error) {
271+
return &ec2.DescribeTagsOutput{
272+
Tags: []types.TagDescription{
273+
{Key: aws.String("Name"), Value: aws.String("TestInstance")},
274+
{Key: aws.String("aws:cloudformation:stack-name"), Value: aws.String("MyStack")},
275+
},
276+
}, nil
277+
},
278+
expectedTags: []string{"Name:TestInstance", "aws:cloudformation:stack-name:MyStack"},
279+
expectedError: assert.NoError,
280+
},
281+
{
282+
name: "DescribeTags returns an error",
283+
instanceIdentity: &EC2Identity{
284+
InstanceID: "i-1234567890abcdef0",
285+
},
286+
mockDescribeTags: func(_ context.Context, _ *ec2.DescribeTagsInput, _ ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error) {
287+
return nil, errors.New("DescribeTags error")
288+
},
289+
expectedTags: nil,
290+
expectedError: assert.Error,
291+
},
292+
}
293+
294+
for _, tt := range tests {
295+
t.Run(tt.name, func(t *testing.T) {
296+
// Mock the EC2 client
297+
mockClient := &mockEC2Client{
298+
DescribeTagsFunc: tt.mockDescribeTags,
299+
}
300+
301+
// Create a background context
302+
ctx := context.Background()
303+
304+
// Call the function under test
305+
tags, err := getTagsWithCreds(ctx, tt.instanceIdentity, mockClient)
306+
307+
// Validate the error
308+
tt.expectedError(t, err)
309+
// Validate the tags
310+
if !equalStringSlices(tags, tt.expectedTags) {
311+
t.Fatalf("Expected tags '%v', got '%v'", tt.expectedTags, tags)
312+
}
313+
})
314+
}
315+
}

0 commit comments

Comments
 (0)