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

b/158262661: Move snake to json segment mapping to per-operation instead of global #218

Merged
merged 14 commits into from
Jul 14, 2020

Conversation

nareddyt
Copy link
Contributor

@nareddyt nareddyt commented Jul 8, 2020

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

…Name mapping

Signed-off-by: Teju Nareddy <nareddyt@google.com>
@nareddyt nareddyt self-assigned this Jul 8, 2020
@nareddyt nareddyt marked this pull request as draft July 8, 2020 00:51
Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Teju Nareddy <nareddyt@google.com>
@nareddyt nareddyt added the bug Something isn't working label Jul 8, 2020
Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Teju Nareddy <nareddyt@google.com>
@nareddyt nareddyt changed the title [WIP] Config generator changes for per-operation snake_case to jsonName mapping Move snake to json segment mapping to per-operation instead of global Jul 9, 2020
Signed-off-by: Teju Nareddy <nareddyt@google.com>
@nareddyt nareddyt marked this pull request as ready for review July 9, 2020 22:57
@nareddyt nareddyt requested review from qiwzhang and TAOXUY July 9, 2020 22:57
src/go/configinfo/service_info.go Outdated Show resolved Hide resolved
src/envoy/http/backend_auth/filter_config.h Outdated Show resolved Hide resolved
src/go/configinfo/service_info.go Outdated Show resolved Hide resolved
Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Teju Nareddy <nareddyt@google.com>
@nareddyt nareddyt requested a review from TAOXUY July 10, 2020 21:46
@nareddyt nareddyt requested a review from qiwzhang July 10, 2020 21:46
Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Teju Nareddy <nareddyt@google.com>
nareddyt added a commit that referenced this pull request Jul 14, 2020
…225)

- Added a script to sort example JSONs automatically when running `make format`
- Redeployed all examples in `cloudesf-testing`. That way, everyone on the team has permissions to deploy and update them.

FYI the diff looks big because this sorts the JSONs alphabetically by key. Semantically, they are identical. The only difference is the name of each API.

Unblocks #218
@nareddyt nareddyt changed the title Move snake to json segment mapping to per-operation instead of global b/158262661: Move snake to json segment mapping to per-operation instead of global Jul 14, 2020
# Conflicts:
#	examples/testdata/path_matcher/envoy_config.json
#	examples/testdata/path_matcher/openapi_swagger.json
#	examples/testdata/path_matcher/service_config_generated.json
Signed-off-by: Teju Nareddy <nareddyt@google.com>
@google-oss-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nareddyt, qiwzhang, TAOXUY

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [TAOXUY,nareddyt,qiwzhang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@qiwzhang
Copy link
Contributor

@nareddyt This is a breaking change. Please don't forget to change version to v7.

@nareddyt
Copy link
Contributor Author

@nareddyt This is a breaking change. Please don't forget to change version to v7.

Yup, I will make a PR changing to v7 as soon as this is submitted. Don't want to update in this PR, diff will be horrible.

@nareddyt nareddyt merged commit 373e123 into GoogleCloudPlatform:master Jul 14, 2020
@nareddyt nareddyt deleted the fix-json-name branch July 14, 2020 17:52
nareddyt added a commit to nareddyt/esp-v2 that referenced this pull request Jul 14, 2020
Due to breaking API change in GoogleCloudPlatform#218

Signed-off-by: Teju Nareddy <nareddyt@google.com>
nareddyt added a commit that referenced this pull request Jul 14, 2020
Due to breaking API change in #218

Signed-off-by: Teju Nareddy <nareddyt@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESPv2 fails to start when service config has duplicated camel_case name to json name mapping.
4 participants