Don't log tracebacks on variable name errors

If a job return an illegal variable name in the return data, or
the user otherwise tries to get the executor to run with such a
variable, log the error but do not log a traceback.

Change-Id: I6a5fb8011a0500d04f5a18d937fe410419f05c6c

zuul/exceptions.py

@@ -54,6 +54,10 @@ class DependencyLimitExceededError(Exception):
     pass
 
 
+class VariableNameError(Exception):
+    pass
+
+
 # Authentication Exceptions
 
 class AuthTokenException(Exception):

zuul/executor/server.py

@@ -41,6 +41,7 @@ import git
 from urllib.parse import urlsplit
 from opentelemetry import trace
 
+from zuul.exceptions import VariableNameError
 from zuul.lib.ansible import AnsibleManager
 from zuul.lib.result_data import get_warnings_from_result_data
 from zuul.lib import yamlutil as yaml
@@ -1599,7 +1600,11 @@ class AnsibleJob(object):
         self.preparePlaybooks(args)
         self.writeLoggingConfig()
         zuul_resources = self.prepareNodes(args)  # set self.host_list
-        self.prepareVars(args, zuul_resources)   # set self.original_hostvars
+        try:
+            # set self.original_hostvars
+            self.prepareVars(args, zuul_resources)
+        except VariableNameError as e:
+            raise ExecutorError(str(e))
         self.writeDebugInventory()
         self.writeRepoStateFile(repos)
 
@@ -1690,6 +1695,8 @@ class AnsibleJob(object):
             secret_data_copy = data.copy()
             secret_data_copy.pop('zuul', None)
             check_varnames(secret_data_copy)
+        except VariableNameError as e:
+            self.log.warning("Unable to load result data: %s", str(e))
         except Exception:
             self.log.exception("Unable to load result data:")
         return data, secret_data

zuul/lib/varnames.py

@@ -13,6 +13,8 @@
 # under the License.
 import re
 
+from zuul.exceptions import VariableNameError
+
 
 VARNAME_RE = re.compile(r'^[A-Za-z0-9_]+$')
 
@@ -21,16 +23,19 @@ def check_varnames(var):
     # We block these in configloader, but block it here too to make
     # sure that a job doesn't pass variables named zuul or nodepool.
     if 'zuul' in var:
-        raise Exception("Defining variables named 'zuul' is not allowed")
+        raise VariableNameError(
+            "Defining variables named 'zuul' is not allowed")
     if 'nodepool' in var:
-        raise Exception("Defining variables named 'nodepool' is not allowed")
+        raise VariableNameError(
+            "Defining variables named 'nodepool' is not allowed")
     if 'unsafe_vars' in var:
-        raise Exception("Defining variables named 'unsafe_vars' "
-                        "is not allowed")
+        raise VariableNameError("Defining variables named 'unsafe_vars' "
+                                "is not allowed")
     for varname in var.keys():
         if not VARNAME_RE.match(varname):
-            raise Exception("Variable names may only contain letters, "
-                            "numbers, and underscores")
+            raise VariableNameError(
+                "Variable names may only contain letters, "
+                "numbers, and underscores")
     # Block some connection related variables so they cannot be
     # overridden by jobs to bypass security mechanisms.
     connection_vars = [
@@ -42,4 +47,5 @@ def check_varnames(var):
     ]
     for conn_var in connection_vars:
         if conn_var in var:
-            raise Exception(f"Variable name '{conn_var}' is not allowed.")
+            raise VariableNameError(
+                f"Variable name '{conn_var}' is not allowed.")