[Fuego] [PATCH] ftc: process jenkins_enabled
Tim.Bird at sony.com
Tim.Bird at sony.com
Thu Feb 28 00:08:44 UTC 2019
See comments inline below.
> -----Original Message-----
> From: Daniel Sangorrin
>
> This patch allows skiping jenkins-related instructions when
> disabled.
>
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin at toshiba.co.jp>
> ---
> scripts/ftc | 68 +++++++++++++++++++++++++++++++++++-------------------
> -------
> 1 file changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/scripts/ftc b/scripts/ftc
> index f654885..633c1c6 100755
> --- a/scripts/ftc
> +++ b/scripts/ftc
> @@ -522,12 +522,7 @@ def wprint(msg):
>
> class config_class:
> def __init__(self, config_path):
> - # hardcode a few config vars
> - self.JENKINS_HOME = "/var/lib/jenkins"
> - jenkins_port = os.environ["JENKINS_PORT"]
> - self.JENKINS_URL = "http://localhost:"+jenkins_port+"/fuego"
> -
> - # now read configuration data from a file
> + # read configuration data from a file
> try:
> data = open(config_path, "r").read()
> except:
> @@ -536,6 +531,20 @@ class config_class:
> conf_map = self.parse_conf(data)
>
> # set values from conf_map
> + self.jenkins_enabled = conf_map.get("jenkins_enabled", "1") == "1"
> + if self.jenkins_enabled:
> + self.JENKINS_HOSTNAME = conf_map.get("jenkins_hostname",
> "localhost")
> + self.JENKINS_HOME = "/var/lib/jenkins"
> + try:
> + self.JENKINS_PORT = os.environ["JENKINS_PORT"]
> + self.JENKINS_URL =
> "http://"+self.JENKINS_HOSTNAME+":"+self.JENKINS_PORT+"/fuego"
> + except:
> + wprint("JENKINS_PORT not found in environment, using port 8080")
> + self.JENKINS_URL =
> "http://"+self.JENKINS_HOSTNAME+":8080/fuego"
> + else:
> + self.JENKINS_HOME = "none"
> + self.JENKINS_URL = "none"
> +
> self.host = conf_map.get("host_name", "localhost")
> self.host_name = conf_map.get("host_name", "localhost")
> self.server_type = conf_map.get("server_type", "fuego")
> @@ -1333,7 +1342,7 @@ def link_key(item):
> return "zzz+"+key
>
>
> -def create_job(board, test):
> +def create_job(conf, board, test):
> flot_link = '<flotile.FlotPublisher plugin="flot at 1.0-SNAPSHOT"/>'
>
> # prepare links for the descriptionsetter plugin
> @@ -1454,8 +1463,7 @@ ftc run-test -b $NODE_NAME -t {testdir} -s
> {testspec} \\
> job_name = board + "." + test.spec + "." + test.name
> print("Creating job " + job_name)
> try:
> - jenkins_port = os.environ["JENKINS_PORT"]
> - subprocess.call('java -jar /var/cache/jenkins/war/WEB-INF/jenkins-
> cli.jar -s http://localhost:'+jenkins_port+'/fuego create-job ' +
> + subprocess.call('java -jar /var/cache/jenkins/war/WEB-INF/jenkins-
> cli.jar -s '+conf.JENKINS_URL+' create-job ' +
This is a nice simplification.
> job_name + ' < ' + tmp, shell=True)
> os.unlink(tmp)
> except Exception as e:
> @@ -1464,7 +1472,7 @@ ftc run-test -b $NODE_NAME -t {testdir} -s
> {testspec} \\
> sys.exit(1)
>
>
> -def create_batch_job(board, testplan, plan_tests):
> +def create_batch_job(conf, board, testplan, plan_tests):
> tmp = "/tmp/fuego_tmp_batch"
> job_list = [board+'.'+test.spec+'.'+test.name for test in plan_tests]
>
> @@ -1504,8 +1512,7 @@ def create_batch_job(board, testplan, plan_tests):
> print("Creating batch job ")
> try:
> job_name = board+'.'+testplan+'.batch'
> - jenkins_port = os.environ["JENKINS_PORT"]
> - subprocess.call('java -jar /var/cache/jenkins/war/WEB-INF/jenkins-
> cli.jar -s http://localhost:'+jenkins_port+'/fuego create-job ' +
> + subprocess.call('java -jar /var/cache/jenkins/war/WEB-INF/jenkins-
> cli.jar -s '+conf.JENKINS_URL+' create-job ' +
> job_name + '< ' + tmp, shell=True)
> os.unlink(tmp)
> except Exception as e:
> @@ -1669,13 +1676,13 @@ def do_add_jobs(conf, options):
> if test_name:
> test = test_class(test_dict)
> for board_name in board_names:
> - create_job(board_name, test)
> + create_job(conf, board_name, test)
> else:
> plan_tests = parse_testplan(testplan, test_dict)
> for board_name in board_names:
> for test in plan_tests:
> - create_job(board_name, test)
> - create_batch_job(board_name, testplan, plan_tests)
> + create_job(conf, board_name, test)
> + create_batch_job(conf, board_name, testplan, plan_tests)
> sys.exit(0)
>
>
> @@ -3914,7 +3921,7 @@ def do_run_test(conf, options):
> build_data.keep_log = "false"
>
> # update all the Jenkins build-related files
> - if fuego_caller != "jenkins":
> + if fuego_caller != "jenkins" and conf.jenkins_enabled:
> job_dir = conf.JENKINS_HOME + "/jobs/" + build_data.job_name
> if not os.path.isdir(job_dir):
> print "Warning: no matching Jenkins job found. Not populating Jenkins
> build directory"
> @@ -4685,7 +4692,10 @@ def do_install_test(conf, options):
>
> # if running as root, switch to jenkins
> # for now, use sudo, but could change to use direct setuid calls
> -def user_check():
> +def user_check(conf):
> + if not conf.jenkins_enabled:
> + return
> +
OK - this is quite dicey from a security standpoint. We really shouldn't
be running things as root, even if we're inside the Docker container.
If ftc is running outside the Docker container, this is especially problematic.
I'm going to put a FIXTHIS on this, but it worries me that we might forget
and not come back to fix this up. If we're not using Jenkins, it might be worthwhile
to define a 'fuego' user account (in the Docker container or on the host), and
switch to that in this routine, so that operations done in the context of a test
are not performed as the root account.
This trusts the tests (which could be coming from a 3rd party or an untrusted source)
way too much.
> import getpass
>
> new_user = "jenkins"
> @@ -4940,13 +4950,13 @@ def main():
> # shows fuego boards
> do_list_boards(conf)
>
> - import jenkins
> - jenkins_port = os.environ["JENKINS_PORT"]
> - server = jenkins.Jenkins('http://localhost:'+jenkins_port+'/fuego')
> + if conf.jenkins_enabled:
> + import jenkins
> + server = jenkins.Jenkins(conf.JENKINS_URL)
>
> if command.startswith("add-job"):
> # adds Jenkins jobs
> - user_check()
> + user_check(conf)
> try:
> do_add_jobs(conf, options)
> except Exception as e:
> @@ -4954,7 +4964,7 @@ def main():
>
> if command.startswith("rm-job"):
> # removes Jenkins jobs
> - user_check()
> + user_check(conf)
> try:
> do_rm_jobs(conf, options)
> except Exception as e:
> @@ -4962,7 +4972,7 @@ def main():
>
> if command.startswith("add-node"):
> # adds Jenkins nodes
> - user_check()
> + user_check(conf)
> try:
> do_add_nodes(conf, options)
> except Exception as e:
> @@ -4970,7 +4980,7 @@ def main():
>
> if command.startswith("rm-node"):
> # removes Jenkins nodes
> - user_check()
> + user_check(conf)
> try:
> do_rm_nodes(conf, options)
> except Exception as e:
> @@ -4978,21 +4988,21 @@ def main():
>
> if command == "list-nodes":
> # shows jenkins nodes
> - user_check()
> + user_check(conf)
> do_list_nodes(conf)
>
> if command == "list-jobs":
> # shows jenkins jobs
> - user_check()
> + user_check(conf)
> do_list_jobs(conf)
>
> if command.startswith("build-job"):
> # build jenkins jobs
> - user_check()
> + user_check(conf)
> do_build_jobs(conf, options)
>
> if command == "add-view":
> - user_check()
> + user_check(conf)
> do_add_view(conf, options)
>
> if command == "list-plans":
> @@ -5046,7 +5056,7 @@ def main():
>
> if command == "run-test":
> # switch users, if needed
> - user_check()
> + user_check(conf)
> rcode, run_id = do_run_test(conf, options)
> # FIXTHIS - should do something with run_id here
> sys.exit(rcode)
> --
> 2.7.4
OK - looks good. I'll add the FIXTHIS I mentioned, and we should consider
the security ramifications of not switching to a non-root account, especially
considering the other work on running outside the container (natively on
the host).
Applied.
-- Tim
More information about the Fuego
mailing list