-
-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add possibility to stop recursion on gnupghome #290
Conversation
If this pull request gets accepted in the end, it should probably be rebased onto master. |
So I changed a lot off indentation and layout. But how on earth have anyone else been able to get travis checks to pass, I wonder. Well now it works. Now rebase and merge, pretty please :-) |
@bahner thanks for the work! a linter plugin was recently updated and is now stricter. Had had not enough time to fix this on all of our modules. I made two inline comments, please have a look. |
@bahner thanks for the updates. can you add a few tests for this? |
I am confused as how to run the unit tests the way things are set up. (I'm a n00b and only use pdk test unit for now). I can't really figure out how to get things setup for running test here. I'm guessing rake test, but that does definitely not work. How should test be invoked correctly? |
I found some checks missing from rake runs, and fixed them. Doing this I mangled hiera_spec.rb to achieve consistent naming of keysdir. When I started I used default and that caused 3 different namings of the same directory. That seems silly. The reason it ended up being /dev/null/keys is that theam YAML eyaml loader sets confdir to "/dev/null" The string "/dev/null" is not mentioned anywhere in the code. So I went with it and changed keysdir to /dev/null/keys instead of /etc/keys. This is a small change, that increases the coverage percentage when I check some file in the eyaml_gpg_spec.rb. Also added a check for the deep_merge class, just becasue it was easy and on my way. I am getting the hang of this now - but if you would ask me what I have done and how, I would have to do answer in a Mr. Miagi-style. Don't know. Firsta time.
Well, that was a mouthful. As stated, please rebase and squash this onto master, when ready. The git log is way too messy. |
You can run the tests locally like this: |
Only let key be used for (de)encryption. A key without passphrase shouldn't sign anything or be used to authenticate anything. You can trust it to decrypt messages destined to be decrypted by it, but you can never trust anything that is signed by it.
My bad, this has nothing to do in this pull request. This reverts commit 1871e63.
describe 'param manage_package => false' do | ||
let(:params) do | ||
{ | ||
eyaml: true, | ||
eyaml_gpg: true, | ||
manage_package: false, | ||
keysdir: '/etc/keys' | ||
keysdir: '/dev/null/keys' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason for changing the parameter here and a few lines below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. For coverage. The same dir "keys" had 3 different names. "So just to use the same dir in all tests, so that they are easier to write. It's very confusing, when the same dir has different paths. I wrote an explanation in the commit, but the commit message drown a bit here.
The coverage percentage gives a more correct measurement of how tested the module is now. The percentage is now around 67% instead of the 30 something, that was reported. Obviously /etc/keys is also just a fake dir and /dev/null van't (easily) be changed. It doesn't come from the code, but most likely from YAML.load(:eyaml). But the path I can't find defined anywhere.
Pull Request (PR) description
When puppet runs it gets an error that it can't handle sockets. Since gpg-agent is integral to gpg now,
that is an error that is unavoidable with thee current setup.
If you do this manually, ie.
chmod -r 0600 $GNUPGHOME
, then there is no need to run this and get an errorevery run.