theforeman / foreman-ansible-modules

Ansible modules for interacting with the Foreman API and various plugin APIs such as Katello
GNU General Public License v3.0
148 stars 166 forks source link

Allow scc_product module without organization parameter #1675

Open lumarel opened 1 year ago

lumarel commented 1 year ago
SUMMARY

The next version of foreman_scc_manager will move where the organization property is needed: https://github.com/ATIX-AG/foreman_scc_manager/pull/126/files

It's currently already optional with the hammer cli for scc_product (and only shows correct output if organization is not defined), which makes it correctly work there. Would have done the change myself already, but as KatelloAnsibleModule overall requires organization I was not sure how to correctly do the change.

ISSUE TYPE
evgeni commented 1 year ago

But the account still requires the org, right?

lumarel commented 1 year ago

Tbh I don't trust my ruby code reading. If I understand it correctly the org is only needed to create a scc_account anymore after the latest change 🤔

evgeni commented 1 year ago

Heh. I'll try to look in detail tomorrow. But my thinking is as follows: even if one doesn't need an Org to find a Product, one needs it to find an Account, so for the overall module call it will still be required.

We have a similar thing in the "repository" module. Repositories are scoped by Product, so technically you don't need an Org if you know the ID of the Product, but finding that requires an Org…

lumarel commented 1 year ago

Unfortunately if you give hammer the organization parameter for scc_product with the list command, it will give you a flawed output (most of the products are missing). This makes you also not able to subscribe to products that should be there, but only if organization is not specified. (already reported and I guess this mentioned PR will fix that).

I might have misunderstood something or the info was lost in transit, but I was told that the scc_accounts will be bound to a org from the beginning, and after that the org is unnecessary, idk, I just ran into this because we wanted to subscribe to products with Ansible and we failed because the output is not complete if the org is provided (which is mandatory for the ansible module)

So... as long as the organization parameter does not reach the scc_product api it should be fine I guess :)

Thank you for looking into it :)

evgeni commented 1 year ago

Unfortunately if you give hammer the organization parameter for scc_product with the list command, it will give you a flawed output (most of the products are missing). This makes you also not able to subscribe to products that should be there, but only if organization is not specified. (already reported and I guess this mentioned PR will fix that).

This sounds like a bug, but yes the linked patch removes a scope = scope.where(:organization => params[:organization_id]) if params[:organization_id].present? from the "list products for an account" endpoint, which used to filter the results by Org and now doesn't anymore.

FWIW, the apidoc for the endpoint never mentioned organization_id, so FAM would never have sent it (as it is very strict to adhere to the apidoc).

I might have misunderstood something or the info was lost in transit, but I was told that the scc_accounts will be bound to a org from the beginning, and after that the org is unnecessary, idk, I just ran into this because we wanted to subscribe to products with Ansible and we failed because the output is not complete if the org is provided (which is mandatory for the ansible module)

So... as long as the organization parameter does not reach the scc_product api it should be fine I guess :)

Looking at the linked PR it should be fine, yes.

(I don't understand why the org_id was removed from the "list accounts" endpoint, but that won't hurt us)

lumarel commented 1 year ago

Okay, so nothing to change here? As the org_id shouldn't reach the API? At least it seems to me with the old version it reaches the API and makes it impossible to subscribe most products 🤔

evgeni commented 1 year ago

That's my understanding, but I am by no means an expert on the SCC side.

Maybe @Manisha15 or @sbernhard can weight in?

nadjaheitmann commented 1 year ago

I tried to dig into this but not sure if I got it all right. In my tests, you can (with the patch applied) do the following: Get a complete product list: hammer scc_manager scc_accounts scc_products list --scc-account scc_test --organization-id 1 hammer scc_manager scc_accounts scc_products list --scc-account-id 1 hammer scc_manager scc_accounts scc_products list --scc-account-id 1 --organization-id 1 If you want to show only subscribed products, you can add the flag --subscribed-only 1.

Providing an account name without the organization does not work for a reason we have not yet figured out: hammer scc_manager scc_accounts scc_products list --scc-account scc_test

Maybe it is related to what @evgeni mentioned:

Heh. I'll try to look in detail tomorrow. But my thinking is as follows: even if one doesn't need an Org to find a Product, one needs it to find an Account, so for the overall module call it will still be required. We have a similar thing in the "repository" module. Repositories are scoped by Product, so technically you don't need an Org if you know the ID of the Product, but finding that requires an Org…

If you have any insights, let me know.

In FAM, you always have to set the organization but I did not experience any trouble subscribing to products using FAM.

@lumarel Not sure if I could help you with that, let me know if there is anything else I should look into.