Changes how pulp-selinux RPM decides when to run restorecon statements #2839
Conversation
@dkliban, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bowlofeggs, @jortel and @beav to be potential reviewers. |
else | ||
return 1 | ||
fi | ||
} |
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.
bash does the implicit return thing, sorta like ruby, so this can just be
function version_less_than {
# Determines if the version passed in as the first argument is less than the version in the second
# argument.
[[ $(echo -e $1'\n'$2|sort -V|head -n 1) != $2 ]]
}
4863c98
to
07a33c5
Compare
oldversion=$(semodule -l | grep pulp-server) | ||
echo ${oldversion:12} > %{_localstatedir}/lib/rpm-state/%{name}/old-version | ||
oldversion=$(rpm -qa pulp-selinux) | ||
echo ${oldversion:13} > %{_localstatedir}/lib/rpm-state/%{name}/old-version |
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.
What is the purpose of this line?
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.
Line 971 takes a string that looks like 'pulp-selinux-2.10.0-0.1' and writes just '2.10.0-0.1' to a file called old-verion. This is done before the uninstalling the pulp-selinux package. The content of that file is then used in the post install step as the argument for the relabel.sh script.
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.
Thank makes sense, thanks
@@ -1,7 +1,13 @@ | |||
#!/usr/bin/env bash | |||
|
|||
function version_less_than { |
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.
For readability can this become:
function version_less_than () {
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.
sure
@dkliban What testing was done with this? |
function version_less_than { | ||
# Determines if the version passed in as the first argument is less than the version in the second | ||
# argument. | ||
[[ $(echo -e $1'\n'$2|sort -V|head -n 1) != $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.
I did several comparisons locally, and I think this statement is exactly what we need.
I tested all the lines of code individually. I created a copy of the relabel.sh file, but replaced all the restorecon statements with echo statements. I also checked that the statements that use rpm -qa pulp-selinux produce the correct version string. I did not build an RPM and try to install it. |
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.
@dkliban this is great work, thanks.
Can you also post a test plan on the bug and send a note to pulp-qe-list identifying that this issue needs manual testing.
Thank you!
RHEL 7.3 was experiencing a bug that was preventing the pulp-selinux RPM from using semodule -l to figure out the installed version of pulp-selinux policies during upgrades. This patch switched to using rpm -qa for determining the version of previously installed SELinux policy. The version comparison logic in relabel.sh only worked for version strings <= 1.9.z. This patch improves this code to make sure upgrades to 2.10.2 don't accidently run unnecesary restorecon statements. closes pulp#2434 https://pulp.plan.io/issues/2424
07a33c5
to
29c2026
Compare
RHEL 7.3 was experiencing an bug that was preventing the pulp-selinux RPM from using semodule -l to
figure out the installed version of pulp-selinux policies during upgrades.
The other change is in relabel.sh. This change fixed version comparison to account for the fact that
2.10.0 is greater than 2.2.0.
closes #2434
https://pulp.plan.io/issues/2424