From 788d9b967075096f5c46c5a0329e476ae04b74f9 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Wed, 2 Apr 2025 14:57:21 +0200 Subject: [PATCH] feat(inventory/instances): prevent modules without explizit class from beeing used --- lib/inventory/default.nix | 41 ++++++++++++ .../distributed-service/inventory-adapter.nix | 28 +++++--- .../distributed-service/tests/default.nix | 10 +++ .../tests/import_module_spec.nix | 65 +++++++++++++++++++ 4 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 lib/inventory/distributed-service/tests/import_module_spec.nix diff --git a/lib/inventory/default.nix b/lib/inventory/default.nix index 9dec489c8..aafba313b 100644 --- a/lib/inventory/default.nix +++ b/lib/inventory/default.nix @@ -39,4 +39,45 @@ acc ++ tagMembers ) [ ] members.tags or [ ]); }; + /** + Checks whether a module has a specific class + + # Arguments + - `module` The module to check. + + # Returns + - `string` | null: The specified class, or null if the class is not set + + # Throws + - If the module is not a valid module + - If the module has a type that is not supported + */ + getModuleClass = + module: + let + loadModuleForClassCheck = + m: + # Logic path adapted from nixpkgs/lib/modules.nix + if lib.isFunction m then + let + args = lib.functionArgs m; + in + m args + else if lib.isAttrs m then + # module doesn't have a _type attribute + if m._type or "module" == "module" then + m + # module has a _type set but it is not "module" + else if m._type == "if" || m._type == "override" then + throw "Module modifiers are not supported yet. Got: ${m._type}" + else + throw "Unsupported module type ${lib.typeOf m}" + else if lib.isList m then + throw "Invalid or unsupported module type ${lib.typeOf m}" + else + import m; + + loaded = loadModuleForClassCheck module; + in + if loaded ? _class then loaded._class else null; } diff --git a/lib/inventory/distributed-service/inventory-adapter.nix b/lib/inventory/distributed-service/inventory-adapter.nix index 305295b15..e3371c6ea 100644 --- a/lib/inventory/distributed-service/inventory-adapter.nix +++ b/lib/inventory/distributed-service/inventory-adapter.nix @@ -63,6 +63,7 @@ let resolvedModule = resolvedModuleSet.${instance.module.name} or (throw "flake doesn't provide clan-module with name ${instance.module.name}"); + moduleClass = clanLib.inventory.getModuleClass resolvedModule; # Every instance includes machines via roles # :: { client :: ... } @@ -86,13 +87,13 @@ let machineName: let machineSettings = instance.roles.${roleName}.machines.${machineName}.settings or { }; - # TODO: tag settings - # Wait for this feature until option introspection for 'settings' is done. - # This might get too complex to handle otherwise. - # settingsViaTags = lib.filterAttrs ( - # tagName: _: machineHasTag machineName tagName - # ) instance.roles.${roleName}.tags; in + # TODO: tag settings + # Wait for this feature until option introspection for 'settings' is done. + # This might get too complex to handle otherwise. + # settingsViaTags = lib.filterAttrs ( + # tagName: _: machineHasTag machineName tagName + # ) instance.roles.${roleName}.tags; { # TODO: Do we want to wrap settings with # setDefaultModuleLocation "inventory.instances.${instanceName}.roles.${roleName}.tags.${tagName}"; @@ -112,20 +113,29 @@ let in { inherit (instance) module; - inherit resolvedModule instanceRoles; + inherit resolvedModule instanceRoles moduleClass; } ) inventory.instances; # 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 = "clan.service"; + class = matchedClass; modules = [ ./service-module.nix # Import the resolved module - (builtins.head instances).instance.resolvedModule + classCheckedModule ] # Include all the instances that correlate to the resolved module ++ (builtins.map (v: { diff --git a/lib/inventory/distributed-service/tests/default.nix b/lib/inventory/distributed-service/tests/default.nix index cc04b8436..d628424c6 100644 --- a/lib/inventory/distributed-service/tests/default.nix +++ b/lib/inventory/distributed-service/tests/default.nix @@ -22,6 +22,15 @@ let }).config; flakeInputsFixture = { + # Example upstream module + upstream.clan.modules = { + uzzi = { + _class = "clan.service"; + manifest = { + name = "uzzi-from-upstream"; + }; + }; + }; }; callInventoryAdapter = @@ -32,6 +41,7 @@ let }; in { + resolve_module_spec = import ./import_module_spec.nix { inherit lib callInventoryAdapter; }; test_simple = let res = callInventoryAdapter { diff --git a/lib/inventory/distributed-service/tests/import_module_spec.nix b/lib/inventory/distributed-service/tests/import_module_spec.nix new file mode 100644 index 000000000..89541868e --- /dev/null +++ b/lib/inventory/distributed-service/tests/import_module_spec.nix @@ -0,0 +1,65 @@ +{ callInventoryAdapter }: +let + # Authored module + # A minimal module looks like this + # It isn't exactly doing anything but it's a valid module that produces an output + modules."A" = { + _class = "clan.service"; + manifest = { + name = "network"; + }; + }; + + modules."B" = + { ... }: + { + options.stuff = "legacy-clan-service"; + }; + + machines = { + jon = { }; + sara = { }; + }; + + resolve = + spec: + callInventoryAdapter { + inherit modules machines; + instances."instance_foo" = { + module = spec; + }; + }; +in +{ + test_import_local_module_by_name = { + expr = (resolve { name = "A"; }).importedModuleWithInstances.instance_foo.resolvedModule; + expected = { + _class = "clan.service"; + manifest = { + name = "network"; + }; + }; + }; + test_import_remote_module_by_name = { + expr = + (resolve { + name = "uzzi"; + input = "upstream"; + }).importedModuleWithInstances.instance_foo.resolvedModule; + expected = { + _class = "clan.service"; + manifest = { + name = "uzzi-from-upstream"; + }; + }; + }; + # 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.*"; + }; + }; +}