From d94bc312c8b2aa7cc7a9e71ee03cd62901618974 Mon Sep 17 00:00:00 2001 From: Jason Perrin Date: Sun, 8 Sep 2019 05:17:06 -0700 Subject: [PATCH] Make octocatalog-diff cache dependent on workspace Also tweak ocf::privatefile a bit --- .gitignore | 3 +++ .octocatalog-diff.cfg.rb | 12 +++++++--- Jenkinsfile | 15 +++++++++++++ Makefile | 19 ++++++++++++---- modules/ocf/manifests/kerberos.pp | 3 +-- modules/ocf/manifests/privatefile.pp | 22 +++++++++++++++---- modules/ocf_apphost/manifests/lets_encrypt.pp | 4 ++-- modules/ocf_ldap/manifests/init.pp | 6 ++--- modules/ocf_mail/manifests/site_ocf.pp | 3 ++- modules/ocf_ocfweb/manifests/dev_config.pp | 3 ++- modules/ocf_www/manifests/lets_encrypt.pp | 4 ++-- 11 files changed, 72 insertions(+), 22 deletions(-) diff --git a/.gitignore b/.gitignore index af92d7e61..e0e2247b3 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,6 @@ # ignore Puppet resource types /.resource_types/ + +# ignore octocatalog-diff cache +/.octocatalog-diff-cache diff --git a/.octocatalog-diff.cfg.rb b/.octocatalog-diff.cfg.rb index af873c73f..66d9279f3 100644 --- a/.octocatalog-diff.cfg.rb +++ b/.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 diff --git a/Jenkinsfile b/Jenkinsfile index ebf7f482f..e86b2cfe1 100644 --- a/Jenkinsfile +++ b/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' diff --git a/Makefile b/Makefile index c0a64970a..582140f5f 100644 --- a/Makefile +++ b/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_@ diff --git a/modules/ocf/manifests/kerberos.pp b/modules/ocf/manifests/kerberos.pp index a7bd55092..0426f8113 100644 --- a/modules/ocf/manifests/kerberos.pp +++ b/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'] } diff --git a/modules/ocf/manifests/privatefile.pp b/modules/ocf/manifests/privatefile.pp index 07c3fa695..d86b3392e 100644 --- a/modules/ocf/manifests/privatefile.pp +++ b/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} } diff --git a/modules/ocf_apphost/manifests/lets_encrypt.pp b/modules/ocf_apphost/manifests/lets_encrypt.pp index 1770c1f8b..cd8198e1e 100644 --- a/modules/ocf_apphost/manifests/lets_encrypt.pp +++ b/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']], } } } diff --git a/modules/ocf_ldap/manifests/init.pp b/modules/ocf_ldap/manifests/init.pp index 3f4da549e..29d7d00b0 100644 --- a/modules/ocf_ldap/manifests/init.pp +++ b/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 git@github.com: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']; } } diff --git a/modules/ocf_mail/manifests/site_ocf.pp b/modules/ocf_mail/manifests/site_ocf.pp index c87a7b75c..89de85d9c 100644 --- a/modules/ocf_mail/manifests/site_ocf.pp +++ b/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 { diff --git a/modules/ocf_ocfweb/manifests/dev_config.pp b/modules/ocf_ocfweb/manifests/dev_config.pp index 79c8f5a4c..6d5feb9f8 100644 --- a/modules/ocf_ocfweb/manifests/dev_config.pp +++ b/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'; } diff --git a/modules/ocf_www/manifests/lets_encrypt.pp b/modules/ocf_www/manifests/lets_encrypt.pp index efe87a355..a40addf19 100644 --- a/modules/ocf_www/manifests/lets_encrypt.pp +++ b/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']], } } }