Make octocatalog-diff cache dependent on workspace

Also tweak ocf::privatefile a bit

.gitignore

@@ -13,3 +13,6 @@
 
 # ignore Puppet resource types
 /.resource_types/
+
+# ignore octocatalog-diff cache
+/.octocatalog-diff-cache

.octocatalog-diff.cfg.rb

@@ -31,16 +31,22 @@ def self.config
       settings[:bootstrap_script] = 'bin/bootstrap'
       settings[:puppet_binary] = '/opt/puppetlabs/bin/puppet'
 
-      # TODO: Set this back to origin/master once my changes are merged to master
+      # TODO: Set this to origin/master once the octocatalog-diff changes are
+      # merged to master
       settings[:from_env] = 'origin/octocatalog-diff-test'
       # This is used to cache third-party/vendored modules so that they don't
-      # have to be installed each time (saves about 2 minutes per run)
+      # have to be installed each time (saves about 2 minutes per run, at least
+      # on a host with relatively slow I/O like a staff VM, this is much faster
+      # on reaper for instance)
       settings[:master_cache_branch] = settings[:from_env]
 
       settings[:validate_references] = %w(before notify require subscribe)
       settings[:header] = :default
 
-      settings[:cached_master_dir] = File.join(ENV['HOME'], '.octocatalog-diff-cache')
+      settings[:cached_master_dir] = File.join(
+        (ENV['WORKSPACE'] || File.join(ENV['HOME'], '.cache')),
+        '.octocatalog-diff-cache'
+      )
       settings[:safe_to_delete_cached_master_dir] = settings[:cached_master_dir]
 
       settings[:basedir] = Dir.pwd

Jenkinsfile

@@ -28,6 +28,21 @@ pipeline {
       }
     }
 
+    stage('octocatalog-diff') {
+      // Don't run this on the master branch yet, since it's really made for
+      // testing PRs and changes, it should always show no diffs on master.
+      // However, it might be useful on master in the future in some kind of
+      // mode to just show that all catalogs actually compile.
+      when {
+        not {
+          branch 'master'
+        }
+      }
+      steps {
+        sh 'make all_diffs'
+      }
+    }
+
     stage('update-prod') {
       when {
         branch 'master'

Makefile

@@ -1,3 +1,5 @@
+WORKSPACE ?= ${HOME}/.cache
+
 .PHONY: all
 all: vendor install-hooks
 
@@ -18,11 +20,19 @@ install-hooks: venv
 vendor: Puppetfile
 	r10k puppetfile install --verbose --color
 
+.PHONY: octocatalog-diff-uncached
+$(WORKSPACE)/.octocatalog-diff-cache:
+octocatalog-diff-uncached:
+	octocatalog-diff \
+		--bootstrap-then-exit \
+		--bootstrapped-from-dir=$(WORKSPACE)/.octocatalog-diff-cache
+
 # Run octocatalog-diff for a particular hostname
 # Add a --debug flag to get much more verbose output
-diff_%:
+diff_%: $(WORKSPACE)/.octocatalog-diff-cache
 	octocatalog-diff \
 		-n $*.ocf.berkeley.edu \
+		--bootstrapped-to-dir=$(WORKSPACE)/.octocatalog-diff-cache \
 		--enc-override environment=production,parameters::use_private_share=false \
 		--ignore 'Ini_setting[puppet.conf/master/storeconfigs]' \
 		--ignore 'Ini_setting[puppet.conf/master/storeconfigs_backend]' \
@@ -33,8 +43,9 @@ diff_%:
 
 # Run octocatalog-diff across all nodes that can be fetched from puppetdb
 # TODO: Make this faster by just selecting a single node from each class we
-# care about (or not selecting all desktops/hozers for example)
-all_diffs:
+# care about (not selecting all desktops/hozers for example)
+#all_diffs: $(WORKSPACE)/.octocatalog-diff-cache
+all_diffs: octocatalog-diff-uncached
 	curl -s --tlsv1 \
 		--cacert /etc/ocfweb/puppet-certs/puppet-ca.pem \
 		--cert /etc/ocfweb/puppet-certs/puppet-cert.pem \
@@ -43,4 +54,4 @@ all_diffs:
 	| jq -r '.[] | .certname' \
 	| cut -d '.' -f 1 \
 	| sort \
-	| xargs -n 1 -P $(shell grep -c ^processor /proc/cpuinfo) -I @ $(MAKE) -i -s diff_@
+	| xargs -n 1 -P $(shell grep -c ^processor /proc/cpuinfo) -I @ $(MAKE) -s diff_@

modules/ocf/manifests/kerberos.pp

@@ -23,14 +23,13 @@
   ocf::privatefile { '/etc/krb5.keytab':
     mode   => '0600',
     source => 'puppet:///private/krb5.keytab',
-    before => Augeas['/etc/ssh/sshd_config/GSSAPIKeyExchange'],
   }
 
   # enable SSH host key verification
   augeas { '/etc/ssh/sshd_config/GSSAPIKeyExchange':
     context => '/files/etc/ssh/sshd_config',
     changes => 'set GSSAPIKeyExchange yes',
-    require => [ Package['openssh-server'], File['/etc/krb5.conf'] ],
+    require => [ Package['openssh-server'], File['/etc/krb5.conf'], Ocf::Privatefile['/etc/krb5.keytab'] ],
     notify  => Service['ssh']
   }
 

modules/ocf/manifests/privatefile.pp

@@ -1,3 +1,20 @@
+# This class exists to abstract out accesses to the puppet private share so
+# that octocatalog-diff can generally ignore them and replace the file contents
+# with dummy contents. It does not and should not have access to the private
+# share, since it could accidentally leak secrets and will show output in
+# jenkins logs that are public.
+#
+# Initially this defined type didn't exist and conditionals using the
+# $::use_private_share conditional were used around everything that used the
+# private share. This worked ok, but led to dependency issues where a resource
+# that would normally exist was then hidden behind a conditional and wouldn't
+# be seen by octocatalog-diff. This approach is better because it can provide a
+# dummy file and a more consistent experience across all usages of the private
+# share.
+#
+# This also gives a convenient interface to enforce file parameters like
+# show_diff and backup that we don't want to be set to true for any private
+# resources.
 define ocf::privatefile(
   String $path = $title,
   Optional[String] $source = undef,
@@ -27,10 +44,7 @@
 
   if $::use_private_share {
     if $content_path {
-      # For some reason, puppet-lint thinks the file function call (or
-      # $content) are top-scope variables when they are not, so the check is
-      # ignored for now.
-      $file_opts = $opts + {content => file($content)} # lint:ignore:variable_scope
+      $file_opts = $opts + {content => file($content_path)}
     } elsif $source {
       $file_opts = $opts + {source => $source}
     }

modules/ocf_apphost/manifests/lets_encrypt.pp

@@ -12,7 +12,6 @@
     source => 'puppet:///private/lets-encrypt-vhost.key',
     owner  => ocfletsencrypt,
     mode   => '0400',
-    before => Cron['lets-encrypt-update'],
   }
 
   if $::host_env == 'prod' {
@@ -21,7 +20,8 @@
       user        => ocfletsencrypt,
       environment => ['MAILTO=root', 'PATH=/bin:/usr/bin:/usr/local/bin'],
       special     => hourly,
-      require     => File['/usr/local/bin/lets-encrypt-update'],
+      require     => [File['/usr/local/bin/lets-encrypt-update'],
+                      Ocf::Privatefile['/etc/ssl/lets-encrypt/le-vhost.key']],
     }
   }
 }

modules/ocf_ldap/manifests/init.pp

@@ -10,6 +10,7 @@
         '/etc/ldap/schema/puppet.schema',
         '/etc/ldap/sasl2/slapd.conf',
         '/etc/ldap/slapd.conf'],
+      Ocf::Privatefile['/etc/ldap/krb5.keytab'],
       Augeas['/etc/default/slapd'],
       Class['ocf::ssl::default'],
     ],
@@ -44,7 +45,6 @@
     group   => openldap,
     mode    => '0600',
     require => Package['slapd', 'heimdal-clients'],
-    notify  => Service['slapd'],
   }
 
   augeas { '/etc/default/slapd':
@@ -94,7 +94,8 @@
       '/var/backups/ldap/.git/hooks/post-commit':
         content => "git push -q [email protected]:ocf/ldap master\n",
         mode    => '0755',
-        require => Package['ldap-git-backup'];
+        require => [Package['ldap-git-backup'],
+                    Ocf::Privatefile['/root/.ssh/id_rsa']];
 
       '/root/.ssh':
         ensure => directory,
@@ -109,7 +110,6 @@
     ocf::privatefile { '/root/.ssh/id_rsa':
       source => 'puppet:///private/id_rsa',
       mode   => '0600',
-      before => File['/var/backups/ldap/.git/hooks/post-commit'];
     }
   }
 

modules/ocf_mail/manifests/site_ocf.pp

@@ -44,6 +44,7 @@
       special => 'hourly',
       require => [
         File['/usr/local/sbin/update-cred-cache'],
+        Ocf::Privatefile['/etc/postfix/ocf/smtp-krb5.keytab'],
         Service['postfix']
       ];
 
@@ -53,6 +54,7 @@
       special => 'reboot',
       require => [
         File['/usr/local/sbin/update-cred-cache'],
+        Ocf::Privatefile['/etc/postfix/ocf/smtp-krb5.keytab'],
         Service['postfix']
       ];
   }
@@ -63,7 +65,6 @@
     group   => root,
     source  => 'puppet:///private/smtp-krb5.keytab',
     require => Package['postfix'],
-    before  => Cron['update-cred-cache', 'update-cred-cache-reboot'],
   }
 
   file {

modules/ocf_ocfweb/manifests/dev_config.pp

@@ -26,7 +26,8 @@
     'octocatalog-diff':;
 
     # Newer puppetdb-termini versions (6.5.0-1stretch for instance) have an
-    # issue with CRLs
+    # issue with CRLs when used with octocatalog-diff, so pin this to 6.4.0 for
+    # now
     'puppetdb-termini':
       ensure => '6.4.0-1stretch';
   }

modules/ocf_www/manifests/lets_encrypt.pp

@@ -12,7 +12,6 @@
     source => 'puppet:///private/lets-encrypt-vhost.key',
     owner  => ocfletsencrypt,
     mode   => '0400',
-    before => Cron['lets-encrypt-update'],
   }
 
   if $::host_env == 'prod' {
@@ -21,7 +20,8 @@
       user        => ocfletsencrypt,
       environment => ['MAILTO=root', 'PATH=/bin:/usr/bin:/usr/local/bin'],
       special     => hourly,
-      require     => File['/usr/local/bin/lets-encrypt-update'],
+      require     => [File['/usr/local/bin/lets-encrypt-update'],
+                      Ocf::Privatefile['/etc/ssl/lets-encrypt/le-vhost.key']],
     }
   }
 }