Finalize pwfilesupport #25

varet80 · 2017-02-28T13:13:24Z

New Feature: Create a Passwd file using Databags

- Generate UserDB
 - Source from databags
 - Compare users with the current if exist

- Kitchen
 - Change Exceptions for centos 7.2 (fall into some bugs)
 - Add databags support
 - Add databags with two users

- User Recipe
 - Change priority on dovecot user creation (group goes after user)

- New AttribugesAttributes
 - Databag name attribute is  `databag_name`
 - Databag item for userdb `databag_users_item`

…cot-cookbook into adding_pwfilesupport

now with reverse verification

…cot-cookbook into adding_pwfilesupport Conflicts: attributes/conf_files.rb

Adding pwfilesupport

- Generate UserDB - Source from databags - Compare users with the current if exist - Kitchen - Change Exceptions for centos 7.2 (fall into some bugs) - Add databags support - Add databags with two users - User Recipe - Change priority on dovecot user creation (group goes after user) - Rename Attributes - Databag attribute is now `databag_name` (more generic) - Databag item for users is now `databag_users_item`

…pport

varet80 · 2017-03-02T13:48:21Z

Hey! thanks for the support

I have already done the new recipe
databags too
I need to add the recipe on a new suite
test the new suite
Move the block under custom resource or create library for it... thinking about it(too big for ruby block)
redoo 2-4

zuazo

OK. Thanks @billiaz! I added some comments to your code.

Don't worry too much about Custom Resources, that may be too much to be included in this PR. We can address it in a future issue/PR anyway.

zuazo · 2017-03-03T13:43:31Z

attributes/conf_files.rb

+  "#{node['dovecot']['conf_path']}/password"
+
+default['dovecot']['conf']['password_file'] = \
+  "#{node['dovecot']['conf_path']}/password"


Same attribute set twice here.

zuazo · 2017-03-03T13:44:13Z

metadata.rb

@@ -27,7 +27,7 @@
 Installs and configures Dovecot, open source IMAP and POP3 email server.
 EOH
 long_description IO.read(File.join(File.dirname(__FILE__), 'README.md'))
-version '3.2.0' # WiP
+version '3.2.0001'


Avoid changing this.

zuazo · 2017-03-03T13:45:40Z

README.md

+
+Creates and configures a password file from local mailboxes based on a data bag.
+
+* `node['dovecot']['databag_users_item']`: The databag item to use (under the databag set)


Maybe add databag_name attribute here?

Also, adding a example content of the data bag would make its usage easier. The same you used for the tests will be enough.

is it better to have it:
node['dovecot']['databag_name']['users_item'] ?

zuazo · 2017-03-03T13:57:02Z

recipes/create_pwfile.rb

+#
+extend Chef::Mixin::ShellOut
+
+include_recipe 'dovecot::from_package'


Better to use include_recipe "dovecot::from_#{node['dovecot']['install_from']}" here as in the ::default recipe.

Will update it on the next commit, Thnx

zuazo · 2017-03-03T14:01:27Z

recipes/user.rb

+else
+  user node['dovecot']['conf']['default_login_user']
+  group node['dovecot']['conf']['default_login_user']
+end


I think this is wrong here. It is not inside a resource block.

I have to check this, it was inherited.

legacy from the previous pull request, cleaning it and testing it

zuazo · 2017-03-03T14:03:25Z

recipes/create_pwfile.rb

+  variables(
+    credentials: credentials
+  )
+  only_if { credentials_updated }


Add sensitive true property to this resource.

zuazo · 2017-03-03T14:09:41Z

recipes/create_pwfile.rb

+                          [crypt, uid, gid, gecos, homedir, shell, extra_fields]
+                        else
+                          [crypt]
+                        end


What's the purpose of that if? Sorry, I don't quite understand it 😞

refactored :) now inside libs
this accepts any type of userdb entry
there have been 2 ways:
user:pass (old)
current:
user:pass:uid:gid:gecos:homedir:shell:extra < some of these doesn't work
this is to force importing properly both, but now after I made some changes it should work better, with less complexity

Refactoring User Creation - Use library with small functions - Add new simpler way of managing the changes - Add documentation and examples Tests - Add unit testing - New kitchen suite - Server specs for the new kitchen suite Rubocop: - Make lines wider

- Fix Style syntax - Add travis checks for the new kitchen unit tests - Fix readme spaces under apache license

varet80 · 2017-03-07T15:32:46Z

README.md

        http://www.apache.org/licenses/LICENSE-2.0
-


fixing it on my next commit with the style changes :(
my lovely editor removes all white space

Added kitchen and travis support - Unfortunatelly not every test is completed as the config changes per dovecot version. (To be done on next versions) - Newer versions require ssl (especially on latest Deb/Ub)

…ignment

coveralls · 2017-03-07T19:50:26Z

Coverage increased (+0.03%) to 98.645% when pulling 469a453 on schubergphilis:finalize_pwfilesupport into a57e307 on zuazo:master.

coveralls · 2017-03-07T19:50:27Z

Coverage increased (+0.03%) to 98.645% when pulling 469a453 on schubergphilis:finalize_pwfilesupport into a57e307 on zuazo:master.

coveralls · 2017-03-07T19:50:27Z

Coverage increased (+0.03%) to 98.645% when pulling 469a453 on schubergphilis:finalize_pwfilesupport into a57e307 on zuazo:master.

zuazo

@billiaz astonishing work! ✨

I have commented some possible improvements and added some notes to help you complete the unit tests.

zuazo · 2017-03-08T00:15:44Z

attributes/conf_files.rb

@@ -23,6 +23,8 @@
 default['dovecot']['conf_files_user'] = 'root'
 default['dovecot']['conf_files_group'] = node['dovecot']['group']
 default['dovecot']['conf_files_mode'] = '00644'
+default['dovecot']['conf']['password_file'] = \


The \ at the en of this line is not needed.

zuazo · 2017-03-08T00:18:49Z

libraries/pwfile.rb

+
+    def self.exists?(localdata)
+      true if ::File.exist?(localdata)
+    end


AFAIK you don't need an if there:

def self.exists?(localdata) ::File.exist?(localdata) end

zuazo · 2017-03-08T00:31:21Z

libraries/pwfile.rb

+
+    def self.file_to_hash(inputfile)
+      output_entries = {}
+      passwordfile = File.open(inputfile, File::RDONLY | File::CREAT, 640)


To use octal numbers, you must add a 0 before the number. So, we need to use 0640 here.

zuazo · 2017-03-08T00:32:53Z

libraries/pwfile.rb

+      end
+      passwordfile.close
+      output_entries
+    end


I think this method can be simplified as follows:

def self.file_to_hash(inputfile) File.open(inputfile, File::RDONLY | File::CREAT, 0640) do |passwordfile| passwordfile.readlines.each_with_object({}) do |line, output_entries| user, data = fileline_to_userdb_hash(line) output_entries[user] = data end end end

zuazo · 2017-03-08T00:35:21Z

libraries/pwfile.rb

+    def self.password_valid?(hashed_pw, plaintext_pw)
+      true unless shell_out(
+        "/usr/bin/doveadm pw -t '#{hashed_pw}' -p '#{plaintext_pw}'"
+      ).exitstatus != 0


The same here, no need for the unless:

def self.password_valid?(hashed_pw, plaintext_pw) shell_out("/usr/bin/doveadm pw -t '#{hashed_pw}' -p '#{plaintext_pw}'") .exitstatus == 0 end

zuazo · 2017-03-08T00:37:35Z

recipes/create_pwfile.rb

+  variables(
+    credentials: credentials
+  )
+  only_if { update_credentials }


I would add a sensitive true here 😉

it is added on my next commit ;)

zuazo · 2017-03-08T08:38:20Z

test/integration/create_pwfile/serverspec/imap_spec.rb

+  it { should connect }
+  it { should authenticate 'vassilis', 'vassilis1234' }
+  it { should authenticate 'dilan', 'password1234' }
+  it { should_not authenticate 'vassilis', 'vassilis123' }


zuazo · 2017-03-08T09:10:46Z

attributes/conf_10_ssl.rb

+default['dovecot']['conf']['ssl'] = \
+  if node['platform_family'] == 'suse' && node['platform_version'].to_i < 13
+    false
+  end


zuazo · 2017-03-08T09:21:44Z

test/unit/recipes/create_pwfile_spec.rb

+        .with_path('/etc/dovecot/password')
+        .with_source('password.erb')
+    end
+  end


This test is failing:

Failures: 1) dovecot::create_pwfile when the module runs creates a file Failure/Error: expect(chef_run). to create_template('/etc/dovecot/password') .with_path('/etc/dovecot/password') .with_source('password.erb') expected "template[/etc/dovecot/password]" actions [] to include :create # ./test/unit/recipes/create_pwfile_spec.rb:29:in `block (3 levels) in <top (required)>'

That's because we need to run the code inside the ruby_block and force it to set update_credentials to true.

It is not easy to understand, but here you have a working version of ChefSpec tests for the create_pwfile recipe:

describe 'dovecot::create_pwfile', order: :random do let(:chef_runner) { ChefSpec::SoloRunner.new } let(:chef_run) { chef_runner.converge(described_recipe) } let(:node) { chef_runner.node } it 'runs databag_to_dovecot_userdb ruby block' do expect(chef_run).to run_ruby_block('databag_to_dovecot_userdb') end describe 'inside ruby_block[databag_to_dovecot_userdb] resource' do let(:chef_runner) { ChefSpec::SoloRunner.new(step_into: %w(ruby_block)) } let(:data_bag_users) do { 'users' => { 'dilan' => 'password1234', 'vassilis' => ['vassilis1234', nil, nil, nil, nil, nil, nil] } } end let(:pwfile) { instance_double('File', readlines: [], close: nil) } let(:encrypted_password) { 'encrypted_password' } before do # Stub ::File.open: allow(::File).to receive(:open).and_call_original allow(::File).to receive(:open).with('/etc/dovecot/password', any_args) .and_return(pwfile) # If passing a block with do...end to ::File.open: # allow(::File).to receive(:open).with('/etc/dovecot/password', any_args) # .and_yield(pwfile) # Stub Data Bag: stub_data_bag_item('dovecot', 'users').and_return(data_bag_users) # Stub some commands excuted inside the ruby_block: # Using: https://github.com/zuazo/filesystem_resize-cookbook/blob/master/test/unit/support/stubs.rb stub_shell_out( '/usr/bin/doveadm pw -s MD5 -p password1234' ).and_return_stdout(encrypted_password) stub_shell_out( '/usr/bin/doveadm pw -s MD5 -p vassilis1234' ).and_return_stdout(encrypted_password) end it 'creates password file template' do expect(chef_run).to create_template('/etc/dovecot/password') .with_path('/etc/dovecot/password') .with_source('password.erb') .with_owner('dovecot') .with_group('dovecot') .with_mode('0640') # .with_sensitve(true) end end end

This example uses the stub_data_bag_item function from here. You need to copy that file to the test/unit/support directory and include it in the test/unit/spec_helper.rb file.

If you change the ::File.open call to use a block, as I have recommended you above, you need to uncomment the line with the and_yield.

Of course, feel free to ask me any questions about this.

unit tests... the worst for me :( I cannot get it yet (though pretty new with these concepts)

varet80 · 2017-03-08T10:05:39Z

Thanks for the help! it is my first real contribution.

- Update recipes with more efficient code - Use the awesome Unit test made by the Maintainer

coveralls · 2017-03-08T10:44:57Z

Coverage decreased (-5.6%) to 92.966% when pulling 39d13f8 on schubergphilis:finalize_pwfilesupport into a57e307 on zuazo:master.

varet80 · 2017-03-08T20:05:53Z

Is there something more I should do to make this pull request ready for merge?

zuazo · 2017-03-09T09:02:18Z

Thanks @billiaz. No, it is ready to be merged. Give me a couple of days because I want to test some of the functions by hand. I'm planning to release this during weekend if you can wait a little.

…bocop LineLength) (related to #25)

zuazo · 2017-03-09T12:55:32Z

OK. I have merged this PR after:

Adding some unit tests for the new library.
Documented the new functions.
Changed the code a little to avoid disabling RuboCop's LineLength.

You can see the changes in 667058b.

zuazo · 2017-03-10T15:08:17Z

Released in 3.2.0.

Thanks for your patience.

stromp and others added 28 commits January 12, 2016 17:22

now with reverse verification

8e055a2

new attrs

db72942

cleaning

58b7d61

cleaning

2ca135c

adding template

8795a17

cleaning up

a94728b

beautify

1e4676f

updated metadata and readme

85bdc3e

swap user and group

be63926

now with reverse verification

c2b010d

new attrs

c872cac

cleaning

4b3b680

cleaning

c8c4450

adding template

c350714

cleaning up

8ca7da1

beautify

2c2795a

updated metadata and readme

01babb1

swap user and group

f4674bc

Merge branch 'adding_pwfilesupport' of github.com:schubergphilis/dove…

f116ec3

…cot-cookbook into adding_pwfilesupport

now with reverse verification

914efcf

Merge pull request #1 from schubergphilis/adding_pwfilesupport

3fc2d10

now with reverse verification

Version 2.4.1001

fcdbb10

now with reverse verification

74e1e86

Merge branch 'adding_pwfilesupport' of github.com:schubergphilis/dove…

a4e8fb7

…cot-cookbook into adding_pwfilesupport Conflicts: attributes/conf_files.rb

Merge pull request #2 from schubergphilis/adding_pwfilesupport

9e8e9ba

Adding pwfilesupport

Merge remote-tracking branch 'upstream/master'

8370366

Merge remote-tracking branch 'upstream/master' into finalize_pwfilesu…

5f8f4bd

…pport

zuazo mentioned this pull request Feb 28, 2017

Adding pwfilesupport #17

Closed

zuazo added the Type: Feature Request label Feb 28, 2017

zuazo requested changes Mar 5, 2017

View reviewed changes

zuazo added Status: Pending Contributor Response and removed Status: Maintainer Review Needed labels Mar 5, 2017

varet80 added 2 commits March 7, 2017 16:09

Small fixes after Last commit

b15165b

- Fix Style syntax - Add travis checks for the new kitchen unit tests - Fix readme spaces under apache license

varet80 commented Mar 7, 2017

View reviewed changes

zuazo added Status: Maintainer Review Needed and removed Status: Pending Contributor Response labels Mar 7, 2017

varet80 added 2 commits March 7, 2017 20:02

Kitchen and Travis modification:

c53fc05

Added kitchen and travis support - Unfortunatelly not every test is completed as the config changes per dovecot version. (To be done on next versions) - Newer versions require ssl (especially on latest Deb/Ub)

Fix attribute conditional to follow Rubocop rule Style/ConditionalAss…

469a453

…ignment

zuazo requested changes Mar 8, 2017

View reviewed changes

zuazo added Status: Pending Contributor Response and removed Status: Maintainer Review Needed labels Mar 8, 2017

Align with Mantainer's requests

39d13f8

- Update recipes with more efficient code - Use the awesome Unit test made by the Maintainer

zuazo added Status: Maintainer Review Needed and removed Status: Pending Contributor Response labels Mar 8, 2017

zuazo approved these changes Mar 8, 2017

View reviewed changes

zuazo removed the Status: In Progress label Mar 8, 2017

zuazo added a commit that referenced this pull request Mar 9, 2017

Add unit tests and doc for pwfile lib and improve its readibility (ru…

667058b

…bocop LineLength) (related to #25)

zuazo merged commit 39d13f8 into zuazo:master Mar 9, 2017


		Creates and configures a password file from local mailboxes based on a data bag.

		* `node['dovecot']['databag_users_item']`: The databag item to use (under the databag set)

Finalize pwfilesupport #25

Finalize pwfilesupport #25

Conversation

varet80 commented Feb 28, 2017

varet80 commented Mar 2, 2017 • edited

zuazo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

zuazo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varet80 commented Mar 8, 2017

coveralls commented Mar 8, 2017 • edited

varet80 commented Mar 8, 2017

zuazo commented Mar 9, 2017

zuazo commented Mar 9, 2017

zuazo commented Mar 10, 2017

varet80 commented Mar 2, 2017 •

edited

coveralls commented Mar 8, 2017 •

edited