Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional logging for default credentials chain #231

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

jamesbornholt
Copy link
Member

We've been finding it difficult to follow the log output of credentials providers when investigating permissions issues. The problem is that the default chain is constructed somewhat dynamically, with some providers only added conditionally, and some dispatching differently depending on environment variables. This patch just adds some more logging in the default chain and in the providers it uses, so we can more easily trace the path of credentials resolution.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

We've been finding it difficult to parse the log output of credentials
providers when investigating permissions issues. The problem is that the
default chain is constructed somewhat dynamically, with some providers
only added conditionally, and some dispatching differently depending on
environment variables. This patch just adds some more logging in the
default chain and in the providers it uses, so we can more easily trace
the path of credentials resolution.

Signed-off-by: James Bornholt <bornholt@amazon.com>
@@ -201,9 +204,17 @@ static int s_credentials_provider_imds_get_credentials_async(
goto error;
}

AWS_LOGF_INFO(
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to dig farther, but I don't think this is an accurate statement. Credentials have not been successfully retrieved, they've only successfully started the retrieval process.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, you're right. Fixed (and added a couple more about each step of the process).

Signed-off-by: James Bornholt <bornholt@amazon.com>
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 70.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 79.55%. Comparing base (0d2aa00) to head (6648ab3).

Files Patch % Lines
source/credentials_provider_default_chain.c 20.00% 4 Missing ⚠️
source/credentials_provider_environment.c 66.66% 1 Missing ⚠️
source/credentials_provider_imds.c 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #231      +/-   ##
==========================================
- Coverage   79.57%   79.55%   -0.02%     
==========================================
  Files          33       33              
  Lines        5841     5860      +19     
==========================================
+ Hits         4648     4662      +14     
- Misses       1193     1198       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

"id=%p: IMDS provider successfully retrieved credentials",
(void *)wrapped_user_data->imds_provider);
} else {
AWS_LOGF_ERROR(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer this to be info level. The imds provider is added in non-ecs environments unless explicitly disabled, and making this error level means error-tagged log lines will show up any time an application is run outside of ec2. Sometimes that's unavoidable, but in general we want to avoid flagging something at error level when it's an optional thing that will fail under ordinary circumstances.

Other than that, looks great.

Copy link
Member Author

Choose a reason for hiding this comment

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

In practice, if we ever get to the point of running the IMDS provider in one of these environments, we're going to spew a bunch of error-level logs anyway. Actually, that's why I didn't notice this — we have the CRT's error-level logs turned off by default in Mountpoint clients because they are so spammy.

Here's what it looks like trying to hit IMDS on my laptop:

2024-03-22T20:17:11.926165Z ERROR awscrt::AuthCredentialsProvider: Failed to resolve role arn during sts web identity provider initialization.
2024-03-22T20:17:13.130559Z ERROR awscrt::AuthCredentialsProvider: (id=0x6000039dc1c0) Failed to source credentials from running process credentials provider with command: ada  credentials print --profile=dev 2>&1, err:The specified header was not found
2024-03-22T20:17:13.137216Z ERROR awscrt::channel-bootstrap: id=0x6000007d8230: Last attempt failed to create socket with error 1049
2024-03-22T20:17:13.137263Z ERROR awscrt::http-connection: static: Client connection failed with error 1049 (AWS_IO_SOCKET_NO_ROUTE_TO_HOST).
2024-03-22T20:17:13.137292Z  WARN awscrt::connection-manager: id=0x13b7063f0: Failed to obtain new connection from http layer, error 1049(socket connect failure, no route to host.)
2024-03-22T20:17:13.137314Z  WARN awscrt::connection-manager: id=0x13b7063f0: Failed to complete connection acquisition with error_code 1049(socket connect failure, no route to host.)
2024-03-22T20:17:13.137331Z  WARN awscrt::Unknown: id=0x13b7062d0: IMDS Client failed to acquire a connection, error code 1049(socket connect failure, no route to host.)
2024-03-22T20:17:13.137363Z ERROR awscrt::Unknown: (id=0x13b7062d0) IMDS client failed to update the token from IMDS.
2024-03-22T20:17:13.248366Z ERROR awscrt::channel-bootstrap: id=0x6000007d8230: Last attempt failed to create socket with error 1052
2024-03-22T20:17:13.248444Z ERROR awscrt::http-connection: static: Client connection failed with error 1052 (AWS_IO_SOCKET_NOT_CONNECTED).
2024-03-22T20:17:13.248465Z  WARN awscrt::connection-manager: id=0x13b7063f0: Failed to obtain new connection from http layer, error 1052(socket not connected.)
2024-03-22T20:17:13.248488Z  WARN awscrt::connection-manager: id=0x13b7063f0: Failed to complete connection acquisition with error_code 1052(socket not connected.)
2024-03-22T20:17:13.248506Z  WARN awscrt::Unknown: id=0x13b7062d0: IMDS Client failed to acquire a connection, error code 1052(socket not connected.)
2024-03-22T20:17:13.248524Z ERROR awscrt::AuthCredentialsProvider: id=0x6000039dc500: IMDS provider failed to retrieve role: socket not connected.
2024-03-22T20:17:13.250612Z ERROR awscrt::AuthCredentialsProvider: (id=0x6000039dc180) Default chain credentials provider failed to source credentials with error 6153(aws-c-auth: AWS_AUTH_CREDENTIALS_PROVIDER_IMDS_SOURCE_FAILURE, Valid credentials could not be sourced by the IMDS provider)
2024-03-22T20:17:13.250865Z ERROR awscrt::AuthSigning: (id=0x600000fd0320) Credentials Provider failed to source credentials with error 6153(aws-c-auth: AWS_AUTH_CREDENTIALS_PROVIDER_IMDS_SOURCE_FAILURE, Valid credentials could not be sourced by the IMDS provider)
2024-03-22T20:17:13.250891Z ERROR awscrt::S3MetaRequest: id=0x13b709080 Meta request could not sign HTTP request due to error code 6146 (Attempt to sign an http request without credentials)
2024-03-22T20:17:13.250910Z ERROR awscrt::S3MetaRequest: id=0x13b709080 Could not prepare request 0x15b605670 due to error 6146 (Attempt to sign an http request without credentials).
2024-03-22T20:17:13.253796Z  WARN list_objects{id=0 bucket="bornholt-test-bucket" continued=false delimiter="" max_keys="500" prefix=""}: mountpoint_s3_client::s3_crt_client: duration=1.32434725s request_id=<unknown> error=ClientError(NoSigningCredentials) meta request failed

That said, I'll change it in the interest of at least not making the spam problem worse :-)

Signed-off-by: James Bornholt <bornholt@amazon.com>
@jamesbornholt jamesbornholt merged commit 075f00c into main Mar 22, 2024
29 checks passed
@jamesbornholt jamesbornholt deleted the bornholt/credentials-chain-logs branch March 22, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants