-
Notifications
You must be signed in to change notification settings - Fork 138
Fix rex_pub_key retrieval for containerized Satellite #22139
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1775,7 +1775,21 @@ def url_katello_ca_rpm(self): | |
|
|
||
| @property | ||
| def rex_pub_key(self): | ||
| return self.execute(f'cat {self.rex_key_path}').stdout.strip() | ||
| if settings.server.install_method == InstallMethod.FOREMANCTL: | ||
| if 'remote-execution' in self.list_foremanctl_features(enabled=True): | ||
| result = self.execute( | ||
| f"podman exec foreman-proxy bash -c 'cat {self.rex_key_path}'" | ||
| ) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Gauravtalreja1 I think fetching this from podman secret makes most sense in case of foremanctl rather than relying on the cat.. We can get around reading files from mounted paths entirely with this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion! I don't have a strong opinion either way, but I'd lean toward sticking with CC @evgeni curious to hear your opinion as well on this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason not to use the APIs we have (Foreman's
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sjha4 I'm not entirely familiar with this API endpoint and don't have a strong opinion against it, but if it works, it would be a much cleaner solution than checking for the public key with two different install-methods |
||
| else: | ||
| raise SatelliteHostError('remote-execution feature is not enabled') | ||
| else: | ||
| result = self.execute(f'cat {self.rex_key_path}') | ||
| key = result.stdout.strip() | ||
| if not key: | ||
| raise ValueError( | ||
| f'Rex public key is empty. Command returned status {result.status}: {result.stderr}' | ||
| ) | ||
| return key | ||
|
|
||
| def is_foremanctl_available(self): | ||
| """Check if foremanctl is installed on the system. | ||
|
|
||
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.
rex is always enabled on sat: theforeman/foremanctl#628
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.
Its not enabled today how we deploy without flavor, but I believe its good to have that check in place as its quick one
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.
Are there plans for something similar with REX enabled on capsule/smart proxies with deploy-proxy. The current default flavor https://github.com/theforeman/foremanctl/blob/master/src/vars/flavors/foreman-proxy-content.yml doesn't have it.
Uh oh!
There was an error while loading. Please reload this page.
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.
yes, maybe in future we'll have dedicate capsule flavor which will have this enabled by default, but I think we do need a quick check today to see if its enabled then fetch pubkey