Skip to content

Remove control for running Database #54

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 8, 2023
Merged

Remove control for running Database #54

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bfd74b9
Fix #17: Checking postgresql status on Ubuntu
professormahi Dec 8, 2023
b402ef6
Fix a typo on using double-quoted strings
professormahi Dec 8, 2023
17eb756
Remove postgre-01
professormahi Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions controls/postgres_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,16 @@
impact 1.0
title 'Postgresql should be running'
desc 'Postgresql should be running.'
describe service(postgres.service) do
it { should be_installed }
it { should be_running }
it { should be_enabled }
if os[:name] == 'ubuntu'
describe command('/etc/init.d/postgresql status') do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still exist on Ubuntu 22.04? Ubuntu uses systemd..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I've tested on both my local machine and also your docker images for molecule.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really a fan of this solution. We depend on a file (that's not really in use anyway) and its output.. What do you think about doing it like in another baseline?

# set OS-dependent filenames and paths
case os[:family]
when 'ubuntu', 'debian'
  process_name = 'postgresql'
when 'redhat', 'fedora'
  process_name = 'postgres'
when 'suse'
  process_name = 'postgresql'
end

Note that these are probably not the correct names..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have some problems with using if here. What about we execute all describe blocks always and group them into a describe.one block (https://docs.chef.io/inspec/profiles/controls/#check-if-at-least-one-condition-passes)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with both comments and starting to fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really a fan of this solution. We depend on a file (that's not really in use anyway) and its output.. What do you think about doing it like in another baseline?

# set OS-dependent filenames and paths
case os[:family]
when 'ubuntu', 'debian'
  process_name = 'postgresql'
when 'redhat', 'fedora'
  process_name = 'postgres'
when 'suse'
  process_name = 'postgresql'
end

Note that these are probably not the correct names..

Is this example from MySQL baseline? If yes, there is no control for "mysql-conf-01" there. Can we remove this control for PostgreSQL, too? It seems the "postgres-03" also checks the state of the Postgres daemon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we dropped it here: dev-sec/mysql-baseline#78

So I think, we should delete the test here, too.

@schurzi, thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds reasonable. Let's do this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 17eb756.

its('stdout') { should include 'active' }
end
else
describe service(postgres.service) do
it { should be_installed }
it { should be_running }
it { should be_enabled }
end
end
end

Expand Down