swarming: Add server-side implementation for supplemental bot_config

appengine/swarming/server/bot_groups_config.py

 # Copyright 2016 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 """Functions to fetch and interpret bots.cfg file with list of bot groups."""
 
 import collections
 import hashlib
 import logging
+import os
 import re
 
 from components import auth
 from components import config
 from components import utils
 from components.config import validation
 
 from proto import bots_pb2
 from server import config as local_config
 
@@ skipping 20 matching lines @@
   'ip_whitelist',
 
   # Tuple with emails of bot owners.
   'owners',
 
   # Dict {key => list of values}. Always contains all the keys specified by
   # 'trusted_dimensions' set in BotsCfg. If BotGroup doesn't define some
   # dimension from that set, the list of value for it will be empty. Key and
   # values are unicode strings.
   'dimensions',
+
+  # Name of the supplemental bot_config.py to inject to the bot during
+  # handshake.
+  'bot_config_script',
 ])
 
 
 # Post-processed and validated read-only form of bots.cfg config. Its structure
 # is optimized for fast lookup of BotGroupConfig by bot_id.
 _BotGroups = collections.namedtuple('_BotGroups', [
   'direct_matches', # dict bot_id => BotGroupConfig
   'prefix_matches', # list of pairs (bot_id_prefix, BotGroupConfig)
   'machine_types',  # dict machine_type.name => BotGroupConfig
   'default_group',  # fallback BotGroupConfig or None if not defined
 ])
 
 
 # Default config to use on unconfigured server.
 _DEFAULT_BOT_GROUPS = _BotGroups(
     direct_matches={},
     prefix_matches=[],
     machine_types={},
     default_group=BotGroupConfig(
         version='default',
         require_luci_machine_token=False,
         require_service_account=None,
         ip_whitelist=auth.BOTS_IP_WHITELIST,
         owners=(),
-        dimensions={}))
+        dimensions={},
+        bot_config_script=''))
 
 
 def _gen_version(fields):
   """Looks at BotGroupConfig fields and derives a digest that summarizes them.
 
   This digest is going to be sent to the bot in /handshake, and bot would
   include it in its state (and thus send it with each /poll). If server detects
   that the bot is using older version of the config, it would ask the bot
   to restart.
 
@@ skipping 53 matching lines @@
     k, v = parts[0], parts[1]
     dimensions.setdefault(k, set()).add(v)
 
   auth_cfg = msg.auth or bots_pb2.BotAuth()
 
   return _make_bot_group_config(
     require_luci_machine_token=auth_cfg.require_luci_machine_token,
     require_service_account=auth_cfg.require_service_account,
     ip_whitelist=auth_cfg.ip_whitelist,
     owners=tuple(msg.owners),
-    dimensions={k: sorted(v) for k, v in dimensions.iteritems()})
+    dimensions={k: sorted(v) for k, v in dimensions.iteritems()},
+    bot_config_script=msg.bot_config_script)
 
 
 def _expand_bot_id_expr(expr):
   """Expands string with bash-like sets (if they are there).
 
   E.g. takes "vm{1..3}-m1" and yields "vm1-m1", "vm2-m1", "vm3-m1". Also
   supports list syntax ({1,2,3}). Either one should be used, but not both, e.g.
   following WILL NOT work: {1..3,4,5}.
 
   Yields original string if it doesn't have '{...}' section.
@@ skipping 226 matching lines @@
         try:
           auth.Identity(auth.IDENTITY_USER, own)
         except ValueError:
           ctx.error('invalid owner email "%s"', own)
 
       # Validate 'dimensions'.
       for dim in entry.dimensions:
         if not local_config.validate_flat_dimension(dim):
           ctx.error('bad dimension %r', dim)
 
+      # Validate 'bot_config_script': the supplemental bot_config.py.
+      if entry.bot_config_script:
+        # Another check confirms that the script itself is valid python.
+        if not entry.bot_config_script.endswith('.py'):
+          ctx.error('Invalid bot_config_script name')
+        if os.path.basename(entry.bot_config_script) != entry.bot_config_script:
+          ctx.error('Invalid bot_config_script name')
+        # The file must exist and must be valid non empty python script.
+        path = 'scripts/' + entry.bot_config_script
+        if not config.get_self_config(path, store_last_good=True)[1]:
+          # Work around a problem in luci-config, this will require a full RPC
+          # but better be safe than sorry.
+          logging.warning('failed to find cached %s', path)
+          if not config.get_self_config(path)[1]:

[Vadim Sh., 2017/02/09 22:22:00]

I think this will not generally work. Luci config has a notion of "good config
set", a config at HEAD is promoted to "good config set" only after it passes all
the validations. get_self_config returns configs from "good config set".

When bot_config_script and the actual scrip are added in the same commit, the
script will not be visible yet from the validation hook, because the commit
(which is being validated right here) is not yet "good".

Consider just checking the name and be done with it.

I think there's also a way to tell luci-config to pass all "scripts/*.py" to
Swarming for validation (to check their syntax). It will be independent
validation rule.

[M-A Ruel, 2017/02/09 23:47:02]

Nodir can confirm if what I do is wrong. It's not a big deal, I just wanted to
help users that do a typo in their config file name but this won't be changed
frequently so it is not a big deal.
It was already implemented, it lives there:
https://github.com/luci/luci-py/blob/master/appengine/swarming/server/bot_cod...

[M-A Ruel, 2017/02/10 18:33:26]

I tested and confirmed it doesn't. Just removed this code and added a comment.

[nodir, 2017/02/10 18:41:43]

Sorry for being late, Vadim's reasoning is correct. The essential problem seems
to be that config validation is per file, cannot validate a set of config files

[M-A Ruel, 2017/02/10 19:53:19]

No big deal, the latest patchset is confirmed to work. I had to do a small fix
on the bot side due to unicode + coding header. <3 python.

+            ctx.error(
+                'Referenced bot_config_script %s must exist in scripts/' %
+                entry.bot_config_script)
+
   # Now verify bot_id_prefix is never a prefix of other prefix. It causes
   # ambiguities.
   for smaller, s_idx in bot_id_prefixes.iteritems():
     for larger, l_idx in bot_id_prefixes.iteritems():
       if smaller == larger:
         continue # we've already checked prefixes have no duplicated
       if larger.startswith(smaller):
         ctx.error(
             'bot_id_prefix "%s", defined in group #%d, is subprefix of "%s", '
             'defined in group #%d; it makes group assigned for bots with '
             'prefix "%s" ambigious', smaller, s_idx, larger, l_idx, larger)