From bee881a64bac108b80a9fd08a1bb0b0535579890 Mon Sep 17 00:00:00 2001 From: Ron Aughenbaugh Date: Wed, 21 Aug 2019 14:46:47 -0400 Subject: [PATCH 1/6] WIP: Manage db.properties with augeas --- .fixtures.yml | 1 + manifests/config.pp | 109 ++++++++++++++++++++++--------- manifests/init.pp | 1 + spec/classes/artifactory_spec.rb | 57 ++++++++++++++++ 4 files changed, 138 insertions(+), 30 deletions(-) diff --git a/.fixtures.yml b/.fixtures.yml index 65a5820..bd363ef 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -1,5 +1,6 @@ fixtures: forge_modules: + augeas: "puppetlabs/augeas_core" stdlib: "puppetlabs/stdlib" java: "puppetlabs/java" yumrepo_core: "puppetlabs/yumrepo_core" diff --git a/manifests/config.pp b/manifests/config.pp index 20c7b9b..7342873 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -15,39 +15,88 @@ $::artifactory::db_password and $::artifactory::db_type ) { + if $::artifactory::use_temp_db_secrets { + file { "${::artifactory::artifactory_home}/etc/.secrets": + ensure => directory, + owner => 'artifactory', + group => 'artifactory', + } - file { "${::artifactory::artifactory_home}/etc/.secrets": - ensure => directory, - owner => 'artifactory', - group => 'artifactory', - } + file { "${::artifactory::artifactory_home}/etc/.secrets/.temp.db.properties": + ensure => file, + content => epp( + 'artifactory/db.properties.epp', + { + db_url => $::artifactory::db_url, + db_username => $::artifactory::db_username, + db_password => $::artifactory::db_password, + db_type => $::artifactory::db_type, + binary_provider_type => $::artifactory::binary_provider_type, + pool_max_active => $::artifactory::pool_max_active, + pool_max_idle => $::artifactory::pool_max_idle, + binary_provider_cache_maxsize => $::artifactory::binary_provider_cache_maxsize, + binary_provider_base_data_dir => $::artifactory::binary_provider_base_data_dir, + binary_provider_filesystem_dir => $::artifactory::binary_provider_filesystem_dir, + binary_provider_cache_dir => $::artifactory::binary_provider_cache_dir, + } + ), + mode => '0640', + owner => 'artifactory', + group => 'artifactory', + } - file { "${::artifactory::artifactory_home}/etc/.secrets/.temp.db.properties": - ensure => file, - content => epp( - 'artifactory/db.properties.epp', - { - db_url => $::artifactory::db_url, - db_username => $::artifactory::db_username, - db_password => $::artifactory::db_password, - db_type => $::artifactory::db_type, - binary_provider_type => $::artifactory::binary_provider_type, - pool_max_active => $::artifactory::pool_max_active, - pool_max_idle => $::artifactory::pool_max_idle, - binary_provider_cache_maxsize => $::artifactory::binary_provider_cache_maxsize, - binary_provider_base_data_dir => $::artifactory::binary_provider_base_data_dir, - binary_provider_filesystem_dir => $::artifactory::binary_provider_filesystem_dir, - binary_provider_cache_dir => $::artifactory::binary_provider_cache_dir, - } - ), - mode => '0640', - owner => 'artifactory', - group => 'artifactory', - } + file { "${::artifactory::artifactory_home}/etc/storage.properties": + ensure => link, + target => "${::artifactory::artifactory_home}/etc/.secrets/.temp.db.properties", + } + + } else { + # Make sure we have correct mode and ownership + file { "${::artifactory::artifactory_home}/etc/db.properties": + ensure => file, + mode => '0640', + owner => 'artifactory', + group => 'artifactory', + } + + $_dbpropchanges = { + 'type' => $::artifactory::db_type, + 'url' => $::artifactory::db_url, + 'driver' => 'oracle.jdbc.OracleDriver', + 'username' => $::artifactory::db_username, + } + $dbpropchanges = $_dbpropchanges.reduce([]) | $memo, $value | { + # lint:ignore:140chars + $memo + "set \"${value[0]}\" \"${value[1]}\"" + # lint:endignore + } + augeas { 'db.properties': + context => '/files/var/opt/jfrog/artifactory/etc/db.properties', + incl => '/var/opt/jfrog/artifactory/etc/db.properties', + lens => 'Properties.lns', + changes => $dbpropchanges, + require => [Class['::artifactory::install']], + notify => Class['::artifactory::service'], + } + + $_dbpropchanges_pw = { + 'password' => $::artifactory::db_password, + } + $dbpropchanges_pw = $_dbpropchanges_pw.reduce([]) | $memo, $value | { + # lint:ignore:140chars + $memo + "set \"${value[0]}\" \"${value[1]}\"" + # lint:endignore + } + augeas { 'db.properties.pw': + context => '/files/var/opt/jfrog/artifactory/etc/db.properties', + incl => '/var/opt/jfrog/artifactory/etc/db.properties', + lens => 'Properties.lns', + changes => $dbpropchanges_pw, + onlyif => 'match /files/var/opt/jfrog/artifactory/etc/db.properties/password size == 0', + require => [Class['::artifactory::install']], + notify => Class['::artifactory::service'], + } - file { "${::artifactory::artifactory_home}/etc/storage.properties": - ensure => link, - target => "${::artifactory::artifactory_home}/etc/.secrets/.temp.db.properties", } if ($::artifactory::jdbc_driver_url) { diff --git a/manifests/init.pp b/manifests/init.pp index 05409b7..2b3c700 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -5,6 +5,7 @@ class artifactory( Boolean $manage_java = true, Boolean $manage_repo = true, + Boolean $use_temp_db_secrets = true, String $yum_name = 'bintray-jfrog-artifactory-rpms', String $yum_baseurl = 'http://jfrog.bintray.com/artifactory-rpms', String $package_name = 'jfrog-artifactory-oss', diff --git a/spec/classes/artifactory_spec.rb b/spec/classes/artifactory_spec.rb index b767afa..569ba80 100644 --- a/spec/classes/artifactory_spec.rb +++ b/spec/classes/artifactory_spec.rb @@ -90,6 +90,63 @@ } end + context 'artifactory class with use_temp_db_secrets set to false' do + let(:params) do + { + 'use_temp_db_secrets' => false, + # super().merge('artifactory_home' => '/var/opt/jfrog/artifactory') + 'jdbc_driver_url' => 'puppet:///modules/my_module/mysql.jar', + 'db_url' => 'oracle://some_url', + 'db_username' => 'foouser', + 'db_password' => 'foopw', + 'db_type' => 'oracle', + } + end + + it { is_expected.to compile.with_all_deps } + + it { + is_expected.to contain_file('/var/opt/jfrog/artifactory/tomcat/lib/mysql.jar').with( + 'source' => 'puppet:///modules/my_module/mysql.jar', + 'mode' => '0775', + 'owner' => 'root', + ) + } + + it { + is_expected.to contain_file('/var/opt/jfrog/artifactory/etc/db.properties').with( + 'ensure' => 'file', + 'mode' => '0640', + 'owner' => 'artifactory', + 'group' => 'artifactory', + ) + } + it do + should contain_augeas('db.properties').with({ + 'changes' => [ + "set \"type\" \"oracle\"", + "set \"url\" \"oracle://some_url\"", + "set \"driver\" \"oracle.jdbc.OracleDriver\"", + "set \"username\" \"foouser\"", + ], + 'require' => ['Class[Artifactory::Install]'], + 'notify' => 'Class[Artifactory::Service]', + }) + end + it do + should contain_augeas('db.properties.pw').with({ + 'changes' => [ + "set \"password\" \"foopw\"", + ], + 'onlyif' => 'match /files/var/opt/jfrog/artifactory/etc/db.properties/password size == 0', + 'require' => ['Class[Artifactory::Install]'], + 'notify' => 'Class[Artifactory::Service]', + }) + end + + + end + context 'artifactory class with manage_java set to false' do let(:params) do { From 4c1b07a6567f8fd001e593729a7a2d3fbc24e6e7 Mon Sep 17 00:00:00 2001 From: Ron Aughenbaugh Date: Thu, 22 Aug 2019 12:20:44 -0400 Subject: [PATCH 2/6] Add optional params to db.properties with augeas --- manifests/config.pp | 82 ++++++++++++++++++++++++++------ spec/classes/artifactory_spec.rb | 7 +++ 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 7342873..3570fd8 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -58,13 +58,62 @@ owner => 'artifactory', group => 'artifactory', } + file { "${::artifactory::artifactory_home}/etc/storage.properties": + ensure => link, + target => "${::artifactory::artifactory_home}/etc/db.properties", + } + + $db_driver = $::artifactory::db_type ? { + 'derby' => 'org.apache.derby.jdbc.EmbeddedDriver', + 'mssql' => 'com.microsoft.sqlserver.jdbc.SQLServerDriver', + 'mysql' => 'com.mysql.jdbc.Driver', + 'oracle' => 'oracle.jdbc.OracleDriver', + 'postgresql' => 'org.postgresql.Driver', + default => 'not valid', + } + + # Following logic in templates/db.properties.epp + case $::artifactory::binary_provider_type { + 'filesystem', + 'fullDb', + 'cachedFS': { + $binary_provider_type = $::artifactory::binary_provider_type + } + 'fullDbDirect': { + $binary_provider_type = undef + } + default: { + $binary_provider_type = 'filesystem' + } + } + + # Following logic in templates/db.properties.epp + if $binary_provider_type == 'filesystem' and ! $::artifactory::binary_provider_filesystem_dir { + $mapped_provider_filesystem_dir = 'filestore' + } else { + $mapped_provider_filesystem_dir = $::artifactory::binary_provider_filesystem_dir + } + if $::artifactory::binary_provider_base_data_dir { + $binary_provider_filesystem_dir = "${::artifactory::binary_provider_base_data_dir}/${mapped_provider_filesystem_dir}" + }else{ + $binary_provider_filesystem_dir = undef + } - $_dbpropchanges = { - 'type' => $::artifactory::db_type, - 'url' => $::artifactory::db_url, - 'driver' => 'oracle.jdbc.OracleDriver', - 'username' => $::artifactory::db_username, + $__dbpropchanges = { + 'type' => $::artifactory::db_type, + 'url' => $::artifactory::db_url, + 'driver' => $db_driver, + 'username' => $::artifactory::db_username, + 'binary.provider.type' => $binary_provider_type, + 'pool.max.active' => $::artifactory::pool_max_active, + 'pool.max.idle' => $::artifactory::pool_max_idle, + 'binary.provider.cache.maxsize' => $::artifactory::binary_provider_cache_maxsize, + 'binary.provider.filesystem.dir' => $binary_provider_filesystem_dir, + 'binary.provider.cache_dir' => $::artifactory::binary_provider_cache_dir, } + # We only care to set values that have actually be defined. + # Therefore empty ones from our collection + $_dbpropchanges = delete_undef_values($__dbpropchanges) $dbpropchanges = $_dbpropchanges.reduce([]) | $memo, $value | { # lint:ignore:140chars $memo + "set \"${value[0]}\" \"${value[1]}\"" @@ -79,19 +128,17 @@ notify => Class['::artifactory::service'], } - $_dbpropchanges_pw = { - 'password' => $::artifactory::db_password, - } - $dbpropchanges_pw = $_dbpropchanges_pw.reduce([]) | $memo, $value | { - # lint:ignore:140chars - $memo + "set \"${value[0]}\" \"${value[1]}\"" - # lint:endignore - } + # We treat db_password differently + # Artifactory likes to encrypt the password after starting. + # Onlyif statement will allow us to set password if the password + # has not be set yet, else it is not touched. + # To update password from hiera, remove the password field in db.properties, + # to update locally, just update and Artifactory will encrypt. augeas { 'db.properties.pw': context => '/files/var/opt/jfrog/artifactory/etc/db.properties', incl => '/var/opt/jfrog/artifactory/etc/db.properties', lens => 'Properties.lns', - changes => $dbpropchanges_pw, + changes => [ "set \"password\" \"$::artifactory::db_password\"" ], onlyif => 'match /files/var/opt/jfrog/artifactory/etc/db.properties/password size == 0', require => [Class['::artifactory::install']], notify => Class['::artifactory::service'], @@ -110,6 +157,13 @@ } } else { + # We are making an assumption that not passing db_username and db_password we are changing to derby + # and do not need db.properties file, but least be explicit in cleaning up. + if $::artifactory::db_type == 'derby' { + file { "${::artifactory::artifactory_home}/etc/db.properties": + ensure => absent, + } + } warning('Database port, hostname, username, password and type must be all be set, or not set. Install proceeding without DB configuration.')#lint:ignore:140chars } } diff --git a/spec/classes/artifactory_spec.rb b/spec/classes/artifactory_spec.rb index 569ba80..cd2eed2 100644 --- a/spec/classes/artifactory_spec.rb +++ b/spec/classes/artifactory_spec.rb @@ -121,6 +121,12 @@ 'group' => 'artifactory', ) } + it { + is_expected.to contain_file('/var/opt/jfrog/artifactory/etc/storage.properties').with( + 'ensure' => 'link', + 'target' => '/var/opt/jfrog/artifactory/etc/db.properties', + ) + } it do should contain_augeas('db.properties').with({ 'changes' => [ @@ -128,6 +134,7 @@ "set \"url\" \"oracle://some_url\"", "set \"driver\" \"oracle.jdbc.OracleDriver\"", "set \"username\" \"foouser\"", + "set \"binary.provider.type\" \"filesystem\"", ], 'require' => ['Class[Artifactory::Install]'], 'notify' => 'Class[Artifactory::Service]', From 07d68525b5b2701a0ac5552e6e4804408abb863c Mon Sep 17 00:00:00 2001 From: Ron Aughenbaugh Date: Thu, 22 Aug 2019 14:44:43 -0400 Subject: [PATCH 3/6] Hardcoding paths is not good --- manifests/config.pp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 3570fd8..795088d 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -120,8 +120,8 @@ # lint:endignore } augeas { 'db.properties': - context => '/files/var/opt/jfrog/artifactory/etc/db.properties', - incl => '/var/opt/jfrog/artifactory/etc/db.properties', + context => "/files${::artifactory::artifactory_home}/etc/db.properties", + incl => "${::artifactory::artifactory_home}/etc/db.properties", lens => 'Properties.lns', changes => $dbpropchanges, require => [Class['::artifactory::install']], @@ -135,11 +135,11 @@ # To update password from hiera, remove the password field in db.properties, # to update locally, just update and Artifactory will encrypt. augeas { 'db.properties.pw': - context => '/files/var/opt/jfrog/artifactory/etc/db.properties', - incl => '/var/opt/jfrog/artifactory/etc/db.properties', + context => "/files${::artifactory::artifactory_home}/etc/db.properties", + incl => "${::artifactory::artifactory_home}/etc/db.properties", lens => 'Properties.lns', changes => [ "set \"password\" \"$::artifactory::db_password\"" ], - onlyif => 'match /files/var/opt/jfrog/artifactory/etc/db.properties/password size == 0', + onlyif => "match /files${::artifactory::artifactory_home}/etc/db.properties/password size == 0", require => [Class['::artifactory::install']], notify => Class['::artifactory::service'], } From b8cc980a87192db67d2c680b7e913d99effc8d19 Mon Sep 17 00:00:00 2001 From: Ron Aughenbaugh Date: Thu, 22 Aug 2019 14:51:54 -0400 Subject: [PATCH 4/6] README update --- README.markdown | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.markdown b/README.markdown index 39be6f6..3b61cb9 100644 --- a/README.markdown +++ b/README.markdown @@ -124,6 +124,15 @@ Sets the location for the jdbc driver. The built-in `file` type is used to retri This is required if using a new data source. +##### `use_temp_db_secrets` + +Set to true(default) if you want Artifactory to delete temporary db.properties file on service start. +https://www.jfrog.com/confluence/display/RTF/Configuring+Security#ConfiguringSecurity-HardeningSecurityforSecrets + +Set to false if you would like db.properties file to be written to ${::artifactory::artifactory_home}/etc/db.properties +and managed with Augeas, taking into account Artifactory encrypts password field on startup. Management with Augeas +allows user to add additional database and storage options to db.properties without Puppet touching. + ##### `db_automate` Set to 'true' if you want Puppet to create a database. Only works with **mysql**. If `true`, we recommend using JDBC connector version 5.1.24. NOTE: Puppet may throw an error the first run while it waits for Artifactory to connect to database From f4b6331073da3941c32652672630fc933ec47a63 Mon Sep 17 00:00:00 2001 From: Ron Aughenbaugh Date: Mon, 26 Aug 2019 08:05:09 -0400 Subject: [PATCH 5/6] Address Rubocop complaints --- spec/classes/artifactory_spec.rb | 36 +++++++++++++------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/spec/classes/artifactory_spec.rb b/spec/classes/artifactory_spec.rb index cd2eed2..902dff0 100644 --- a/spec/classes/artifactory_spec.rb +++ b/spec/classes/artifactory_spec.rb @@ -128,30 +128,24 @@ ) } it do - should contain_augeas('db.properties').with({ - 'changes' => [ - "set \"type\" \"oracle\"", - "set \"url\" \"oracle://some_url\"", - "set \"driver\" \"oracle.jdbc.OracleDriver\"", - "set \"username\" \"foouser\"", - "set \"binary.provider.type\" \"filesystem\"", - ], - 'require' => ['Class[Artifactory::Install]'], - 'notify' => 'Class[Artifactory::Service]', - }) + is_expected.to contain_augeas('db.properties').with('changes' => [ + 'set "type" "oracle"', + 'set "url" "oracle://some_url"', + 'set "driver" "oracle.jdbc.OracleDriver"', + 'set "username" "foouser"', + 'set "binary.provider.type" "filesystem"', + ], + 'require' => ['Class[Artifactory::Install]'], + 'notify' => 'Class[Artifactory::Service]') end it do - should contain_augeas('db.properties.pw').with({ - 'changes' => [ - "set \"password\" \"foopw\"", - ], - 'onlyif' => 'match /files/var/opt/jfrog/artifactory/etc/db.properties/password size == 0', - 'require' => ['Class[Artifactory::Install]'], - 'notify' => 'Class[Artifactory::Service]', - }) + is_expected.to contain_augeas('db.properties.pw').with('changes' => [ + 'set "password" "foopw"', + ], + 'onlyif' => 'match /files/var/opt/jfrog/artifactory/etc/db.properties/password size == 0', + 'require' => ['Class[Artifactory::Install]'], + 'notify' => 'Class[Artifactory::Service]') end - - end context 'artifactory class with manage_java set to false' do From 713916339a975f77cad20b35d9d7a78dd3ba609f Mon Sep 17 00:00:00 2001 From: Ron Aughenbaugh Date: Mon, 26 Aug 2019 08:10:54 -0400 Subject: [PATCH 6/6] Address Puppet Lint issues --- manifests/config.pp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 795088d..fc6a5a7 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -53,10 +53,10 @@ } else { # Make sure we have correct mode and ownership file { "${::artifactory::artifactory_home}/etc/db.properties": - ensure => file, - mode => '0640', - owner => 'artifactory', - group => 'artifactory', + ensure => file, + mode => '0640', + owner => 'artifactory', + group => 'artifactory', } file { "${::artifactory::artifactory_home}/etc/storage.properties": ensure => link, @@ -138,7 +138,7 @@ context => "/files${::artifactory::artifactory_home}/etc/db.properties", incl => "${::artifactory::artifactory_home}/etc/db.properties", lens => 'Properties.lns', - changes => [ "set \"password\" \"$::artifactory::db_password\"" ], + changes => [ "set \"password\" \"${::artifactory::db_password}\"" ], onlyif => "match /files${::artifactory::artifactory_home}/etc/db.properties/password size == 0", require => [Class['::artifactory::install']], notify => Class['::artifactory::service'],