On Tue, May 13, 2014 at 10:43 AM, 'Jose A. Lopes' via ganeti-devel <
[email protected]> wrote:

> * Extend 'backend.DiagnoseRPC' to stat 'create_untrusted', which
>   indicates the OS is untrusted.
> * Extend queries to include the 'trusted' field, which is calculated
>   from the whether the OS contains a 'create_untrusted' script.
> * Fix typo in docstring.
> * Extend client to show OS trusted field.
> * Extend cluster verify, which performs OS related checks, to account
>   for trusted/untrusted scripts.
>
> Signed-off-by: Jose A. Lopes <[email protected]>
> ---
>  lib/backend.py                 | 25 +++++++++++++++++++++++--
>  lib/client/gnt_os.py           | 14 ++++++++++----
>  lib/cmdlib/cluster.py          | 17 ++++++++++++-----
>  lib/cmdlib/operating_system.py | 17 +++++++++++------
>  lib/objects.py                 | 10 ++++++++++
>  lib/query.py                   |  6 +++++-
>  src/Ganeti/Constants.hs        |  7 +++++--
>  7 files changed, 76 insertions(+), 20 deletions(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index 8740d1e..f8798bd 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -2976,11 +2976,13 @@ def DiagnoseOS(top_dirs=None):
>            variants = os_inst.supported_variants
>            parameters = os_inst.supported_parameters
>            api_versions = os_inst.api_versions
> +          trusted = False if os_inst.create_script_untrusted else True
>          else:
>            diagnose = os_inst
>            variants = parameters = api_versions = []
> +          trusted = True
>          result.append((name, os_path, status, diagnose, variants,
> -                       parameters, api_versions))
> +                       parameters, api_versions, trusted))
>
>    return result
>
> @@ -3021,6 +3023,9 @@ def _TryOSFromDisk(name, base_dir=None):
>    # an optional one
>    os_files = dict.fromkeys(constants.OS_SCRIPTS, True)
>
> +  os_files[constants.OS_SCRIPT_CREATE] = False
> +  os_files[constants.OS_SCRIPT_CREATE_UNTRUSTED] = False
> +
>    if max(api_versions) >= constants.OS_API_V15:
>      os_files[constants.OS_VARIANTS_FILE] = False
>
> @@ -3050,6 +3055,21 @@ def _TryOSFromDisk(name, base_dir=None):
>          return False, ("File '%s' under path '%s' is not executable" %
>                         (filename, os_dir))
>
> +  if not constants.OS_SCRIPT_CREATE in os_files and \
> +        not constants.OS_SCRIPT_CREATE_UNTRUSTED in os_files:
> +    return False, ("A create script (trusted or untrusted) under path
> '%s'"
> +                   " must exist" % os_dir)
> +
> +  if constants.OS_SCRIPT_CREATE in os_files:
> +    create_script = os_files[constants.OS_SCRIPT_CREATE]
> +  else:
> +    create_script = None
>

A more concise way of writing this is:

create_script = os_files.get(constants.OS_SCRIPT_CREATE, None)


> +
> +  if constants.OS_SCRIPT_CREATE_UNTRUSTED in os_files:
> +    create_script_untrusted =
> os_files[constants.OS_SCRIPT_CREATE_UNTRUSTED]
> +  else:
> +    create_script_untrusted = None
> +
>    variants = []
>    if constants.OS_VARIANTS_FILE in os_files:
>      variants_file = os_files[constants.OS_VARIANTS_FILE]
> @@ -3073,7 +3093,8 @@ def _TryOSFromDisk(name, base_dir=None):
>      parameters = [v.split(None, 1) for v in parameters]
>
>    os_obj = objects.OS(name=name, path=os_dir,
> -                      create_script=os_files[constants.OS_SCRIPT_CREATE],
> +                      create_script=create_script,
> +                      create_script_untrusted=create_script_untrusted,
>                        export_script=os_files[constants.OS_SCRIPT_EXPORT],
>                        import_script=os_files[constants.OS_SCRIPT_IMPORT],
>                        rename_script=os_files[constants.OS_SCRIPT_RENAME],
> diff --git a/lib/client/gnt_os.py b/lib/client/gnt_os.py
> index e6fe970..d456142 100644
> --- a/lib/client/gnt_os.py
> +++ b/lib/client/gnt_os.py
> @@ -42,7 +42,8 @@ def ListOS(opts, args):
>    @return: the desired exit code
>
>    """
> -  op = opcodes.OpOsDiagnose(output_fields=["name", "variants"], names=[])
> +  op = opcodes.OpOsDiagnose(output_fields=["name", "variants"],
> +                            names=[])
>    result = SubmitOpCode(op, opts=opts)
>
>    if not opts.no_headers:
> @@ -76,7 +77,7 @@ def ShowOSInfo(opts, args):
>    op = opcodes.OpOsDiagnose(output_fields=["name", "valid", "variants",
>                                             "parameters", "api_versions",
>                                             "blacklisted", "hidden",
> "os_hvp",
> -                                           "osparams"],
> +                                           "osparams", "trusted"],
>                              names=[])
>    result = SubmitOpCode(op, opts=opts)
>
> @@ -90,7 +91,7 @@ def ShowOSInfo(opts, args):
>    total_osparams = {}
>
>    for (name, valid, variants, parameters, api_versions, blk, hid, os_hvp,
> -       osparams) in result:
> +       osparams, trusted) in result:
>      total_os_hvp.update(os_hvp)
>      total_osparams.update(osparams)
>      if do_filter:
> @@ -112,6 +113,7 @@ def ShowOSInfo(opts, args):
>        ToStdout("  - parameters:")
>        for pname, pdesc in parameters:
>          ToStdout("    - %s: %s", pname, pdesc)
> +    ToStdout("  - trusted: %s", trusted)
>      ToStdout("")
>
>    if args:
> @@ -178,7 +180,7 @@ def DiagnoseOS(opts, args):
>        nodes_hidden[node_name] = []
>        if node_info: # at least one entry in the per-node list
>          (fo_path, fo_status, fo_msg, fo_variants,
> -         fo_params, fo_api) = node_info.pop(0)
> +         fo_params, fo_api, fo_trusted) = node_info.pop(0)
>          fo_msg = "%s (path: %s)" % (_OsStatus(fo_status, fo_msg), fo_path)
>          if fo_api:
>            max_os_api = max(fo_api)
> @@ -198,6 +200,10 @@ def DiagnoseOS(opts, args):
>                         utils.CommaJoin([v[0] for v in fo_params]))
>            else:
>              fo_msg += " [no parameters]"
> +        if fo_trusted:
> +          fo_msg += " [trusted]"
> +        else:
> +          fo_msg += " [untrusted]"
>          if fo_status:
>            nodes_valid[node_name] = fo_msg
>          else:
> diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
> index 5418c09..aba6987 100644
> --- a/lib/cmdlib/cluster.py
> +++ b/lib/cmdlib/cluster.py
> @@ -2801,7 +2801,7 @@ class LUClusterVerifyGroup(LogicalUnit,
> _VerifyErrors):
>      """
>      remote_os = nresult.get(constants.NV_OSLIST, None)
>      test = (not isinstance(remote_os, list) or
> -            not compat.all(isinstance(v, list) and len(v) == 7
> +            not compat.all(isinstance(v, list) and len(v) == 8
>                             for v in remote_os))
>
>      self._ErrorIf(test, constants.CV_ENODEOS, ninfo.name,
> @@ -2815,7 +2815,8 @@ class LUClusterVerifyGroup(LogicalUnit,
> _VerifyErrors):
>      os_dict = {}
>
>      for (name, os_path, status, diagnose,
> -         variants, parameters, api_ver) in nresult[constants.NV_OSLIST]:
> +         variants, parameters, api_ver,
> +         trusted) in nresult[constants.NV_OSLIST]:
>
>        if name not in os_dict:
>          os_dict[name] = []
> @@ -2824,7 +2825,8 @@ class LUClusterVerifyGroup(LogicalUnit,
> _VerifyErrors):
>        # JSON lacking a real tuple type, fix it:
>        parameters = [tuple(v) for v in parameters]
>        os_dict[name].append((os_path, status, diagnose,
> -                            set(variants), set(parameters), set(api_ver)))
> +                            set(variants), set(parameters), set(api_ver),
> +                            trusted))
>
>      nimg.oslist = os_dict
>
> @@ -2842,7 +2844,7 @@ class LUClusterVerifyGroup(LogicalUnit,
> _VerifyErrors):
>      beautify_params = lambda l: ["%s: %s" % (k, v) for (k, v) in l]
>      for os_name, os_data in nimg.oslist.items():
>        assert os_data, "Empty OS status for OS %s?!" % os_name
> -      f_path, f_status, f_diag, f_var, f_param, f_api = os_data[0]
> +      f_path, f_status, f_diag, f_var, f_param, f_api, f_trusted =
> os_data[0]
>        self._ErrorIf(not f_status, constants.CV_ENODEOS, ninfo.name,
>                      "Invalid OS %s (located at %s): %s",
>                      os_name, f_path, f_diag)
> @@ -2858,7 +2860,7 @@ class LUClusterVerifyGroup(LogicalUnit,
> _VerifyErrors):
>        if test:
>          continue
>        assert base.oslist[os_name], "Base node has empty OS status?"
> -      _, b_status, _, b_var, b_param, b_api = base.oslist[os_name][0]
> +      _, b_status, _, b_var, b_param, b_api, b_trusted =
> base.oslist[os_name][0]
>        if not b_status:
>          # base OS is invalid, skipping
>          continue
> @@ -2871,6 +2873,11 @@ class LUClusterVerifyGroup(LogicalUnit,
> _VerifyErrors):
>                        " [%s] vs. [%s]", kind, os_name,
>                        self.cfg.GetNodeName(base.uuid),
>                        utils.CommaJoin(sorted(a)),
> utils.CommaJoin(sorted(b)))
> +      for kind, a, b in [("trusted", f_trusted, b_trusted)]:
> +        self._ErrorIf(a != b, constants.CV_ENODEOS, ninfo.name,
> +                      "OS %s for %s differs from reference node %s:"
> +                      " %s vs. %s", kind, os_name,
> +                      self.cfg.GetNodeName(base.uuid), a, b)


>      # check any missing OSes
>      missing = set(base.oslist.keys()).difference(nimg.oslist.keys())
> diff --git a/lib/cmdlib/operating_system.py
> b/lib/cmdlib/operating_system.py
> index 9a857b8..f36e7ca 100644
> --- a/lib/cmdlib/operating_system.py
> +++ b/lib/cmdlib/operating_system.py
> @@ -51,7 +51,7 @@ class OsQuery(QueryBase):
>
>    @staticmethod
>    def _DiagnoseByOS(rlist):
> -    """Remaps a per-node return list into an a per-os per-node dictionary
> +    """Remaps a per-node return list into a per-os per-node dictionary
>
>      @param rlist: a map with node names as keys and OS objects as values
>
> @@ -76,7 +76,7 @@ class OsQuery(QueryBase):
>        if nr.fail_msg or not nr.payload:
>          continue
>        for (name, path, status, diagnose, variants,
> -           params, api_versions) in nr.payload:
> +           params, api_versions, trusted) in nr.payload:
>          if name not in all_os:
>            # build a list of nodes for this os containing empty lists
>            # for each node in node_list
> @@ -85,8 +85,8 @@ class OsQuery(QueryBase):
>              all_os[name][nuuid] = []
>          # convert params from [name, help] to (name, help)
>          params = [tuple(v) for v in params]
> -        all_os[name][node_uuid].append((path, status, diagnose,
> -                                        variants, params, api_versions))
> +        all_os[name][node_uuid].append((path, status, diagnose, variants,
> +                                        params, api_versions, trusted))
>      return all_os
>
>    def _GetQueryData(self, lu):
> @@ -110,13 +110,14 @@ class OsQuery(QueryBase):
>        variants = set()
>        parameters = set()
>        api_versions = set()
> +      trusted = True
>
>        for idx, osl in enumerate(os_data.values()):
>          info.valid = bool(info.valid and osl and osl[0][1])
>          if not info.valid:
>            break
>
> -        (node_variants, node_params, node_api) = osl[0][3:6]
> +        (node_variants, node_params, node_api, node_trusted) = osl[0][3:7]
>          if idx == 0:
>            # First entry
>            variants.update(node_variants)
> @@ -128,9 +129,13 @@ class OsQuery(QueryBase):
>            parameters.intersection_update(node_params)
>            api_versions.intersection_update(node_api)
>
> +        if not node_trusted:
> +          trusted = False
> +
>        info.variants = list(variants)
>        info.parameters = list(parameters)
>        info.api_versions = list(api_versions)
> +      info.trusted = trusted
>
>        for variant in variants:
>          name = "+".join([os_name, variant])
> @@ -181,7 +186,7 @@ class LUOsDiagnose(NoHooksLU):
>
>    def CheckArguments(self):
>      self.oq = OsQuery(self._BuildFilter(self.op.output_fields,
> self.op.names),
> -                       self.op.output_fields, False)
> +                      self.op.output_fields, False)
>
>    def ExpandNames(self):
>      self.oq.ExpandNames(self)
> diff --git a/lib/objects.py b/lib/objects.py
> index 4584517..2401150 100644
> --- a/lib/objects.py
> +++ b/lib/objects.py
> @@ -1268,6 +1268,7 @@ class OS(ConfigObject):
>      "path",
>      "api_versions",
>      "create_script",
> +    "create_script_untrusted",
>      "export_script",
>      "import_script",
>      "rename_script",
> @@ -1311,6 +1312,15 @@ class OS(ConfigObject):
>      """
>      return cls.SplitNameVariant(name)[1]
>
> +  def IsTrusted(self):
> +    """Returns whether this OS is trusted.
> +
> +    @rtype: bool
> +    @return: L{True} if this OS is trusted, L{False} otherwise
> +
> +    """
> +    return not self.create_script_untrusted
> +
>
>  class ExtStorage(ConfigObject):
>    """Config object representing an External Storage Provider.
> diff --git a/lib/query.py b/lib/query.py
> index 1ff55d1..b617d7d 100644
> --- a/lib/query.py
> +++ b/lib/query.py
> @@ -2449,7 +2449,8 @@ class OsInfo(objects.ConfigObject):
>      "parameters",
>      "node_status",
>      "os_hvp",
> -    "osparams"
> +    "osparams",
> +    "trusted"
>      ]
>
>
> @@ -2488,6 +2489,9 @@ def _BuildOsFields():
>      (_MakeField("osparams", "OsParameters", QFT_OTHER,
>                  "Operating system specific parameters"),
>       None, 0, _GetItemAttr("osparams")),
> +    (_MakeField("trusted", "Trusted", QFT_BOOL,
> +                "Whether this OS is trusted"),
> +     None, 0, _GetItemAttr("trusted")),
>      ]
>
>    return _PrepareFieldList(fields, [])
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 91d4873..e89ad6a 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -1314,6 +1314,9 @@ rpcConnectTimeout = 5
>  osScriptCreate :: String
>  osScriptCreate = "create"
>
> +osScriptCreateUntrusted :: String
> +osScriptCreateUntrusted = "create_untrusted"
> +
>  osScriptExport :: String
>  osScriptExport = "export"
>
> @@ -1327,8 +1330,8 @@ osScriptVerify :: String
>  osScriptVerify = "verify"
>
>  osScripts :: [String]
> -osScripts = [osScriptCreate, osScriptExport, osScriptImport,
> osScriptRename,
> -             osScriptVerify]
> +osScripts = [osScriptCreate, osScriptCreateUntrusted, osScriptExport,
> +             osScriptImport, osScriptRename, osScriptVerify]
>
>  osApiFile :: String
>  osApiFile = "ganeti_api_version"
> --
> 1.9.1.423.g4596e3a
>
>
The suggestion is optional, LGTM otherwise

Reply via email to