-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
[rpm_key] Fix to import first key on the system #31514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments in code. Also, I think this would be better with a integration test included but I can add that later, no worries.
@@ -170,11 +170,15 @@ def execute_command(self, cmd): | |||
return stdout, stderr | |||
|
|||
def is_key_imported(self, keyid): | |||
cmd=self.rpm + ' -q gpg-pubkey --qf "%{description}" | ' + self.gpg + ' --no-tty --batch --with-colons --fixed-list-mode -' | |||
cmd = self.rpm + ' -q gpg-pubkey' | |||
rc, _, _ = self.module.run_command(cmd, use_unsafe_shell=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use_unsafe_shell
is not needed here, also can you please not use _
(the reason being that if we ever get around to i18n'ing our code, we'll be using _
to mark strings), so:
rc, stdout, stderr = self.module.run_command(cmd)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, OK
rc, _, _ = self.module.run_command(cmd, use_unsafe_shell=True) | ||
if rc != 0: # No key is installed on system | ||
return False | ||
cmd += ' --qf "%{description}" | ' + self.gpg + ' --no-tty --batch --with-colons --fixed-list-mode -' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can reuse stdout
from the previous command here, but not a blocker for merge.
Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks @lukas-bednar !
cherry-picked to stable-2.4 for the 2.4.1rc1 release. |
* [rpm_key] Fix to import first key on the system Fixes: ansible#31483 * [rpm_key] removed unsafe_shell and "throwaway" underscore * [rpm_key] adding test to add the first key on system
* [rpm_key] Fix to import first key on the system Fixes: ansible#31483 * [rpm_key] removed unsafe_shell and "throwaway" underscore * [rpm_key] adding test to add the first key on system
SUMMARY
Fixes #31483
ISSUE TYPE
COMPONENT NAME
rpm_key
ANSIBLE VERSION
ADDITIONAL INFORMATION