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.

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

Regression in host #413

Closed
gcornacchia opened this issue Jul 10, 2023 · 36 comments
Closed

Regression in host #413

gcornacchia opened this issue Jul 10, 2023 · 36 comments
Assignees
Labels
bug Something isn't working

Comments

@gcornacchia
Copy link
Contributor

APIM-CLI version

1.14.0

API-Management version

7.7.20211130

Bug description

hi,
we found a regression in our system while testing the latest version 1.14.0.

consider the following situation

json api config file
{ ....
"backendBasePath": "http://myhost:597"
...

}

swagger file:

{
"host": "http://fakehost"
}

with 1.13.0 and replaceHostInSwagger=true the backendBasePath wins over swagger host (our desired behaviour).

with 1.14.0 where (replaceHostInSwagger is no longer considered) the swagger host replace backendBasePath e we don't want that because the swagger file is the same for each environment but the real host must change (according to backendbasepath in json config file) and in some case, we cannot force the developer to remove the field from swagger.

so basically we would like to have replaceHostInSwagger parameter back.
this parameter in combination with ovverrideSpecBasePath must support all the scenarios in order to let the user decide the source of each field (host and base path)

host -> from swagger or from json config file?
basepath -> from swagger or from json config file?

what do you think @rathnapandi ?

Steps to reproduce

No response

Relevant log output

No response

@gcornacchia
Copy link
Contributor Author

in the pull request you can find that it's not necessary the replaceHostInSwagger parameter but just to uniform the behavior of overrideSpecBasePath parameter.

@rathnapandi
Copy link
Member

@gcornacchia

We are using overridebasepath flag if we want to change the basepath in swagger specification. other changes like host:protocol, shceme can be overridden via frontend api.

I have merged your changes; can you please review it.

@gcornacchia
Copy link
Contributor Author

yes, thanks, I'll check it now

@gcornacchia
Copy link
Contributor Author

hi @rathnapandi , I have the same regression discussed on top of this issue.

consider always the same configuration:

json api config file
{ ....
"backendBasePath": "http://myhost:597/"
...

}

swagger file:

{
....
"host": "http://localhost/"
....
}

My backend API on API Manager is created with host "localhost" which is incorrect because I want to point to http://myhost:597 (as described in json api config file) and I cannot force the swagger to remove "host" field.

In this case, How should I manage this situation?

What do you mean with this "other changes like host:protocol, shceme can be overridden via frontend api."?

thanks

@gcornacchia
Copy link
Contributor Author

please @rathnapandi, with Axway customer we are introducing the automation for quotas but we need a stable and updated version of apim-cli in order to proceed.

@rathnapandi
Copy link
Member

Hi @gcornacchia,

We will override the host/port/protocol via backend service url specified in the image. The backend URL updated with the value of backendBasepath. The value will be updated based on the environments ( dev, qa, prod).

image

Thanks
Rathna

@gcornacchia
Copy link
Contributor Author

gcornacchia commented Jul 21, 2023

so @rathnapandi you're telling me that the backend base path will override host/port/protocol of the swagger file as it was in my pull request or something similar?
We look forward to your changes, let us know asap, thanks

@rathnapandi
Copy link
Member

@gcornacchia,

We do not override anything in swagger file by default, it will update basePath (see the below sample json) element in frontend API (it is represented as Backend Service url in API manager UI). During runtime (when the request hits gateway), the basePath value from frontend api will be used instead of protocol, host, port defined in swagger.

"serviceProfiles": {
            "_default": {
                "apiId": "c3cc77e7-28a0-42b6-b533-9178ecd89857",
                "basePath": "https://petstore.swagger.io"
            }
        },

But in some cases, we will update the swagger if swagger does not contain host, port, or scheme. The values will be updated from the element defined in backendBasepath defined in apiconfig.json. If basePath is empty in swagger, it will be hardcoded as / according to swagger2 specification.

If you want to change the basePath on backend / swagger. Front API won't provide an option to change backend basePath, there are two options.

  1. Change the basePath to a swagger file.
  2. use Overridebasepath flag in apimcli, it will take the basepath from backendBasepath defined in apiconfig.json and replace basePath in swagger.

I think we don't required any code changes.

Thanks
Rathna

@gcornacchia
Copy link
Contributor Author

gcornacchia commented Jul 25, 2023

Hi @rathnapandi I dont understand why the parameter replaceHostInSwagger has been removed in some version after 1.13.0, it's the logic we need in order to proceed with the update of all apim-cli. The reason is that we want to have full control of what is deployed in api manager in json config file without having to deal with what is defined in swaggers (I'm always referring about host field) , as described in the doc https://github.com/Axway-API-Management-Plus/apim-cli/wiki/2.1.2-API-Configuration-file#parameter-backendbasepath

@lturricchia
Copy link

lturricchia commented Jul 28, 2023

Hello @rathnapandi , I'm dealing with the same problem of @gcornacchia in this topic, so please help me to understand:

--> Take as example this piece of api description json file input for apim-cli:
{
...
"apiDefinition": "interface.json",
"backendBasepath": "http://192.0.0.0:8080",
...

--> and take this swagger interface json piece:
{
"swagger": "2.0",
...
"host": "some.invalid.IP.address",
"basePath": "/v2",
...
"schemes": [
"https",
"http"
],
"paths": {
"/pet/{petId}/uploadImage": {

There are 3 fields object of discussion:

-- api description json file -> field "backendBasepath": the actual protocol+host+port we want to use, as you said on Jul 20, overwriting the eventual host/port/protocol defined in swagger interface and called in outbound.

-- swagger interface file-> field "basePath": this is the path to use after the backendBasepath from api description json file to call the api, we never need to overwrite this path.

-- swagger interface file-> field "host": undesired and for us always to be overwritten, varying for the different environments.
Until 1.13.0, as in your doc https://github.com/Axway-API-Management-Plus/apim-cli/wiki/2.1.2-API-Configuration-file#parameter-backendbasepath , we were using backendBasepath in the api description json file together with the replaceHostInSwagger=true parameter, and it was woking to use "backendBasepath" field instead of "host" field.
Starting from 1.14.0 it's no longer working and we end up with (see examples above) some.invalid.IP.address/v2 released on Manager instead of http://192.0.0.0:8080/v2.

Don't you have the same problem using examples as above? How can it be solved? Is there some different parameter I should use to say apim-cli to ignore the host field and overwrite it with backendBasepath?

Thanks in advance for your patience

@rathnapandi rathnapandi self-assigned this Jul 31, 2023
@rathnapandi
Copy link
Member

Hi @gcornacchia,

I would like to understand what you are trying to solve to replace the host in swagger file.

From my perspective the flag replaceHostInSwagger does not add any value as we are overriding host name via front end api.

Thanks
Rathna

@rathnapandi
Copy link
Member

Hi @lturricchia,

Sorry, I am not clear with the statement "Starting from 1.14.0 it's no longer working and we end up with (see examples above) some.invalid.IP.address/v2 released on Manager instead of http://192.0.0.0:8080/v2."

Can you please share some screenshots with an issue?

Thanks
Rathna

@lturricchia
Copy link

Hi @rathnapandi,
I must specify: in our problem with host filed, we are referring to the Backend API: here some screen to better explain:

these are the swagger interface and the api description json as example:
swagger-host
apijson-backendbasepath

here is what happens with previous versions of apim-cli (up to 1.13.0), which is our desired behaviour:
deploy-lib113

here is what happens with current version (using last build from develop - commit Aug 1 1:13AM GMT+2):
deploy-lib114develop2ago23

as you can see, using the same files, using the last version the deployed Backend API gets the undesired swagger host field localhost:1234 instead of the BackendBasePath url in api description json file.

@rathnapandi
Copy link
Member

Hi @lturricchia,

There is no use with Base URL defined in Backend API. The front-end API's outbound section Backend Service URL will be used for API calls which will be changed based on deployed environment like test, stage, preprod and production.

image

Please do let me know your thoughts.

Thanks
Rathna

@lturricchia
Copy link

lturricchia commented Aug 3, 2023

Hi @rathnapandi , to better understand, could you kindly link me the Axway documentation you are using as reference? Is it explained to avoid use of Base URL of Backend API? That's the logic we use to route to backend, we haven't got the Backend service URL filed in outbound of our Frontend APIs.

Do I read correctly form your screenshot that you are in a APIGateway -> SomeCloudService scenario? We use routing policy, so we are in a APIGateway -> RouteToPolicy scenario (as in screen), could you explain me if/how the Backend service URL is managed respectively?

feapi-outbound-routing

Thanks in advance for your patience

@lturricchia
Copy link

lturricchia commented Aug 10, 2023

Hi @rathnapandi ,
hope I'm not disturbing you on holiday, in case have a nice time, waiting for your reply later asap.

Any news about our topic?
To further explain, what we are doing in our Custom Routing Policy is to use a Connect to URL filter to call the Backend API at Base URL (the field now compromised by Apim-cli, as above).

I think this use we make of routing policy is in line with what PolicyStudio supports and suggests for use with API Manager, here some references describing the same we did (put aside the "Amplify" header):

-- here is how we configured our SSL Custom Routing Policy https://axway-open-docs.netlify.app/docs/apim_administration/apimgr_admin/api_mgmt_custom_policies/#create-the-custom-ssl-routing-policy-in-policy-studio

-- here is reported the respective use of the Base URL field from Backend API https://axway-open-docs.netlify.app/docs/apim_administration/apimgr_admin/api_mgmt_custom_policies/#configure-the-custom-sslrouting-policy-in-api-manager

-- here it's specified that when a routing policy is used, the Frontend API field Backend service URL can be ignored using the URL configured in the routing policy (field destinationURL, then using the Backend API field Base URL) https://axway-open-docs.netlify.app/docs/apim_administration/apimgr_admin/api_mgmt_custom_policies/#routing-policy

That's why we need the Backend API field Base URL to be set by Apim-cli with the one in api description json, ignoring the swagger field (see above).

What do you think? Did you find any documentation specifying that the Backend API field Base URL is no longer supported for routing?

waiting for your reply,
thanks for your attention

@JuIngong
Copy link

Hey,
We had the same issues that's completely breaking our usage of the tool.
For a quick fix I did a local change to uniform how the flag overridebasepath is working on the OAS3 with the Swagger 2
In the Swagger2xSpecification.java, method configureBasePath, i added
image
With this I'm getting back the expected result

@gcornacchia
Copy link
Contributor Author

hi @rathnapandi , I'm back from a brief period off work.

I cite your sentence:

"There is no use with Base URL defined in Backend API. The front-end API's outbound section Backend Service URL will be used for API calls which will be changed based on deployed environment like test, stage, preprod and production."

I ask myself if we've ever thought about changes in API Manager logic itself.

We are encountering the problem in 7.7 nov 2021. @JuIngong what is your reference API Manager version?

Maybe something has changed in the following releases and field host in the backend API is completely ignored in latest versions? in this scenario apim-cli must deal with these differences in order to not break compatibility.

@JuIngong
Copy link

Indeed, we are using an older core version as well (if I'm not mistaking, depending on the env we have both 7.7.20210830 and 7.7.20211130)

@rathnapandi
Copy link
Member

rathnapandi commented Aug 22, 2023

Hi @JuIngong,

Can you share the issue you are facing? We can override the host and scheme / protocol via frontend api.

Hi @gcornacchia,

I am sure that nothing changed in terms of handling host filed on backend and frontend API. do you have multiple scheme ( https and http) in swagger 2 spec?

Thanks
Rathna

@JuIngong
Copy link

Same issue described in this issue, when using a swagger (2.0) which has a host specified, even if in my apm-import the backend basepath is specified, it is neither overrode in the outbound backend field of the Frontend API and neither in the Backend API (which, as you said is not important).
But if i'm using an Open API (3.x.x) file the host is overrode as expected

@gcornacchia
Copy link
Contributor Author

hi @rathnapandi,
Consider the following attachment
PetStore.zip

it contains a simple API apim-cli config file and dependencies (swagger file and trusted certificate).

in this case, the frontend API points to the backend defined in the host field in swagger and not the backendBasePath field in json.

Can you reproduce this issue? just enable the Community organization to API development.
I also tested with latest version (may 2023) and the behavior is the same.

@gcornacchia
Copy link
Contributor Author

you can try the frontend API by calling a GET to https://host:port/petstore/pet/1

@rathnapandi
Copy link
Member

Hi @gcornacchia,

Once issue I see in the config file is path parameter. it should be /petstore not /petstore/

I have imported the API with command ./apim.sh api import -c PetStore.json -u apiadmin -p changeme -h 10.10.10.100

This is what I see in Frontend API's outbound section. I have tested the API; it failed in connecting to the backend. Everything looks good to me.

Screenshot 2023-08-23 at 9 42 36 AM

To have working API, use "backendBasepath": "https://petstore.swagger.io" in conig file

Am i missing anything here?

@gcornacchia
Copy link
Contributor Author

hi @rathnapandi, I forgot to mention all of the variables:

apim-cli version: 1.14.1

apim-cli configuration file:

force=true
quotaMode=replace
clientAppsMode=replace
clientsOrgsMode=ignore
replaceHostInSwagger=true
zeroDowntimeUpdate=true
overrideSpecBasePath=true
api.defaults.state=published

//the following variabiles are defined in each stage properties file
username=<>
password=<>
host=<>
port=<>

import script:

./apim.sh api import -c PetStore.json -s <staging environment>

and that's the result:

image

this is the log (info level):

2023-08-24 11:04:26,580 [APIManagerCLI] INFO : API-Manager CLI: 1.14.1
2023-08-24 11:04:26,582 [APIManagerCLI] INFO : Module: API - I M P O R T (1.14.1)
2023-08-24 11:04:28,844 [APIManagerAdapter] INFO : Successfully connected to API-Manager (7.7.20211130) on: https://192.168.144.105:8076
2023-08-24 11:04:29,439 [APISpecificationFactory] INFO : Reading API-Definition (Swagger/WSDL) from file: \petstore.json (absolute path)
2023-08-24 11:04:29,446 [APISpecificationFactory] INFO : Detected: Swagger 2.0 specification.
2023-08-24 11:04:30,643 [APIImportConfigAdapter] INFO : Handling configured client-applications.
2023-08-24 11:04:30,644 [APIImportApp] INFO : Lookup actual API based on Path: /petstore
2023-08-24 11:04:31,670 [APIImportManager] INFO : No existing API found, creating new!
2023-08-24 11:04:32,514 [CreateNewAPI] INFO : Create published API: Petstore_v1 1.0 based on Swagger 2.0 specification.
2023-08-24 11:04:36,454 [ManageClientApps] INFO : All desired applications: [] have already a subscription. Nothing to do.
2023-08-24 11:04:36,454 [CreateNewAPI] INFO : Successfully created published API: Petstore_v1 1.0 (ID: 1ac7c0e9-a0ca-4de3-9b50-a9bff98af3c6)

@rathnapandi
Copy link
Member

Hi @gcornacchia,

Thanks for sharing the test data, I could reproduce the issue. Below line is blocking the basepath update on front end api.

if (backendBasePath != null && !CoreParameters.getInstance().isOverrideSpecBasePath()) {

I am fixing the issue as suggested by you and @JuIngong.

rathnapandi added a commit that referenced this issue Aug 24, 2023
@gcornacchia
Copy link
Contributor Author

I will try the code in develop branch and I will give you a feedback

@gcornacchia
Copy link
Contributor Author

gcornacchia commented Aug 25, 2023

hi @rathnapandi, I've tested the code in develop branch and it seems all ok regarding the handling of backend base url where host is defined in interface file.
But... I've found another regression in handling base path in OAS3 interface, consider the following attachment:

PetStoreOAS3.zip

in json config file there is a backendBasePath with no relative path.

  "apiDefinition": "./oas/petstore.json",
  "backendBasepath": "https://petstore.swagger.io",
  "applications": [],
  "clientOrganizations": 

in oas3 file there a url field with only relative path:

  "servers": [
    {
      "url": "/v2"
    }
  ],

After import with latest version the "/v2" path disappears from the backend api and the API call to backend is not pointing to correct url.

image

I remember the same problem were solved in another issue #397 for swagger 2.0 (I'm referring to lines 113-120 of class Swagger2xspecification.java).
Maybe it's missing the same check for OAS3?

@gcornacchia
Copy link
Contributor Author

@rathnapandi consider my pull request #428

@rathnapandi
Copy link
Member

Thanks @gcornacchia for reporting, I will review and get back to you if needed.

@rathnapandi
Copy link
Member

Hi @gcornacchia,

For your case the value should be "backendBasepath": "https://petstore.swagger.io/v2".
We are using flag overrideSpecBasePath to change basepath.
if you want to just change hostname, change the value of overrideSpecBasePath=false or remove it. That will update frontend api outbound section Backend service url with value from backendBasepath .

Also, if host is not defined in openapi specification, apim cli will take hostname from backendBasepath and change the openapi specfication.

@gcornacchia
Copy link
Contributor Author

gcornacchia commented Aug 28, 2023

HI @rathnapandi,
I add some considerations:

  1. In our deploy pipelines, we cannot change the value of overrideSpecBasePath parameter at runtime as we must handle both swagger 2.0 or OAS3 APIS without prior knowledge.

  2. The intention of my pull request was to reach the direction defined in a table that I've uploaded in a past PR (do you remember?)

image

the idea was to handle both swagger 2.0 and oas3 interfaces in the same coherent way.

Tell me what do you think about it?
in case you don't want to merge the PR please, tell us when you want to release 1.14.2 version (with the "host" bug fixed)

In any case, an updated wiki documentation is needed as it's completely outdated.

@rathnapandi
Copy link
Member

Hi @gcornacchia,

We are following the openapi specification for updating host .

image

Reference the section https://swagger.io/docs/specification/api-host-and-base-path/

The next release is planned on 1st September, will update documentation as well.

@gcornacchia
Copy link
Contributor Author

yes ok but in my mind oas3 and swagger 2 are not handled in the same way.
If I'm using swagger 2.0 I can use the base path from swagger field and specify only host/port in backendbasepath (in json config file).
If I'm using oas3 I cannot use base path (from servers.url field) and I have to specify host/port/basepath in backendBasePath (in json config file).

Anyway, I will try to use a workaround before calling apim-cli.

If you prepare a 1.14.2 pre-release package I can do more tests before Friday. Let me know

@rathnapandi
Copy link
Member

Thanks @gcornacchia for your support. Released 1.14.2 with the fix. please do let me know if you see any other issues.

@gcornacchia
Copy link
Contributor Author

Hi @rathnapandi,
I've done further tests.
Everything is ok regarding the handling of "host" field, but I've found an issue about quota handling, I've opened issue #434.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants