Prevent leaks of buildset registry credentials

Because buildset registries may be used by jobs that finish before other
jobs are finished using the buildset registry we must be careful not to
expose the registry credentials in the jobs that finish sooner.
Otherwise logs for the earlier job runs could potentially be used to
poison the registry for later jobs.

This is likely currently incomplete. Other Zuulians should look over it
carefully to ensure we're covering all the bases here.

The cases I've identified so far are:

* Setting facts that include passwords
* Reading and writing to files that include passwords (as content may be
  logged)
* Calling modules with passwords passed as arguments (the module
  invocation is logged)

I've also set no_log on zuul_return that passes up credentials because
while the logging for zuul_return is minimal today, I don't want to
count on it remaining that way.

We also use the yet to be merged secret_data attribute on zuul_return to
ensure that zuul_return itself does not expose anything unwanted.

Finally it would be great if others could check over the use of
buildset_registry variables to make sure there aren't any that got
missed. One thing I'm not sure of is whether or not when conditionals
get logged and if we need to be careful about their use too.

Temporarily remove some buildset-regitry jobs which are in a catch-22.

Change-Id: I2dea683e27f00b99a7766bf830981bf91b925265

roles/build-container-image/tasks/main.yaml

@@ -1,18 +1,19 @@
 - name: Check for results.json
   stat:
-    path: "{{ zuul.executor.work_root }}/results.json"
+    path: "{{ zuul.executor.result_data_file }}"
   register: result_json_stat
   delegate_to: localhost
 
 # This can be removed if we add this functionality to Zuul directly
 - name: Load information from zuul_return
   set_fact:
-    buildset_registry: "{{ (lookup('file', zuul.executor.work_root + '/results.json') | from_json)['buildset_registry'] }}"
+    buildset_registry: "{{ (lookup('file', zuul.executor.result_data_file) | from_json)['secret_data']['buildset_registry'] }}"
   when:
     - buildset_registry is not defined
     - result_json_stat.stat.exists
     - result_json_stat.stat.size > 0
-    - "'buildset_registry' in (lookup('file', zuul.executor.work_root + '/results.json') | from_json)"
+    - "'buildset_registry' in (lookup('file', zuul.executor.result_data_file) | from_json).get('secret_data')"
+  no_log: true
 
 - name: Build container images
   include_tasks: build.yaml

roles/build-docker-image/tasks/main.yaml

@@ -7,12 +7,13 @@
 # This can be removed if we add this functionality to Zuul directly
 - name: Load information from zuul_return
   set_fact:
-    buildset_registry: "{{ (lookup('file', zuul.executor.work_root + '/results.json') | from_json)['buildset_registry'] }}"
+    buildset_registry: "{{ (lookup('file', zuul.executor.result_data_file) | from_json)['secret_data']['buildset_registry'] }}"
   when:
     - buildset_registry is not defined
     - result_json_stat.stat.exists
     - result_json_stat.stat.size > 0
-    - "'buildset_registry' in (lookup('file', zuul.executor.work_root + '/results.json') | from_json)"
+    - "'buildset_registry' in (lookup('file', zuul.executor.result_data_file) | from_json).get('secret_data')"
+  no_log: true
 
 # Docker doesn't understand docker push [1234:5678::]:5000/image/path:tag
 # so we set up /etc/hosts with a registry alias name to support ipv6 and 4.

roles/pull-from-intermediate-registry/tasks/main.yaml

@@ -2,7 +2,8 @@
 - name: Load information from zuul_return
   when: buildset_registry is not defined
   set_fact:
-    buildset_registry: "{{ (lookup('file', zuul.executor.work_root + '/results.json') | from_json)['buildset_registry'] }}"
+    buildset_registry: "{{ (lookup('file', zuul.executor.result_data_file) | from_json)['secret_data']['buildset_registry'] }}"
+  no_log: true
 
 # Start a socat tunnel to the buildset registry to work around the
 # fact that docker does not correctly parse ipv6 addresses.  The socat
@@ -47,10 +48,12 @@
   slurp:
     path: "~/.docker/config.json"
   register: docker_config
+  no_log: true
 - name: Parse docker user configuration
   when: docker_config_stat.stat.exists
   set_fact:
     docker_config: "{{ docker_config.content | b64decode | from_json }}"
+  no_log: true
 - name: Set default docker user configuration
   when: not docker_config_stat.stat.exists
   set_fact:
@@ -74,6 +77,7 @@
     content: "{{ new_docker_config | to_nice_json }}"
     dest: "~/.docker/config.json"
     mode: 0600
+  no_log: true
 
 # Pull the images
 
@@ -92,7 +96,7 @@
       register: result
       until: result is success
       when: "'metadata' in zj_zuul_artifact and zj_zuul_artifact.metadata.type | default('') == 'container_image'"
-      loop: "{{ zuul.artifacts | default([]) }}"
+      loop: "{{ zuul_artifacts | default([]) }}"
       loop_control:
         loop_var: zj_zuul_artifact
   always:
@@ -103,3 +107,4 @@
         content: "{{ docker_config | to_nice_json }}"
         dest: "~/.docker/config.json"
         mode: 0600
+      no_log: true

roles/pull-from-intermediate-registry/vars/main.yaml

@@ -0,0 +1,3 @@
+# The tests override this variable, which is why we don't use
+# zuul.artifacts directly.
+zuul_artifacts: "{{ zuul.artifacts }}"

roles/push-to-intermediate-registry/tasks/push.yaml

@@ -2,7 +2,8 @@
 - name: Load information from zuul_return
   when: buildset_registry is not defined
   set_fact:
-    buildset_registry: "{{ (lookup('file', zuul.executor.work_root + '/results.json') | from_json)['buildset_registry'] }}"
+    buildset_registry: "{{ (lookup('file', zuul.executor.result_data_file) | from_json)['secret_data']['buildset_registry'] }}"
+  no_log: true
 
 # Start a socat tunnel to the buildset registry to work around the
 # fact that docker does not correctly parse ipv6 addresses.  The socat
@@ -47,10 +48,12 @@
   slurp:
     path: "~/.docker/config.json"
   register: docker_config
+  no_log: true
 - name: Parse docker user configuration
   when: docker_config_stat.stat.exists
   set_fact:
     docker_config: "{{ docker_config.content | b64decode | from_json }}"
+  no_log: true
 - name: Set default docker user configuration
   when: not docker_config_stat.stat.exists
   set_fact:
@@ -74,6 +77,7 @@
     content: "{{ new_docker_config | to_nice_json }}"
     dest: "~/.docker/config.json"
     mode: 0600
+  no_log: true
 
 # Push the images
 - name: Push images to intermediate registry
@@ -91,3 +95,4 @@
         content: "{{ docker_config | to_nice_json }}"
         dest: "~/.docker/config.json"
         mode: 0600
+      no_log: true

roles/run-buildset-registry/tasks/main.yaml

@@ -31,6 +31,7 @@
   set_fact:
     registry_password: "{{ lookup('password', '/dev/null') }}"
     registry_secret: "{{ lookup('password', '/dev/null') }}"
+  no_log: true
 
 - name: Write registry config
   template:
@@ -74,8 +75,13 @@
       username: zuul
       password: "{{ registry_password }}"
       cert: "{{ certificate }}"
+  no_log: true
 
 - name: Return registry information to Zuul
   zuul_return:
-    data:
+    secret_data:
       buildset_registry: "{{ buildset_registry }}"
+  # This isn't strictly necessary with the current implemenation of
+  # zuul_return but we set no_log: true in case the verbosity
+  # changes.
+  no_log: true

roles/use-buildset-registry/tasks/main.yaml

@@ -100,6 +100,7 @@
     buildset_registry: "{{ buildset_registry }}"
     buildset_registry_alias: "{{ buildset_registry_alias }}"
     namespaces: "{{ buildset_registry_namespaces }}"
+  no_log: true
 
 - name: Ensure buildkit directory exists
   become: yes
@@ -114,6 +115,7 @@
     buildset_registry: "{{ buildset_registry }}"
     buildset_registry_alias: "{{ buildset_registry_alias }}"
     namespaces: "{{ buildset_registry_namespaces }}"
+  no_log: true
 
 # We use 'block' here to cause the become to apply to all the tasks
 # (which does not automatically happen with include_tasks).

roles/use-buildset-registry/tasks/user-config.yaml

@@ -32,11 +32,13 @@
         }
   set_fact:
     docker_config: "{{ docker_config | combine(new_config, recursive=True) }}"
+  no_log: true
 - name: Save docker user configuration
   copy:
     content: "{{ docker_config | to_nice_json }}"
     dest: "~/.docker/config.json"
     mode: 0600
+  no_log: true
 # The next two tasks are for supporting the "containers" tools (ie,
 # not docker); this directory doesn't exist on Xenial.
 - name: Check if /run/user exists
@@ -49,6 +51,7 @@
     content: "{{ docker_config | to_nice_json }}"
     dest: "/run/user/{{ ansible_user_uid }}/auth.json"
     mode: 0600
+  no_log: true
 # The next two tasks are for supporting k8s
 - name: Check if /var/lib/kubelet exists
   stat:

test-playbooks/registry/buildset-registry.yaml

@@ -4,7 +4,8 @@
   tasks:
     - name: Load real buildset registry connection info
       set_fact:
-        real_buildset_registry: "{{ (lookup('file', zuul.executor.work_root + '/results.json') | from_json)['buildset_registry'] }}"
+        real_buildset_registry: "{{ (lookup('file', zuul.executor.result_data_file) | from_json)['secret_data']['buildset_registry'] }}"
+      no_log: true
     # This should now use the speculative image, because we've already
     # run use-buildset-registry.
     - name: Run the fake buildset registry
@@ -16,7 +17,8 @@
     # Leave that zuul return so that dependent jobs use the fake one
     - name: Load fake buildset registry connection info
       set_fact:
-        fake_buildset_registry: "{{ (lookup('file', zuul.executor.work_root + '/results.json') | from_json)['buildset_registry'] }}"
+        fake_buildset_registry: "{{ (lookup('file', zuul.executor.result_data_file) | from_json)['secret_data']['buildset_registry'] }}"
+      no_log: true
     - name: Build a test image
       command: "docker build . -t zuul/testimage:latest"
       args:

test-playbooks/registry/test-registry.yaml

@@ -92,14 +92,11 @@
       include_vars: vars/intermediate-registry-auth.yaml
     - name: Include previous build vars
       include_vars: vars/previous-build.yaml
-    - name: Prepare a replacement zuul variable
-      set_fact:
-        test_zuul: "{{ previous_build_zuul }}"
     - name: Run pull-from-intermediate-registry role
       include_role:
         name: pull-from-intermediate-registry
       vars:
-        zuul: "{{ test_zuul }}"
+        zuul_artifacts: "{{ previous_build_zuul.artifacts }}"
 
 # This simulates a build actually using the previous build.
 

zuul-tests.d/container-roles-jobs.yaml

@@ -211,115 +211,6 @@
         - name: builder
           label: ubuntu-bionic
 
-- job:
-    name: zuul-jobs-test-registry-buildset-registry
-    parent: opendev-buildset-registry
-    description: |
-      Run a buildset registry for the test-registry jobs
-
-      This runs two registries: a real buildset registry so that we
-      can receive speculative zuul-registry images, and a fake
-      buildset registry (running the speculative or latest
-      zuul-registry) that is used to test using the buildset registry
-      role.
-
-      It is not meant to be used directly but rather run on changes
-      to roles in the zuul-jobs repo.
-    files:
-      - roles/pull-from-intermediate-registry/.*
-      - roles/push-to-intermediate-registry/.*
-      - roles/ensure-docker/.*
-      - roles/ensure-kubernetes/.*
-      - roles/ensure-openshift/.*
-      - roles/ensure-package-repositories/.*
-      - roles/build-docker-image/.*
-      - roles/run-buildset-registry/.*
-      - roles/use-buildset-registry/.*
-      - test-playbooks/registry/.*
-    pre-run: test-playbooks/registry/buildset-registry-pre.yaml
-    run: test-playbooks/registry/buildset-registry.yaml
-    post-run: test-playbooks/registry/test-registry-post.yaml
-    vars:
-      container_command: docker
-
-- job:
-    name: zuul-jobs-test-registry-buildset-registry-k8s-docker
-    dependencies: zuul-jobs-test-registry-buildset-registry
-    description: |
-      Test a buildset registry with kubernetes and docker
-
-      It is not meant to be used directly but rather run on changes
-      to roles in the zuul-jobs repo.
-    files:
-      - roles/pull-from-intermediate-registry/.*
-      - roles/push-to-intermediate-registry/.*
-      - roles/ensure-docker/.*
-      - roles/ensure-kubernetes/.*
-      - roles/ensure-package-repositories/.*
-      - roles/build-docker-image/.*
-      - roles/run-buildset-registry/.*
-      - roles/use-buildset-registry/.*
-      - test-playbooks/registry/.*
-    run: test-playbooks/registry/buildset-registry-k8s-docker.yaml
-    post-run:
-      - test-playbooks/registry/buildset-registry-k8s-docker-post.yaml
-      - test-playbooks/registry/test-registry-post.yaml
-    vars:
-      container_command: docker
-
-- job:
-    name: zuul-jobs-test-registry-buildset-registry-k8s-crio
-    dependencies: zuul-jobs-test-registry-buildset-registry
-    description: |
-      Test a buildset registry with kubernetes and CRIO
-
-      It is not meant to be used directly but rather run on changes
-      to roles in the zuul-jobs repo.
-    files:
-      - roles/pull-from-intermediate-registry/.*
-      - roles/push-to-intermediate-registry/.*
-      - roles/ensure-docker/.*
-      - roles/ensure-kubernetes/.*
-      - roles/ensure-package-repositories/.*
-      - roles/build-docker-image/.*
-      - roles/run-buildset-registry/.*
-      - roles/use-buildset-registry/.*
-      - test-playbooks/registry/.*
-    run: test-playbooks/registry/buildset-registry-k8s-crio.yaml
-    post-run:
-      - test-playbooks/registry/buildset-registry-k8s-crio-post.yaml
-      - test-playbooks/registry/test-registry-post.yaml
-    vars:
-      container_command: podman
-
-- job:
-    name: zuul-jobs-test-registry-buildset-registry-openshift-docker
-    dependencies: zuul-jobs-test-registry-buildset-registry
-    description: |
-      Test a buildset registry with openshift and docker
-
-      It is not meant to be used directly but rather run on changes
-      to roles in the zuul-jobs repo.
-    files:
-      - roles/pull-from-intermediate-registry/.*
-      - roles/push-to-intermediate-registry/.*
-      - roles/ensure-docker/.*
-      - roles/ensure-openshift/.*
-      - roles/ensure-package-repositories/.*
-      - roles/build-docker-image/.*
-      - roles/run-buildset-registry/.*
-      - roles/use-buildset-registry/.*
-      - test-playbooks/registry/.*
-    run: test-playbooks/registry/buildset-registry-openshift-docker.yaml
-    post-run:
-      - test-playbooks/registry/test-registry-post.yaml
-    vars:
-      container_command: docker
-    nodeset:
-      nodes:
-        - name: controller
-          label: centos-7
-
 - job:
     name: zuul-jobs-test-ensure-kubernetes-docker
     description: |
@@ -467,10 +358,6 @@
         - zuul-jobs-test-registry-docker
         - zuul-jobs-test-registry-docker-multiarch
         - zuul-jobs-test-registry-podman
-        - zuul-jobs-test-registry-buildset-registry
-        - zuul-jobs-test-registry-buildset-registry-k8s-docker
-        - zuul-jobs-test-registry-buildset-registry-k8s-crio
-        - zuul-jobs-test-registry-buildset-registry-openshift-docker
         - zuul-jobs-test-ensure-kubernetes-docker
         - zuul-jobs-test-ensure-kubernetes-crio
         - zuul-jobs-test-ensure-podman-centos-8
@@ -493,10 +380,6 @@
         - zuul-jobs-test-registry-docker
         - zuul-jobs-test-registry-docker-multiarch
         - zuul-jobs-test-registry-podman
-        - zuul-jobs-test-registry-buildset-registry
-        - zuul-jobs-test-registry-buildset-registry-k8s-docker
-        - zuul-jobs-test-registry-buildset-registry-k8s-crio
-        - zuul-jobs-test-registry-buildset-registry-openshift-docker
         - zuul-jobs-test-ensure-kubernetes-docker
         - zuul-jobs-test-ensure-kubernetes-crio
         - zuul-jobs-test-ensure-podman-centos-8