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

fix a bug that prevents launch-time passphrases w/ cred plugins #4807

Conversation

ryanpetrello
Copy link
Contributor

with the advent of credential plugins there's no way for us to actually
know
the RSA key value at the time the credential is created, because
the order of operations is:

  1. Create the credential with a specified passphrase
  2. Associate a new dynamic inventory source pointed at some third party
    provider (hashi, cyberark, etc...)

this commit removes the code that warns you about an extraneous
passphrase (if you don't specify a private key)

additionally, the code for determining whether or not a credential
requires a password/phrase at launch time has been updated to test
private key validity based on the actual value stored in the third party
provider

see: #4791

@@ -151,7 +151,7 @@ def needs_ssh_password(self):
@property
def has_encrypted_ssh_key_data(self):
try:
ssh_key_data = decrypt_field(self, 'ssh_key_data')
ssh_key_data = self.get_input('ssh_key_data')
Copy link
Contributor Author

@ryanpetrello ryanpetrello Sep 23, 2019

Choose a reason for hiding this comment

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

I hate that we have to actually pull the value from the third party credential here (because it's in the HTTP request), but I can't think of any other way to do this correctly.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@omgjlk
Copy link
Contributor

omgjlk commented Sep 23, 2019

I tried editing the AWX container contents to apply this patch manually, then restarted web/task containers. It still doesn't seem to actually USE the passphrase, or at least the error screen still shows a prompt for the passphrase for the key data.

@ryanpetrello
Copy link
Contributor Author

ryanpetrello commented Sep 23, 2019

Odd. I was able to apply a passphrase before/after this PR and run an adhoc command on my personal server using a passphrase I specified at launch time:

image

image

@omgjlk
Copy link
Contributor

omgjlk commented Sep 23, 2019

My bad, I had edited the web container venv, rather than the task venv. Once I edited the task one it seemed to work.

@ryanpetrello ryanpetrello force-pushed the cred-plugins-with-promptable-passphrase branch from a79fc3e to 505acb5 Compare September 26, 2019 17:17
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@ryanpetrello
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@ryanpetrello ryanpetrello force-pushed the cred-plugins-with-promptable-passphrase branch from 505acb5 to 4844005 Compare September 26, 2019 21:01
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

with the advent of credential plugins there's no way for us to *actually
know* the RSA key value at the time the credential is _created_, because
the order of operations is:

1.  Create the credential with a specified passphrase
2.  Associate a new dynamic inventory source pointed at some third party
    provider (hashi, cyberark, etc...)

this commit removes the code that warns you about an extraneous
passphrase (if you don't specify a private key)

additionally, the code for determining whether or not a credential
_requires_ a password/phrase at launch time has been updated to test
private key validity based on the *actual* value from the third party
provider

see: ansible#4791
@ryanpetrello ryanpetrello force-pushed the cred-plugins-with-promptable-passphrase branch from 4844005 to d30d51d Compare September 26, 2019 21:14
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

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