From f7d5a529aa30d42e405bd073bd9580e593917f9b Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Tue, 8 Apr 2025 15:03:58 +0200 Subject: [PATCH 1/2] chore(inventory/services): dont check _class of legacy clanModules. Let the module system handle the error. Once we shift to deferred modules, things get more complicated and we cannot check the module class eagerly --- .../build-inventory/builder/default.nix | 42 +++++-------------- .../build-inventory/builder/interface.nix | 3 -- .../build-inventory/builder/roles.nix | 2 +- lib/inventory/eval-clan-modules/default.nix | 2 +- lib/inventory/frontmatter/default.nix | 29 +++++++++---- 5 files changed, 35 insertions(+), 43 deletions(-) diff --git a/lib/inventory/build-inventory/builder/default.nix b/lib/inventory/build-inventory/builder/default.nix index e3b3bad38..18b7bc22d 100644 --- a/lib/inventory/build-inventory/builder/default.nix +++ b/lib/inventory/build-inventory/builder/default.nix @@ -56,7 +56,7 @@ let assertions = { }; }; - legacyResolveImports = + resolveImports = { supportedRoles, resolvedRolesPerInstance, @@ -168,36 +168,16 @@ in ./roles.nix ]; - isClanModule = - let - firstRole = import (getRoleFile (builtins.head config.supportedRoles)); - loadModuleForClassCheck = - m: - if lib.isFunction m then - let - args = lib.functionArgs m; - in - m args - else - m; - module = loadModuleForClassCheck (firstRole); - in - if (module) ? _class then module._class == "clan" else false; - # The actual result - machineImports = - if config.isClanModule then - throw "Clan modules are not supported yet." - else - legacyResolveImports { - supportedRoles = config.supportedRoles; - resolvedRolesPerInstance = config.resolvedRolesPerInstance; - inherit - serviceConfigs - serviceName - machineName - getRoleFile - ; - }; + machineImports = resolveImports { + supportedRoles = config.supportedRoles; + resolvedRolesPerInstance = config.resolvedRolesPerInstance; + inherit + serviceConfigs + serviceName + machineName + getRoleFile + ; + }; # Assertions assertions = { diff --git a/lib/inventory/build-inventory/builder/interface.nix b/lib/inventory/build-inventory/builder/interface.nix index 584ccafc3..90f161865 100644 --- a/lib/inventory/build-inventory/builder/interface.nix +++ b/lib/inventory/build-inventory/builder/interface.nix @@ -54,9 +54,6 @@ in matchedRoles = mkOption { type = types.listOf types.str; }; - isClanModule = mkOption { - type = types.bool; - }; machinesRoles = mkOption { type = types.attrsOf (types.listOf types.str); }; diff --git a/lib/inventory/build-inventory/builder/roles.nix b/lib/inventory/build-inventory/builder/roles.nix index 1eb29f47d..364b72d62 100644 --- a/lib/inventory/build-inventory/builder/roles.nix +++ b/lib/inventory/build-inventory/builder/roles.nix @@ -14,7 +14,7 @@ in { # Roles resolution # : List String - supportedRoles = clanLib.modules.getRoles inventory.modules serviceName; + supportedRoles = clanLib.modules.getRoles "inventory.modules" inventory.modules serviceName; matchedRoles = builtins.attrNames ( lib.filterAttrs (_: ms: builtins.elem machineName ms) config.machinesRoles ); diff --git a/lib/inventory/eval-clan-modules/default.nix b/lib/inventory/eval-clan-modules/default.nix index 2aed71757..de944efe4 100644 --- a/lib/inventory/eval-clan-modules/default.nix +++ b/lib/inventory/eval-clan-modules/default.nix @@ -74,7 +74,7 @@ let roles = if builtins.elem "inventory" frontmatter.features or [ ] then assert lib.isPath module; - clanLib.modules.getRoles allModules moduleName + clan-core.lib.modules.getRoles "Documentation: inventory.modules" allModules moduleName else [ ]; in diff --git a/lib/inventory/frontmatter/default.nix b/lib/inventory/frontmatter/default.nix index fee0e29ed..ad9f63745 100644 --- a/lib/inventory/frontmatter/default.nix +++ b/lib/inventory/frontmatter/default.nix @@ -36,7 +36,7 @@ let lib.evalModules { specialArgs = { inherit moduleName resolvedRoles instanceName; - allRoles = getRoles allModules moduleName; + allRoles = getRoles "inventory.modules" allModules moduleName; }; modules = [ (getFrontmatter allModules.${moduleName} moduleName) @@ -56,15 +56,30 @@ let ]; }).options; + # This is a legacy function + # Old modules needed to define their roles by directory + # This means if this function gets anything other than a string/path it will throw getRoles = - allModules: serviceName: - lib.mapAttrsToList (name: _value: trimExtension name) ( + scope: allModules: serviceName: + let + module = + allModules.${serviceName} + or (throw "(Legacy) ClanModule not found: '${serviceName}'. Make sure the module is added to ${scope}"); + moduleType = (lib.typeOf module); + checked = if builtins.elem moduleType ["string" "path"] then true else throw "(Legacy) ClanModule must be a 'path' or 'string' pointing to a directory: Got 'typeOf inventory.modules.${serviceName}' => ${moduleType} "; + modulePath = lib.seq checked module + "/roles"; + checkedPath = if builtins.pathExists modulePath then modulePath else throw '' + (Legacy) ClanModule must have a 'roles' directory' + + Fixes: + - Provide a 'roles' subdirectory + - Use the newer 'clan.service' modules. (Recommended) + ''; + in + lib.seq checkedPath lib.mapAttrsToList (name: _value: trimExtension name) ( lib.filterAttrs (name: type: type == "regular" && lib.hasSuffix ".nix" name) ( builtins.readDir ( - if allModules ? ${serviceName} then - allModules.${serviceName} + "/roles" - else - throw "ClanModule not found: '${serviceName}'. Make sure the module is added in the 'clanModules' attribute of clan-core." + checkedPath ) ) ); From 9af20ad8b5d2d410e95c8e26e3d119df5a61087b Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Tue, 8 Apr 2025 15:09:22 +0200 Subject: [PATCH 2/2] chore(inventory/instances): don't eagerly test the service modules _class --- .../distributed-service/inventory-adapter.nix | 15 ++------- .../tests/import_module_spec.nix | 9 ------ lib/inventory/frontmatter/default.nix | 31 +++++++++++++------ lib/inventory/tests/default.nix | 1 - 4 files changed, 24 insertions(+), 32 deletions(-) diff --git a/lib/inventory/distributed-service/inventory-adapter.nix b/lib/inventory/distributed-service/inventory-adapter.nix index e3371c6ea..a92649322 100644 --- a/lib/inventory/distributed-service/inventory-adapter.nix +++ b/lib/inventory/distributed-service/inventory-adapter.nix @@ -120,22 +120,13 @@ let # TODO: Eagerly check the _class of the resolved module importedModulesEvaluated = lib.mapAttrs ( _module_ident: instances: - let - matchedClass = "clan.service"; - instance = (builtins.head instances).instance; - classCheckedModule = - if instance.moduleClass == matchedClass then - instance.resolvedModule - else - (throw ''Module '${instance.module.name}' is not a valid '${matchedClass}' module. Got module with class:${builtins.toJSON instance.moduleClass}''); - in (lib.evalModules { - class = matchedClass; + class = "clan.service"; modules = [ ./service-module.nix - # Import the resolved module - classCheckedModule + # Import the resolved module. + (builtins.head instances).instance.resolvedModule ] # Include all the instances that correlate to the resolved module ++ (builtins.map (v: { diff --git a/lib/inventory/distributed-service/tests/import_module_spec.nix b/lib/inventory/distributed-service/tests/import_module_spec.nix index 4d15cdbd9..15db21107 100644 --- a/lib/inventory/distributed-service/tests/import_module_spec.nix +++ b/lib/inventory/distributed-service/tests/import_module_spec.nix @@ -53,13 +53,4 @@ in }; }; }; - # Currently this should fail - # TODO: Can we implement a default wrapper to make migration easy? - test_import_local_legacy_module = { - expr = (resolve { name = "B"; }).allMachines; - expectedError = { - type = "ThrownError"; - msg = "Module 'B' is not a valid 'clan.service' module.*"; - }; - }; } diff --git a/lib/inventory/frontmatter/default.nix b/lib/inventory/frontmatter/default.nix index ad9f63745..243fb4c61 100644 --- a/lib/inventory/frontmatter/default.nix +++ b/lib/inventory/frontmatter/default.nix @@ -66,21 +66,32 @@ let allModules.${serviceName} or (throw "(Legacy) ClanModule not found: '${serviceName}'. Make sure the module is added to ${scope}"); moduleType = (lib.typeOf module); - checked = if builtins.elem moduleType ["string" "path"] then true else throw "(Legacy) ClanModule must be a 'path' or 'string' pointing to a directory: Got 'typeOf inventory.modules.${serviceName}' => ${moduleType} "; + checked = + if + builtins.elem moduleType [ + "string" + "path" + ] + then + true + else + throw "(Legacy) ClanModule must be a 'path' or 'string' pointing to a directory: Got 'typeOf inventory.modules.${serviceName}' => ${moduleType} "; modulePath = lib.seq checked module + "/roles"; - checkedPath = if builtins.pathExists modulePath then modulePath else throw '' - (Legacy) ClanModule must have a 'roles' directory' + checkedPath = + if builtins.pathExists modulePath then + modulePath + else + throw '' + (Legacy) ClanModule must have a 'roles' directory' - Fixes: - - Provide a 'roles' subdirectory - - Use the newer 'clan.service' modules. (Recommended) - ''; + Fixes: + - Provide a 'roles' subdirectory + - Use the newer 'clan.service' modules. (Recommended) + ''; in lib.seq checkedPath lib.mapAttrsToList (name: _value: trimExtension name) ( lib.filterAttrs (name: type: type == "regular" && lib.hasSuffix ".nix" name) ( - builtins.readDir ( - checkedPath - ) + builtins.readDir (checkedPath) ) ); diff --git a/lib/inventory/tests/default.nix b/lib/inventory/tests/default.nix index 3fd897c2a..55d01df3d 100644 --- a/lib/inventory/tests/default.nix +++ b/lib/inventory/tests/default.nix @@ -34,7 +34,6 @@ in }; expected = { legacyModule = { - isClanModule = false; }; }; };