Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle module router request response format #3838

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Handle module router request response format #3838

merged 1 commit into from
Jul 12, 2023

Conversation

LightOfHeaven1994
Copy link
Contributor

@LightOfHeaven1994 LightOfHeaven1994 commented Jul 4, 2023 •
edited
Loading

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

I catch the issue when was running insights-client against ephemeral environment. I don't have module-router-request application deployed there and registering host failed:

2023-07-04 04:02:14,506  NETWORK insights.client.connection GET https://env-ephemeral-ng0e91-gvcod9fw.apps.c-rh-c-eph.8p0c.p1.openshiftapps.com/api/module-update-router/v1/channel?module=insights-core
2023-07-04 04:02:14,615  NETWORK insights.client.connection HTTP Status: 200 OK
2023-07-04 04:02:14,615  NETWORK insights.client.connection HTTP Response Text: <!DOCTYPE html>
<html lang="en-US">

<head>
    <meta charset="UTF-8">
    <title>
        Hybrid Cloud Console
    </title>
    <meta http-equiv="Content-Security-Policy" content="default-src 'self' https: wss: data: blob: 'unsafe-inline' 'unsafe-eval' https://*.redhat.com/ https://www.redhat.com/  https://*.openshift.com/ https://api.stage.openshift.com/ https://identity.api.openshift.com/ https://www.youtube.com/ https://redhat.sc.omtrdc.net/ https://assets.adobedtm.com/ https://www.redhat.com/ https://*.storage.googleapis.com/ https://*.hotjar.com:* https://*.hotjar.io wss://*.hotjar.com https://stage.quay.io/ https://quay.io/;">
    <meta http-equiv="Pragma" content="no-cache">
    <meta http-equiv="cache-control" content="no-cache, no-store, must-revalidate">
    <meta http-equiv="x-ua-compatible" content="ie=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
    <link rel="icon" type="image/png" href="https://access.redhat.com/webassets/avalon/g/favicon.ico">
    <script type="text/javascript">
        window.insights = {
            chrome: {
                isChrome2: true
            }
        }
    </script>
    <script id="dpal" src="https://www.redhat.com/ma/dpal.js" type="text/javascript"></script>
<base href="/"><link href="/apps/chrome/js/main.1014b7422bbb96c7.css" rel="stylesheet"></head>

<body class="pf-m-redhat-font">
    <div id="chrome-entry"></div>
    <div id="consent_blackbar"></div>
<script defer src="/apps/chrome/js/chrome-root.1014b7422bbb96c7.js"></script><script defer src="/apps/chrome/js/chrome.1014b7422bbb96c7.js"></script></body>
</html>

The issue here is that response code is 200 but content is not in json format. So if it doesn't have correct format it has to point to /release the same way as if you have ConnectionError

@LightOfHeaven1994
Copy link
Contributor Author

Tested the fix locally and it works great

[root@eshamard-insights-rhel91-ephemeral cloud-user]# INSIGHTS_GPG=false BYPASS_GPG=true EGG=/home/cloud-user/insights.zip insights-client --register
Module update router response is not valid for https://env-ephemeral-ng0e91-gvcod9fw.apps.c-rh-c-eph.8p0c.p1.openshiftapps.com/api/module-update-router/v1/channel?module=insights-core. Expected json format but got text/html; charset=utf-8. Defaulting to /release

@xiangce
Copy link
Contributor

xiangce commented Jul 12, 2023

@LightOfHeaven1994 - Since I'm not familiar with the procedures of the module update router on the client side, do you think it's necessary to ask the client team to have a look at and approve this change?

@LightOfHeaven1994
Copy link
Contributor Author

@xiangce I think if client QE @patchkez approve it we are good (since we found this bug together)

@patchkez
Copy link
Collaborator

/lgtm

@LightOfHeaven1994
Copy link
Contributor Author

@xiangce we are good to go, thanks

@xiangce xiangce merged commit f9c0ec2 into RedHatInsights:master Jul 12, 2023
7 checks passed
xiangce pushed a commit that referenced this pull request Jul 13, 2023
@LightOfHeaven1994 @xiangce
Handle module router request response format (#3838)
85d5bfa 
Signed-off-by: eshamard <[email protected]>
(cherry picked from commit f9c0ec2)
xiangce pushed a commit that referenced this pull request Sep 6, 2024
@LightOfHeaven1994
Handle module router request response format (#3838)
4cd60b0 
Signed-off-by: eshamard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LightOfHeaven1994 @xiangce @patchkez