diff --git a/.github/actions/install-nix-action/action.yaml b/.github/actions/install-nix-action/action.yaml index 00d02d6a2f2..535ae9d08fd 100644 --- a/.github/actions/install-nix-action/action.yaml +++ b/.github/actions/install-nix-action/action.yaml @@ -24,8 +24,8 @@ inputs: description: "Github token" required: true use_cache: - description: "Whether to setup magic-nix-cache" - default: true + description: "Whether to setup github actions cache (not implemented currently)" + default: false required: false runs: using: "composite" @@ -122,10 +122,3 @@ runs: source-url: ${{ inputs.experimental-installer-version != 'latest' && 'https://artifacts.nixos.org/experimental-installer/tag/${{ inputs.experimental-installer-version }}/${{ env.EXPERIMENTAL_INSTALLER_ARTIFACT }}' || '' }} nix-package-url: ${{ inputs.dogfood == 'true' && steps.download-nix-installer.outputs.tarball-path || (inputs.tarball_url || '') }} extra-conf: ${{ inputs.extra_nix_config }} - - uses: DeterminateSystems/magic-nix-cache-action@565684385bcd71bad329742eefe8d12f2e765b39 # v13 - if: ${{ inputs.use_cache == 'true' }} - with: - diagnostic-endpoint: '' - use-flakehub: false - use-gha-cache: true - source-revision: 92d9581367be2233c2d5714a2640e1339f4087d8 # main diff --git a/.version b/.version index ba13d3caf21..0b82b8d70e2 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -2.33.1 +2.33.2 diff --git a/src/libstore/aws-creds.cc b/src/libstore/aws-creds.cc index dfdd81abbc4..9b0ddefdca8 100644 --- a/src/libstore/aws-creds.cc +++ b/src/libstore/aws-creds.cc @@ -4,15 +4,20 @@ # include # include "nix/store/s3-url.hh" -# include "nix/util/finally.hh" # include "nix/util/logging.hh" -# include "nix/util/url.hh" -# include "nix/util/util.hh" # include # include # include +// C library headers for SSO provider support +# include + +// C library headers for custom logging +# include + +# include + # include # include @@ -30,6 +35,141 @@ AwsAuthError::AwsAuthError(int errorCode) namespace { +/** + * Map AWS log level to Nix verbosity. + * AWS levels: AWS_LL_NONE=0, AWS_LL_FATAL=1, AWS_LL_ERROR=2, AWS_LL_WARN=3, + * AWS_LL_INFO=4, AWS_LL_DEBUG=5, AWS_LL_TRACE=6 + * + * We map very conservatively because the AWS SDK is extremely noisy. What AWS + * considers "info" includes low-level details like "Initializing epoll" and + * "Starting event-loop thread". What it considers "errors" includes expected + * conditions like missing ~/.aws/config or IMDS being unavailable on non-EC2. + * + * To avoid spamming users, we only show FATAL at default verbosity. Everything + * else requires -vvvvv (lvlDebug) or higher to see. + */ +static Verbosity awsLogLevelToVerbosity(enum aws_log_level level) +{ + switch (level) { + case AWS_LL_FATAL: + return lvlError; + case AWS_LL_ERROR: + case AWS_LL_WARN: + case AWS_LL_INFO: + return lvlDebug; + case AWS_LL_DEBUG: + case AWS_LL_TRACE: + return lvlVomit; + // AWS_LL_NONE and AWS_LL_COUNT are enum sentinels, not real log levels + case AWS_LL_NONE: + case AWS_LL_COUNT: + return lvlDebug; + } + unreachable(); +} + +/** + * Custom AWS logger that routes logs through Nix's logging infrastructure. + * + * The AWS CRT C++ wrapper (ApiHandle::InitializeLogging) only supports FILE* + * or filename-based logging. The underlying C library supports custom loggers + * via aws_logger struct with a vtable containing callback functions. + */ +static int nixAwsLoggerLog( + struct aws_logger * logger, enum aws_log_level logLevel, aws_log_subject_t subject, const char * format, ...) +{ + Verbosity nixLevel = awsLogLevelToVerbosity(logLevel); + if (nixLevel > verbosity) + return AWS_OP_SUCCESS; /* Bail out early to avoid formatting the message unnecessarily. */ + + va_list args; + va_start(args, format); + std::array buffer{}; + auto res = vsnprintf(buffer.data(), buffer.size(), format, args); + va_end(args); + if (res < 0) /* Skip garbage debug messages in case the SDK is busted. */ + return AWS_OP_SUCCESS; + + const char * subjectName = aws_log_subject_name(subject); + printMsgUsing(nix::logger, nixLevel, "(aws:%s) %s", subjectName ? subjectName : "unknown", chomp(buffer.data())); + return AWS_OP_SUCCESS; +} + +/** + * Get current log level for a subject - determines which messages will be logged. + * Must be consistent with awsLogLevelToVerbosity mapping. + */ +static aws_log_level nixAwsLoggerGetLevel(struct aws_logger * logger, aws_log_subject_t subject) +{ + // Map Nix verbosity back to AWS log level (inverse of awsLogLevelToVerbosity) + if (verbosity >= lvlVomit) + return AWS_LL_TRACE; + if (verbosity >= lvlDebug) + return AWS_LL_INFO; // error/warn/info are all mapped to lvlDebug + return AWS_LL_FATAL; +} + +static void initialiseAwsLogger() +{ + static std::once_flag initialised; /* aws_logger_set must only be called once */ + std::call_once(initialised, []() { + static aws_logger_vtable nixAwsLoggerVtable = { + .log = nixAwsLoggerLog, + .get_log_level = nixAwsLoggerGetLevel, + .clean_up = [](struct aws_logger *) {}, // No resources to clean up + .set_log_level = nullptr, + }; + + static aws_logger nixAwsLogger = { + .vtable = &nixAwsLoggerVtable, + .allocator = nullptr, + .p_impl = nullptr, + }; + + aws_logger_set(&nixAwsLogger); + }); +} + +/** + * Helper function to wrap a C credentials provider in the C++ interface. + * This replicates the static s_CreateWrappedProvider from aws-crt-cpp. + */ +static std::shared_ptr createWrappedProvider( + aws_credentials_provider * rawProvider, Aws::Crt::Allocator * allocator = Aws::Crt::ApiAllocator()) +{ + if (rawProvider == nullptr) { + return nullptr; + } + + auto provider = Aws::Crt::MakeShared(allocator, rawProvider, allocator); + return std::static_pointer_cast(provider); +} + +/** + * Create an SSO credentials provider using the C library directly. + * The C++ wrapper doesn't expose SSO, so we call the C library and wrap the result. + * Returns nullptr if SSO provider creation fails (e.g., profile doesn't have SSO config). + */ +static std::shared_ptr createSSOProvider( + const std::string & profileName, + Aws::Crt::Io::ClientBootstrap * bootstrap, + Aws::Crt::Io::TlsContext * tlsContext, + Aws::Crt::Allocator * allocator = Aws::Crt::ApiAllocator()) +{ + aws_credentials_provider_sso_options options; + AWS_ZERO_STRUCT(options); + + options.bootstrap = bootstrap->GetUnderlyingHandle(); + options.tls_ctx = tlsContext ? tlsContext->GetUnderlyingHandle() : nullptr; + if (!profileName.empty()) { + options.profile_name_override = aws_byte_cursor_from_c_str(profileName.c_str()); + } + + // Create the SSO provider - will return nullptr if SSO isn't configured for this profile + // createWrappedProvider handles nullptr gracefully + return createWrappedProvider(aws_credentials_provider_new_sso(allocator, &options), allocator); +} + static AwsCredentials getCredentialsFromProvider(std::shared_ptr provider) { if (!provider || !provider->IsValid()) { @@ -79,18 +219,25 @@ class AwsCredentialProviderImpl : public AwsCredentialProvider public: AwsCredentialProviderImpl() { - // Map Nix's verbosity to AWS CRT log level - Aws::Crt::LogLevel logLevel; - if (verbosity >= lvlVomit) { - logLevel = Aws::Crt::LogLevel::Trace; - } else if (verbosity >= lvlDebug) { - logLevel = Aws::Crt::LogLevel::Debug; - } else if (verbosity >= lvlChatty) { - logLevel = Aws::Crt::LogLevel::Info; - } else { - logLevel = Aws::Crt::LogLevel::Warn; + // Install custom logger that routes AWS CRT logs through Nix's logging infrastructure. + // This ensures AWS logs respect Nix's verbosity settings and are formatted consistently. + initialiseAwsLogger(); + + // Create a shared TLS context for SSO (required for HTTPS connections) + auto allocator = Aws::Crt::ApiAllocator(); + auto tlsCtxOptions = Aws::Crt::Io::TlsContextOptions::InitDefaultClient(allocator); + tlsContext = + std::make_shared(tlsCtxOptions, Aws::Crt::Io::TlsMode::CLIENT, allocator); + if (!tlsContext || !*tlsContext) { + warn("failed to create TLS context for AWS SSO; SSO authentication will be unavailable"); + tlsContext = nullptr; + } + + // Get bootstrap (lives as long as apiHandle) + bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap(); + if (!bootstrap) { + throw AwsAuthError("failed to create AWS client bootstrap"); } - apiHandle.InitializeLogging(logLevel, stderr); } AwsCredentials getCredentialsRaw(const std::string & profile); @@ -111,6 +258,8 @@ class AwsCredentialProviderImpl : public AwsCredentialProvider private: Aws::Crt::ApiHandle apiHandle; + std::shared_ptr tlsContext; + Aws::Crt::Io::ClientBootstrap * bootstrap; boost::concurrent_flat_map> credentialProviderCache; }; @@ -118,23 +267,58 @@ class AwsCredentialProviderImpl : public AwsCredentialProvider std::shared_ptr AwsCredentialProviderImpl::createProviderForProfile(const std::string & profile) { - debug( - "[pid=%d] creating new AWS credential provider for profile '%s'", - getpid(), - profile.empty() ? "(default)" : profile.c_str()); - - if (profile.empty()) { - Aws::Crt::Auth::CredentialsProviderChainDefaultConfig config; - config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap(); - return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderChainDefault(config); + // profileDisplayName is only used for debug logging - SDK uses its default profile + // when ProfileNameOverride is not set + const char * profileDisplayName = profile.empty() ? "(default)" : profile.c_str(); + + debug("[pid=%d] creating new AWS credential provider for profile '%s'", getpid(), profileDisplayName); + + // Build a custom credential chain: Environment → SSO → Profile → IMDS + // This works for both default and named profiles, ensuring consistent behavior + // including SSO support and proper TLS context for STS-based role assumption. + Aws::Crt::Auth::CredentialsProviderChainConfig chainConfig; + auto allocator = Aws::Crt::ApiAllocator(); + + auto addProviderToChain = [&](std::string_view name, auto createProvider) { + if (auto provider = createProvider()) { + chainConfig.Providers.push_back(provider); + debug("Added AWS %s Credential Provider to chain for profile '%s'", name, profileDisplayName); + } else { + debug("Skipped AWS %s Credential Provider for profile '%s'", name, profileDisplayName); + } + }; + + // 1. Environment variables (highest priority) + addProviderToChain("Environment", [&]() { + return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderEnvironment(allocator); + }); + + // 2. SSO provider (try it, will fail gracefully if not configured) + if (tlsContext) { + addProviderToChain("SSO", [&]() { return createSSOProvider(profile, bootstrap, tlsContext.get(), allocator); }); + } else { + debug("Skipped AWS SSO Credential Provider for profile '%s': TLS context unavailable", profileDisplayName); } - Aws::Crt::Auth::CredentialsProviderProfileConfig config; - config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap(); - // This is safe because the underlying C library will copy this string - // c.f. https://github.com/awslabs/aws-c-auth/blob/main/source/credentials_provider_profile.c#L220 - config.ProfileNameOverride = Aws::Crt::ByteCursorFromCString(profile.c_str()); - return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderProfile(config); + // 3. Profile provider (for static credentials and role_arn/source_profile with STS) + addProviderToChain("Profile", [&]() { + Aws::Crt::Auth::CredentialsProviderProfileConfig profileConfig; + profileConfig.Bootstrap = bootstrap; + profileConfig.TlsContext = tlsContext.get(); + if (!profile.empty()) { + profileConfig.ProfileNameOverride = Aws::Crt::ByteCursorFromCString(profile.c_str()); + } + return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderProfile(profileConfig, allocator); + }); + + // 4. IMDS provider (for EC2 instances, lowest priority) + addProviderToChain("IMDS", [&]() { + Aws::Crt::Auth::CredentialsProviderImdsConfig imdsConfig; + imdsConfig.Bootstrap = bootstrap; + return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderImds(imdsConfig, allocator); + }); + + return Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderChain(chainConfig, allocator); } AwsCredentials AwsCredentialProviderImpl::getCredentialsRaw(const std::string & profile) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 10ba0e78b77..b43563459bd 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -563,8 +563,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() { DerivationBuildingGoal & goal; - DerivationBuildingGoalCallbacks( - DerivationBuildingGoal & goal, std::unique_ptr & builder) + DerivationBuildingGoalCallbacks(DerivationBuildingGoal & goal) : goal{goal} { } @@ -633,15 +632,15 @@ Goal::Co DerivationBuildingGoal::tryToBuild() /* If we have to wait and retry (see below), then `builder` will already be created, so we don't need to create it again. */ - builder = externalBuilder ? makeExternalDerivationBuilder( - *localStoreP, - std::make_unique(*this, builder), - std::move(params), - *externalBuilder) - : makeDerivationBuilder( - *localStoreP, - std::make_unique(*this, builder), - std::move(params)); + builder = + externalBuilder + ? makeExternalDerivationBuilder( + *localStoreP, + std::make_unique(*this), + std::move(params), + *externalBuilder) + : makeDerivationBuilder( + *localStoreP, std::make_unique(*this), std::move(params)); } if (auto builderOutOpt = builder->startBuild()) { @@ -1177,18 +1176,6 @@ DerivationBuildingGoal::checkPathValidity(std::map & Goal::Done DerivationBuildingGoal::doneSuccess(BuildResult::Success::Status status, SingleDrvOutputs builtOutputs) { - buildResult.inner = BuildResult::Success{ - .status = status, - .builtOutputs = std::move(builtOutputs), - }; - - logger->result( - act ? act->id : getCurActivity(), - resBuildResult, - nlohmann::json(KeyedBuildResult( - buildResult, - DerivedPath::Built{.drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All{}}))); - mcRunningBuilds.reset(); if (status == BuildResult::Success::Built) @@ -1196,15 +1183,11 @@ Goal::Done DerivationBuildingGoal::doneSuccess(BuildResult::Success::Status stat worker.updateProgress(); - return amDone(ecSuccess, std::nullopt); -} - -Goal::Done DerivationBuildingGoal::doneFailure(BuildError ex) -{ - buildResult.inner = BuildResult::Failure{ - .status = ex.status, - .errorMsg = fmt("%s", Uncolored(ex.info().msg)), - }; + auto res = Goal::doneSuccess( + BuildResult::Success{ + .status = status, + .builtOutputs = std::move(builtOutputs), + }); logger->result( act ? act->id : getCurActivity(), @@ -1213,6 +1196,11 @@ Goal::Done DerivationBuildingGoal::doneFailure(BuildError ex) buildResult, DerivedPath::Built{.drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All{}}))); + return res; +} + +Goal::Done DerivationBuildingGoal::doneFailure(BuildError ex) +{ mcRunningBuilds.reset(); if (ex.status == BuildResult::Failure::TimedOut) @@ -1224,7 +1212,22 @@ Goal::Done DerivationBuildingGoal::doneFailure(BuildError ex) worker.updateProgress(); - return amDone(ecFailed, {std::move(ex)}); + auto res = Goal::doneFailure( + ecFailed, + BuildResult::Failure{ + .status = ex.status, + .errorMsg = fmt("%s", Uncolored(ex.info().msg)), + }, + std::move(ex)); + + logger->result( + act ? act->id : getCurActivity(), + resBuildResult, + nlohmann::json(KeyedBuildResult( + buildResult, + DerivedPath::Built{.drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All{}}))); + + return res; } } // namespace nix diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 1908f957313..97188d30c31 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -452,27 +452,6 @@ UnkeyedRealisation DerivationGoal::assertPathValidity() Goal::Done DerivationGoal::doneSuccess(BuildResult::Success::Status status, UnkeyedRealisation builtOutput) { - buildResult.inner = BuildResult::Success{ - .status = status, - .builtOutputs = {{ - wantedOutput, - { - std::move(builtOutput), - DrvOutput{ - .drvHash = outputHash, - .outputName = wantedOutput, - }, - }, - }}, - }; - - logger->result( - getCurActivity(), - resBuildResult, - nlohmann::json(KeyedBuildResult( - buildResult, - DerivedPath::Built{.drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All{}}))); - mcExpectedBuilds.reset(); if (status == BuildResult::Success::Built) @@ -480,15 +459,20 @@ Goal::Done DerivationGoal::doneSuccess(BuildResult::Success::Status status, Unke worker.updateProgress(); - return amDone(ecSuccess, std::nullopt); -} - -Goal::Done DerivationGoal::doneFailure(BuildError ex) -{ - buildResult.inner = BuildResult::Failure{ - .status = ex.status, - .errorMsg = fmt("%s", Uncolored(ex.info().msg)), - }; + auto res = Goal::doneSuccess( + BuildResult::Success{ + .status = status, + .builtOutputs = {{ + wantedOutput, + { + std::move(builtOutput), + DrvOutput{ + .drvHash = outputHash, + .outputName = wantedOutput, + }, + }, + }}, + }); logger->result( getCurActivity(), @@ -497,6 +481,11 @@ Goal::Done DerivationGoal::doneFailure(BuildError ex) buildResult, DerivedPath::Built{.drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All{}}))); + return res; +} + +Goal::Done DerivationGoal::doneFailure(BuildError ex) +{ mcExpectedBuilds.reset(); if (ex.status == BuildResult::Failure::TimedOut) @@ -508,7 +497,22 @@ Goal::Done DerivationGoal::doneFailure(BuildError ex) worker.updateProgress(); - return amDone(ecFailed, {std::move(ex)}); + auto res = Goal::doneFailure( + ecFailed, + BuildResult::Failure{ + .status = ex.status, + .errorMsg = fmt("%s", Uncolored(ex.info().msg)), + }, + std::move(ex)); + + logger->result( + getCurActivity(), + resBuildResult, + nlohmann::json(KeyedBuildResult( + buildResult, + DerivedPath::Built{.drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All{}}))); + + return res; } } // namespace nix diff --git a/src/libstore/build/derivation-trampoline-goal.cc b/src/libstore/build/derivation-trampoline-goal.cc index 963156aa584..cfa0c538f95 100644 --- a/src/libstore/build/derivation-trampoline-goal.cc +++ b/src/libstore/build/derivation-trampoline-goal.cc @@ -98,7 +98,13 @@ Goal::Co DerivationTrampolineGoal::init() trace("outer load and build derivation"); if (nrFailed != 0) { - co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); + co_return doneFailure( + ecFailed, + BuildResult::Failure{ + .status = BuildResult::Failure::DependencyFailed, + .errorMsg = fmt("failed to obtain derivation of '%s'", drvReq->to_string(worker.store)), + }, + Error("failed to obtain derivation of '%s'", drvReq->to_string(worker.store))); } StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 6266329e7e5..1c5c6bfe48c 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -133,6 +133,19 @@ Co Goal::await(Goals new_waitees) co_return Return{}; } +Goal::Done Goal::doneSuccess(BuildResult::Success success) +{ + buildResult.inner = std::move(success); + return amDone(ecSuccess); +} + +Goal::Done Goal::doneFailure(ExitCode result, BuildResult::Failure failure, std::optional ex) +{ + assert(result == ecFailed || result == ecNoSubstituters); + buildResult.inner = std::move(failure); + return amDone(result, std::move(ex)); +} + Goal::Done Goal::amDone(ExitCode result, std::optional ex) { trace("done"); diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index b2e321c7238..00411565e77 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -31,32 +31,36 @@ PathSubstitutionGoal::~PathSubstitutionGoal() Goal::Done PathSubstitutionGoal::doneSuccess(BuildResult::Success::Status status) { - buildResult.inner = BuildResult::Success{ - .status = status, - }; + auto res = Goal::doneSuccess( + BuildResult::Success{ + .status = status, + }); logger->result( getCurActivity(), resBuildResult, nlohmann::json(KeyedBuildResult(buildResult, DerivedPath::Opaque{storePath}))); - return amDone(ecSuccess); + return res; } Goal::Done PathSubstitutionGoal::doneFailure(ExitCode result, BuildResult::Failure::Status status, std::string errorMsg) { debug(errorMsg); - buildResult.inner = BuildResult::Failure{ - .status = status, - .errorMsg = std::move(errorMsg), - }; + + auto res = Goal::doneFailure( + result, + BuildResult::Failure{ + .status = status, + .errorMsg = std::move(errorMsg), + }); logger->result( getCurActivity(), resBuildResult, nlohmann::json(KeyedBuildResult(buildResult, DerivedPath::Opaque{storePath}))); - return amDone(result); + return res; } Goal::Co PathSubstitutionGoal::init() diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index e5cf3726676..35ca62df0e8 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -125,7 +125,16 @@ struct curlFileTransfer : public FileTransfer { result.urls.push_back(request.uri.to_string()); - requestHeaders = curl_slist_append(requestHeaders, "Accept-Encoding: zstd, br, gzip, deflate, bzip2, xz"); + /* Don't set Accept-Encoding for S3 requests that use AWS SigV4 signing. + curl's SigV4 implementation signs all headers including Accept-Encoding, + but some S3-compatible services (like GCS) modify this header in transit, + causing signature verification to fail. + See https://github.com/NixOS/nix/issues/15019 */ +#if NIX_WITH_AWS_AUTH + if (!request.awsSigV4Provider) +#endif + requestHeaders = + curl_slist_append(requestHeaders, "Accept-Encoding: zstd, br, gzip, deflate, bzip2, xz"); if (!request.expectedETag.empty()) requestHeaders = curl_slist_append(requestHeaders, ("If-None-Match: " + request.expectedETag).c_str()); if (!request.mimeType.empty()) @@ -464,6 +473,11 @@ struct curlFileTransfer : public FileTransfer curl_easy_setopt(req, CURLOPT_CUSTOMREQUEST, "DELETE"); if (request.data) { + // Restart the source to ensure it's at the beginning. + // This is necessary for retries, where the source was + // already consumed by a previous attempt. + request.data->source->restart(); + if (request.method == HttpMethod::Post) { curl_easy_setopt(req, CURLOPT_POST, 1L); curl_easy_setopt(req, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) request.data->sizeHint); diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index c51424d0ea6..1e84b7eee65 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -194,15 +194,22 @@ struct ExternalBuilder std::vector args; }; +struct DerivationBuilderDeleter +{ + void operator()(DerivationBuilder * builder) noexcept; +}; + +using DerivationBuilderUnique = std::unique_ptr; + #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows -std::unique_ptr makeDerivationBuilder( +DerivationBuilderUnique makeDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params); /** * @param handler Must be chosen such that it supports the given * derivation. */ -std::unique_ptr makeExternalDerivationBuilder( +DerivationBuilderUnique makeExternalDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params, diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index be95c796b05..e745c12c7ba 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -5,6 +5,7 @@ #include "nix/store/parsed-derivations.hh" #include "nix/store/derivation-options.hh" #include "nix/store/build/derivation-building-misc.hh" +#include "nix/store/build/derivation-builder.hh" #include "nix/store/outputs-spec.hh" #include "nix/store/store-api.hh" #include "nix/store/pathlocks.hh" @@ -89,7 +90,7 @@ private: */ std::unique_ptr hook; - std::unique_ptr builder; + DerivationBuilderUnique builder; #endif BuildMode buildMode; diff --git a/src/libstore/include/nix/store/build/goal.hh b/src/libstore/include/nix/store/build/goal.hh index f048c75687c..d26803532db 100644 --- a/src/libstore/include/nix/store/build/goal.hh +++ b/src/libstore/include/nix/store/build/goal.hh @@ -393,9 +393,28 @@ protected: * Signals that the goal is done. * `co_return` the result. If you're not inside a coroutine, you can ignore * the return value safely. + * + * Prefer using `doneSuccess` or `doneFailure` instead, which ensure + * `buildResult` is set correctly. */ Done amDone(ExitCode result, std::optional ex = {}); + /** + * Signals successful completion of the goal. + * Sets `buildResult` and calls `amDone`. + */ + Done doneSuccess(BuildResult::Success success); + + /** + * Signals failed completion of the goal. + * Sets `buildResult` and calls `amDone`. + * + * @param result The exit code (ecFailed or ecNoSubstituters) + * @param failure The failure details including status and error message + * @param ex Optional exception to store/log + */ + Done doneFailure(ExitCode result, BuildResult::Failure failure, std::optional ex = {}); + public: virtual void cleanup() {} diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 6556adc27d8..52d60ad2fda 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -167,6 +167,8 @@ if s3_aws_auth.enabled() deps_other += aws_crt_cpp aws_c_common = cxx.find_library('aws-c-common', required : true) deps_other += aws_c_common + aws_c_auth = cxx.find_library('aws-c-auth', required : true) + deps_other += aws_c_auth endif configdata_pub.set('NIX_WITH_AWS_AUTH', s3_aws_auth.enabled().to_int()) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index d097333d43f..d4c7debbf35 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -104,10 +104,13 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder { } - ~DerivationBuilderImpl() + /** + * Cleanup code to run when destroying any DerivationBuilderImpl implementation. + */ + void cleanupOnDestruction() noexcept { /* Careful: we should never ever throw an exception from a - destructor. */ + noexcept function. */ try { killChild(); } catch (...) { @@ -2000,7 +2003,20 @@ StorePath DerivationBuilderImpl::makeFallbackPath(const StorePath & path) namespace nix { -std::unique_ptr makeDerivationBuilder( +void DerivationBuilderDeleter::operator()(DerivationBuilder * builder) noexcept +{ + if (!builder) /* Idempotent and handles nullptr as any deleter must. */ + return; + + if (auto builderImpl = dynamic_cast(builder)) + /* Note that this might call into virtual functions, which we can't do in a destructor of + the DerivationBuilderImpl itself. */ + builderImpl->cleanupOnDestruction(); + + delete builder; +} + +std::unique_ptr makeDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params) { bool useSandbox = false; @@ -2029,7 +2045,7 @@ std::unique_ptr makeDerivationBuilder( } if (params.drv.platform == "wasm32-wasip1") - return std::make_unique(store, std::move(miscMethods), std::move(params)); + return DerivationBuilderUnique(new WasiDerivationBuilder(store, std::move(miscMethods), std::move(params))); if (store.storeDir != store.config->realStoreDir.get()) { #ifdef __linux__ @@ -2054,17 +2070,19 @@ std::unique_ptr makeDerivationBuilder( throw Error("feature 'uid-range' is only supported in sandboxed builds"); #ifdef __APPLE__ - return std::make_unique(store, std::move(miscMethods), std::move(params), useSandbox); + return DerivationBuilderUnique( + new DarwinDerivationBuilder(store, std::move(miscMethods), std::move(params), useSandbox)); #elif defined(__linux__) if (useSandbox) - return std::make_unique(store, std::move(miscMethods), std::move(params)); + return DerivationBuilderUnique( + new ChrootLinuxDerivationBuilder(store, std::move(miscMethods), std::move(params))); - return std::make_unique(store, std::move(miscMethods), std::move(params)); + return DerivationBuilderUnique(new LinuxDerivationBuilder(store, std::move(miscMethods), std::move(params))); #else if (useSandbox) throw Error("sandboxing builds is not supported on this platform"); - return std::make_unique(store, std::move(miscMethods), std::move(params)); + return DerivationBuilderUnique(new DerivationBuilderImpl(store, std::move(miscMethods), std::move(params))); #endif } diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 7ddb6e093b1..6c1a00f9106 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -106,13 +106,14 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl } }; -std::unique_ptr makeExternalDerivationBuilder( +DerivationBuilderUnique makeExternalDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params, const ExternalBuilder & handler) { - return std::make_unique(store, std::move(miscMethods), std::move(params), handler); + return DerivationBuilderUnique( + new ExternalDerivationBuilder(store, std::move(miscMethods), std::move(params), handler)); } } // namespace nix diff --git a/src/libutil/unix/file-descriptor.cc b/src/libutil/unix/file-descriptor.cc index d90342ff0ec..501f07aec65 100644 --- a/src/libutil/unix/file-descriptor.cc +++ b/src/libutil/unix/file-descriptor.cc @@ -8,9 +8,12 @@ #include #include +#if defined(__linux__) +# include /* pull __NR_* definitions */ +#endif + #if defined(__linux__) && defined(__NR_openat2) # define HAVE_OPENAT2 1 -# include # include #else # define HAVE_OPENAT2 0 @@ -323,7 +326,7 @@ Descriptor unix::openFileEnsureBeneathNoSymlinks(Descriptor dirFd, const CanonPa { assert(!path.rel().starts_with('/')); /* Just in case the invariant is somehow broken. */ assert(!path.isRoot()); -#ifdef __linux__ +#if HAVE_OPENAT2 auto maybeFd = linux::openat2( dirFd, path.rel_c_str(), flags, static_cast(mode), RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS); if (maybeFd) { diff --git a/tests/functional/build.sh b/tests/functional/build.sh index 51f2e2423f3..6a4c90a4378 100755 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -158,12 +158,14 @@ printf "" | nix build --no-link --stdin --json | jq --exit-status '. == []' printf "%s\n" "$drv^*" | nix build --no-link --stdin --json | jq --exit-status '.[0]|has("drvPath")' # --keep-going and FOD -out="$(nix build -f fod-failing.nix -L 2>&1)" && status=0 || status=$? +out="$(nix build -f fod-failing.nix -j1 -L 2>&1)" && status=0 || status=$? test "$status" = 1 -# one "hash mismatch" error, one cancelled build +# Only the hash mismatch error for the first failing goal (x1). +# The other goals (x2, x3, x4) are cancelled and not reported as failures. test "$(<<<"$out" grep -cE '^error:')" = 1 -test "$(<<<"$out" grep -cE '(cancelled)')" = 3 -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation" +<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'" +<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'" +<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'" out="$(nix build -f fod-failing.nix -L x1 x2 x3 --keep-going 2>&1)" && status=0 || status=$? test "$status" = 1 diff --git a/tests/functional/dyn-drv/failing-outer.sh b/tests/functional/dyn-drv/failing-outer.sh index 596efe43dbd..dcf3e830ed5 100644 --- a/tests/functional/dyn-drv/failing-outer.sh +++ b/tests/functional/dyn-drv/failing-outer.sh @@ -10,3 +10,36 @@ if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not expectStderr "$expected" nix-build ./text-hashed-output.nix -A failingWrapper --no-out-link \ | grepQuiet "build of resolved derivation '.*use-dynamic-drv-in-non-dynamic-drv-wrong.drv' failed" + +# Test that error messages are not empty when a producer derivation fails. +# This exercises the nrFailed path in DerivationTrampolineGoal::init(). +# +# Using `nix build --expr` with builtins.outputOf creates a top-level +# DerivationTrampolineGoal that goes through buildPathsWithResults. +# When the producer fails, the nrFailed path must use doneFailure (not amDone) +# to set buildResult.inner with a proper error message. +# +# Without the fix, the error message would be empty because amDone doesn't +# set buildResult.inner, so rethrow() throws Error("") - an empty message. + +out=$(nix build --impure --no-link --expr ' + let + config = import (builtins.getEnv "_NIX_TEST_BUILD_DIR" + "/config.nix"); + inherit (config) mkDerivation; + + # A CA derivation that fails before producing a .drv + failingProducer = mkDerivation { + name = "failing-producer"; + buildCommand = "echo This producer fails; exit 1"; + __contentAddressed = true; + outputHashMode = "text"; + outputHashAlgo = "sha256"; + }; + in + # Build the dynamic derivation output directly - this creates a top-level + # DerivationTrampolineGoal, not a nested one inside a DerivationGoal + builtins.outputOf failingProducer.outPath "out" +' 2>&1) || true + +# The error message must NOT be empty - it should mention the failed derivation +echo "$out" | grepQuiet "failed to obtain derivation of" diff --git a/tests/nixos/s3-binary-cache-store.nix b/tests/nixos/s3-binary-cache-store.nix index 33e869d1954..6869c6c35eb 100644 --- a/tests/nixos/s3-binary-cache-store.nix +++ b/tests/nixos/s3-binary-cache-store.nix @@ -145,7 +145,7 @@ in else: machine.fail(f"nix path-info {pkg}") - def setup_s3(populate_bucket=[], public=False, versioned=False): + def setup_s3(populate_bucket=[], public=False, versioned=False, profiles=None): """ Decorator that creates/destroys a unique bucket for each test. Optionally pre-populates bucket with specified packages. @@ -155,9 +155,22 @@ in populate_bucket: List of packages to upload before test runs public: If True, make the bucket publicly accessible versioned: If True, enable versioning on the bucket before populating + profiles: Dict of AWS profiles to create, e.g.: + {"valid": {"access_key": "...", "secret_key": "..."}, + "invalid": {"access_key": "WRONG", "secret_key": "WRONG"}} + Profiles are created on the client machine at /root/.aws/credentials """ def decorator(test_func): def wrapper(): + # Restart nix-daemon on both machines to clear the credential provider cache. + # The AwsCredentialProviderImpl singleton persists in the daemon process, + # and its cache can cause credentials from previous tests to be reused. + # We reset-failed first to avoid systemd's start rate limiting. + server.succeed("systemctl reset-failed nix-daemon.service nix-daemon.socket") + server.succeed("systemctl restart nix-daemon") + client.succeed("systemctl reset-failed nix-daemon.service nix-daemon.socket") + client.succeed("systemctl restart nix-daemon") + bucket = str(uuid.uuid4()) server.succeed(f"mc mb minio/{bucket}") try: @@ -165,6 +178,15 @@ in server.succeed(f"mc anonymous set download minio/{bucket}") if versioned: server.succeed(f"mc version enable minio/{bucket}") + if profiles: + # Build credentials file content + creds_content = "" + for name, creds in profiles.items(): + creds_content += f"[{name}]\n" + creds_content += f"aws_access_key_id = {creds['access_key']}\n" + creds_content += f"aws_secret_access_key = {creds['secret_key']}\n\n" + client.succeed("mkdir -p /root/.aws") + client.succeed(f"cat > /root/.aws/credentials << 'AWSCREDS'\n{creds_content}AWSCREDS") if populate_bucket: store_url = make_s3_url(bucket) for pkg in populate_bucket: @@ -172,6 +194,9 @@ in test_func(bucket) finally: server.succeed(f"mc rb --force minio/{bucket}") + # Clean up AWS profiles if created + if profiles: + client.succeed("rm -rf /root/.aws") # Clean up client store - only delete if path exists for pkg in PKGS.values(): client.succeed(f"[ ! -e {pkg} ] || nix store delete --ignore-liveness {pkg}") @@ -200,7 +225,40 @@ in "Credential provider caching failed" ) - print("✓ Credential provider created once and cached") + @setup_s3() + def test_aws_log_integration(bucket): + """Test that AWS SDK logs are properly routed through Nix logger""" + print("\n=== Testing AWS Log Integration ===") + + store_url = make_s3_url(bucket) + + # With default verbosity, AWS noise should NOT appear + # All AWS messages are demoted to lvlDebug or lvlVomit + output_default = server.succeed( + f"{ENV_WITH_CREDS} nix copy --to '{store_url}' {PKGS['A']} 2>&1" + ) + + if "(aws:" in output_default: + print("Output at default verbosity:") + print(output_default) + raise Exception("Found AWS noise at default verbosity") + + # With --debug (lvlDebug), we should see AWS messages with (aws:subject) prefix + output_debug = server.succeed( + f"{ENV_WITH_CREDS} nix copy --debug --to '{store_url}' {PKGS['B']} 2>&1" + ) + + # Check for the (aws:subject) prefix format + if "(aws:" not in output_debug: + print("Output at --debug verbosity:") + print(output_debug) + raise Exception("Expected to see (aws:subject) prefix in debug output") + + # Should also see Nix's own credential provider creation message + if "creating new AWS credential provider" not in output_debug: + print("Debug output:") + print(output_debug) + raise Exception("Expected to see credential provider creation at debug level") @setup_s3(populate_bucket=[PKGS['A']]) def test_fetchurl_basic(bucket): @@ -216,8 +274,6 @@ in f"'builtins.fetchurl {{ name = \"foo\"; url = \"{cache_info_url}\"; }}'" ) - print("✓ builtins.fetchurl works with s3:// URLs") - @setup_s3() def test_error_message_formatting(bucket): """Verify error messages display URLs correctly""" @@ -236,8 +292,6 @@ in print(error_msg) raise Exception("Error message formatting failed - should show actual URL, not %s placeholder") - print("✓ Error messages format URLs correctly") - @setup_s3(populate_bucket=[PKGS['A']]) def test_fork_credential_preresolution(bucket): """Test credential pre-resolution in forked processes""" @@ -277,8 +331,6 @@ in print(output) raise Exception("Expected to find FileTransfer creation in forked process") - print(" ✓ Forked process creates fresh FileTransfer") - # Verify pre-resolution in parent required_messages = [ "Pre-resolving AWS credentials for S3 URL in builtin:fetchurl", @@ -291,8 +343,6 @@ in print(output) raise Exception(f"Missing expected message: {msg}") - print(" ✓ Parent pre-resolves credentials") - # Verify child uses pre-resolved credentials if "Using pre-resolved AWS credentials from parent process" not in output: print("Debug output:") @@ -316,8 +366,6 @@ in print(output) raise Exception(f"Child process (pid={child_pid}) should NOT create new credential providers") - print(" ✓ Child uses pre-resolved credentials (no new providers)") - @setup_s3(populate_bucket=[PKGS['A'], PKGS['B'], PKGS['C']]) def test_store_operations(bucket): """Test nix store info and copy operations""" @@ -335,8 +383,6 @@ in if not store_info.get("url"): raise Exception("Store should have a URL") - print(f" ✓ Store URL: {store_info['url']}") - # Test copy from store verify_packages_in_store(client, PKGS['A'], should_exist=False) @@ -354,9 +400,6 @@ in verify_packages_in_store(client, [PKGS['A'], PKGS['B'], PKGS['C']]) - print(" ✓ nix copy works") - print(" ✓ Credentials cached on client") - @setup_s3(populate_bucket=[PKGS['A'], PKGS['B']], public=True) def test_public_bucket_operations(bucket): """Test store operations on public bucket without credentials""" @@ -366,7 +409,6 @@ in # Verify store info works without credentials client.succeed(f"nix store info --store '{store_url}' >&2") - print(" ✓ nix store info works without credentials") # Get and validate store info JSON info_json = client.succeed(f"nix store info --json --store '{store_url}'") @@ -375,8 +417,6 @@ in if not store_info.get("url"): raise Exception("Store should have a URL") - print(f" ✓ Store URL: {store_info['url']}") - # Verify packages are not yet in client store verify_packages_in_store(client, [PKGS['A'], PKGS['B']], should_exist=False) @@ -389,8 +429,6 @@ in # Verify packages were copied successfully verify_packages_in_store(client, [PKGS['A'], PKGS['B']]) - print(" ✓ nix copy from public bucket works without credentials") - @setup_s3(populate_bucket=[PKGS['A']]) def test_url_format_variations(bucket): """Test different S3 URL parameter combinations""" @@ -399,12 +437,10 @@ in # Test parameter order variation (region before endpoint) url1 = f"s3://{bucket}?region={REGION}&endpoint={ENDPOINT}" client.succeed(f"{ENV_WITH_CREDS} nix store info --store '{url1}' >&2") - print(" ✓ Parameter order: region before endpoint works") # Test parameter order variation (endpoint before region) url2 = f"s3://{bucket}?endpoint={ENDPOINT}®ion={REGION}" client.succeed(f"{ENV_WITH_CREDS} nix store info --store '{url2}' >&2") - print(" ✓ Parameter order: endpoint before region works") @setup_s3(populate_bucket=[PKGS['A']]) def test_concurrent_fetches(bucket): @@ -458,9 +494,6 @@ in providers_created = output.count("creating new AWS credential provider") transfers_created = output.count("builtin:fetchurl creating fresh FileTransfer instance") - print(f" ✓ {providers_created} credential providers created") - print(f" ✓ {transfers_created} FileTransfer instances created") - if transfers_created != 5: print("Debug output:") print(output) @@ -486,14 +519,10 @@ in pkg_hash = get_package_hash(PKGS['B']) verify_content_encoding(server, bucket, f"{pkg_hash}.narinfo", "gzip") - print(" ✓ .narinfo has Content-Encoding: gzip") - # Verify client can download and decompress client.succeed(f"{ENV_WITH_CREDS} nix copy --from '{store_url}' --no-check-sigs {PKGS['B']}") verify_packages_in_store(client, PKGS['B']) - print(" ✓ Client decompressed .narinfo successfully") - @setup_s3() def test_compression_mixed(bucket): """Test mixed compression (narinfo=xz, ls=gzip)""" @@ -510,18 +539,14 @@ in # Verify .narinfo has xz compression verify_content_encoding(server, bucket, f"{pkg_hash}.narinfo", "xz") - print(" ✓ .narinfo has Content-Encoding: xz") # Verify .ls has gzip compression verify_content_encoding(server, bucket, f"{pkg_hash}.ls", "gzip") - print(" ✓ .ls has Content-Encoding: gzip") # Verify client can download with mixed compression client.succeed(f"{ENV_WITH_CREDS} nix copy --from '{store_url}' --no-check-sigs {PKGS['C']}") verify_packages_in_store(client, PKGS['C']) - print(" ✓ Client downloaded package with mixed compression") - @setup_s3() def test_compression_disabled(bucket): """Verify no compression by default""" @@ -533,8 +558,6 @@ in pkg_hash = get_package_hash(PKGS['A']) verify_no_compression(server, bucket, f"{pkg_hash}.narinfo") - print(" ✓ No compression applied by default") - @setup_s3() def test_nix_prefetch_url(bucket): """Test that nix-prefetch-url retrieves actual file content from S3, not empty files (issue #8862)""" @@ -554,8 +577,6 @@ in "nix hash file --type sha256 --base32 /tmp/test-file.txt" ).strip() - print(f" ✓ Uploaded test file to S3 ({test_file_size} bytes)") - # Use nix-prefetch-url to download from S3 s3_url = make_s3_url(bucket, path="/test-file.txt") @@ -575,8 +596,6 @@ in f"Hash mismatch: expected {expected_hash}, got {prefetch_hash}" ) - print(" ✓ nix-prefetch-url completed with correct hash") - # Verify the downloaded file is NOT empty (the bug in #8862) file_size = int(client.succeed(f"stat -c %s {store_path}").strip()) @@ -588,16 +607,12 @@ in f"File size mismatch: expected {test_file_size}, got {file_size}" ) - print(f" ✓ File has correct size ({file_size} bytes, not empty)") - # Verify actual content matches by comparing hashes instead of printing entire file downloaded_hash = client.succeed(f"nix hash file --type sha256 --base32 {store_path}").strip() if downloaded_hash != expected_hash: raise Exception(f"Content hash mismatch: expected {expected_hash}, got {downloaded_hash}") - print(" ✓ File content verified correct (hash matches)") - @setup_s3(populate_bucket=[PKGS['A']], versioned=True) def test_versioned_urls(bucket): """Test that versionId parameter is accepted in S3 URLs""" @@ -611,7 +626,6 @@ in f"{ENV_WITH_CREDS} nix eval --impure --expr " f"'builtins.fetchurl {{ name = \"cache-info\"; url = \"{cache_info_url}\"; }}'" ) - print(" ✓ Fetch without versionId works") # List versions to get a version ID # MinIO output format: [timestamp] size tier versionId versionNumber method filename @@ -625,8 +639,6 @@ in raise Exception("Could not extract version ID from MinIO output") version_id = version_match.group(1) - print(f" ✓ Found version ID: {version_id}") - # Version ID should not be "null" since versioning was enabled before upload if version_id == "null": raise Exception("Version ID is 'null' - versioning may not be working correctly") @@ -637,7 +649,6 @@ in f"{ENV_WITH_CREDS} nix eval --impure --expr " f"'builtins.fetchurl {{ name = \"cache-info-versioned\"; url = \"{versioned_url}\"; }}'" ) - print(" ✓ Fetch with versionId parameter works") @setup_s3() def test_multipart_upload_basic(bucket): @@ -673,13 +684,9 @@ in print(output) raise Exception(f"Expected '{expected_msg}' in output") - print(f" ✓ Multipart upload used with {expected_parts} parts") - client.succeed(f"{ENV_WITH_CREDS} nix copy --from '{store_url}' {large_pkg} --no-check-sigs") verify_packages_in_store(client, large_pkg, should_exist=True) - print(" ✓ Large file downloaded and verified") - @setup_s3() def test_multipart_threshold(bucket): """Test that files below threshold use regular upload""" @@ -702,13 +709,9 @@ in if "using S3 regular upload" not in output: raise Exception("Expected regular upload to be used") - print(" ✓ Regular upload used for file below threshold") - client.succeed(f"{ENV_WITH_CREDS} nix copy --no-check-sigs --from '{store_url}' {PKGS['A']}") verify_packages_in_store(client, PKGS['A'], should_exist=True) - print(" ✓ Small file uploaded and verified") - @setup_s3() def test_multipart_with_log_compression(bucket): """Test multipart upload with compressed build logs""" @@ -760,7 +763,99 @@ in print(output) raise Exception("Expected multipart completion message") - print(" ✓ Compressed log uploaded with multipart") + @setup_s3( + populate_bucket=[PKGS['A']], + profiles={ + "valid": {"access_key": ACCESS_KEY, "secret_key": SECRET_KEY}, + "invalid": {"access_key": "INVALIDKEY", "secret_key": "INVALIDSECRET"}, + } + ) + def test_profile_credentials(bucket): + """Test that profile-based credentials work without environment variables""" + print("\n=== Testing Profile-Based Credentials ===") + + store_url = make_s3_url(bucket, profile="valid") + + # Verify store info works with profile credentials (no env vars) + client.succeed(f"HOME=/root nix store info --store '{store_url}' >&2") + + # Verify we can copy from the store using profile + verify_packages_in_store(client, PKGS['A'], should_exist=False) + client.succeed(f"HOME=/root nix copy --no-check-sigs --from '{store_url}' {PKGS['A']}") + verify_packages_in_store(client, PKGS['A']) + + # Clean up the package we just copied so we can test invalid profile + client.succeed(f"nix store delete --ignore-liveness {PKGS['A']}") + verify_packages_in_store(client, PKGS['A'], should_exist=False) + + # Verify invalid profile fails when trying to copy + invalid_url = make_s3_url(bucket, profile="invalid") + client.fail(f"HOME=/root nix copy --no-check-sigs --from '{invalid_url}' {PKGS['A']} 2>&1") + + @setup_s3( + populate_bucket=[PKGS['A']], + profiles={ + "wrong": {"access_key": "WRONGKEY", "secret_key": "WRONGSECRET"}, + } + ) + def test_env_vars_precedence(bucket): + """Test that environment variables take precedence over profile credentials""" + print("\n=== Testing Environment Variables Precedence ===") + + # Use profile with wrong credentials, but provide correct creds via env vars + store_url = make_s3_url(bucket, profile="wrong") + + # Ensure package is not in client store + verify_packages_in_store(client, PKGS['A'], should_exist=False) + + # This should succeed because env vars (correct) override profile (wrong) + output = client.succeed( + f"HOME=/root {ENV_WITH_CREDS} nix copy --no-check-sigs --debug --from '{store_url}' {PKGS['A']} 2>&1" + ) + + # Verify the credential chain shows Environment provider was added + if "Added AWS Environment Credential Provider" not in output: + print("Debug output:") + print(output) + raise Exception("Expected Environment provider to be added to chain") + + # Clean up the package so we can test again without env vars + client.succeed(f"nix store delete --ignore-liveness {PKGS['A']}") + verify_packages_in_store(client, PKGS['A'], should_exist=False) + + # Without env vars, same URL should fail (proving profile creds are actually wrong) + client.fail(f"HOME=/root nix copy --no-check-sigs --from '{store_url}' {PKGS['A']} 2>&1") + + @setup_s3( + populate_bucket=[PKGS['A']], + profiles={ + "testprofile": {"access_key": ACCESS_KEY, "secret_key": SECRET_KEY}, + } + ) + def test_credential_provider_chain(bucket): + """Test that debug logging shows which providers are added to the chain""" + print("\n=== Testing Credential Provider Chain Logging ===") + + store_url = make_s3_url(bucket, profile="testprofile") + + output = client.succeed( + f"HOME=/root nix store info --debug --store '{store_url}' 2>&1" + ) + + # For a named profile, we expect to see these providers in the chain + expected_providers = ["Environment", "Profile", "IMDS"] + for provider in expected_providers: + msg = f"Added AWS {provider} Credential Provider to chain for profile 'testprofile'" + if msg not in output: + print("Debug output:") + print(output) + raise Exception(f"Expected to find: {msg}") + + # SSO should be skipped (no SSO config for this profile) + if "Skipped AWS SSO Credential Provider for profile 'testprofile'" not in output: + print("Debug output:") + print(output) + raise Exception("Expected SSO provider to be skipped") # ============================================================================ # Main Test Execution @@ -780,6 +875,7 @@ in # Run tests (each gets isolated bucket via decorator) test_credential_caching() + test_aws_log_integration() test_fetchurl_basic() test_error_message_formatting() test_fork_credential_preresolution() @@ -795,9 +891,8 @@ in test_multipart_upload_basic() test_multipart_threshold() test_multipart_with_log_compression() - - print("\n" + "="*80) - print("✓ All S3 Binary Cache Store Tests Passed!") - print("="*80) + test_profile_credentials() + test_env_vars_precedence() + test_credential_provider_chain() ''; }