OpCodes: modify InstanceCreate for private, secret params

Modify InstanceCreate to accept process private and secret parameters.
Signed-off-by: Santi Raffa <rsanti@google.com>
Reviewed-by: Jose A. Lopes <jabolopes@google.com>

lib/backend.py

@@ -3266,6 +3266,10 @@ def FinalizeExport(instance, snap_disks):
   for name, value in instance.osparams.items():
     config.set(constants.INISECT_OSP, name, str(value))
 
+  config.add_section(constants.INISECT_OSP_PRIVATE)
+  for name, value in instance.osparams_private.items():
+    config.set(constants.INISECT_OSP_PRIVATE, name, str(value.Get()))
+
   utils.WriteFile(utils.PathJoin(destdir, constants.EXPORT_CONF_FILE),
                   data=config.Dumps())
   shutil.rmtree(finaldestdir, ignore_errors=True)

lib/cli.py

@@ -2794,6 +2794,8 @@ def GenericInstanceCreate(mode, opts, args):
                                 hvparams=hvparams,
                                 beparams=opts.beparams,
                                 osparams=opts.osparams,
+                                osparams_private=opts.osparams_private,
+                                osparams_secret=opts.osparams_secret,
                                 mode=mode,
                                 start=start,
                                 os_type=os_type,

lib/client/gnt_instance.py

@@ -1485,7 +1485,8 @@ add_opts = [
 
 commands = {
   "add": (
-    AddInstance, [ArgHost(min=1, max=1)], COMMON_CREATE_OPTS + add_opts,
+    AddInstance, [ArgHost(min=1, max=1)],
+    COMMON_CREATE_OPTS + add_opts + [OSPARAMS_PRIVATE_OPT, OSPARAMS_SECRET_OPT],
     "[...] -t disk-type -n node[:secondary-node] -o os-type <name>",
     "Creates and adds a new instance to the cluster"),
   "batch-create": (
@@ -1548,7 +1549,7 @@ commands = {
      m_pri_node_opt, m_sec_node_opt, m_clust_opt, m_inst_opt, m_node_tags_opt,
      m_pri_node_tags_opt, m_sec_node_tags_opt, m_inst_tags_opt, SELECT_OS_OPT]
     + SUBMIT_OPTS + [DRY_RUN_OPT, PRIORITY_OPT, OSPARAMS_OPT,
-                     OSPARAMS_NOLOG_OPT, OSPARAMS_NOLOG_NOSAVE_OPT],
+                     OSPARAMS_PRIVATE_OPT, OSPARAMS_SECRET_OPT],
     "[-f] <instance>", "Reinstall a stopped instance"),
   "remove": (
     RemoveInstance, ARGS_ONE_INSTANCE,
@@ -1572,7 +1573,7 @@ commands = {
     SetInstanceParams, ARGS_ONE_INSTANCE,
     [BACKEND_OPT, DISK_OPT, FORCE_OPT, HVOPTS_OPT, NET_OPT] + SUBMIT_OPTS +
     [DISK_TEMPLATE_OPT, SINGLE_NODE_OPT, OS_OPT, FORCE_VARIANT_OPT,
-     OSPARAMS_OPT, OSPARAMS_NOLOG_OPT, DRY_RUN_OPT, PRIORITY_OPT, NWSYNC_OPT,
+     OSPARAMS_OPT, OSPARAMS_PRIVATE_OPT, DRY_RUN_OPT, PRIORITY_OPT, NWSYNC_OPT,
      OFFLINE_INST_OPT, ONLINE_INST_OPT, IGNORE_IPOLICY_OPT, RUNTIME_MEM_OPT,
      NOCONFLICTSCHECK_OPT, NEW_PRIMARY_OPT, HOTPLUG_OPT,
      HOTPLUG_IF_POSSIBLE_OPT],

lib/cmdlib/instance.py

@@ -37,6 +37,7 @@ from ganeti import masterd
 from ganeti import netutils
 from ganeti import objects
 from ganeti import pathutils
+from ganeti import serializer
 import ganeti.rpc.node as rpc
 from ganeti import utils
 
@@ -847,6 +848,12 @@ class LUInstanceCreate(LogicalUnit):
         if name not in self.op.osparams:
           self.op.osparams[name] = value
 
+    if einfo.has_section(constants.INISECT_OSP_PRIVATE):
+      # use the parameters, without overriding
+      for name, value in einfo.items(constants.INISECT_OSP_PRIVATE):
+        if name not in self.op.osparams_private:
+          self.op.osparams_private[name] = serializer.Private(value, descr=name)
+
   def _RevertToDefaults(self, cluster):
     """Revert the instance parameters to the default values.
 
@@ -873,6 +880,12 @@ class LUInstanceCreate(LogicalUnit):
       if name in os_defs and os_defs[name] == self.op.osparams[name]:
         del self.op.osparams[name]
 
+    os_defs_ = cluster.SimpleFillOS(self.op.os_type, {},
+                                    os_params_private={})
+    for name in self.op.osparams_private.keys():
+      if name in os_defs_ and os_defs_[name] == self.op.osparams_private[name]:
+        del self.op.osparams_private[name]
+
   def _CalculateFileStorageDir(self):
     """Calculate final instance file storage dir.
 
@@ -977,7 +990,17 @@ class LUInstanceCreate(LogicalUnit):
     self.be_full = _ComputeFullBeParams(self.op, cluster)
 
     # build os parameters
-    self.os_full = cluster.SimpleFillOS(self.op.os_type, self.op.osparams)
+    if self.op.osparams_private is None:
+      self.op.osparams_private = serializer.PrivateDict()
+    if self.op.osparams_secret is None:
+      self.op.osparams_secret = serializer.PrivateDict()
+
+    self.os_full = cluster.SimpleFillOS(
+      self.op.os_type,
+      self.op.osparams,
+      os_params_private=self.op.osparams_private,
+      os_params_secret=self.op.osparams_secret
+    )
 
     # now that hvp/bep are in final format, let's reset to defaults,
     # if told to do so
@@ -1332,6 +1355,7 @@ class LUInstanceCreate(LogicalUnit):
                             hvparams=self.op.hvparams,
                             hypervisor=self.op.hypervisor,
                             osparams=self.op.osparams,
+                            osparams_private=self.op.osparams_private,
                             )
 
     if self.op.tags:
@@ -1428,7 +1452,9 @@ class LUInstanceCreate(LogicalUnit):
           feedback_fn("* running the instance OS create scripts...")
           # FIXME: pass debug option from opcode to backend
           os_add_result = \
-            self.rpc.call_instance_os_add(self.pnode.uuid, (iobj, None), False,
+            self.rpc.call_instance_os_add(self.pnode.uuid,
+                                          (iobj, self.op.osparams_secret),
+                                          False,
                                           self.op.debug_level)
           if pause_sync:
             feedback_fn("* resuming disk sync")
@@ -3008,9 +3034,9 @@ class LUInstanceSetParams(LogicalUnit):
                                 hvspecs)
 
     # osparams processing
-    if self.op.osparams or self.op.osparams_private_cluster:
+    if self.op.osparams or self.op.osparams_private:
       public_parms = self.op.osparams or {}
-      private_parms = self.op.osparams_private_cluster or {}
+      private_parms = self.op.osparams_private or {}
       dupe_keys = utils.GetRepeatedKeys(public_parms, private_parms)
 
       if dupe_keys:

man/gnt-instance.rst

@@ -37,6 +37,8 @@ ADD
 | [{-B|\--backend-parameters} *BEPARAMS*]
 | [{-H|\--hypervisor-parameters} *HYPERVISOR* [: option=*value*... ]]
 | [{-O|\--os-parameters} *param*=*value*... ]
+| [--os-parameters-private *param*=*value*... ]
+| [--os-parameters-secret *param*=*value*... ]
 | [\--file-storage-dir *dir\_path*] [\--file-driver {loop \| blktap \| blktap2}]
 | {{-n|\--node} *node[:secondary-node]* \| {-I|\--iallocator} *name*}
 | {{-o|\--os-type} *os-type*}
@@ -827,6 +829,18 @@ a hypothetical ``dhcp`` parameter to yes can be achieved by::
 
     gnt-instance add -O dhcp=yes ...
 
+You can also specify OS parameters that should not be logged but reused
+at the next reinstall with ``--os-parameters-private`` and OS parameters
+that should not be logged or saved to configuration with
+``--os-parameters-secret``. Bear in mind that:
+
+  * Launching the daemons in debug mode will cause debug logging to
+    happen, which leaks private and secret parameters to the log files.
+    Do not use the debug mode in production. Deamons will emit a warning
+    on startup if they are in debug mode.
+  * You will have to pass again all ``--os-parameters-secret`` parameters
+    should you want to reinstall this instance.
+
 The ``-I (--iallocator)`` option specifies the instance allocator plugin
 to use (``.`` means the default allocator). If you pass in this option
 the allocator will select nodes for this instance automatically, so you

src/Ganeti/Constants.hs

@@ -1104,6 +1104,9 @@ inisectIns = "instance"
 inisectOsp :: String
 inisectOsp = "os"
 
+inisectOspPrivate :: String
+inisectOspPrivate = "os_private"
+
 -- * Dynamic device modification
 
 ddmAdd :: String

src/Ganeti/OpCodes.hs

@@ -438,6 +438,8 @@ $(genOpCode "OpCode"
      , pInstNics
      , pNoInstall
      , pInstOsParams
+     , pInstOsParamsPrivate
+     , pInstOsParamsSecret
      , pInstOs
      , pPrimaryNode
      , pPrimaryNodeUuid

src/Ganeti/OpParams.hs

@@ -120,6 +120,8 @@ module Ganeti.OpParams
   , pClusterOsParams
   , pClusterOsParamsPrivate
   , pInstOsParams
+  , pInstOsParamsPrivate
+  , pInstOsParamsSecret
   , pCandidatePoolSize
   , pMaxRunningJobs
   , pUidPool
@@ -1098,6 +1100,18 @@ pInstOsParams =
   defaultField [| toJSObject [] |] $
   simpleField "osparams" [t| JSObject JSValue |]
 
+pInstOsParamsPrivate :: Field
+pInstOsParamsPrivate =
+  withDoc "Private OS parameters for instance" .
+  optionalField $
+  simpleField "osparams_private" [t| JSObject (Private JSValue) |]
+
+pInstOsParamsSecret :: Field
+pInstOsParamsSecret =
+  withDoc "Secret OS parameters for instance" .
+  optionalField $
+  simpleField "osparams_secret" [t| JSObject (Private JSValue) |]
+
 pPrimaryNode :: Field
 pPrimaryNode =
   withDoc "Primary node for an instance" $

test/hs/Test/Ganeti/OpCodes.hs

@@ -255,18 +255,46 @@ instance Arbitrary OpCodes.OpCode where
           return Nothing <*> genMaybe genNodeNameNE <*> return Nothing <*>
           genMaybe genNameNE <*> arbitrary
       "OP_INSTANCE_CREATE" ->
-        OpCodes.OpInstanceCreate <$> genFQDN <*> arbitrary <*>
-          arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*>
-          pure emptyJSObject <*> arbitrary <*> arbitrary <*> arbitrary <*>
-          genMaybe genNameNE <*> pure emptyJSObject <*> arbitrary <*>
-          genMaybe genNameNE <*> arbitrary <*> arbitrary <*> arbitrary <*>
-          arbitrary <*> arbitrary <*> arbitrary <*> pure emptyJSObject <*>
-          genMaybe genNameNE <*> genMaybe genNodeNameNE <*> return Nothing <*>
-          genMaybe genNodeNameNE <*> return Nothing <*> genMaybe (pure []) <*>
-          genMaybe genNodeNameNE <*> arbitrary <*> genMaybe genNodeNameNE <*>
-          return Nothing <*> genMaybe genNodeNameNE <*> genMaybe genNameNE <*>
-          arbitrary <*> arbitrary <*> (genTags >>= mapM mkNonEmpty) <*>
-          arbitrary
+        OpCodes.OpInstanceCreate
+          <$> genFQDN                         -- instance_name
+          <*> arbitrary                       -- force_variant
+          <*> arbitrary                       -- wait_for_sync
+          <*> arbitrary                       -- name_check
+          <*> arbitrary                       -- ignore_ipolicy
+          <*> arbitrary                       -- opportunistic_locking
+          <*> pure emptyJSObject              -- beparams
+          <*> arbitrary                       -- disks
+          <*> arbitrary                       -- disk_template
+          <*> arbitrary                       -- file_driver
+          <*> genMaybe genNameNE              -- file_storage_dir
+          <*> pure emptyJSObject              -- hvparams
+          <*> arbitrary                       -- hypervisor
+          <*> genMaybe genNameNE              -- iallocator
+          <*> arbitrary                       -- identify_defaults
+          <*> arbitrary                       -- ip_check
+          <*> arbitrary                       -- conflicts_check
+          <*> arbitrary                       -- mode
+          <*> arbitrary                       -- nics
+          <*> arbitrary                       -- no_install
+          <*> pure emptyJSObject              -- osparams
+          <*> genMaybe arbitraryPrivateJSObj  -- osparams_private
+          <*> genMaybe arbitraryPrivateJSObj  -- osparams_secret
+          <*> genMaybe genNameNE              -- os_type
+          <*> genMaybe genNodeNameNE          -- pnode
+          <*> return Nothing                  -- pnode_uuid
+          <*> genMaybe genNodeNameNE          -- snode
+          <*> return Nothing                  -- snode_uuid
+          <*> genMaybe (pure [])              -- source_handshake
+          <*> genMaybe genNodeNameNE          -- source_instance_name
+          <*> arbitrary                       -- source_shutdown_timeout
+          <*> genMaybe genNodeNameNE          -- source_x509_ca
+          <*> return Nothing                  -- src_node
+          <*> genMaybe genNodeNameNE          -- src_node_uuid
+          <*> genMaybe genNameNE              -- src_path
+          <*> arbitrary                       -- compress
+          <*> arbitrary                       -- start
+          <*> (genTags >>= mapM mkNonEmpty)   -- tags
+          <*> arbitrary                       -- instance_communication
       "OP_INSTANCE_MULTI_ALLOC" ->
         OpCodes.OpInstanceMultiAlloc <$> arbitrary <*> genMaybe genNameNE <*>
         pure []

test/py/cmdlib/instance_unittest.py

@@ -465,6 +465,7 @@ class TestLUInstanceCreate(CmdlibTestCase):
                          osparams={
                            self.os_name_variant: {}
                          },
+                         osparams_private={},
                          identify_defaults=True)
     self.ExecOpCode(op)