publish_early: rework API

Commit ba6c3331 does not handle non-string
values properly. While fixing this bug, I also found that '-init' was bad:
sections were uninstalled on next runs, which is a problem with recipes
whose install() return paths.

slapos/recipe/neoppod.py

@@ -32,30 +32,26 @@ from .librecipe import GenericBaseRecipe
 class Cluster(object):
 
   def __init__(self, buildout, name, options):
-    self.buildout = buildout
-    self.options = options
-
-  def publish_early(self, publish_dict):
-    masters = publish_dict.setdefault('masters', '')
+    masters = options.setdefault('masters', '')
     result_dict = {
       'connection-admin': [],
       'connection-master': [],
     }
     node_list = []
-    for node in sorted(self.options['nodes'].split()):
-      node = self.buildout[node]
+    for node in sorted(options['nodes'].split()):
+      node = buildout[node]
       node_list.append(node)
       for k, v in result_dict.iteritems():
         x = node[k]
         if x:
           v.append(x)
-    publish_dict['admins'] = ' '.join(result_dict.pop('connection-admin'))
+    options['admins'] = ' '.join(result_dict.pop('connection-admin'))
     x = ' '.join(result_dict.pop('connection-master'))
     if masters != x:
-      publish_dict['masters'] = x
+      options['masters'] = x
       for node in node_list:
         node['config-masters'] = x
-        node.recipe.__init__(self.buildout, node.name, node)
+        node.recipe.__init__(buildout, node.name, node)
 
   install = update = lambda self: None
 

slapos/recipe/publish_early.py

@@ -28,20 +28,14 @@
 from collections import defaultdict
 from .librecipe import unwrap, wrap, GenericSlapRecipe
 
-def patchOptions(options, override):
-  def get(option, *args, **kw):
-    try:
-      return override[option]
-    except KeyError:
-      return options_get(option, *args, **kw)
-  try:
-    options_get = options._get
-  except AttributeError:
-    options_get = options.get
-    options.get = get
-  else:
-    options._get = get
-
+def volatileOptions(options, volatile):
+  def copy():
+    copy = options_copy()
+    for key in volatile:
+      copy.pop(key, None)
+    return copy
+  options_copy = options.copy
+  options.copy = copy
 
 class Recipe(GenericSlapRecipe):
   """
@@ -57,8 +51,6 @@ class Recipe(GenericSlapRecipe):
     -init =
       foo gen-foo:x
       bar gen-bar:y
-    -update =
-      baz update-baz:z
     bar = z
 
     [gen-foo]
@@ -69,72 +61,74 @@ class Recipe(GenericSlapRecipe):
     -extends = publish-early
     ...
 
-  ${publish-early:foo} is initialized with the value of the published
-  parameter 'foo', or ${gen-foo:x} if it hasn't been published yet
-  (and in this case, it is published immediately as a way to save the value).
+  Just before the recipe of [gen-foo] is instantiated, 'x' is overridden with
+  the published value 'foo' if it exists. If its __init__ modifies 'x', the new
+  value is published. To prevent [gen-foo] from being accessed too early, 'x'
+  is then removed and the value can only be accessed with ${publish-early:foo}.
+
+  Generated values don't end up in the buildout installed file, which is good
+  if they're secret. Note however that buildout won't detect if values change
+  and it may only call update().
+
   ${publish-early:bar} is forced to 'z' (${gen-bar:y} ignored):
   a line like 'bar = z' is usually rendered conditionally with Jinja2.
-
-  The '-update' option has the same syntax than '-init'. The recipes of the
-  specified sections must implement 'publish_early(publish_dict)':
-  - it is always called, just before early publishing
-  - publish_dict is a dict with already published values
-  - 'publish_early' can change published values by modifying publish_dict.
-
-  In the above example:
-  - publish_dict is {'z': ...}
-  - during the execution of 'publish_early', other sections can access the
-    value with ${update-baz:z}
-  - once [publish-early] is initialized, the value should be accessed with
-    ${publish-early:bar} ([update-baz] does not have it if it's accessed
-    before [publish-early])
   """
   def __init__(self, buildout, name, options):
     GenericSlapRecipe.__init__(self, buildout, name, options)
     init = defaultdict(dict)
-    update = defaultdict(dict)
-    for d, k in (init, '-init'), (update, '-update'):
-      for line in options.get(k, '').splitlines():
-        if line:
-          k, v = line.split()
-          if k not in options:
-            section, v = v.split(':')
-            d[section][k] = v
-    if init or update:
+    for line in options['-init'].splitlines():
+      if line:
+        k, v = line.split()
+        if k not in options:
+          section, v = v.split(':')
+          init[section][k] = v
+    if init:
       self.slap.initializeConnection(self.server_url, self.key_file,
         self.cert_file)
       computer_partition = self.slap.registerComputerPartition(
         self.computer_id, self.computer_partition_id)
       published_dict = unwrap(computer_partition.getConnectionParameterDict())
 
+      Options = buildout.Options
+      if 'Options' in buildout.__dict__:
+        def revertOptions():
+          buildout.Options = Options
+      else:
+        def revertOptions():
+          try:
+            del buildout.Options
+          except AttributeError:
+            pass
+      def newOptions(buildout, section, data):
+        assert section == init_section, (section, init_section)
+        revertOptions()
+        self = buildout.Options(buildout, section, data)
+        self.update(override)
+        return self
+
       publish = False
       publish_dict = {}
-      for section, init in init.iteritems():
-        for k, v in init.iteritems():
-          try:
-            publish_dict[k] = published_dict[k]
-          except KeyError:
-            publish_dict[k] = buildout[section][v]
+      try:
+        for init_section, init in init.iteritems():
+          override = {}
+          for k, v in init.iteritems():
+            try:
+              override[v] = published_dict[k]
+            except KeyError:
+              pass
+          buildout.Options = newOptions
+          init_section = buildout[init_section]
+          assert buildout.Options is Options
+          new = {}
+          for k, v in init.iteritems():
+            try:
+              publish_dict[k] = new[v] = init_section.pop(v)
+            except KeyError:
+              pass
+          if new != override:
             publish = True
-
-      for section, update in update.iteritems():
-        override = {}
-        for k, v in update.iteritems():
-          try:
-            override[v] = published_dict[k]
-          except KeyError:
-            pass
-        section = buildout[section]
-        patchOptions(section, override)
-        old = override.copy()
-        section.recipe.publish_early(override)
-        if override != old:
-          publish = True
-        for k, v in update.iteritems():
-          try:
-            publish_dict[k] = override[v]
-          except KeyError:
-            pass
+      finally:
+        revertOptions()
 
       if publish:
         computer_partition.setConnectionDict(wrap(publish_dict))
@@ -143,6 +137,7 @@ class Recipe(GenericSlapRecipe):
         if k != 'recipe' and not k.startswith('-')]
       publish += publish_dict
       publish_dict['-publish'] = ' '.join(publish)
-      patchOptions(options, publish_dict)
+      volatileOptions(options, list(publish_dict))
+      options.update(publish_dict)
 
   install = update = lambda self: None

slapos/recipe/random.py

@@ -36,8 +36,8 @@ import errno
 import os
 import random
 import string
-
-from slapos.recipe.librecipe import GenericBaseRecipe
+from .librecipe import GenericBaseRecipe
+from .publish_early import volatileOptions
 
 class Integer(object):
   """
@@ -54,7 +54,9 @@ class Integer(object):
     Resulting integer.
   """
   def __init__(self, buildout, name, options):
-    options['value'] = random.randint(int(options['minimum']), int(options['maximum']))
+    if 'value' not in options:
+      options['value'] = random.randint(int(options['minimum']),
+                                        int(options['maximum']))
 
   def install(self):
     pass
@@ -65,10 +67,9 @@ class Time(object):
   """Generate a random time from a 24h time clock"""
 
   def __init__(self, buildout, name, options):
-    self.name = name
-    self.buildout = buildout
-    self.options = options
-    self.options['time'] = "%d:%d" % (random.randint(0, 23), random.randint(0, 59))
+    if 'time' not in options:
+      options['time'] = "%u:%02u" % (
+        random.randint(0, 23), random.randint(0, 59))
 
   def install(self):
     pass
@@ -76,26 +77,33 @@ class Time(object):
   update = install
 
 
-class Mac(GenericBaseRecipe):
+class Mac(object):
 
   def __init__(self, buildout, name, options):
-    if os.path.exists(options['storage-path']):
-      open_file = open(options['storage-path'], 'r')
-      options['mac-address'] = open_file.read()
-      open_file.close()
-
-    if options.get('mac-address', '') == '':
-      # First octet has to represent a locally administered address
-      octet_list = [254] + [random.randint(0x00, 0xff) for x in range(5)]
-      options['mac-address'] = ':'.join(['%02x' % x for x in octet_list])
-    return GenericBaseRecipe.__init__(self, buildout, name, options)
+    self.storage_path = options['storage-path']
+    mac = options.get('mac-address')
+    if not mac:
+      try:
+        with open(self.storage_path) as f:
+          mac = f.read()
+      except IOError as e:
+        if e.errno != errno.ENOENT:
+          raise
+      if not mac:
+        # First octet has to represent a locally administered address
+        octet_list = [254] + [random.randint(0x00, 0xff) for x in range(5)]
+        mac = ':'.join(['%02x' % x for x in octet_list])
+        self.update = self.install
+      options['mac-address'] = mac
+    self.mac = mac
 
   def install(self):
-    open_file = open(self.options['storage-path'], 'w')
-    open_file.write(self.options['mac-address'])
-    open_file.close()
-    return [self.options['storage-path']]
+    with open(self.storage_path, 'w') as f:
+      f.write(self.mac)
+    return self.storage_path
 
+  def update(self):
+    pass
 
 def generatePassword(length):
   return ''.join(random.SystemRandom().sample(string.ascii_lowercase, length))
@@ -130,30 +138,24 @@ class Password(object):
     except KeyError:
       self.storage_path = options['storage-path'] = os.path.join(
         buildout['buildout']['parts-directory'], name)
-    passwd = None
-    if self.storage_path:
-      try:
-        with open(self.storage_path) as f:
-          passwd = f.read().strip('\n')
-      except IOError as e:
-        if e.errno != errno.ENOENT:
-          raise
+    passwd = options.get('passwd')
     if not passwd:
-      passwd = self.generatePassword(int(options.get('bytes', '8')))
-      self.update = self.install
-    self.passwd = passwd
+      if self.storage_path:
+        try:
+          with open(self.storage_path) as f:
+            passwd = f.read().strip('\n')
+        except IOError as e:
+          if e.errno != errno.ENOENT:
+            raise
+      if not passwd:
+        passwd = self.generatePassword(int(options.get('bytes', '8')))
+        self.update = self.install
+      options['passwd'] = passwd
     # Password must not go into .installed file, for 2 reasons:
     # security of course but also to prevent buildout to always reinstall.
-    def get(option, *args, **kw):
-      return passwd if option == 'passwd' else options_get(option, *args, **kw)
-
-    try:
-      options_get = options._get
-    except AttributeError:
-      options_get = options.get
-      options.get = get
-    else:
-      options._get = get
+    # publish_early already does it, but this recipe may also be used alone.
+    volatileOptions(options, ('passwd',))
+    self.passwd = passwd
 
   generatePassword = staticmethod(generatePassword)
 
@@ -179,4 +181,4 @@ class Password(object):
         return self.storage_path
 
   def update(self):
-    return ()
+    pass