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

Hide RHN,yum in main output when Ubuntu/Debian is detected #220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BryanQuigley
Copy link

I tried to code this so I don't think it will break when used for RHEL, but it seems like there a lot of corner cases.

Figured getting an ack that this is a good direction before doing extensive testing. Thanks!

@superjamie
Copy link
Collaborator

Sorry for the delay. Yes this is a good direction.

If you don't want to source os-release then you could grep it. Quick pseudocode sketch to give the idea:

[[ -r "$1/etc/os-release" ]] &&
  if   [[ egrep "ID=\"?(rhel|fedora|centos)" ]]; then
    packaging=rpm
  elif [[ egrep "ID=\"?(ubuntu|debian)" ]]; then
    packaging=deb
  fi

I'd worry more about variables colliding than /etc being used as an attack vector, but perhaps I am not sufficiently paranoid ;)

@BryanQuigley
Copy link
Author

Oh, "# I don't like blindly sourcing a file -- that provides a vector to screw with this script..." was already present. I don't mind it, nor do I think it's super likely to mess things up.

We've been using my branch internally at Canonical for Ubuntu as part of ScoutPlane tool for months so the basics still work great.

My biggest worry is that I could have broken some RH/N specific bits that I can't test.

if source "$1/etc/os-release" 2>/dev/null; then
distro_release="${c[Imp]}[ID]${c[0]} $ID"
if [[ "$ID" == "rhel" || "$ID" == "fedora" || "$ID" == "centos" ]]; then
packaging=rpm
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add packaging to the local vars of OSINFO()?

echo -e " ${c[H2]}RHN:${c[0]} $rhn_serverURL$rhnProxyStuff"
echo -e " ${c[H2]}RHSM:${c[0]} $rhsm_hostname$rhsmProxyStuff"
fi
if [ $packaging = yum ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be rpm?

@superjamie
Copy link
Collaborator

Looking at this again, I think the order which xsos determines and displays OS release is wrong now.

It was originally written a long time ago to think of RHEL first, but these days Ubuntu should also be a first-class citizen. We could handle other distros which carry recent sosreport packages like AlmaLinux, Debian, Fedora, Mageia.

I'm inclined to close this PR and re-order/rewrite _CHECK_DISTRO to be more distro-agnostic, so still implement the idea of this PR plus end up with an even better result. I will have a think.

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.

3 participants