Skip to content

RS-322: Unify uninstall commands #526

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 10 commits into from
Nov 19, 2021
Merged

Conversation

vikin91
Copy link
Contributor

@vikin91 vikin91 commented Nov 15, 2021

This PR is a follow-up to the following tickets:

It unifies the command used to uninstall undesired packages from the Docker image.
The unified command was proposed as rpm --verbose -e --nodeps $(rpm -qa curl '*rpm*' '*dnf*' '*libsolv*' '*hawkey*' 'yum*') with exception for scanner to not remove rpm.

Additionally, it was discussed in Slack (link to the discussion in the ticket https://stack-rox.atlassian.net/browse/RS-322) that we should not remove subscription-manager as it cannot be used to install potentially malicious components.

How tested

Did docker build and docker run afterwards and manually confirmed that package managers are removed but rpm and subscription-manager kept.

Unable to build this image locally

➜ make scanner-image
env: Studio: No such file or directory
...
env: Studio: No such file or directory
make: *** [scanner-image-builder] Error 127

so I trust that CI would complain if, for example, rpm binary would be missing.

@vikin91 vikin91 marked this pull request as ready for review November 17, 2021 13:05
Copy link
Contributor

@msugakov msugakov left a comment

Choose a reason for hiding this comment

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

LGTM.
I recommend getting an approval from @RTann and/or @connorgorman before merging.

@vikin91 vikin91 requested a review from RTann November 17, 2021 14:30
Copy link
Collaborator

@RTann RTann left a comment

Choose a reason for hiding this comment

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

When do we want to have this merged? I plan on cutting a new scanner release today, but if this needs to go into 67, then I will wait

@RTann
Copy link
Collaborator

RTann commented Nov 17, 2021

When do we want to have this merged? I plan on cutting a new scanner release today, but if this needs to go into 67, then I will wait

This was answered on Slack. It is not a blocker for 67, so I will still cut the release today

@vikin91 vikin91 force-pushed the pr/RS-322-unify-uninstall-cmd branch from 8f6a92a to a3bc8e5 Compare November 18, 2021 07:59
@vikin91 vikin91 merged commit dd3f349 into master Nov 19, 2021
@vikin91 vikin91 deleted the pr/RS-322-unify-uninstall-cmd branch November 19, 2021 08:42
RTann pushed a commit that referenced this pull request Jan 27, 2022
* X-Smart-Branch-Parent: master

* Unify uninstallation commands in Dockerfiles (keeping rpm)
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.

4 participants