Skip to content

release 1.1: Update envoy to enable path normalization#2166

Merged
duderino merged 3 commits intoistio:release-1.1from
lambdai:my11
Apr 11, 2019
Merged

release 1.1: Update envoy to enable path normalization#2166
duderino merged 3 commits intoistio:release-1.1from
lambdai:my11

Conversation

@lambdai
Copy link
Copy Markdown
Contributor

@lambdai lambdai commented Apr 10, 2019

On top of #2158

And introduce the path normalization.

  • Update envoy dependency to 1.10 at 04/10/2019
  • Update .bazelrc to adopt the compilation option to enable the path normalization by default
  • Update WORKSPACE so that it support envoy build from http github repo and local envoy. The latter is heavily used by debugging.

Q: How to build or test the path normalization?
A: Any make/bazel command should pickup the compilation option automatically. See .bazelrc

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 10, 2019
@googlebot
Copy link
Copy Markdown
Collaborator

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label Apr 10, 2019
envoy commit date: 04/10/2019
bazel version: 0.22.0

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@googlebot
Copy link
Copy Markdown
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels Apr 10, 2019
@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 10, 2019
@lambdai lambdai changed the title Update envoy to enable path normalization release 1.1: Update envoy to enable path normalization Apr 10, 2019
Comment thread WORKSPACE
)

# envoy commit date 04/10/2019
# bazel version: 0.22.0
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice touch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be another pain if we forgot to update the comment... We will see.

Comment thread WORKSPACE
#)

load("@envoy//bazel:api_repositories.bzl", "envoy_api_dependencies")
envoy_api_dependencies()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you have to add this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Envoy is splitting /api to external repository. It's WIP and I am seeing error when I introduce envoy using local_repository.

With this fix, proxy build works perfectly no matter we introduce envoy by http or by local_repository.

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Comment thread WORKSPACE
#local_repository(
# name = "envoy",
# path = LOCAL_ENVOY_PROJECT
#)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did stuff for building against a local envoy slip in here? I think it's fine to keep it here commented out, but if you do, please add a few comments describing what it's for

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave here because I probably pasted these lines 10 times in the last 2 weeks... I haven't figure out if we can use bazel arg to switch between local or http_archive
Let me add the comment.

Copy link
Copy Markdown
Contributor Author

@lambdai lambdai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add the comment to local_repository

Comment thread WORKSPACE
#)

load("@envoy//bazel:api_repositories.bzl", "envoy_api_dependencies")
envoy_api_dependencies()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Envoy is splitting /api to external repository. It's WIP and I am seeing error when I introduce envoy using local_repository.

With this fix, proxy build works perfectly no matter we introduce envoy by http or by local_repository.

Comment thread WORKSPACE
#local_repository(
# name = "envoy",
# path = LOCAL_ENVOY_PROJECT
#)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave here because I probably pasted these lines 10 times in the last 2 weeks... I haven't figure out if we can use bazel arg to switch between local or http_archive
Let me add the comment.

Comment thread WORKSPACE
)

# envoy commit date 04/10/2019
# bazel version: 0.22.0
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be another pain if we forgot to update the comment... We will see.

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@duderino
Copy link
Copy Markdown

/lgtm will merge once tests pass

@istio-testing
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: duderino, silentdai

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@duderino duderino merged commit bf8b9cd into istio:release-1.1 Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants