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

Install: Be consistent with use of the "sudo" - v1 #9552

Closed
wants to merge 1 commit into from

Conversation

0xEniola
Copy link
Contributor

@0xEniola 0xEniola commented Oct 4, 2023

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/5720

Describe changes:

  • Added the "sudo" command to commands that require administrative privileges before they can be executed.

Included the "sudo" command to commands that need to be executed with the super-user privilege.
@jufajardini jufajardini added the outreachy Contributions made by Outreachy applicants label Oct 4, 2023
@jasonish
Copy link
Member

jasonish commented Oct 4, 2023

Thanks for your contribution. With respect to the details of the commit itself, could you make the following changes:

Comment on lines +270 to 271
sudo echo "deb http://http.debian.net/debian buster-backports main" > \
/etc/apt/sources.list.d/backports.list
Copy link
Member

Choose a reason for hiding this comment

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

This particular command will actually fail when sudo is added, instead it should probably be:

echo "deb http://http.debian.net/debian buster-backports main" | sudo tee -a /etc/apt/sources.list.d/backports.list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! Thank you for the feedback.

@jasonish
Copy link
Member

jasonish commented Oct 4, 2023

We probably also need a .. note:: or whatever the syntax is that sudo must be installed to follow the directions as-is. If you install a minimal Debian, Ubuntu or RedHat EL like system, sudo often isn't there.

Copy link
Member

@jasonish jasonish left a comment

Choose a reason for hiding this comment

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

Some updates required. Thanks!

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #9552 (512eb9d) into master (9157070) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9552      +/-   ##
==========================================
- Coverage   82.20%   82.19%   -0.01%     
==========================================
  Files         968      968              
  Lines      274275   274275              
==========================================
- Hits       225461   225435      -26     
- Misses      48814    48840      +26     
Flag Coverage Δ
fuzzcorpus 64.05% <ø> (+<0.01%) ⬆️
suricata-verify 60.91% <ø> (-0.05%) ⬇️
unittests 62.87% <ø> (-0.01%) ⬇️

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

@0xEniola
Copy link
Contributor Author

0xEniola commented Oct 4, 2023

We probably also need a .. note:: or whatever the syntax is that sudo must be installed to follow the directions as-is. If you install a minimal Debian, Ubuntu or RedHat EL like system, sudo often isn't there.

Installing sudo on a minimal system, one will have to be able to log in to root, and if the root password was not set at the time of installation, one will have to chroot into the system from a live disc/usb to set the root password.

@jasonish
Copy link
Member

jasonish commented Oct 4, 2023

We probably also need a .. note:: or whatever the syntax is that sudo must be installed to follow the directions as-is. If you install a minimal Debian, Ubuntu or RedHat EL like system, sudo often isn't there.

Installing sudo on a minimal system, one will have to be able to log in to root, and if the root password was not set at the time of installation, one will have to chroot into the system from a live disc/usb to set the root password.

Where I often get stuck is... I install minimal system, usually in a container. Then cut and paste a page of instructions that use sudo, but sudo isn't there. Usually a quick apt install sudo fixes that but it can save someone reaching out to forums for support by having a one line item in the documentation.

@0xEniola
Copy link
Contributor Author

0xEniola commented Oct 4, 2023

We probably also need a .. note:: or whatever the syntax is that sudo must be installed to follow the directions as-is. If you install a minimal Debian, Ubuntu or RedHat EL like system, sudo often isn't there.

Installing sudo on a minimal system, one will have to be able to log in to root, and if the root password was not set at the time of installation, one will have to chroot into the system from a live disc/usb to set the root password.

Where I often get stuck is... I install minimal system, usually in a container. Then cut and paste a page of instructions that use sudo, but sudo isn't there. Usually a quick apt install sudo fixes that but it can save someone reaching out to forums for support by having a one line item in the documentation.

re: a quick apt install sudo fixes that.

You know majority of these packages cannot be used without sudo or root privileges.

@jasonish
Copy link
Member

jasonish commented Oct 5, 2023

You know majority of these packages cannot be used without sudo or root privileges.

Yes, but you can build, install, run Suricata-Verify, and do a lot of development before ever needing sudo. But its kind of beside the point, its about providing instructions that just work, with a note about where they might not work for you, like on a minimal install, or someone setting up their environment in docker.

@0xEniola
Copy link
Contributor Author

0xEniola commented Oct 5, 2023

You know majority of these packages cannot be used without sudo or root privileges.

Yes, but you can build, install, run Suricata-Verify, and do a lot of development before ever needing sudo. But its kind of beside the point, its about providing instructions that just work, with a note about where they might not work for you, like on a minimal install, or someone setting up their environment in docker.

Yes I understand.

What I'm saying is, in the case a user wants to take the sudo route, to install sudo on Minimal, they'll have to boot live into their system and chroot into the system.

So I'll have to put these details into the doc then, right?

@jasonish
Copy link
Member

jasonish commented Oct 5, 2023

You know majority of these packages cannot be used without sudo or root privileges.

Yes, but you can build, install, run Suricata-Verify, and do a lot of development before ever needing sudo. But its kind of beside the point, its about providing instructions that just work, with a note about where they might not work for you, like on a minimal install, or someone setting up their environment in docker.

Yes I understand.

What I'm saying is, in the case a user wants to take the sudo route, to install sudo on Minimal, they'll have to boot live into their system and chroot into the system.

So I'll have to put these details into the doc then, right?

No, I've never had to do that. Usually when you don't have sudo already installed, you're dropped in as root. A container is a great example of this, or even many GitHub/Copilot dev containers. Our instructions here will fail if I just go cut and paste them. So just simply a note that the following instructions require sudo. Sphinx provides a markup for these types of note with something like .. note:

@0xEniola
Copy link
Contributor Author

0xEniola commented Oct 5, 2023

You know majority of these packages cannot be used without sudo or root privileges.

Yes, but you can build, install, run Suricata-Verify, and do a lot of development before ever needing sudo. But its kind of beside the point, its about providing instructions that just work, with a note about where they might not work for you, like on a minimal install, or someone setting up their environment in docker.

Yes I understand.

What I'm saying is, in the case a user wants to take the sudo route, to install sudo on Minimal, they'll have to boot live into their system and chroot into the system.

So I'll have to put these details into the doc then, right?

No, I've never had to do that. Usually when you don't have sudo already installed, you're dropped in as root. A container is a great example of this, or even many GitHub/Copilot dev containers. Our instructions here will fail if I just go cut and paste them. So just simply a note that the following instructions require sudo. Sphinx provides a markup for these types of note with something like .. note:

OK! I understand you now.

So I should just specify at the beginning of each distro sub-heading a note that sudo is required to be installed for the instructions to work.

OK! On it.
Thank you for your assistance, Jason.

@0xEniola
Copy link
Contributor Author

0xEniola commented Oct 5, 2023

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/5720

Describe changes:

  • Added the "sudo" command to commands that require administrative privileges before they can be executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
outreachy Contributions made by Outreachy applicants
3 participants