Bug 2158575 - Katello API activation_key/:id/product_content does not expose per_page
Summary: Katello API activation_key/:id/product_content does not expose per_page
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: API
Version: 6.12.0
Hardware: x86_64
OS: Linux
medium
medium
Target Milestone: 6.12.2
Assignee: satellite6-bugs
QA Contact: Lukáš Hellebrandt
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-01-05 18:41 UTC by Odilon Sousa
Modified: 2023-02-08 14:35 UTC (History)
4 users (show)

Fixed In Version: tfm-rubygem-katello-4.5.0.24-1
Doc Type: If docs needed, set a value
Doc Text:
Clone Of: 2154512
Environment:
Last Closed: 2023-02-08 14:35:01 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Foreman Issue Tracker 35633 0 Normal Closed Activation Keys "product_content" API doesn't expose the "per_page" parameter 2023-01-05 18:42:08 UTC
Foreman Issue Tracker 35879 0 Normal Duplicate Katello API activation_key/:id/product_content does not expose per_page 2023-01-05 18:42:08 UTC
Red Hat Issue Tracker SAT-14623 0 None None None 2023-01-06 13:18:42 UTC
Red Hat Issue Tracker SAT-14659 0 None None None 2023-01-05 18:42:08 UTC
Red Hat Issue Tracker SAT-14744 0 None None None 2023-01-05 19:12:27 UTC
Red Hat Product Errata RHBA-2023:0668 0 None None None 2023-02-08 14:35:17 UTC

Comment 3 Lukáš Hellebrandt 2023-01-30 18:12:40 UTC
My findings:

1) Tried listing product content in AK using API and Hammer. In both cases, a full list was returned with proper pagination.
2) APIdoc correctly contains information about per_page: and page: attributes.
3) The playbook is de-facto idempotent since it only creates the AK once, and with proper overrides. However, the task is still marked as "changed" when run repeatedly.

Comment 4 Lukáš Hellebrandt 2023-01-30 19:05:50 UTC
The module itself is flawed, as you can see by adding these debug outputs in the respective place in the module's source code:

module.warn(f"PRODUCT_CONTENT: {product_content}")
for label, override in desired_content_overrides.items():
  pop = current_content_overrides.pop(label, None)
  if override is not None and override != pop:
    module.warn(f"ADDING {label}, override: {override}, pop: {pop}")
    changed_content_overrides.append({'content_label': label, 'value': override})

You see it only gets the first page, doesn't list, then it's missing the overrides on the second page and sets them again. I think this is the same behavior that occured in the version where the issue was reported against.

The API, its docs and Hammer are fixed though, now it's about the module.

Odilon, is it OK to close this BZ based on the fact that it's about API, not the module - and perhaps file a new BZ about the module itself?

Comment 5 Odilon Sousa 2023-01-31 15:48:44 UTC
(In reply to Lukáš Hellebrandt from comment #4)
> The module itself is flawed, as you can see by adding these debug outputs in
> the respective place in the module's source code:
> 
> module.warn(f"PRODUCT_CONTENT: {product_content}")
> for label, override in desired_content_overrides.items():
>   pop = current_content_overrides.pop(label, None)
>   if override is not None and override != pop:
>     module.warn(f"ADDING {label}, override: {override}, pop: {pop}")
>     changed_content_overrides.append({'content_label': label, 'value':
> override})
> 
> You see it only gets the first page, doesn't list, then it's missing the
> overrides on the second page and sets them again. I think this is the same
> behavior that occured in the version where the issue was reported against.
> 
> The API, its docs and Hammer are fixed though, now it's about the module.
> 
> Odilon, is it OK to close this BZ based on the fact that it's about API, not
> the module - and perhaps file a new BZ about the module itself?

I think we can close this one, and validate since it was created for the API, and create a new one. This Bug was cloned from https://bugzilla.redhat.com/show_bug.cgi?id=2154512 we should link the new bug to the one for 6.13 as well if it's get the same result there.

Comment 6 Evgeni Golov 2023-01-31 16:34:56 UTC
(In reply to Lukáš Hellebrandt from comment #4)
> The module itself is flawed, as you can see by adding these debug outputs in
> the respective place in the module's source code:
> 
> module.warn(f"PRODUCT_CONTENT: {product_content}")
> for label, override in desired_content_overrides.items():
>   pop = current_content_overrides.pop(label, None)
>   if override is not None and override != pop:
>     module.warn(f"ADDING {label}, override: {override}, pop: {pop}")
>     changed_content_overrides.append({'content_label': label, 'value':
> override})
> 
> You see it only gets the first page, doesn't list, then it's missing the
> overrides on the second page and sets them again. I think this is the same
> behavior that occured in the version where the issue was reported against.
> 
> The API, its docs and Hammer are fixed though, now it's about the module.
> 
> Odilon, is it OK to close this BZ based on the fact that it's about API, not
> the module - and perhaps file a new BZ about the module itself?

The module fix is in https://bugzilla.redhat.com/show_bug.cgi?id=2134605 and is fixed in the 3.8.0 version of the collection, which one are you using?

Comment 7 Lukáš Hellebrandt 2023-01-31 17:19:28 UTC
ansible-collection-redhat-satellite-3.6.0-1.el8sat.noarch on Sat 6.12.2, the installer didn't update the collection - should it have? Also, I can't update it manually. Note this BZ is for a 6.12.2 z-stream version, maybe it wasn't packaged properly?

Comment 8 Evgeni Golov 2023-01-31 17:35:46 UTC
(In reply to Lukáš Hellebrandt from comment #7)
> ansible-collection-redhat-satellite-3.6.0-1.el8sat.noarch on Sat 6.12.2, the
> installer didn't update the collection - should it have? Also, I can't
> update it manually. Note this BZ is for a 6.12.2 z-stream version, maybe it
> wasn't packaged properly?

No, I don't think we have 3.8 in 6.12 yet. Carry on, I guess.

The thing mentioned in the BZ is fixed, the other has 6.12.z+ so it will show up eventually.

If you think the ansible feature also needs to be present (which the text might indicate, while the title does not), more work will be needed.

Comment 9 Lukáš Hellebrandt 2023-02-01 10:50:14 UTC
VERIFIED using steps from comment 3 on Sat 6.12.2 snap 1.0.

The only thing remaining to fix in relation to this BZ is bug 2134605.

Comment 14 errata-xmlrpc 2023-02-08 14:35:01 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Satellite 6.12.2 Async Update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:0668


Note You need to log in before you can comment on or make changes to this bug.