Tried to implement test config before reload

[treenerd]

Hi togeather again;

I tried to implement test the configuration before chef is restarts/reloads anything.
I recognized that there was a problem with configuration testing, cause of "wrong" table permissions in the postgresql database. (Not sure if this was no problem with peer authentication)

Take a look on that changes, and tell me what you thinking about them.

Kitchen tests with Ubuntu14, Debian78, CentOS66 worked; There are some issues with CentOS7 cause of undefined addresses, I'll try to solve that as soon as I can.

[GitBytes]

@treenerd I see where you are going with this but based on the kinds of changes you are trying to make I need to take some time to review them. Mind if I look at this a little later today and get back to you? Thanks! BR, GitBytes

[treenerd]

@GitBytes
Take the time you need, it's Sunday.

[GitBytes]

@treenerd

Hello! I have taken some time to better evaluate your path on these commits. Here is my breakdown:

• Commit treenerd@bf57203
  • I would be good with commit on the basis that it is in the kitchen environment but is also not necessary as it is in the kitchen environment. If a user wishes to include this attribute change within their local test config, that is great but I think we will hold off with changing this for now just to keep it simple essentially.
• Commit treenerd@ddaaa46
  • I see this as being potentially useful in certain deployments. We should keep this and associated bits with this change.
• Commit treenerd@2aeaf39
  • See previous comment.
• Commit treenerd@6fcee62
  • client.rb
    • Change reload to restart for the bareos-fd service. Otherwise it is great!
  • server.rb
    • This looks ok but there needs to be some fine tuning bits I can think of in other places of the recipe(s) permission wise. Once merged I can work on overall security bits a bit more and get this more aligned with doc.bareos.org.
  • storage.rb
    • Change reload to restart for the bareos-sd service. Looks good otherwise.
• Commit treenerd@8951628
  • database.rb
    • In my humble opinion, the change to the bareos user (lines 59-62) feels like a security risk. Unless strongly defended I have to vote no on this change. @sitle Please feel free to review this opinion but I strongly think the bareos user needs to maintain a shell of /bin/false in /etc/passwd.
    • I am not sure I am fond of the use of the database cookbook, solely because it is force building and installing postgresql from a ruby gem (chef_gem pg in ruby.rb recipe of the postgresql cookbook) instead of using the available package manager (i.e. yum/apt). Building a gem like this at runtime shouldn't be necessary unless certain cases exist plus it adds a gem dependency list that is otherwise not necessary. I am seeing various possible issues associated with this method so I think we should hold off and continue to initialize the database using the classic scripts included with the bareos installation packages and install from classic package manager methods. @sitle Please feel free to comment on this. @treenerd If I have missed something in my analysis of the cookbook dependency tree and how it all functions I apologize and would like to invite any further discussion. I am not completely closed off to this idea necessarily as I could have missed something but this is stemming from what I can gather after a bit of looking through things.
• Last commits seem to be an extension of last bullet.

@sitle @treenerd Please look through my comments when you have time. Sorry for the wall of text in advance. Apparently, I am a detailed reviewer but that makes it fun right?! ;-)

BR, GitBytes

[treenerd]

@GitBytes

Hello again; Thank you for the fast review.

• Commit treenerd/chef-bareos@bf57203
  • I understand. I had troubles with the bareos-dir tests if I would use sockets with (peer) authentification. It always failed because authentification was not possible. Perhaps you have an Idea to avoid this problem. On the other side the cookbook is now prepared to use external postgresql databases. It would also be possible to autogenerate the bareos dbpasswd for the bareos installation if it is not set as attribute (Just as an idea).
    So at the end it's no problem for me to add the attribute in the kitchen.yml when I need it.
• Commit treenerd/chef-bareos@ddaaa46
  • So if no dbpassword is set I think it will fail. We could check if the password is empty, don't parse the address into the configfile?
• Commit treenerd/chef-bareos@2aeaf39
  • Sorry for splitting up the commits to much as necessary.
• Commit treenerd/chef-bareos@6fcee62
  • Client.rb: Okay I'll rewrite that a bit.
  • server.rb: please explain a bit because I can't see the issues with the permission herre. 640 is also default permission for the other configuration files of bareos.
  • storage.rb: Okay I'll rewrite that a bit
• Commit treenerd/chef-bareos@8951628
  • :-D Hm; Yes I see what you mean. I did this as workaround, because when postgres user is creating the tables, bareos-dir tests failed with wrong table permissions. Perhaps you have an idea why the tables are created with the owner postgres and the bareos-dir test fails in that situation. (I'm no postgresql expert)
  • AH, I see. This was no problem with CentOS installations. The compiling is a workaround for the Ubuntu installations and omnibus for the postgresql cookbook https://tickets.opscode.com/browse/COOK-1406 . But I can understand what you mean.
• Last commits:
  • Never used rubocop before, now I know how to use it; Thanks

Nice to work with you
treenerd

[treenerd]

@GitBytes @sitle
Just for your information;

I asked a little about the reload/restart topic in the bareos forum.
https://groups.google.com/forum/#!topic/bareos-users/o0F0fgIb7Sg

[GitBytes]

@treenerd That is some good information you got from them. I'll have to look that over again and see if I can work that stuff in where appropriate unless you get to it first. I have been swamped this past week even though I would love to work on this project more.. :(

Regarding the other post(s) I haven't had time to comment on.. I think the one thing I wanted to explain was about the server.rb bits you asked about. So, the bareos user isn't meant to be logged into normally, in my opinion anyway. By changing the ability of the bareos user in passwd to use /bin/sh, you are opening up that user to be broken into. If you leave it as /bin/false or /bin/nologin you are preventing the use of the bareos user having a shell environment (service account). It should still do all the daemon stuff it needs to properly but wont open up the users shell access, which, if you have root you can gain access to the bareos user with an su - bareos -s /bin/sh if you are troubleshooting. The 640 bits I think will work just fine and is easy enough to fix if there happens to somehow be a problem down the road. I was mainly concerned with giving shell access to bareos account. Made me feel a bit uneasy.

Just to note, under the security section of the Bareos doc (http://doc.bareos.org/master/html/bareos-manual-main-reference.html#QQ2-1-526) the File Daemon must be run as root, the Storage Daemon is recommended to run as root but is not required, and the director does not have to be run as root. In fact, I think I am running my director as bareos. I mention this to give scope to what files will need access by root and which can be designated non-root (bareos) access, and I do like moving things to 640 because at most we want just root and bareos to have access and others none. I really only post this as informational for my reference too.

Also, I do like the autogenerate password bits you discussed, I had the same thought and want to try to implement that utilizing the OpenSSL Cookbook more or less like the other passwords in the cookbook.

I think there is a way to avoid the psql problem you mentioned via the pg_hba.conf options out there (md5 versus ident) but that may take some testing to work it out. Like I said I would love to spend more time on things I just haven't had the time this past week overall...

I like the idea on this:

We could check if the password is empty, don't parse the address into the configfile?

I am glad this whole process has introduced you to rubocop, very useful linting tool. I am not sure what else to hammer out here other than what we have discussed. Feel free to make any further changes and we will get things closed up here or we can maybe get a new PR if we really have to but that is up to you. Thanks!

BR, GitBytes (Ian)

[treenerd]

@GitBytes

Hi back again,
Used my brain a bit and at the end it was easy to get rid of the shell for the bareos user.

[GitBytes]

@treenerd

Used my brain a bit and at the end it was easy to get rid of the shell for the bareos user.

No worries! Thanks for doing that, I feel much better now! =)

[treenerd]

@GitBytes

So now also the check works with the socket postgresql connection. and thats default db configuration again. should be easier for you to test because of less changes.

[GitBytes]

@treenerd
Have you completed kitchen tests with Ubuntu in this current state? I expect this to pass I just want to ask before we move forward.

@sitle
How is this looking in its current state to you sir? Let me know your thoughts and we will get this PR closed. Thanks!

[GitBytes]

@sitle
Also, I think it would actually be a good idea to make the develop branch the main branch we work out of. Then maybe we can look at some kind of release cycle in terms of supermarket releases after gathering some amount of updates/changes, blah blah blah.. I don't think I have rights to make that change, I'll have to double check.. Let me know whenever no rush on that obviously. Thanks!

[treenerd]

@GitBytes

So now I rolled back the changes in the database recipe, you are totaly right to get rid of the database cookbook. I changed the role a bit, otherwise the database scripts couldn't read the bareos-dir.conf because it wouldn't exist. I also changed the check a bit, because there where issues in ubuntu with 'su - bareos ...' . And I don't know why but I had to converge the debian server twice to get it run.
Perhaps you have an idea why.

Cya
Treenerd

[GitBytes]

@treenerd I'm not sure without looking over the kitchen output really from the first run. I am not sure of a concise way to post that output in this thread.. In theory I should be able to look back and see which bit failed on initial startup. I am suspecting the restarting of the director though for some reason perhaps... Let me know if you can not find anything, I will try to validate this case at my site and see what I can find as well.

[GitBytes]

@treenerd We should be able to kill this line now I think?

[treenerd]

Yes sure, simply forgot to put it out, thank you for the reminder.

[GitBytes]

@treenerd
Looking good at the moment. If we can just figure out the Ubuntu issue we should be good to merge from here essentially I think. @sitle can you confirm?

[treenerd]

@GitBytes @sitle

So for Debian78, Ubuntu14 and CentOS66 it is working now.
Next steps are the changes for Centos7, but I will put that stuff in a new PR.

[treenerd]

Just for information; I got the issues with CentOS7.

There was an bug in vagrant 1.6.5. Which made problems with CentOS7.
hashicorp/vagrant#4784

And the second thing is, even the bareos-director in CentOS7 only supports restart action.

[sitle]

@treenerd Feel free to add your name on the documentation if you agree with that.

@treenerd @GitBytes Thanks for your work.