-
Notifications
You must be signed in to change notification settings - Fork 168
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
ESPv2 fails to start when service config has duplicated camel_case name to json name mapping. #185
Comments
I would suspect that "--backend=http://dev:8081" is not correct. please make sure that your local backend is listening on port 8081, and your docker network is correct. |
I found this error: Somehow, either your service_name is wrong, |
@TAOXUY there is not much info for this error, even under --enable_debug flag. Can we add more debug info for easy debugging? |
@someone1 did you remove your service name in the log, or you are actually pass in this flag as '--service', '' This is the first log line: Starting Config Manager with args: ['bin/configmanager', '--logtostderr', '--backend_address', 'http://dev:8081', '--rollout_strategy', 'managed', '--v', '1', '--listener_port', '8089', '--service', '', '--disable_tracing', '--cors_preset', 'basic', '--cors_allow_origin', 'http://localhost:8080', '--cors_allow_origin_regex', '', '--cors_allow_methods', 'GET, POST, PUT, PATCH, DELETE, OPTIONS', '--cors_allow_headers', 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization', '--cors_expose_headers', 'Content-Length,Content-Range', '--service_account_key', '/.google-application-credentials.json', '--non_gcp', '--suppress_envoy_headers=false'] |
This is the same backend I use and work with
I'm using the same service account JSON file and configuration version I use with I also tried launching this on CloudRun with the same result using the
Logs from Cloud Run
I removed it from the log |
I guess given that it doesn't work both on Cloud Run using the build step and locally, this may be related to the OpenAPI spec not loading properly in ESPv2? It works as-is in ESPv1. Feel free to adjust the title as necessary. |
This is related to some validation I added in #118. If you're running into this error, then this could result in undefined behavior with OpenAPI path templating. Could you share the open API spec and the service config with us? If needed, you can also email it directly to me (nareddyt@google.com). You can download the service config with this command:
|
I'll email it over shortly! Some Updates from testing locally:
For now I'll pin the version to 2.7 |
I do see this in the logs with 2.7:
Again - I'm using the same exact JSON credentials file as I do with ESPv1 and it has the project owner role so I'm not sure what other underlying issue there may be. It obviously does load the configuration since it's passing requests through to my backend and notifies me if I navigate to an unknown path. |
Basically what this error is saying is that if you have a GET /test1/{number_value} ESPv2 is unsure which
ESPv2 will end up choosing based on the ordering of the service config, which is not well defined. As far as I can tell, this was never validated in ESPv1. I believe this change is in 2.9.0, which would explain why 2.7.0 works. Unsure about the error in 2.8.0, seems unrelated. |
@nareddyt with that We are going to add more debug log in configmgr to help to debug this problem. |
@qiwzhang no, this will cause config manager to bail out. I think this is the correct behavior. Otherwise, ESPv2 will not be predictable in which query param to use for CONSTANT ADDRESS. This was bought up during the security review. Maybe we can relax the constraint so this validation error only happens when the service config has CONSTANT ADDRESS path translation. Currently, the error occurs even if the user only configures APPEND_PATH_TO_ADDRESS, which doesn't make sense (query params are not required for this path translation). @someone1 emailed me the OpenAPI spec and service config, so I want to take a closer look. |
@nareddyt I filed b/158262661 |
We discussed offline how to better map query params in ESPv2 to prevent this duplication from occurring. I will implement next week, hopefully get is fixed by the next release. |
Hi - It is my understanding it is not possible to run the ESPv2 containers locally for gRPC backends. Is this an accurate understanding? If that is not the case, is there a docker compose or example repo to follow along? Thank you. |
Running ESPv2 with docker run in localhost is possible.
The step is similar to this doc |
As specific for @someone1 original issue, we have some bugs in handling camel_case to jsonName conversion. Specifically, his service config some how has some duplicated mapping from camel_case names to json names, ESPv2 fails with such service cofnig, ESPv1 is fine. We are work on the fix for this case. It has nothing to do with running ESPv2 locally. |
…ead of global (#218) In OpenAPI specs, a path template jsonName may conflict with the jsonName of well known `google.protobuf.*` types. To prevent this, define the snake_to_json segment mapping per operation instead of globally in the path matcher filter. Then the validation of name conflicts will only be per-operation, greatly making the validation less strict. Also optimize when this needs to be configured in the path matcher config. We only pass this when we need path parameter extraction under constant address backend routing. Diff is too large already. I will follow-up with PRs for: - Adding integration tests. - Incrementing api version (breaking API change). Ref: https://groups.google.com/u/1/g/google-cloud-endpoints/c/hxyuDx1yrvQ Closes #185 Signed-off-by: Teju Nareddy <nareddyt@google.com>
@nareddyt - should this be working now with 2.14? It works when run on Cloud Run but not locally. It seems to get a lot further, actually downloading the config and listening to requests and the previous snake_case error message no longer appears. However, I still only get a single request before the entire container locks up, and it never reaches my backend: HTTP Response (Status Code 500):
Relevant logs around the request:
I can open a new issue if this is unrelated - and I'd be happy to share my entire log privately if it helps any. I'm using the same ESPv2 startup options as mentioned earlier in this issue. It continues to work fine with ESPv2.7.0. I should mention, the 401/403 error messages may be symptomatic of an error earlier in the stack. The following works fine with the same service account file so I'm pretty sure it has permissions: curl -s \
-H "Authorization: Bearer $(gcloud auth application-default print-access-token)" \
-H "Content-Type: application/json" \
https://servicecontrol.googleapis.com/v1/services/web-api-dot-<project-id>.appspot.com:check \
--data '{
"serviceConfigId": "2020-07-16r1",
operation: {
"operationId": "<removed>",
"consumerId": "api_key:<removed>",
"operationName": "<removed>",
"startTime": "2020-07-22T22:01:23.045123456Z",
"endTime": "2020-07-22T22:01:23.045123456Z",
}
}' |
This is unrelated, let me open a new issue. |
Moving discussion from Google Group:
I'm trying to run ESPv2 locally to replace ESPv1 during development. However, although ESPv1 works just fine, ESPv2 keeps bouncing the container. I've set my service account to project owner to eliminate permission issues.
Here is my Docker compose:
Error log output (this constantly repeats):
And the standard log output (this also repeats):
The text was updated successfully, but these errors were encountered: