swarming: Add server-side implementation for supplemental bot_config

appengine/swarming/server/bot_groups_config.py

Index: appengine/swarming/server/bot_groups_config.py
diff --git a/appengine/swarming/server/bot_groups_config.py b/appengine/swarming/server/bot_groups_config.py
index 962a36087159b45a6dc360a5e357410a9ba8def0..1be560ac9d2587ca80a5ec70f415024d3abfe726 100644
--- a/appengine/swarming/server/bot_groups_config.py
+++ b/appengine/swarming/server/bot_groups_config.py
@@ -7,6 +7,7 @@
 import collections
 import hashlib
 import logging
+import os
 import re
 
 from components import auth
@@ -47,6 +48,10 @@ BotGroupConfig = collections.namedtuple('BotGroupConfig', [
   # 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',
 ])
 
 
@@ -71,7 +76,8 @@ _DEFAULT_BOT_GROUPS = _BotGroups(
         require_service_account=None,
         ip_whitelist=auth.BOTS_IP_WHITELIST,
         owners=(),
-        dimensions={}))
+        dimensions={},
+        bot_config_script=''))
 
 
 def _gen_version(fields):
@@ -145,7 +151,8 @@ def _bot_group_proto_to_tuple(msg, trusted_dimensions):
     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):
@@ -392,6 +399,24 @@ def validate_settings(cfg, ctx):
         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():