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

adjustments to sudo call and log test #98

Merged
merged 5 commits into from
Nov 5, 2021
Merged

adjustments to sudo call and log test #98

merged 5 commits into from
Nov 5, 2021

Conversation

veggiesaurus
Copy link
Collaborator

@veggiesaurus veggiesaurus commented Nov 5, 2021

Closes #97

@confluence do you think we need to warn users about the additional line in the sudoers file that is now needed?

@confluence
Copy link
Collaborator

I think that there should be some kind of warning. This may be a good time to add a changelog, so that it can be included in the package metadata. We should probably also add the changelog to the documentation.

Is there any way for an npm package to print messages for the user upon installation?

@confluence
Copy link
Collaborator

I wonder if a safer option would be 1) to give the user explicit control over the sudo arguments as well as the sudoers config, and 2) leave the previous setup as the default. This would make upgrades safe, and also makes sense because we're compensating for an old version of sudo rather than changing to accommodate a new version.

If we did that, we could just add a note to the extended configuration explaining what to do with an older version of sudo.

We could also automatically detect the sudo version and pick the correct arguments (instead of exposing this to the user).

@veggiesaurus
Copy link
Collaborator Author

I wonder if a safer option would be 1) to give the user explicit control over the sudo arguments as well as the sudoers config, and 2) leave the previous setup as the default. This would make upgrades safe, and also makes sense because we're compensating for an old version of sudo rather than changing to accommodate a new version.

That's a good point. I've actually just tried bionic and centos7/8 docker images, and they all seem to have a new enough sudo version, so I suspect this is related to RHEL7 in particular.

@veggiesaurus
Copy link
Collaborator Author

veggiesaurus commented Nov 5, 2021

@confluence I have now added the config option you suggested, please re-evaluate

config/example_sudoers_conf.stub Outdated Show resolved Hide resolved
@veggiesaurus veggiesaurus merged commit 15dd981 into dev Nov 5, 2021
@veggiesaurus veggiesaurus deleted the angus/sudo_fix branch November 5, 2021 12:02
veggiesaurus added a commit that referenced this pull request Nov 25, 2021
@veggiesaurus veggiesaurus mentioned this pull request Nov 25, 2021
veggiesaurus added a commit that referenced this pull request Nov 25, 2021
* backport #98

* version bump
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.

Use entry in sudoers config instead of --preserve-env when starting backend process
2 participants