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.

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

Conversation

professormahi
Copy link
Contributor

In the Ubuntu operating system, the name of the PostgreSQL service is postgresql.

@schurzi schurzi linked an issue Dec 8, 2023 that may be closed by this pull request
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.

Signed-off-by: Mahdi Fooladgar <[email protected]>
@schurzi schurzi changed the title Fix #17: Checking postgresql status on Ubuntu Remove check for running Database Dec 8, 2023
@schurzi schurzi changed the title Remove check for running Database Remove control for running Database Dec 8, 2023
@schurzi schurzi merged commit bf66b20 into dev-sec:master Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch back to simple InSpec service description
3 participants