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

Change usage of STRAP_INTERACTIVE to STRAP_SUDO_PROMPT #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mpatnode
Copy link

So I was a little frustrated/surprised that setting STRAP_SUDO_PROMPT to false didn't stop strap from asking for my password. Seemed to make sense to me, but I can see how it might break existing usage.

lib/ansible.sh Outdated
@@ -16,7 +16,7 @@ strap::lib::import exec || . exec.sh

STRAP_HOME="${STRAP_HOME:-}"; [[ -n "$STRAP_HOME" ]] || { echo "STRAP_HOME is not set" >&2; exit 1; }
STRAP_USER_HOME="${STRAP_USER_HOME:-}"; [[ -n "$STRAP_USER_HOME" ]] || { echo "STRAP_USER_HOME is not set" >&2; exit 1; }
STRAP_INTERACTIVE="${STRAP_INTERACTIVE:-}"; [[ -n "$STRAP_INTERACTIVE" ]] || STRAP_INTERACTIVE=true
STRAP_SUDO_PROMPT="${STRAP_SUDO_PROMPT:-}"; [[ -n "$STRAP_SUDO_PROMPT" ]] || STRAP_SUDO_PROMPT=true
Copy link
Author

Choose a reason for hiding this comment

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

So one backwards compat option could be to change this to:

STRAP_SUDO_PROMPT="${STRAP_SUDO_PROMPT:-$STRAP_INTERACTIVE}"; [[ -n "$STRAP_SUDO_PROMPT" ]] || STRAP_SUDO_PROMPT=true

Happy to make that change if folks think it would be useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

That change looks better from a backwards-compatibility stance IMO

Copy link
Author

Choose a reason for hiding this comment

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

👍

@lhazlewood
Copy link
Contributor

@kelvinzhu-okta and @seanwang-okta can you guys take a look at this and verify?

@lhazlewood
Copy link
Contributor

@mpatnode can you please help me understand the change? I could be missing something, but it looks like just a variable rename - why does renaming a variable change anything?

@mpatnode
Copy link
Author

mpatnode commented Aug 27, 2020

STRAP_SUDO_PROMPT is already being used in sudo.sh. STRAP_INTERACTIVE appears to be a bigger hammer which says don't expect a TTY, while STRAP_SUDO_PROMPT says stop asking for the sudo password. The latter is all I want for my usage case, I don't want to disable all interactive prompts.

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.

None yet

2 participants