Merge pull request #1275 from adisbladis/xml-to-json

Move from xml intermediate Nix representation to JSON

Author: grahamc

doc/plugins/authoring.rst

@@ -113,6 +113,46 @@ Important Notes
              os.path.dirname(os.path.abspath(__file__)) + "/nix"
          ]
 
+5. Resource subclasses must now work with Python objects instead of XML
+
+   This old-style ResourceDefinition subclass:
+
+   .. code-block:: python
+
+     class NeatCloudMachineDefinition(nixops.resources.ResourceDefinition):
+
+         def __init__(self, xml):
+             super().__init__(xml)
+             self.store_keys_on_machine = (
+                 xml.find("attrs/attr[@name='storeKeysOnMachine']/bool").get("value")
+                 == "true"
+             )
+
+   Should now look like:
+
+   .. code-block:: python
+
+     class NeatCloudMachineOptions(nixops.resources.ResourceOptions):
+         storeKeysOnMachine: bool
+
+     class NeatCloudMachineDefinition(nixops.resources.ResourceDefinition):
+
+         config: MachineOptions
+
+         store_keys_on_machine: bool
+
+         def __init__(self, name: str, config: nixops.resources.ResourceEval):
+             super().__init__(name, config)
+             self.store_keys_on_machine = config.storeKeysOnMachine
+
+   ``ResourceEval`` is an immutable ``typing.Mapping`` implementation.
+   Also note that ``ResourceEval`` has turned Nix lists into Python tuples, dictionaries into ResourceEval objects and so on.
+   ``typing.Tuple`` cannot be used as it's fixed-size, use ``typing.Sequence`` instead.
+
+   ``ResourceOptions`` is an immutable object that provides type validation both with ``mypy`` _and_ at runtime.
+   Any attributes which are not explicitly typed are passed through as-is.
+
+
 On with Poetry
 ----
 

nix/keys.nix

@@ -23,6 +23,7 @@ let
     options.keyFile = mkOption {
       default = null;
       type = types.nullOr types.path;
+      apply = toString;
       description = ''
         When non-null, contents of the specified file will be deployed to the
         specified key on the target machine.  If the key name is
@@ -155,8 +156,8 @@ in
   config = {
 
     assertions = flip mapAttrsToList config.deployment.keys (key: opts: {
-      assertion = (opts.text == null && opts.keyFile != null) ||
-                  (opts.text != null && opts.keyFile == null);
+      assertion = (opts.text == null && opts.keyFile != "") ||
+                  (opts.text != null && opts.keyFile == "");
       message = "Deployment key '${key}' must have either a 'text' or a 'keyFile' specified.";
     });
 

nixops/backends/__init__.py

@@ -3,52 +3,48 @@
 import os
 import re
 import subprocess
-from typing import Dict, Any, List, Optional, Union, Set
+from typing import Mapping, Any, List, Optional, Union, Set, Sequence
 import nixops.util
 import nixops.resources
 import nixops.ssh_util
-import xml.etree.ElementTree as ET
+
+
+class KeyOptions(nixops.resources.ResourceOptions):
+    text: Optional[str]
+    keyFile: Optional[str]
+    destDir: str
+    user: str
+    group: str
+    permissions: str
+
+
+class MachineOptions(nixops.resources.ResourceOptions):
+    targetPort: int
+    alwaysActivate: bool
+    owners: Sequence[str]
+    hasFastConnection: bool
+    keys: Mapping[str, KeyOptions]
+    nixosRelease: str
 
 
 class MachineDefinition(nixops.resources.ResourceDefinition):
     """Base class for NixOps machine definitions."""
 
-    def __init__(self, xml, config={}) -> None:
-        nixops.resources.ResourceDefinition.__init__(self, xml, config)
-        self.ssh_port = int(xml.find("attrs/attr[@name='targetPort']/int").get("value"))
-        self.always_activate = (
-            xml.find("attrs/attr[@name='alwaysActivate']/bool").get("value") == "true"
-        )
-        self.owners = [
-            e.get("value")
-            for e in xml.findall("attrs/attr[@name='owners']/list/string")
-        ]
-        self.has_fast_connection = (
-            xml.find("attrs/attr[@name='hasFastConnection']/bool").get("value")
-            == "true"
-        )
+    config: MachineOptions
 
-        def _extract_key_options(x: ET.Element) -> Dict[str, str]:
-            opts = {}
-            for (key, xmlType) in (
-                ("text", "string"),
-                ("keyFile", "path"),
-                ("destDir", "string"),
-                ("user", "string"),
-                ("group", "string"),
-                ("permissions", "string"),
-            ):
-                elem = x.find("attrs/attr[@name='{0}']/{1}".format(key, xmlType))
-                if elem is not None:
-                    value = elem.get("value")
-                    if value is not None:
-                        opts[key] = value
-            return opts
-
-        self.keys = {
-            k.get("name"): _extract_key_options(k)
-            for k in xml.findall("attrs/attr[@name='keys']/attrs/attr")
-        }
+    ssh_port: int
+    always_activate: bool
+    owners: List[str]
+    has_fast_connection: bool
+    keys: Mapping[str, KeyOptions]
+
+    def __init__(self, name: str, config: nixops.resources.ResourceEval):
+        super().__init__(name, config)
+        self.ssh_port = config["targetPort"]
+        self.always_activate = config["alwaysActivate"]
+        self.owners = config["owners"]
+        self.has_fast_connection = config["hasFastConnection"]
+        self.keys = {k: KeyOptions(**v) for k, v in config["keys"].items()}
 
 
 class MachineState(nixops.resources.ResourceState):
@@ -61,7 +57,7 @@ class MachineState(nixops.resources.ResourceState):
     ssh_pinged: bool = nixops.util.attr_property("sshPinged", False, bool)
     ssh_port: int = nixops.util.attr_property("targetPort", 22, int)
     public_vpn_key: Optional[str] = nixops.util.attr_property("publicVpnKey", None)
-    keys: Dict[str, str] = nixops.util.attr_property("keys", {}, "json")
+    keys: Mapping[str, str] = nixops.util.attr_property("keys", {}, "json")
     owners: List[str] = nixops.util.attr_property("owners", [], "json")
 
     # Nix store path of the last global configuration deployed to this
@@ -202,7 +198,7 @@ def remove_backup(self, backup_id, keep_physical=False):
             "don't know how to remove a backup for machine ‘{0}’".format(self.name)
         )
 
-    def get_backups(self) -> Dict[str, Dict[str, Any]]:
+    def get_backups(self) -> Mapping[str, Mapping[str, Any]]:
         self.warn("don't know how to list backups for ‘{0}’".format(self.name))
         return {}
 
@@ -259,11 +255,10 @@ def send_keys(self) -> None:
             # so keys will probably end up being written to DISK instead of
             # into memory.
             return
+
         for k, opts in self.get_keys().items():
             self.log("uploading key ‘{0}’...".format(k))
             tmp = self.depl.tempdir + "/key-" + self.name
-            if "destDir" not in opts:
-                raise Exception("Key '{}' has no 'destDir' specified.".format(k))
 
             destDir = opts["destDir"].rstrip("/")
             self.run_command(
@@ -274,10 +269,10 @@ def send_keys(self) -> None:
                 ).format(destDir)
             )
 
-            if "text" in opts:
+            if opts["text"] is not None:
                 with open(tmp, "w+") as f:
                     f.write(opts["text"])
-            elif "keyFile" in opts:
+            elif opts["keyFile"] is not None:
                 self._logged_exec(["cp", opts["keyFile"], tmp])
             else:
                 raise Exception(

nixops/backends/none.py

@@ -1,27 +1,30 @@
 # -*- coding: utf-8 -*-
+from typing import Dict, Optional
 import os
 import sys
 import nixops.util
 
-from nixops.backends import MachineDefinition, MachineState
+from nixops.backends import MachineDefinition, MachineState, MachineOptions
 from nixops.util import attr_property, create_key_pair
+import nixops.resources
 
 
 class NoneDefinition(MachineDefinition):
     """Definition of a trivial machine."""
 
+    _target_host: str
+    _public_ipv4: Optional[str]
+
+    config: MachineOptions
+
     @classmethod
     def get_type(cls):
         return "none"
 
-    def __init__(self, xml, config):
-        MachineDefinition.__init__(self, xml, config)
-        self._target_host = xml.find("attrs/attr[@name='targetHost']/string").get(
-            "value"
-        )
-
-        public_ipv4 = xml.find("attrs/attr[@name='publicIPv4']/string")
-        self._public_ipv4 = None if public_ipv4 is None else public_ipv4.get("value")
+    def __init__(self, name: str, config: nixops.resources.ResourceEval):
+        super().__init__(name, config)
+        self._target_host = config["targetHost"]
+        self._public_ipv4 = config.get("publicIPv4", None)
 
 
 class NoneState(MachineState):

nixops/deployment.py

@@ -9,7 +9,6 @@
 import sqlite3
 import threading
 from collections import defaultdict
-from xml.etree import ElementTree
 import re
 from datetime import datetime, timedelta
 import getpass
@@ -440,16 +439,15 @@ def evaluate_args(self) -> Any:
         except subprocess.CalledProcessError:
             raise NixEvalError
 
-    def evaluate_config(self, attr):
+    def evaluate_config(self, attr) -> Dict:
         try:
-            # FIXME: use --json
-            xml = subprocess.check_output(
+            _json = subprocess.check_output(
                 ["nix-instantiate"]
                 + self.extra_nix_eval_flags
                 + self._eval_flags(self.nix_exprs)
                 + [
                     "--eval-only",
-                    "--xml",
+                    "--json",
                     "--strict",
                     "--arg",
                     "checkConfigurationOptions",
@@ -461,25 +459,19 @@ def evaluate_config(self, attr):
                 text=True,
             )
             if DEBUG:
-                print("XML output of nix-instantiate:\n" + xml, file=sys.stderr)
+                print("JSON output of nix-instantiate:\n" + _json, file=sys.stderr)
         except OSError as e:
             raise Exception("unable to run ‘nix-instantiate’: {0}".format(e))
         except subprocess.CalledProcessError:
             raise NixEvalError
 
-        tree = ElementTree.fromstring(xml)
-
-        # Convert the XML to a more Pythonic representation. This is
-        # in fact the same as what json.loads() on the output of
-        # "nix-instantiate --json" would yield.
-        config = nixops.util.xml_expr_to_python(tree.find("*"))
-        return (tree, config)
+        return json.loads(_json)
 
     def evaluate_network(self, action: str = "") -> None:
         if not self.network_attr_eval:
             # Extract global deployment attributes.
             try:
-                (_, config) = self.evaluate_config("info.network")
+                config = self.evaluate_config("info.network")
             except Exception as e:
                 if action not in ("destroy", "delete"):
                     raise e
@@ -494,22 +486,20 @@ def evaluate(self) -> None:
         self.definitions = {}
         self.evaluate_network()
 
-        (tree, config) = self.evaluate_config("info")
+        config = self.evaluate_config("info")
+
+        tree = None
 
         # Extract machine information.
-        for x in tree.findall("attrs/attr[@name='machines']/attrs/attr"):
-            name = x.get("name")
-            cfg = config["machines"][name]
-            defn = _create_definition(x, cfg, cfg["targetEnv"])
+        for name, cfg in config["machines"].items():
+            defn = _create_definition(name, cfg, cfg["targetEnv"])
             self.definitions[name] = defn
 
         # Extract info about other kinds of resources.
-        for x in tree.findall("attrs/attr[@name='resources']/attrs/attr"):
-            res_type = x.get("name")
-            for y in x.findall("attrs/attr"):
-                name = y.get("name")
+        for res_type, cfg in config["resources"].items():
+            for name, y in cfg.items():
                 defn = _create_definition(
-                    y, config["resources"][res_type][name], res_type
+                    name, config["resources"][res_type][name], res_type
                 )
                 self.definitions[name] = defn
 
@@ -604,13 +594,13 @@ def do_machine(m: nixops.backends.MachineState) -> None:
             attrs_list = attrs_per_resource[m.name]
 
             # Set system.stateVersion if the Nixpkgs version supports it.
-            nixos_version = nixops.util.parse_nixos_version(defn.config["nixosRelease"])
+            nixos_version = nixops.util.parse_nixos_version(defn.config.nixosRelease)
             if nixos_version >= ["15", "09"]:
                 attrs_list.append(
                     {
                         ("system", "stateVersion"): Call(
                             RawValue("lib.mkDefault"),
-                            m.state_version or defn.config["nixosRelease"],
+                            m.state_version or defn.config.nixosRelease,
                         )
                     }
                 )
@@ -1674,16 +1664,14 @@ def _subclasses(cls: Any) -> List[Any]:
     return [cls] if not sub else [g for s in sub for g in _subclasses(s)]
 
 
-def _create_definition(xml: Any, config: Dict[str, Any], type_name: str) -> Any:
+def _create_definition(
+    name: str, config: Dict[str, Any], type_name: str
+) -> nixops.resources.ResourceDefinition:
     """Create a resource definition object from the given XML representation of the machine's attributes."""
 
     for cls in _subclasses(nixops.resources.ResourceDefinition):
         if type_name == cls.get_resource_type():
-            # FIXME: backward compatibility hack
-            if len(inspect.getargspec(cls.__init__).args) == 2:
-                return cls(xml)
-            else:
-                return cls(xml, config)
+            return cls(name, nixops.resources.ResourceEval(config))
 
     raise nixops.deployment.UnknownBackend(
         "unknown resource type ‘{0}’".format(type_name)