Skip to content
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

[Security] Disable unused background services: wpa_supplicant and cups. #2656

Conversation

gmarciani
Copy link
Contributor

Description of changes

Disable unused background services: wpa_supplicant and cups.

Test

  • Spec tests for aws-parallelcluster-platform
  • Kitchen tests ./kitchen.ec2.sh platform-install test disable-services

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.48%. Comparing base (7d414fe) to head (a647004).
Report is 5 commits behind head on develop.

❗ Current head a647004 differs from pull request most recent head b6da231. Consider uploading reports for the commit b6da231 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2656   +/-   ##
========================================
  Coverage    76.48%   76.48%           
========================================
  Files           22       22           
  Lines         2220     2220           
========================================
  Hits          1698     1698           
  Misses         522      522           
Flag Coverage Δ
unittests 76.48% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gmarciani gmarciani force-pushed the wip/mgiacomo/390/disable-unused-services branch from 26d6f95 to addc64f Compare February 19, 2024 11:50
@gmarciani gmarciani force-pushed the wip/mgiacomo/390/disable-unused-services branch from a647004 to ef49507 Compare April 16, 2024 17:31
Signed-off-by: Giacomo Marciani <mgiacomo@amazon.com>
@gmarciani gmarciani force-pushed the wip/mgiacomo/390/disable-unused-services branch from ef49507 to b6da231 Compare April 16, 2024 17:35
@@ -27,3 +27,13 @@
service 'log4j-cve-2021-44228-hotpatch' do
action %i(disable stop mask)
end unless on_docker?

# Necessary on Ubuntu and Amazon Linux 2
service 'cups' do
Copy link
Contributor

Choose a reason for hiding this comment

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

This recipe is not following the general code structure.

It's not strictly related to your change, but when there is code "os specific" we should create a Chef Resource for it.

The idea is to have in the recipes only common code, the recipe should be the orchestrator that will call the resource to perform OS specific actions.

In this way if we want to fix/test/search code for a specific OS we don't need to search in all the recipes but we can rely on the resources.
Then if we want to add/remove support for a specific OS we just need to update the resources avoiding to touch the recipes.

In this case we can create a resource called "service_manager" or something like this and call it directly in in the install recipe, with a different flavour for alinux2, ubuntu, u22, etc.

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. Refactoring captured in our backlog. We will decouple the refactoring from this change though.

@gmarciani gmarciani closed this Jul 11, 2024
@gmarciani
Copy link
Contributor Author

Closed in favour of #2775

@gmarciani gmarciani reopened this Jul 11, 2024
@gmarciani gmarciani closed this Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants