search

ceph / ceph-cookbook

Chef cookbooks for Ceph
Apache License 2.0
100 stars 108 forks source link

Flag sensitive execute and file resources #199

Closed scarvalhojr closed 8 years ago

scarvalhojr commented 9 years ago

Use sensitive attribute on execute and file resources that may expose sensitive data. This avoids keys and secrets appearing on chef-client logs.

scarvalhojr commented 9 years ago

Foodcritic isn't recognising the sensitive attribute. It seems that a fix is on the way though: https://github.com/acrmp/foodcritic/issues/289

guilhem commented 9 years ago

I agree with this feature, but it seems to be only for chef12. Can you add a test (defined? maybe) to not fail on chef11?

scarvalhojr commented 9 years ago

I've added a check for old Chef servers.

guilhem commented 9 years ago

@scarvalhojr do you test it on chef11? I fear some problem like here: https://github.com/opscode-cookbooks/aws/pull/110/files#r25011292

scarvalhojr commented 9 years ago

I'm actually running this on a Chef server 11.1 and chef-client 11.12.8 without the need to check if sensitive is supported. If you prefer to be on the safe side and use instance_methods.include, I'm happy to do it.

guilhem commented 9 years ago

I really prefer your way ;) If it works let's go for it. I will test it on TK.

@hufman I'm waiting your go

scarvalhojr commented 9 years ago

Let me know how your test goes and if you need any changes.

scarvalhojr commented 9 years ago

@guilhem, did you manage to test this change? I have another change for the client LWRP and I'm wondering if I should keep the sensitive attribute or not. Thanks

scarvalhojr commented 9 years ago

Can't seem to silence Foodcritic's false positive...

hufman commented 9 years ago

Try making the ~FC009 comment be attached to the first line of the file resource, and not to the actual attribute. This commit seems to do this, and the error message points to the first line of the resource.

Otherwise it looks great!

hufman commented 8 years ago

Can you rebase this to the current master? I merged a bunch of PRs and caused this one to conflict, I'm sorry!

scarvalhojr commented 8 years ago

Done

On Wed, Aug 5, 2015 at 7:24 PM, Walter Huf notifications@github.com wrote:

Can you rebase this to the current master? I merged a bunch of PRs and caused this one to conflict, I'm sorry!

— Reply to this email directly or view it on GitHub https://github.com/ceph/ceph-cookbook/pull/199#issuecomment-128099292.

hufman commented 8 years ago

Looks good to me!