Move certificate bundle to auth bundle - #22277
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdate the foremanctl capsule bundle test to target the new auth-bundle command instead of certificate-bundle and validate that OAuth credentials are included in the bundle tarball in addition to existing certificate checks. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Now that
foremanctl_capsule_cert_pathsalso validates oauth files, consider renaming the helper (and/or its docstring/return values) to better reflect that it deals with the full auth bundle rather than only certificate paths, so future readers don’t assume it’s certificate-only.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that `foremanctl_capsule_cert_paths` also validates oauth files, consider renaming the helper (and/or its docstring/return values) to better reflect that it deals with the full auth bundle rather than only certificate paths, so future readers don’t assume it’s certificate-only.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
PRT Result |
|
|
PRT Result |
|
|
PRT Result |
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider renaming
foremanctl_capsule_cert_paths(and related variables likecapsule_certs) to something like*_auth_bundle_pathsto better reflect that the helper now validates OAuth files in addition to certificates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider renaming `foremanctl_capsule_cert_paths` (and related variables like `capsule_certs`) to something like `*_auth_bundle_paths` to better reflect that the helper now validates OAuth files in addition to certificates.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
PRT Result |
|
|
@arvind4501 Can you remove |
|
|
PRT Result |
|
|
PRT Result |
| @pytest.mark.rhel_ver_match('9') | ||
| def test_positive_foremanctl_certificate_bundle(module_sat_ready_rhel, rhel_contenthost): | ||
| """Verify foremanctl certificate-bundle generation and renewal for a capsule. | ||
| def test_positive_foremanctl_auth_bundle(module_sat_ready_rhel, rhel_contenthost): |
There was a problem hiding this comment.
I would recommend using module_target_sat instead of module_sat_ready_rhel. There might be a conflict caused by using module_sat_ready_rhel, as it is created only when we don't have the setup available.
|
20ff8d8 to
08f2ba2
Compare
|
|
PRT Result |
Problem Statement
theforeman/foremanctl#698 changes the old certificate bundle command and adds oauth/ credentials into the bundle
Solution
Change according to theforeman/foremanctl#698
Related Issues
Summary by Sourcery
Tests: