From 02962a0637c9e40eac95c75c613caa68bb872405 Mon Sep 17 00:00:00 2001 From: Petro Sidlovskyy Date: Wed, 24 Apr 2024 14:28:44 +0300 Subject: [PATCH 1/2] Change credentials priority for linux: rotating credentials should be prioritized over EC2 instance role ones --- .../instancecreds/instancecreds_linux.go | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/ecs-agent/credentials/instancecreds/instancecreds_linux.go b/ecs-agent/credentials/instancecreds/instancecreds_linux.go index 88ac75ee454..8118971fc61 100644 --- a/ecs-agent/credentials/instancecreds/instancecreds_linux.go +++ b/ecs-agent/credentials/instancecreds/instancecreds_linux.go @@ -28,20 +28,30 @@ import ( // GetCredentials returns the instance credentials chain. This is the default chain // credentials plus the "rotating shared credentials provider", so credentials will // be checked in this order: +// // 1. Env vars (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY). +// // 2. Shared credentials file (https://docs.aws.amazon.com/ses/latest/DeveloperGuide/create-shared-credentials-file.html) (file at ~/.aws/credentials containing access key id and secret access key). +// // 3. EC2 role credentials. This is an IAM role that the user specifies when they launch their EC2 container instance (ie ecsInstanceRole (https://docs.aws.amazon.com/AmazonECS/latest/developerguide/instance_IAM_role.html)). +// // 4. Rotating shared credentials file located at /rotatingcreds/credentials +// +// In the case of external instance `RotatingSharedCredentialsProvider` provider should be prioritized over +// `EC2RoleProvider` one as SSM credentials are going to be ignored in case agent is deployed to EC2 +// with instance role which doesn't have permissions needed to run ECS Agent func GetCredentials(isExternal bool) *credentials.Credentials { mu.Lock() - if credentialChain == nil { - credProviders := defaults.CredProviders(defaults.Config(), defaults.Handlers()) + credProviders := defaults.CredProviders(defaults.Config(), defaults.Handlers()) + if isExternal { + credProviders = append(credProviders[:2], append([]credentials.Provider{providers.NewRotatingSharedCredentialsProvider()}, credProviders[2:]...)...) + } else { credProviders = append(credProviders, providers.NewRotatingSharedCredentialsProvider()) - credentialChain = credentials.NewCredentials(&credentials.ChainProvider{ - VerboseErrors: false, - Providers: credProviders, - }) } + credentialChain = credentials.NewCredentials(&credentials.ChainProvider{ + VerboseErrors: false, + Providers: credProviders, + }) mu.Unlock() // credentials.Credentials is concurrency-safe, so lock not needed here From 884a8c256c9887ddfd39b591a35eb8d7b3f3fc17 Mon Sep 17 00:00:00 2001 From: Petro Sidlovskyy Date: Wed, 24 Apr 2024 14:34:18 +0300 Subject: [PATCH 2/2] Add checking if credentialChain is nil --- .../instancecreds/instancecreds_linux.go | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/ecs-agent/credentials/instancecreds/instancecreds_linux.go b/ecs-agent/credentials/instancecreds/instancecreds_linux.go index 8118971fc61..98b01b6232b 100644 --- a/ecs-agent/credentials/instancecreds/instancecreds_linux.go +++ b/ecs-agent/credentials/instancecreds/instancecreds_linux.go @@ -42,16 +42,18 @@ import ( // with instance role which doesn't have permissions needed to run ECS Agent func GetCredentials(isExternal bool) *credentials.Credentials { mu.Lock() - credProviders := defaults.CredProviders(defaults.Config(), defaults.Handlers()) - if isExternal { - credProviders = append(credProviders[:2], append([]credentials.Provider{providers.NewRotatingSharedCredentialsProvider()}, credProviders[2:]...)...) - } else { - credProviders = append(credProviders, providers.NewRotatingSharedCredentialsProvider()) + if credentialChain == nil { + credProviders := defaults.CredProviders(defaults.Config(), defaults.Handlers()) + if isExternal { + credProviders = append(credProviders[:2], append([]credentials.Provider{providers.NewRotatingSharedCredentialsProvider()}, credProviders[2:]...)...) + } else { + credProviders = append(credProviders, providers.NewRotatingSharedCredentialsProvider()) + } + credentialChain = credentials.NewCredentials(&credentials.ChainProvider{ + VerboseErrors: false, + Providers: credProviders, + }) } - credentialChain = credentials.NewCredentials(&credentials.ChainProvider{ - VerboseErrors: false, - Providers: credProviders, - }) mu.Unlock() // credentials.Credentials is concurrency-safe, so lock not needed here