fix: skip package profile upload when disabled in config#3714
Merged
Conversation
Reviewer's GuideThis PR makes package profile upload during registration respect the rhsm.conf Sequence diagram for registration package profile upload respecting report_package_profilesequenceDiagram
actor User
participant RegisterCLI as RegisterCLICommand
participant RHSMConf as RhsmConfig
participant EntCertLib as EntitlementCertLib
participant ProfileMgr as ProfileManager
participant Rhsmcertd as RhsmcertdDaemon
User->>RegisterCLI: run register
RegisterCLI->>EntCertLib: update()
EntCertLib-->>RegisterCLI: updated entitlements
RegisterCLI->>RHSMConf: get_int(report_package_profile)
RHSMConf-->>RegisterCLI: value (0 or 1)
alt report_package_profile == 1
RegisterCLI->>RegisterCLI: _upload_profile(consumer)
RegisterCLI->>Rhsmcertd: send SIGUSR1
Rhsmcertd-->>RegisterCLI: signal received
Rhsmcertd->>ProfileMgr: update_check(cp, consumer_uuid)
ProfileMgr-->>Rhsmcertd: profile upload result
else report_package_profile != 1
RegisterCLI->>RegisterCLI: log skip message
end
RegisterCLI->>RegisterCLI: _request_validity_check()
RegisterCLI-->>User: registration result
Flow diagram for deciding whether to upload package profile on registrationflowchart TD
A[Start registration] --> B[Update entitlement certificates]
B --> C[Read rhsm.conf rhsm.report_package_profile]
C --> D{report_package_profile == 1?}
D -- Yes --> E[Call _upload_profile]
E --> F[Send SIGUSR1 to rhsmcertd]
F --> G[rhsmcertd uploads package profile using ProfileManager.update_check without force]
G --> H[Request validity check]
D -- No --> I[Log skipping package profile upload]
I --> H[Request validity check]
H --> J[End]
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:
- The two new tests for enabled/disabled package profile upload duplicate most of their setup and assertions; consider refactoring them into a single parametrized helper to reduce repetition and keep future changes in sync.
- The check
conf["rhsm"].get_int("report_package_profile") == 1hardcodes the magic value1; if possible, prefer a boolean or a named constant to make the intent clearer and avoid relying on specific numeric values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The two new tests for enabled/disabled package profile upload duplicate most of their setup and assertions; consider refactoring them into a single parametrized helper to reduce repetition and keep future changes in sync.
- The check `conf["rhsm"].get_int("report_package_profile") == 1` hardcodes the magic value `1`; if possible, prefer a boolean or a named constant to make the intent clearer and avoid relying on specific numeric values.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
a0123cb to
7a2ca35
Compare
cnsnyder
reviewed
Mar 4, 2026
cnsnyder
reviewed
Mar 4, 2026
91932ce to
1d23974
Compare
Coverage (computed on Fedora latest) •
|
||||||||||||||||||||||||||||||||||||||
1d23974 to
59ddcdd
Compare
jirihnidek
requested changes
Mar 5, 2026
Contributor
jirihnidek
left a comment
There was a problem hiding this comment.
Thanks for the PR. I have one request to revert one small changes in your code. The reason is explained in the comment.
ce3f3d2 to
2a9076b
Compare
jirihnidek
approved these changes
Mar 5, 2026
Contributor
jirihnidek
left a comment
There was a problem hiding this comment.
Thanks for your updates. LGTM now.
This patch checks the rhsm config to ensure the "report_package_profile" setting is not ignored during registration. Previously, this setting was ignored and the package profile was forced to upload. Resolves: RHEL-73923 AI Usage Disclosure: This commit contains test cases that were created with the assistance of AI tools. Signed-off-by: Jason Jerome <[email protected]>
2a9076b to
95b9371
Compare
Contributor
|
LGTM from a pre-merge functional test perspective. Please merge to main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch checks the rhsm config to ensure the
"report_package_profile" setting is not ignored during registration.
Previously, this setting was ignored and the package profile was forced to upload.
Resolves: RHEL-73923