-
Notifications
You must be signed in to change notification settings - Fork 133
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
Container support and dokken tests in travis CI #199
Conversation
c6ea48a
to
bb2377a
Compare
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.
@artem-sidorenko this is a great addition and I like that we test in do as well as Container. The only thing I see is that we want to be very clear what will be deactivated if we run inside of a container. For mid term it seems not right to say container or not, but instead we focus more eg. on activate sysctl config. Does this make sense?
README.md
Outdated
@@ -35,6 +35,7 @@ It will not: | |||
|
|||
## Attributes | |||
|
|||
* `['os-hardening']['container']['enable']` - true if this is a container. Default value is based on the information provided by ohai |
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.
I think we need to make more clear what the impact of the setting is
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.
There is another PR #200. Should we maybe restructure default.rb in order to have a generic way to control which recipes should get included via default recipe? Something like this?
# default.rb
# generic recipes
%w(packages limits).each do |rp|
node.default['os-hardening']['recipes'][rp] = true
end
# recipes which are not suitable for containers
unless node['virtualization']['system'] =~ /^(lxc|docker)$/ && node['virtualization']['role'] == 'guest' do
node.default['os-hardening']['recipes']['sysctl'] = true
node.default['os-hardening']['recipes']['autditd'] = true
end
# selinux recipe should be included only on RH based systems
node.default['os-hardening']['recipes']['selinux'] =
node['platform_family'] == 'rhel' || node['platform_family'] == 'fedora'
....
node['os-hardening']['recipes'].each do |recipe, state|
include_recipe "#{cookbook_name}::#{recipe}" if state
end
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, this looks good and we keep the decision in default.rb
See dev-sec/chef-os-hardening#199 for reference Signed-off-by: Artem Sidorenko <[email protected]>
See dev-sec/chef-os-hardening#199 for reference Signed-off-by: Artem Sidorenko <[email protected]>
40d0874
to
62e9355
Compare
@chris-rock can you please have a look here again? There are some failing tests with kitchen-dokken, I configured them as allowed to fail and going to fix that in other PRs (mostly linux-baseline) |
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.
Great improvement @artem-sidorenko
@@ -37,7 +37,7 @@ platforms: | |||
- name: opensuse-leap-42 | |||
driver_config: | |||
box: bento/opensuse-leap-42.1 | |||
- name: amzn2 | |||
- name: amazonlinux-2 |
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.
The right call :-)
README.md
Outdated
@@ -35,6 +35,7 @@ It will not: | |||
|
|||
## Attributes | |||
|
|||
* `['os-hardening']['recipes'][RECIPE_NAME]` - allows the fine control over which recipes should be run via default recipe. See below for more details |
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.
Okay for now but we should rename this to ['os-hardening']['components']['component_name]
README.md
Outdated
```ruby | ||
# some attribute file | ||
# do not include sysctl and auditd recipes | ||
override['os-hardening']['recipes']['sysctl'] = false |
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.
I would call this [os-hardening]['components'], since this frees us from having a 1:1 relationship which we may not want in the future. Except from that this makes perfect sense.
b8df8e5
to
4e57419
Compare
Try to detect the good defaults via ohai. Allow overriding of recipes Signed-off-by: Artem Sidorenko <[email protected]>
and DO tests on the full VMs if possible Signed-off-by: Artem Sidorenko <[email protected]>
as rsyslog isn't installed within containers, syslog group doesn't exist and the group of /var/log is root
They will be addressed in a dedicated PRs, esp to the linux-baseline
The idea is to pick up the #184 here and to add container based tests at least for forks. The logic looks like this:
@chris-rock whats your view on this? Do you have a good suggestion/idea how we can exclude set of tests in the baseline within this setup?
Fixes #200