Skip to content

Commit

Permalink
Unrevert #4424 and #4431 - aws-sdk-go-v2 credentials chain (#4438)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tinnywang
tinnywang authored Dec 2, 2024
1 parent de95b39 commit 7975bb3
Show file tree
Hide file tree
Showing 743 changed files with 124,846 additions and 115 deletions.
11 changes: 10 additions & 1 deletion agent/app/agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import (
apierrors "github.com/aws/amazon-ecs-agent/ecs-agent/api/errors"
"github.com/aws/amazon-ecs-agent/ecs-agent/credentials"
"github.com/aws/amazon-ecs-agent/ecs-agent/credentials/instancecreds"
"github.com/aws/amazon-ecs-agent/ecs-agent/credentials/providers"
"github.com/aws/amazon-ecs-agent/ecs-agent/doctor"
"github.com/aws/amazon-ecs-agent/ecs-agent/ec2"
"github.com/aws/amazon-ecs-agent/ecs-agent/eventstream"
Expand All @@ -68,6 +69,7 @@ import (
"github.com/aws/amazon-ecs-agent/ecs-agent/tcs/model/ecstcs"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/retry"
"github.com/aws/amazon-ecs-agent/ecs-agent/wsclient"
awsv2 "github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
aws_credentials "github.com/aws/aws-sdk-go/aws/credentials"
Expand Down Expand Up @@ -146,6 +148,7 @@ type ecsAgent struct {
dockerClient dockerapi.DockerClient
containerInstanceARN string
credentialProvider *aws_credentials.Credentials
credentialsCache *awsv2.CredentialsCache
stateManagerFactory factory.StateManager
saveableOptionFactory factory.SaveableOption
pauseLoader loader.Loader
Expand Down Expand Up @@ -231,6 +234,11 @@ func newAgent(blackholeEC2Metadata bool, acceptInsecureCert *bool) (agent, error
metadataManager = containermetadata.NewManager(dockerClient, cfg)
}

credentialsCache := providers.NewInstanceCredentialsCache(
cfg.External.Enabled(),
providers.NewRotatingSharedCredentialsProviderV2(),
nil,
)
initialSeqNumber := int64(-1)
return &ecsAgent{
ctx: ctx,
Expand All @@ -244,6 +252,7 @@ func newAgent(blackholeEC2Metadata bool, acceptInsecureCert *bool) (agent, error
// to mimic roughly the way it's instantiated by the SDK for a default
// session.
credentialProvider: instancecreds.GetCredentials(cfg.External.Enabled()),
credentialsCache: credentialsCache,
stateManagerFactory: factory.NewStateManager(),
saveableOptionFactory: factory.NewSaveableOption(),
pauseLoader: pause.New(),
Expand Down Expand Up @@ -781,7 +790,7 @@ func (agent *ecsAgent) registerContainerInstance(
client ecs.ECSClient,
additionalAttributes []*ecsmodel.Attribute) error {
// Preflight request to make sure they're good
if preflightCreds, err := agent.credentialProvider.Get(); err != nil || preflightCreds.AccessKeyID == "" {
if preflightCreds, err := agent.credentialsCache.Retrieve(context.TODO()); err != nil || !preflightCreds.HasKeys() {
seelog.Errorf("Error getting valid credentials: %s", err)
}

Expand Down
124 changes: 62 additions & 62 deletions agent/app/agent_test.go
View file

Large diffs are not rendered by default.

98 changes: 46 additions & 52 deletions agent/app/agent_unix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ import (
"github.com/aws/amazon-ecs-agent/ecs-agent/eventstream"
md "github.com/aws/amazon-ecs-agent/ecs-agent/manageddaemon"

awsv2 "github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
)
Expand All @@ -74,7 +74,7 @@ func TestDoStartTaskENIHappyPath(t *testing.T) {
monitoShutdownEvents := make(chan bool)

cniClient := mock_ecscni.NewMockCNIClient(ctrl)
mockCredentialsProvider := app_mocks.NewMockProvider(ctrl)
mockCredentialsProvider := app_mocks.NewMockCredentialsProvider(ctrl)
mockPauseLoader := mock_loader.NewMockLoader(ctrl)
mockUdevMonitor := mock_udev.NewMockUdev(ctrl)
mockMetadata := mock_ec2.NewMockEC2MetadataClient(ctrl)
Expand All @@ -88,7 +88,6 @@ func TestDoStartTaskENIHappyPath(t *testing.T) {

// These calls are expected to happen, but cannot be ordered as they are
// invoked via go routines, which will lead to occasional test failues
mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes()
dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes()
dockerClient.EXPECT().ListContainers(gomock.Any(), gomock.Any(), gomock.Any()).Return(
Expand Down Expand Up @@ -135,7 +134,7 @@ func TestDoStartTaskENIHappyPath(t *testing.T) {
cniClient.EXPECT().Capabilities(ecscni.ECSIPAMPluginName).Return(cniCapabilities, nil),
cniClient.EXPECT().Capabilities(ecscni.ECSAppMeshPluginName).Return(cniCapabilities, nil),
cniClient.EXPECT().Capabilities(ecscni.ECSBranchENIPluginName).Return(cniCapabilities, nil),
mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil),
mockCredentialsProvider.EXPECT().Retrieve(gomock.Any()).Return(awsv2.Credentials{}, nil),
cniClient.EXPECT().Version(ecscni.VPCENIPluginName).Return("v1", nil),
cniClient.EXPECT().Version(ecscni.ECSBranchENIPluginName).Return("v2", nil),
mockMobyPlugins.EXPECT().Scan().Return([]string{}, nil),
Expand Down Expand Up @@ -170,15 +169,15 @@ func TestDoStartTaskENIHappyPath(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
// Cancel the context to cancel async routines
agent := &ecsAgent{
ctx: ctx,
cfg: &cfg,
credentialProvider: credentials.NewCredentials(mockCredentialsProvider),
dataClient: data.NewNoopClient(),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
eniWatcher: eniWatcher,
cniClient: cniClient,
ec2MetadataClient: mockMetadata,
ctx: ctx,
cfg: &cfg,
credentialsCache: awsv2.NewCredentialsCache(mockCredentialsProvider),
dataClient: data.NewNoopClient(),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
eniWatcher: eniWatcher,
cniClient: cniClient,
ec2MetadataClient: mockMetadata,
terminationHandler: func(state dockerstate.TaskEngineState, dataClient data.Client, taskEngine engine.TaskEngine, cancel context.CancelFunc) {
},
mobyPlugins: mockMobyPlugins,
Expand Down Expand Up @@ -441,7 +440,7 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) {
ctrl, credentialsManager, state, imageManager, client,
dockerClient, _, _, execCmdMgr, _ := setup(t)
defer ctrl.Finish()
mockCredentialsProvider := app_mocks.NewMockProvider(ctrl)
mockCredentialsProvider := app_mocks.NewMockCredentialsProvider(ctrl)
mockControl := mock_control.NewMockControl(ctrl)
mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl)
mockPauseLoader := mock_loader.NewMockLoader(ctrl)
Expand All @@ -453,7 +452,6 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) {
dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes()
dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes()
imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1)
mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
ec2MetadataClient.EXPECT().PrimaryENIMAC().Return("mac", nil)
ec2MetadataClient.EXPECT().VPCID(gomock.Eq("mac")).Return("vpc-id", nil)
ec2MetadataClient.EXPECT().SubnetID(gomock.Eq("mac")).Return("subnet-id", nil)
Expand All @@ -479,7 +477,7 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) {

gomock.InOrder(
mockControl.EXPECT().Init().Return(nil),
mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil),
mockCredentialsProvider.EXPECT().Retrieve(gomock.Any()).Return(awsv2.Credentials{}, nil),
mockMobyPlugins.EXPECT().Scan().Return([]string{}, nil),
dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any()).Return([]string{}, nil),
Expand Down Expand Up @@ -510,11 +508,11 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
// Cancel the context to cancel async routines
agent := &ecsAgent{
ctx: ctx,
cfg: &cfg,
credentialProvider: credentials.NewCredentials(mockCredentialsProvider),
pauseLoader: mockPauseLoader,
dockerClient: dockerClient,
ctx: ctx,
cfg: &cfg,
credentialsCache: awsv2.NewCredentialsCache(mockCredentialsProvider),
pauseLoader: mockPauseLoader,
dockerClient: dockerClient,
terminationHandler: func(state dockerstate.TaskEngineState, dataClient data.Client, taskEngine engine.TaskEngine, cancel context.CancelFunc) {
},
mobyPlugins: mockMobyPlugins,
Expand Down Expand Up @@ -547,7 +545,7 @@ func TestDoStartCgroupInitErrorPath(t *testing.T) {
dockerClient, _, _, execCmdMgr, _ := setup(t)
defer ctrl.Finish()

mockCredentialsProvider := app_mocks.NewMockProvider(ctrl)
mockCredentialsProvider := app_mocks.NewMockCredentialsProvider(ctrl)
mockControl := mock_control.NewMockControl(ctrl)
mockPauseLoader := mock_loader.NewMockLoader(ctrl)
var discoverEndpointsInvoked sync.WaitGroup
Expand All @@ -556,7 +554,6 @@ func TestDoStartCgroupInitErrorPath(t *testing.T) {
dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes()
dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes()
imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1)
mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
mockPauseLoader.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil).AnyTimes()
mockServiceConnectManager := mock_serviceconnect.NewMockManager(ctrl)
Expand All @@ -580,11 +577,11 @@ func TestDoStartCgroupInitErrorPath(t *testing.T) {
// Cancel the context to cancel async routines
defer cancel()
agent := &ecsAgent{
ctx: ctx,
cfg: &cfg,
credentialProvider: credentials.NewCredentials(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
ctx: ctx,
cfg: &cfg,
credentialsCache: awsv2.NewCredentialsCache(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
terminationHandler: func(state dockerstate.TaskEngineState, dataClient data.Client, taskEngine engine.TaskEngine, cancel context.CancelFunc) {
},
resourceFields: &taskresource.ResourceFields{
Expand All @@ -603,7 +600,7 @@ func TestDoStartGPUManagerHappyPath(t *testing.T) {
ctrl, credentialsManager, state, imageManager, client,
dockerClient, _, _, execCmdMgr, _ := setup(t)
defer ctrl.Finish()
mockCredentialsProvider := app_mocks.NewMockProvider(ctrl)
mockCredentialsProvider := app_mocks.NewMockCredentialsProvider(ctrl)
mockGPUManager := mock_gpu.NewMockGPUManager(ctrl)
mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl)
ec2MetadataClient := mock_ec2.NewMockEC2MetadataClient(ctrl)
Expand All @@ -630,7 +627,6 @@ func TestDoStartGPUManagerHappyPath(t *testing.T) {
dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes()
dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes()
imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1)
mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
ec2MetadataClient.EXPECT().PrimaryENIMAC().Return("mac", nil)
ec2MetadataClient.EXPECT().VPCID(gomock.Eq("mac")).Return("vpc-id", nil)
ec2MetadataClient.EXPECT().SubnetID(gomock.Eq("mac")).Return("subnet-id", nil)
Expand All @@ -657,7 +653,7 @@ func TestDoStartGPUManagerHappyPath(t *testing.T) {

gomock.InOrder(
mockGPUManager.EXPECT().Initialize().Return(nil),
mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil),
mockCredentialsProvider.EXPECT().Retrieve(gomock.Any()).Return(awsv2.Credentials{}, nil),
mockMobyPlugins.EXPECT().Scan().Return([]string{}, nil),
dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any()).Return([]string{}, nil),
Expand Down Expand Up @@ -691,11 +687,11 @@ func TestDoStartGPUManagerHappyPath(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
// Cancel the context to cancel async routines
agent := &ecsAgent{
ctx: ctx,
cfg: &cfg,
credentialProvider: credentials.NewCredentials(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
ctx: ctx,
cfg: &cfg,
credentialsCache: awsv2.NewCredentialsCache(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
terminationHandler: func(state dockerstate.TaskEngineState, dataClient data.Client, taskEngine engine.TaskEngine, cancel context.CancelFunc) {
},
mobyPlugins: mockMobyPlugins,
Expand Down Expand Up @@ -728,7 +724,7 @@ func TestDoStartGPUManagerInitError(t *testing.T) {
dockerClient, _, _, execCmdMgr, _ := setup(t)
defer ctrl.Finish()

mockCredentialsProvider := app_mocks.NewMockProvider(ctrl)
mockCredentialsProvider := app_mocks.NewMockCredentialsProvider(ctrl)
mockGPUManager := mock_gpu.NewMockGPUManager(ctrl)
mockPauseLoader := mock_loader.NewMockLoader(ctrl)
var discoverEndpointsInvoked sync.WaitGroup
Expand All @@ -737,7 +733,6 @@ func TestDoStartGPUManagerInitError(t *testing.T) {
dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes()
dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes()
imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1)
mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
mockGPUManager.EXPECT().Initialize().Return(errors.New("init error"))
mockPauseLoader.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil).AnyTimes()
Expand All @@ -754,11 +749,11 @@ func TestDoStartGPUManagerInitError(t *testing.T) {
// Cancel the context to cancel async routines
defer cancel()
agent := &ecsAgent{
ctx: ctx,
cfg: &cfg,
credentialProvider: credentials.NewCredentials(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
ctx: ctx,
cfg: &cfg,
credentialsCache: awsv2.NewCredentialsCache(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
terminationHandler: func(state dockerstate.TaskEngineState, dataClient data.Client, taskEngine engine.TaskEngine, cancel context.CancelFunc) {
},
resourceFields: &taskresource.ResourceFields{
Expand All @@ -779,7 +774,7 @@ func TestDoStartTaskENIPauseError(t *testing.T) {
defer ctrl.Finish()

cniClient := mock_ecscni.NewMockCNIClient(ctrl)
mockCredentialsProvider := app_mocks.NewMockProvider(ctrl)
mockCredentialsProvider := app_mocks.NewMockCredentialsProvider(ctrl)
mockPauseLoader := mock_loader.NewMockLoader(ctrl)
mockMetadata := mock_ec2.NewMockEC2MetadataClient(ctrl)
mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl)
Expand All @@ -789,7 +784,6 @@ func TestDoStartTaskENIPauseError(t *testing.T) {

// These calls are expected to happen, but cannot be ordered as they are
// invoked via go routines, which will lead to occasional test failures
mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes()
dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes()
dockerClient.EXPECT().ListContainers(gomock.Any(), gomock.Any(), gomock.Any()).Return(
Expand All @@ -803,13 +797,13 @@ func TestDoStartTaskENIPauseError(t *testing.T) {
cfg.ENITrunkingEnabled = config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled}
ctx, _ := context.WithCancel(context.TODO())
agent := &ecsAgent{
ctx: ctx,
cfg: &cfg,
credentialProvider: credentials.NewCredentials(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
cniClient: cniClient,
ec2MetadataClient: mockMetadata,
ctx: ctx,
cfg: &cfg,
credentialsCache: awsv2.NewCredentialsCache(mockCredentialsProvider),
dockerClient: dockerClient,
pauseLoader: mockPauseLoader,
cniClient: cniClient,
ec2MetadataClient: mockMetadata,
terminationHandler: func(state dockerstate.TaskEngineState, dataClient data.Client, taskEngine engine.TaskEngine, cancel context.CancelFunc) {
},
mobyPlugins: mockMobyPlugins,
Expand Down
1 change: 1 addition & 0 deletions agent/app/generate_mocks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@
package app

//go:generate mockgen -destination=mocks/credentials_mocks.go -copyright_file=../../scripts/copyright_file github.com/aws/aws-sdk-go/aws/credentials Provider
//go:generate mockgen -destination=mocks/credentials_provider_mocks.go -package mock_credentials -copyright_file=../../scripts/copyright_file github.com/aws/aws-sdk-go-v2/aws CredentialsProvider
65 changes: 65 additions & 0 deletions agent/app/mocks/credentials_provider_mocks.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 7975bb3

Please sign in to comment.