diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults-null.drv b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults-null.drv new file mode 120000 index 000000000000..210f500f75db --- /dev/null +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults-null.drv @@ -0,0 +1 @@ +../../../../../tests/functional/derivation/ca/advanced-attributes-structured-attrs-defaults-null.drv \ No newline at end of file diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults-null.json b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults-null.json new file mode 100644 index 000000000000..9a1146f8f512 --- /dev/null +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults-null.json @@ -0,0 +1,45 @@ +{ + "args": [ + "-c", + "echo hello > $out" + ], + "builder": "/bin/bash", + "env": { + "dev": "/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz", + "out": "/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "advanced-attributes-structured-attrs-defaults-null", + "outputs": { + "dev": { + "hashAlgo": "sha256", + "method": "nar" + }, + "out": { + "hashAlgo": "sha256", + "method": "nar" + } + }, + "structuredAttrs": { + "builder": "/bin/bash", + "name": "advanced-attributes-structured-attrs-defaults-null", + "outputChecks": { + "out": { + "allowedReferences": null, + "allowedRequisites": null + } + }, + "outputHashAlgo": "sha256", + "outputHashMode": "recursive", + "outputs": [ + "out", + "dev" + ], + "system": "my-system" + }, + "system": "my-system", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults-null.drv b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults-null.drv new file mode 120000 index 000000000000..76efee5b6e01 --- /dev/null +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults-null.drv @@ -0,0 +1 @@ +../../../../../tests/functional/derivation/ia/advanced-attributes-structured-attrs-defaults-null.drv \ No newline at end of file diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults-null.json b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults-null.json new file mode 100644 index 000000000000..7a247447072e --- /dev/null +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults-null.json @@ -0,0 +1,41 @@ +{ + "args": [ + "-c", + "echo hello > $out" + ], + "builder": "/bin/bash", + "env": { + "dev": "/nix/store/390jivcxmgr11md7knrcyzwv9v2v64cc-advanced-attributes-structured-attrs-defaults-null-dev", + "out": "/nix/store/s579dvk7r4jvp7rjmzq1gy3bf9sp7b4k-advanced-attributes-structured-attrs-defaults-null" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "advanced-attributes-structured-attrs-defaults-null", + "outputs": { + "dev": { + "path": "390jivcxmgr11md7knrcyzwv9v2v64cc-advanced-attributes-structured-attrs-defaults-null-dev" + }, + "out": { + "path": "s579dvk7r4jvp7rjmzq1gy3bf9sp7b4k-advanced-attributes-structured-attrs-defaults-null" + } + }, + "structuredAttrs": { + "builder": "/bin/bash", + "name": "advanced-attributes-structured-attrs-defaults-null", + "outputChecks": { + "out": { + "allowedReferences": null, + "allowedRequisites": null + } + }, + "outputs": [ + "out", + "dev" + ], + "system": "my-system" + }, + "system": "my-system", + "version": 4 +} diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index 296ffed619b2..7cc45df7cd67 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -126,6 +126,7 @@ TEST_ATERM_JSON(advancedAttributes, "advanced-attributes-defaults"); TEST_ATERM_JSON(advancedAttributes_defaults, "advanced-attributes"); TEST_ATERM_JSON(advancedAttributes_structuredAttrs, "advanced-attributes-structured-attrs-defaults"); TEST_ATERM_JSON(advancedAttributes_structuredAttrs_defaults, "advanced-attributes-structured-attrs"); +TEST_ATERM_JSON(advancedAttributes_structuredAttrs_defaults_null, "advanced-attributes-structured-attrs-defaults-null"); #undef TEST_ATERM_JSON @@ -351,6 +352,62 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_default testRequiredSystemFeatures("advanced-attributes-structured-attrs-defaults.drv", {"ca-derivations"}); }; +/** + * Test that null values for allowedReferences and allowedRequisites are + * treated as "not set" (no restriction), same as if the field was missing. + * + * The outputChecks map will have an entry for "out" (since outputChecks.out + * exists in the nix file), but the OutputChecks for that entry should have + * default/empty values for the nullable fields. + */ +DerivationOptions advancedAttributes_structuredAttrs_defaults_null = { + .outputChecks = + std::map::OutputChecks>{ + // null values result in nullopt/empty, same as if not specified + {"out", DerivationOptions::OutputChecks{}}, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph = {}, + .additionalSandboxProfile = "", + .noChroot = false, + .impureHostDeps = {}, + .impureEnvVars = {}, + .allowLocalNetworking = false, + .requiredSystemFeatures = {}, + .preferLocalBuild = false, + .allowSubstitutes = true, +}; + +TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_defaults_null) +{ + this->readTest("advanced-attributes-structured-attrs-defaults-null.drv", [&](auto encoded) { + auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); + + auto options = derivationOptionsFromStructuredAttrs( + *this->store, got.inputDrvs, got.env, get(got.structuredAttrs), true, this->mockXpSettings); + + EXPECT_TRUE(got.structuredAttrs); + + EXPECT_EQ(options, advancedAttributes_structuredAttrs_defaults_null); + + EXPECT_EQ(options.canBuildLocally(*this->store, got), false); + EXPECT_EQ(options.willBuildLocally(*this->store, got), false); + EXPECT_EQ(options.substitutesAllowed(), true); + EXPECT_EQ(options.useUidRange(got), false); + }); +}; + +TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_defaults_null) +{ + testRequiredSystemFeatures("advanced-attributes-structured-attrs-defaults-null.drv", {}); +}; + +TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_defaults_null) +{ + testRequiredSystemFeatures("advanced-attributes-structured-attrs-defaults-null.drv", {"ca-derivations"}); +}; + TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) { DerivationOptions expected = { diff --git a/src/libstore/derivation-options.cc b/src/libstore/derivation-options.cc index 2ead0c444c91..d8da28062f28 100644 --- a/src/libstore/derivation-options.cc +++ b/src/libstore/derivation-options.cc @@ -21,78 +21,43 @@ static std::optional getStringAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name) { if (parsed) { - auto i = parsed->structuredAttrs.find(name); - if (i == parsed->structuredAttrs.end()) - return {}; - else { - if (!i->second.is_string()) - throw Error("attribute '%s' of must be a string", name); - return i->second.get(); - } + if (auto * i = get(parsed->structuredAttrs, name)) + return getString(*i); + return {}; } else { - auto i = env.find(name); - if (i == env.end()) - return {}; - else - return i->second; + if (auto * i = get(env, name)) + return *i; + return {}; } } static bool getBoolAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name, bool def) { if (parsed) { - auto i = parsed->structuredAttrs.find(name); - if (i == parsed->structuredAttrs.end()) - return def; - else { - if (!i->second.is_boolean()) - throw Error("attribute '%s' must be a Boolean", name); - return i->second.get(); - } + if (auto * i = get(parsed->structuredAttrs, name)) + return getBoolean(*i); + return def; } else { - auto i = env.find(name); - if (i == env.end()) - return def; - else - return i->second == "1"; + if (auto * i = get(env, name)) + return *i == "1"; + return def; } } -static std::optional -getStringsAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name) +static std::optional +getStringSetAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name) { if (parsed) { - auto i = parsed->structuredAttrs.find(name); - if (i == parsed->structuredAttrs.end()) - return {}; - else { - if (!i->second.is_array()) - throw Error("attribute '%s' must be a list of strings", name); - auto & a = getArray(i->second); - Strings res; - for (auto j = a.begin(); j != a.end(); ++j) { - if (!j->is_string()) - throw Error("attribute '%s' must be a list of strings", name); - res.push_back(j->get()); - } - return res; - } + if (auto * i = get(parsed->structuredAttrs, name)) + return getStringSet(*i); + return {}; } else { - auto i = env.find(name); - if (i == env.end()) - return {}; - else - return tokenizeString(i->second); + if (auto * i = get(env, name)) + return tokenizeString(*i); + return {}; } } -static std::optional -getStringSetAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name) -{ - auto ss = getStringsAttr(env, parsed, name); - return ss ? (std::optional{StringSet{ss->begin(), ss->end()}}) : (std::optional{}); -} - template using OutputChecks = DerivationOptions::OutputChecks; @@ -233,51 +198,108 @@ DerivationOptions derivationOptionsFromStructuredAttrs( std::map> res; if (auto * outputChecks = get(structuredAttrs, "outputChecks")) { for (auto & [outputName, output_] : getObject(*outputChecks)) { - OutputChecks checks; - auto & output = getObject(output_); - if (auto maxSize = get(output, "maxSize")) - checks.maxSize = maxSize->get(); - - if (auto maxClosureSize = get(output, "maxClosureSize")) - checks.maxClosureSize = maxClosureSize->get(); - - auto get_ = - [&](const std::string & name) -> std::optional>> { - if (auto i = get(output, name)) { - std::set> res; - for (auto j = i->begin(); j != i->end(); ++j) { - if (!j->is_string()) - throw Error("attribute '%s' must be a list of strings", name); - res.insert(parseRef(j->get())); - } - return res; - } - return {}; + auto getRefSet = [&](const nlohmann::json & j) -> std::set> { + std::set> res; + for (auto & s : getStringList(j)) + res.insert(parseRef(s)); + return res; }; res.insert_or_assign( outputName, OutputChecks{ .maxSize = [&]() -> std::optional { - if (auto maxSize = get(output, "maxSize")) - return maxSize->get(); - else - return std::nullopt; + if (auto * i = get(output, "maxSize")) { + try { + return *i; + } catch (Error & e) { + e.addTrace( + {}, + "while parsing attribute 'outputChecks.\"%s\".maxSize'", + outputName); + throw; + } + } + return {}; }(), .maxClosureSize = [&]() -> std::optional { - if (auto maxClosureSize = get(output, "maxClosureSize")) - return maxClosureSize->get(); - else - return std::nullopt; + if (auto * i = get(output, "maxClosureSize")) { + try { + return *i; + } catch (Error & e) { + e.addTrace( + {}, + "while parsing attribute 'outputChecks.\"%s\".maxClosureSize'", + outputName); + throw; + } + } + return {}; + }(), + .allowedReferences = [&]() -> std::optional>> { + if (auto * i = get(output, "allowedReferences")) { + if (auto * j = getNullable(*i)) { + try { + return getRefSet(*j); + } catch (Error & e) { + e.addTrace( + {}, + "while parsing attribute 'outputChecks.\"%s\".allowedReferences'", + outputName); + throw; + } + } + } + return {}; }(), - .allowedReferences = get_("allowedReferences"), .disallowedReferences = - get_("disallowedReferences").value_or(std::set>{}), - .allowedRequisites = get_("allowedRequisites"), + [&] { + if (auto * i = get(output, "disallowedReferences")) { + try { + return getRefSet(*i); + } catch (Error & e) { + e.addTrace( + {}, + "while parsing attribute 'outputChecks.\"%s\".disallowedReferences'", + outputName); + throw; + } + } + return std::set>{}; + }(), + .allowedRequisites = [&]() -> std::optional>> { + if (auto * i = get(output, "allowedRequisites")) { + if (auto * j = getNullable(*i)) { + try { + return getRefSet(*j); + } catch (Error & e) { + e.addTrace( + {}, + "while parsing attribute 'outputChecks.\"%s\".allowedRequisites'", + outputName); + throw; + } + } + } + return {}; + }(), .disallowedRequisites = - get_("disallowedRequisites").value_or(std::set>{}), + [&] { + if (auto * i = get(output, "disallowedRequisites")) { + try { + return getRefSet(*i); + } catch (Error & e) { + e.addTrace( + {}, + "while parsing attribute 'outputChecks.\"%s\".disallowedRequisites'", + outputName); + throw; + } + } + return std::set>{}; + }(), }); } } @@ -307,13 +329,13 @@ DerivationOptions derivationOptionsFromStructuredAttrs( std::map res; if (parsed) { - auto & structuredAttrs = parsed->structuredAttrs; - - if (auto * udr = get(structuredAttrs, "unsafeDiscardReferences")) { - for (auto & [outputName, output] : getObject(*udr)) { - if (!output.is_boolean()) - throw Error("attribute 'unsafeDiscardReferences.\"%s\"' must be a Boolean", outputName); - res.insert_or_assign(outputName, output.get()); + if (auto * udr = get(parsed->structuredAttrs, "unsafeDiscardReferences")) { + try { + for (auto & [outputName, output] : getObject(*udr)) + res.insert_or_assign(outputName, getBoolean(output)); + } catch (Error & e) { + e.addTrace({}, "while parsing attribute 'unsafeDiscardReferences'"); + throw; } } } @@ -340,9 +362,13 @@ DerivationOptions derivationOptionsFromStructuredAttrs( std::map> ret; if (parsed) { - auto * e = optionalValueAt(parsed->structuredAttrs, "exportReferencesGraph"); - if (!e || !e->is_object()) + auto * e = get(parsed->structuredAttrs, "exportReferencesGraph"); + if (!e) return ret; + if (!e->is_object()) { + warn("'exportReferencesGraph' in structured attrs is not a JSON object, ignoring"); + return ret; + } for (auto & [key, storePathsJson] : getObject(*e)) { StringSet ss; flatten(storePathsJson, ss); @@ -591,8 +617,8 @@ DerivationOptions adl_serializer OutputChecksVariant { auto outputChecks = getObject(valueAt(json, "outputChecks")); - auto forAllOutputsOpt = optionalValueAt(outputChecks, "forAllOutputs"); - auto perOutputOpt = optionalValueAt(outputChecks, "perOutput"); + auto forAllOutputsOpt = get(outputChecks, "forAllOutputs"); + auto perOutputOpt = get(outputChecks, "perOutput"); if (forAllOutputsOpt && !perOutputOpt) { return static_cast>(*forAllOutputsOpt); diff --git a/src/libutil/include/nix/util/json-utils.hh b/src/libutil/include/nix/util/json-utils.hh index ec513ca25d61..555c982cc6e6 100644 --- a/src/libutil/include/nix/util/json-utils.hh +++ b/src/libutil/include/nix/util/json-utils.hh @@ -75,6 +75,15 @@ Strings getStringList(const nlohmann::json & value); StringMap getStringMap(const nlohmann::json & value); StringSet getStringSet(const nlohmann::json & value); +template +static inline std::optional ptrToOwned(const nlohmann::json * ptr) +{ + if (ptr) + return std::optional{*ptr}; + else + return std::nullopt; +} + } // namespace nix namespace nlohmann { @@ -114,13 +123,4 @@ struct adl_serializer> } }; -template -static inline std::optional ptrToOwned(const json * ptr) -{ - if (ptr) - return std::optional{*ptr}; - else - return std::nullopt; -} - } // namespace nlohmann diff --git a/tests/functional/derivation/advanced-attributes-structured-attrs-defaults-null.nix b/tests/functional/derivation/advanced-attributes-structured-attrs-defaults-null.nix new file mode 100644 index 000000000000..04ab58b1911b --- /dev/null +++ b/tests/functional/derivation/advanced-attributes-structured-attrs-defaults-null.nix @@ -0,0 +1,39 @@ +{ contentAddress }: + +let + caArgs = + if contentAddress then + { + __contentAddressed = true; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; + } + else + { }; + + derivation' = args: derivation (caArgs // args); + + system = "my-system"; + +in +derivation' { + inherit system; + name = "advanced-attributes-structured-attrs-defaults-null"; + builder = "/bin/bash"; + args = [ + "-c" + "echo hello > $out" + ]; + outputs = [ + "out" + "dev" + ]; + __structuredAttrs = true; + outputChecks = { + out = { + # Test that null is treated as "not set" (no restriction) + allowedReferences = null; + allowedRequisites = null; + }; + }; +} diff --git a/tests/functional/derivation/ca/advanced-attributes-structured-attrs-defaults-null.drv b/tests/functional/derivation/ca/advanced-attributes-structured-attrs-defaults-null.drv new file mode 100644 index 000000000000..0d74732d1d9a --- /dev/null +++ b/tests/functional/derivation/ca/advanced-attributes-structured-attrs-defaults-null.drv @@ -0,0 +1 @@ +Derive([("dev","","r:sha256",""),("out","","r:sha256","")],[],[],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults-null\",\"outputChecks\":{\"out\":{\"allowedReferences\":null,\"allowedRequisites\":null}},\"outputHashAlgo\":\"sha256\",\"outputHashMode\":\"recursive\",\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}"),("dev","/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz"),("out","/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9")]) \ No newline at end of file diff --git a/tests/functional/derivation/ia/advanced-attributes-structured-attrs-defaults-null.drv b/tests/functional/derivation/ia/advanced-attributes-structured-attrs-defaults-null.drv new file mode 100644 index 000000000000..9e3755eca515 --- /dev/null +++ b/tests/functional/derivation/ia/advanced-attributes-structured-attrs-defaults-null.drv @@ -0,0 +1 @@ +Derive([("dev","/nix/store/390jivcxmgr11md7knrcyzwv9v2v64cc-advanced-attributes-structured-attrs-defaults-null-dev","",""),("out","/nix/store/s579dvk7r4jvp7rjmzq1gy3bf9sp7b4k-advanced-attributes-structured-attrs-defaults-null","","")],[],[],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults-null\",\"outputChecks\":{\"out\":{\"allowedReferences\":null,\"allowedRequisites\":null}},\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}"),("dev","/nix/store/390jivcxmgr11md7knrcyzwv9v2v64cc-advanced-attributes-structured-attrs-defaults-null-dev"),("out","/nix/store/s579dvk7r4jvp7rjmzq1gy3bf9sp7b4k-advanced-attributes-structured-attrs-defaults-null")]) \ No newline at end of file