From 0a42b0722bb50a7a2e105bc34127eda77a2113d0 Mon Sep 17 00:00:00 2001 From: Patrick Moore Date: Fri, 9 May 2014 13:51:12 -0400 Subject: [PATCH] [COOK-4619] - no way to unset recipient_delimiter Signed-off-by: Sean OMeara --- .kitchen.yml | 4 +- .rubocop.yml | 4 ++ README.md | 1 - attributes/default.rb | 1 - metadata.rb | 66 +++++++++---------- recipes/aliases.rb | 2 +- recipes/client.rb | 2 +- recipes/default.rb | 20 +++--- recipes/sasl_auth.rb | 10 +-- spec/default_spec.rb | 16 ++++- test/fixtures/cookbooks/fake/metadata.rb | 6 +- .../fixtures/cookbooks/fake/recipes/omnios.rb | 6 +- .../default/serverspec/default_spec.rb | 2 +- .../sasl_auth/serverspec/sasl_auth_spec.rb | 6 +- 14 files changed, 78 insertions(+), 68 deletions(-) create mode 100644 .rubocop.yml diff --git a/.kitchen.yml b/.kitchen.yml index 1da2578..0571893 100644 --- a/.kitchen.yml +++ b/.kitchen.yml @@ -15,8 +15,8 @@ platforms: run_list: - recipe[apt] -- name: centos-6.4 -- name: centos-5.9 +- name: centos-6.5 +- name: centos-5.10 - name: omnios-r151006c driver: box: omnios-r151006c diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..31892cf --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,4 @@ +Encoding: + Enabled: False +LineLength: + Max: 200 diff --git a/README.md b/README.md index b6b5215..a983206 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,6 @@ This change in namespace to `node['postfix']['main']` should allow for greater f * `node['postfix']['main']['inet_interfaces']` - set to `loopback-only`, or `all` for server recipe * `node['postfix']['main']['alias_maps']` - set to `hash:/etc/aliases` * `node['postfix']['main']['mailbox_size_limit']` - set to `0` (disabled) -* `node['postfix']['main']['recipient_delimiter']` - set to `+` * `node['postfix']['main']['mydestination']` - default fqdn, hostname, localhost.localdomain, localhost * `node['postfix']['main']['smtpd_use_tls']` - (yes/no); default yes. See conditional cert/key attributes. - `node['postfix']['main']['smtpd_tls_cert_file']` - conditional attribute, set to full path of server's x509 certificate. diff --git a/attributes/default.rb b/attributes/default.rb index ff265c1..becfc34 100644 --- a/attributes/default.rb +++ b/attributes/default.rb @@ -49,7 +49,6 @@ default['postfix']['main']['smtpd_use_tls'] = 'yes' default['postfix']['main']['smtp_use_tls'] = 'yes' default['postfix']['main']['alias_maps'] = ["hash:#{node['postfix']['aliases_db']}"] default['postfix']['main']['mailbox_size_limit'] = 0 -default['postfix']['main']['recipient_delimiter'] = '+' default['postfix']['main']['smtp_sasl_auth_enable'] = 'no' default['postfix']['main']['mynetworks'] = '127.0.0.0/8' default['postfix']['main']['inet_interfaces'] = 'loopback-only' diff --git a/metadata.rb b/metadata.rb index 857caf0..ec931de 100644 --- a/metadata.rb +++ b/metadata.rb @@ -1,51 +1,51 @@ # encoding: utf-8 -name 'postfix' -description 'Installs and configures postfix for client or outbound relayhost, or to do SASL auth' -maintainer 'Opscode, Inc.' -maintainer_email 'cookbooks@opscode.com' -license 'Apache 2.0' -version '3.1.9' -recipe 'postfix', 'Installs and configures postfix' -recipe 'postfix::sasl_auth', 'Set up postfix to auth to a server with sasl' -recipe 'postfix::aliases', 'Manages /etc/aliases' -recipe 'postfix::client', 'Searches for the relayhost based on an attribute' -recipe 'postfix::server', 'Sets the mail_type attribute to master' +name 'postfix' +description 'Installs and configures postfix for client or outbound relayhost, or to do SASL auth' +maintainer 'Opscode, Inc.' +maintainer_email 'cookbooks@opscode.com' +license 'Apache 2.0' +version '3.1.9' +recipe 'postfix', 'Installs and configures postfix' +recipe 'postfix::sasl_auth', 'Set up postfix to auth to a server with sasl' +recipe 'postfix::aliases', 'Manages /etc/aliases' +recipe 'postfix::client', 'Searches for the relayhost based on an attribute' +recipe 'postfix::server', 'Sets the mail_type attribute to master' -%w{ubuntu debian redhat centos amazon scientific smartos}.each do |os| +%w(ubuntu debian redhat centos amazon scientific smartos).each do |os| supports os end attribute 'postfix/main', -display_name: 'postfix/main', -description: 'Hash of Postfix main.cf attributes', -type: 'hash' + display_name: 'postfix/main', + description: 'Hash of Postfix main.cf attributes', + type: 'hash' attribute 'postfix/aliases', -display_name: 'Postfix Aliases', -description: "Hash of Postfix aliases mapping a name to a value. Example 'root' => 'operator@example.com'. See aliases man page for details.", -type: 'hash' + display_name: 'Postfix Aliases', + description: "Hash of Postfix aliases mapping a name to a value. Example 'root' => 'operator@example.com'. See aliases man page for details.", + type: 'hash' attribute 'postfix/mail_type', -display_name: 'Postfix Mail Type', -description: 'Is this node a client or server?', -default: 'client' + display_name: 'Postfix Mail Type', + description: 'Is this node a client or server?', + default: 'client' attribute 'postfix/smtp_sasl_user_name', -display_name: 'Postfix SMTP SASL Username', -description: 'User to auth SMTP via SASL', -default: '' + display_name: 'Postfix SMTP SASL Username', + description: 'User to auth SMTP via SASL', + default: '' attribute 'postfix/smtp_sasl_passwd', -display_name: 'Postfix SMTP SASL Password', -description: 'Password for smtp_sasl_user_name', -default: '' + display_name: 'Postfix SMTP SASL Password', + description: 'Password for smtp_sasl_user_name', + default: '' attribute 'postfix/relayhost_role', -display_name: "Postfix Relayhost's role", -description: 'String containing the role name', -default: 'relayhost' + display_name: "Postfix Relayhost's role", + description: 'String containing the role name', + default: 'relayhost' attribute 'postfix/use_procmail', -display_name: 'Postfix Use procmail?', -description: 'Whether procmail should be used as the local delivery agent for a server', -default: 'no' + display_name: 'Postfix Use procmail?', + description: 'Whether procmail should be used as the local delivery agent for a server', + default: 'no' diff --git a/recipes/aliases.rb b/recipes/aliases.rb index 166a125..76947a5 100644 --- a/recipes/aliases.rb +++ b/recipes/aliases.rb @@ -18,7 +18,7 @@ include_recipe 'postfix' execute 'update-postfix-aliases' do command 'newaliases' - environment :PATH => "#{ENV['PATH']}:/opt/omni/bin:/opt/omni/sbin" if platform_family?('omnios') + environment PATH: "#{ENV['PATH']}:/opt/omni/bin:/opt/omni/sbin" if platform_family?('omnios') action :nothing end diff --git a/recipes/client.rb b/recipes/client.rb index 493f4b8..c13751d 100644 --- a/recipes/client.rb +++ b/recipes/client.rb @@ -25,7 +25,7 @@ end query = "role:#{node['postfix']['relayhost_role']}" relayhost = '' -results = [] +# results = [] if node.run_list.roles.include?(node['postfix']['relayhost_role']) relayhost << node['ipaddress'] diff --git a/recipes/default.rb b/recipes/default.rb index ca20be4..07fdf8d 100644 --- a/recipes/default.rb +++ b/recipes/default.rb @@ -20,9 +20,7 @@ package 'postfix' -if node['postfix']['use_procmail'] - package 'procmail' -end +package 'procmail' if node['postfix']['use_procmail'] case node['platform_family'] when 'rhel', 'fedora' @@ -37,7 +35,7 @@ when 'rhel', 'fedora' not_if '/usr/bin/test /etc/alternatives/mta -ef /usr/sbin/sendmail.postfix' end when 'omnios' - manifest_path = ::File.join(Chef::Config[:file_cache_path], "manifest-postfix.xml") + manifest_path = ::File.join(Chef::Config[:file_cache_path], 'manifest-postfix.xml') # we need to manage the postfix group and user # and then subscribe to the package install because it creates a @@ -62,13 +60,13 @@ when 'omnios' owner 'root' group 'root' mode 00644 - notifies :run, "execute[load postfix manifest]", :immediately + notifies :run, 'execute[load postfix manifest]', :immediately end - execute "load postfix manifest" do + execute 'load postfix manifest' do action :nothing command "svccfg import #{manifest_path}" - notifies :restart, "service[postfix]" + notifies :restart, 'service[postfix]' end end @@ -77,21 +75,21 @@ execute 'update-postfix-sender_canonical' do action :nothing end -if !node['postfix']['sender_canonical_map_entries'].empty? +unless node['postfix']['sender_canonical_map_entries'].empty? template "#{node['postfix']['conf_dir']}/sender_canonical" do owner 'root' group 0 - mode '0644' + mode '0644' notifies :run, 'execute[update-postfix-sender_canonical]' notifies :reload, 'service[postfix]' end - if !node['postfix']['main'].key?('sender_canonical_maps') + unless node['postfix']['main'].key?('sender_canonical_maps') node.set['postfix']['main']['sender_canonical_maps'] = "hash:#{node['postfix']['conf_dir']}/sender_canonical" end end -%w{main master}.each do |cfg| +%w(main master).each do |cfg| template "#{node['postfix']['conf_dir']}/#{cfg}.cf" do source "#{cfg}.cf.erb" owner 'root' diff --git a/recipes/sasl_auth.rb b/recipes/sasl_auth.rb index 18be42f..a019630 100644 --- a/recipes/sasl_auth.rb +++ b/recipes/sasl_auth.rb @@ -27,15 +27,15 @@ sasl_pkgs = [] # version specifics for RHEL. case node['platform_family'] when 'debian' - sasl_pkgs = %w{libsasl2-2 libsasl2-modules ca-certificates} + sasl_pkgs = %w(libsasl2-2 libsasl2-modules ca-certificates) when 'rhel' if node['platform_version'].to_i < 6 - sasl_pkgs = %w{cyrus-sasl cyrus-sasl-plain openssl} + sasl_pkgs = %w(cyrus-sasl cyrus-sasl-plain openssl) else - sasl_pkgs = %w{cyrus-sasl cyrus-sasl-plain ca-certificates} + sasl_pkgs = %w(cyrus-sasl cyrus-sasl-plain ca-certificates) end when 'fedora' - sasl_pkgs = %w{cyrus-sasl cyrus-sasl-plain ca-certificates} + sasl_pkgs = %w(cyrus-sasl cyrus-sasl-plain ca-certificates) end sasl_pkgs.each do |pkg| @@ -55,5 +55,5 @@ template node['postfix']['sasl_password_file'] do mode 0400 notifies :run, 'execute[postmap-sasl_passwd]', :immediately notifies :restart, 'service[postfix]' - variables(:settings => node['postfix']['sasl']) + variables(settings: node['postfix']['sasl']) end diff --git a/spec/default_spec.rb b/spec/default_spec.rb index 9b4da58..790c563 100644 --- a/spec/default_spec.rb +++ b/spec/default_spec.rb @@ -4,7 +4,7 @@ describe 'postfix::default' do before do stub_command('/usr/bin/test /etc/alternatives/mta -ef /usr/sbin/sendmail.postfix').and_return(true) end - + context 'on Centos 6.5' do let(:chef_run) do ChefSpec::Runner.new(platform: 'centos', version: 6.5).converge(described_recipe) @@ -13,6 +13,10 @@ describe 'postfix::default' do it '[COOK-4423] renders file main.cf with /etc/pki/tls/cert.pem' do expect(chef_run).to render_file('/etc/postfix/main.cf').with_content(%r{smtp_tls_CAfile += +/etc/pki/tls/cert.pem}) end + + it '[COOK-4619] does not set recipient_delimiter' do + expect(chef_run).to_not render_file('/etc/postfix/main.cf').with_content('recipient_delimiter') + end end context 'on SmartOS' do @@ -21,7 +25,11 @@ describe 'postfix::default' do end it '[COOK-4423] renders file main.cf without smtp_use_tls' do - expect(chef_run).to render_file('/opt/local/etc/postfix/main.cf').with_content(%r{smtp_use_tls += +no}) + expect(chef_run).to render_file('/opt/local/etc/postfix/main.cf').with_content(/smtp_use_tls += +no/) + end + + it '[COOK-4619] does not set recipient_delimiter' do + expect(chef_run).to_not render_file('/etc/postfix/main.cf').with_content('recipient_delimiter') end end @@ -33,5 +41,9 @@ describe 'postfix::default' do it '[COOK-4423] renders file main.cf with /etc/postfix/cacert.pem' do expect(chef_run).to render_file('/etc/postfix/main.cf').with_content(%r{smtp_tls_CAfile += +/etc/postfix/cacert.pem}) end + + it '[COOK-4619] does not set recipient_delimiter' do + expect(chef_run).to_not render_file('/etc/postfix/main.cf').with_content('recipient_delimiter') + end end end diff --git a/test/fixtures/cookbooks/fake/metadata.rb b/test/fixtures/cookbooks/fake/metadata.rb index ade03f3..9ded389 100644 --- a/test/fixtures/cookbooks/fake/metadata.rb +++ b/test/fixtures/cookbooks/fake/metadata.rb @@ -1,3 +1,3 @@ -name "fake" -version "0.0.1" -description "Not a real cookbook, used for testing only." +name 'fake' +version '0.0.1' +description 'Not a real cookbook, used for testing only.' diff --git a/test/fixtures/cookbooks/fake/recipes/omnios.rb b/test/fixtures/cookbooks/fake/recipes/omnios.rb index 08af3bb..006c5f6 100644 --- a/test/fixtures/cookbooks/fake/recipes/omnios.rb +++ b/test/fixtures/cookbooks/fake/recipes/omnios.rb @@ -1,5 +1,5 @@ -execute "pkg set-publisher -g http://pkg.omniti.com/omniti-ms/ ms.omniti.com" do - not_if "pkg publisher ms.omniti.com" +execute 'pkg set-publisher -g http://pkg.omniti.com/omniti-ms/ ms.omniti.com' do + not_if 'pkg publisher ms.omniti.com' end -execute "pkg refresh --full" +execute 'pkg refresh --full' diff --git a/test/integration/default/serverspec/default_spec.rb b/test/integration/default/serverspec/default_spec.rb index 7b0775b..53daf0b 100644 --- a/test/integration/default/serverspec/default_spec.rb +++ b/test/integration/default/serverspec/default_spec.rb @@ -28,7 +28,7 @@ describe 'postfix::default' do context 'configures' do describe file('/etc/postfix/main.cf') do - its(:content) { should match /^# Generated by Chef for / } + its(:content) { should match(/^# Generated by Chef for /) } end end end diff --git a/test/integration/sasl_auth/serverspec/sasl_auth_spec.rb b/test/integration/sasl_auth/serverspec/sasl_auth_spec.rb index 51f388a..0d8cefd 100644 --- a/test/integration/sasl_auth/serverspec/sasl_auth_spec.rb +++ b/test/integration/sasl_auth/serverspec/sasl_auth_spec.rb @@ -19,12 +19,10 @@ describe 'postfix::sasl_auth' do let(:sasl_passwd_file) { '/etc/postfix/sasl_passwd' } it 'manages postfix sasl_passwd' do - expect(file(sasl_passwd_file).content). - to match(/^# This file is generated by Chef for/) + expect(file(sasl_passwd_file).content).to match(/^# This file is generated by Chef for/) end it 'configures postfix to use the sasl_passwd file' do - expect(file('/etc/postfix/main.cf').content). - to match(/^\s*smtp_sasl_password_maps\s*=.*#{sasl_passwd_file}\s*$/) + expect(file('/etc/postfix/main.cf').content).to match(/^\s*smtp_sasl_password_maps\s*=.*#{sasl_passwd_file}\s*$/) end end