Skip to content

Better Handling for Expired Credentials #13460

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.

Sign up for GitHub

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

Merged
edolstra merged 3 commits into from
Jul 14, 2025
Merged

Better Handling for Expired Credentials #13460

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a48632f
Better Handling for Expired Credentials
jgbailey-well Jul 12, 2025
22d6969
Update src/libstore/s3-binary-cache-store.cc
m4dc4p Jul 12, 2025
f786c0b
Update src/libstore/s3-binary-cache-store.cc
m4dc4p Jul 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/libstore/s3-binary-cache-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ namespace nix {
struct S3Error : public Error
{
Aws::S3::S3Errors err;
Aws::String exceptionName;

template<typename... Args>
S3Error(Aws::S3::S3Errors err, const Args & ... args)
: Error(args...), err(err) { };
S3Error(Aws::S3::S3Errors err, Aws::String exceptionName, const Args & ... args)
: Error(args...), err(err), exceptionName(exceptionName) { };
};

/* Helper: given an Outcome<R, E>, return R in case of success, or
Expand All @@ -51,6 +52,7 @@ R && checkAws(std::string_view s, Aws::Utils::Outcome<R, E> && outcome)
if (!outcome.IsSuccess())
throw S3Error(
outcome.GetError().GetErrorType(),
outcome.GetError().GetExceptionName(),
fmt(
"%s: %s (request id: %s)",
s,
Expand Down Expand Up @@ -226,7 +228,13 @@ S3Helper::FileTransferResult S3Helper::getObject(

} catch (S3Error & e) {
if ((e.err != Aws::S3::S3Errors::NO_SUCH_KEY) &&
(e.err != Aws::S3::S3Errors::ACCESS_DENIED)) throw;
(e.err != Aws::S3::S3Errors::ACCESS_DENIED) &&
// Expired tokens are not really an error, more of a caching problem. Should be treated same as 403.
//
// AWS unwilling to provide a specific error type for the situation (https://github.com/aws/aws-sdk-cpp/issues/1843)
// so use this hack
(e.exceptionName != "ExpiredToken")
) throw;
}

auto now2 = std::chrono::steady_clock::now();
Expand Down Expand Up @@ -325,15 +333,22 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore
{
stats.head++;

// error: AWS error fetching 'vjgpmfn7s6vkynymnk8jfx2fcxnsbd6b.narinfo': Unable to parse ExceptionName: ExpiredToken Message: The provided token has expired.
Comment thread
m4dc4p marked this conversation as resolved.
Outdated
auto res = s3Helper.client->HeadObject(
Aws::S3::Model::HeadObjectRequest()
.WithBucket(config->bucketName)
.WithKey(path));

printError("Checking for file");

Comment thread
m4dc4p marked this conversation as resolved.
Outdated
if (!res.IsSuccess()) {
auto & error = res.GetError();
if (error.GetErrorType() == Aws::S3::S3Errors::RESOURCE_NOT_FOUND
|| error.GetErrorType() == Aws::S3::S3Errors::NO_SUCH_KEY
// Expired tokens are not really an error, more of a caching problem. Should be treated same as 403.
// AWS unwilling to provide a specific error type for the situation (https://github.com/aws/aws-sdk-cpp/issues/1843)
// so use this hack
|| (error.GetErrorType() == Aws::S3::S3Errors::UNKNOWN && error.GetExceptionName() == "ExpiredToken")
// If bucket listing is disabled, 404s turn into 403s
|| error.GetErrorType() == Aws::S3::S3Errors::ACCESS_DENIED)
return false;
Expand Down