Skip to content

docs: simplify Linux sudo instructions in README #1096

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 1 commit into from
Sep 28, 2021
Merged

docs: simplify Linux sudo instructions in README #1096

merged 1 commit into from
Sep 28, 2021

Conversation

jnm
Copy link
Contributor

@jnm jnm commented Sep 21, 2021

Describe an easier way to pass the SENTRY_IMAGE environment variable to install.sh when using sudo that doesn't require modifying the sudo configuration.

Demonstration

Simple test script:

jnm@world$ cat > install.sh
#!/usr/bin/env bash
echo "It looks like you are $(whoami) and want to install $SENTRY_IMAGE"

jnm@world$ chmod +x install.sh

Basic environment variable test:

jnm@world$ SENTRY_IMAGE=whee ./install.sh
It looks like you are jnm and want to install whee

Oops, sudo doesn't forward the variable from its own environment by default (#922):

jnm@world$ SENTRY_IMAGE=whee sudo ./install.sh
[sudo] password for jnm:
It looks like you are root and want to install

However, there's an easier way than modifying the sudo configuration:

jnm@world$ sudo SENTRY_IMAGE=whee ./install.sh
It looks like you are root and want to install whee

My environment is Ubuntu 20.04 (with the KDE neon desktop environment):

jnm@world$ sudo --version
Sudo version 1.8.31
Sudoers policy plugin version 1.8.31
Sudoers file grammar version 46
Sudoers I/O plugin version 1.8.31

jnm@world$ lsb_release -a
No LSB modules are available.
Distributor ID: Neon
Description:    KDE neon User Edition 5.22
Release:        20.04
Codename:       focal

This method is also described in the same StackOverflow discussion that was referenced by #922. Thanks for your consideration, and thanks for making Sentry available for on-premise installations!

Describe an easier way to pass the `SENTRY_IMAGE` environment variable to `install.sh` when using `sudo` that doesn't require modifying the `sudo` configuration
@chadwhitacre
Copy link
Member

Thanks for the PR, @jnm! Looks good at a skim, give me a day to think about it a bit more and also invite @BYK's input to make sure there's no real reason we were doing it the other way. 🤔

Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Great simplification, thanks a lot!

@BYK
Copy link
Member

BYK commented Sep 21, 2021

Will wait till @chadwhitacre and @scefali to chime in before merging.

Copy link
Contributor

@scefali scefali left a comment

Choose a reason for hiding this comment

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

@BYK I have no opinion on this as long as the README is accurate

Copy link
Member

@chadwhitacre chadwhitacre left a comment

Choose a reason for hiding this comment

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

Nice simplification. Thanks @jnm!

@BYK BYK merged commit f2e2dc2 into getsentry:master Sep 28, 2021
@BYK
Copy link
Member

BYK commented Sep 28, 2021

Sorry, did not realize we forgot to merge this in 🙂

@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants