theforeman / foreman_rh_cloud

a plugin to Foreman that generates and uploads reports to the Red Hat cloud
GNU General Public License v3.0
6 stars 32 forks source link

Requests to /api/insights/v1/system/<uuid>/reports lack trailing slash #895

Open PaulWay opened 2 months ago

PaulWay commented 2 months ago

At the moment the Foreman client seems to be making requests on behalf of the Insights client to:

/api/insights/v1/system/<uuid>/reports?branch_id=<branch_id>.

This lacks a trailing slash, which causes the API to return a 301 redirect to:

/api/insights/v1/system/<uuid>/reports/?branch_id=<branch_id>.

Which the client immediately follows. This causes two requests and two jobs to process one request, leading to increased work and delays in both Satellite and the Insights Advisor API.

We would like to make sure that only the latter URL is requested.

Versions affected:

Satellite 6.11.5.6, 6.11.5.7, 6.12.5.3, 6.13.1, 6.13.7, 6.14.2, 6.14.4, 6.14.4.1, 6.15.1, 6.15.2 foreman_rh_cloud 5.0.46, 6.0.45, 7.0.45, 7.0.48, 8.0.51, 9.0.56 insights-client 3.1.9, 3.2.2

ShimShtein commented 2 months ago

Also tracked in https://issues.redhat.com/browse/SAT-27374

PaulWay commented 2 months ago

I'm happy to submit a patch or PR for this, I'm just struggling to see where this URL is constructed or comes from.

lfu commented 1 week ago

@PaulWay Could you provide the steps to reproduce the issue? Thanks.

PaulWay commented 1 week ago

Sure. Simply watch the requests going out to URLs starting with https://console.redhat.com/api/insights/v1/ - you'll see that they don't contain a trailing slash. The requests library being used automatically follows the 301 redirect so the request eventually resolves, but it wastes an entire request/response cycle. Look in the foreman log.

PaulWay commented 1 week ago

This should work with any of the Satellite versions I gave above.

chris1984 commented 1 week ago

@PaulWay I ran insights-client on my client system connected to a Satellite 6.16 and see this having a trailing slash and 1 path that does not, but it's to a different endpoint than what you described. Is there a way to get my client to hit the api path you are hitting?

[root@satellite ~]# tail -f /var/log/foreman/production.log | grep -i reports
2024-11-05T15:33:55 [I|app|37b67a45] Started GET "/redhat_access/r/insights/platform/insights/v1/system/9e5f25ae-985d-4137-ac15-f31ff57af003/reports/" for 108.61.158.191 at 2024-11-05 15:33:55 -0500
2024-11-05T15:33:55 [I|app|37b67a45]   Parameters: {"path"=>"platform/insights/v1/system/9e5f25ae-985d-4137-ac15-f31ff57af003/reports"}

2024-11-05T15:35:41 [I|app|cd0cde98] Started GET "/redhat_access/r/insights/platform/insights/v1/system/9e5f25ae-985d-4137-ac15-f31ff57af003/reports/" for 108.61.158.191 at 2024-11-05 15:35:41 -0500
2024-11-05T15:35:41 [I|app|cd0cde98]   Parameters: {"path"=>"platform/insights/v1/system/9e5f25ae-985d-4137-ac15-f31ff57af003/reports"}
PaulWay commented 1 week ago

Hi @chris1984 - thanks for that. I can see that system having recently checked in using the Satellite compatibility API:

image

(Sorry for the image, it's hard to copy and paste log text directly from Kibana.) The logs are most recent at top so they should be read from the bottom up.

So you can see there that even though Foreman says it's requesting the /reports/ URL, for some reason it's requesting the /reports URL first.

lfu commented 1 week ago

@PaulWay Thanks for your reply. I'm wondering if you could show me the detail steps in Satellite and where to check the results so anyone new to the project could re-produce the issue. So far I did not see the two requests in foreman logs when registering a client via global registration.

PaulWay commented 1 week ago

Hi @lfu - I'm sorry, I'm not the expert in how Satellite and Foreman work. I can show you that Satellite is making those requests. I need your help to find out why.

lfu commented 1 week ago

@PaulWay 😄 Just show me how the Satellite is making those requests. And I'm here to help find out the reason.

PaulWay commented 1 week ago

I'm asking you to find out how Satellite is making those requests. I don't know. All I know is that it's making them.

lfu commented 1 week ago

@PaulWay 😄 This is funny. I just want to know the steps that you have done that makes the issue you are reporting.

PaulWay commented 1 week ago

1) Have a running Satellite of the given versions. 2) Register it to work with Insights. 3) Register a RHEL host to the Satellite. 4) Make sure the RHEL host performs an Insights client upload by running insights-client. 5) This should generate the requests for reports.

ShimShtein commented 1 week ago

@lfu to add to what Paul has described: in Foreman, the part that is responsible to make the actual call to the cloud is here: https://github.com/theforeman/foreman_rh_cloud/blob/48f9d74ba8a671c74cec3a79b2646c38172a42a3/app/services/foreman_rh_cloud/cloud_request_forwarder.rb#L7 I suppose we should investigate deeper to understand why some requests do get the trailing slash and some don't.

Also make sure to sync with @chris1984, since I think he was working on this.