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

Envoy gloo bump for inja escaped characters #8499

Merged
merged 23 commits into from
Jul 28, 2023

Conversation

jbohanon
Copy link
Contributor

@jbohanon jbohanon commented Jul 21, 2023

Description

This gets the latest envoy-gloo in order to test functionality with new Inja feature and new custom callback

Control plane changes to accommodate this new functionality:
Settings CRD:

glooOptions:
  transformationOptions:
    escapeCharacters: true

TransformationStages:
Setting can be configured as part of stagedTransformations on VirtualHostOptions, RouteOptions, or WeightedDestinationOptions

Context

See context here

Checklist:

  • I included a concise, user-facing changelog (for details, see https://github.com/solo-io/go-utils/tree/main/changelogutils) which references the issue that is resolved.
  • If I updated APIs (our protos) or helm values, I ran make -B install-go-tools generated-code to ensure there will be no code diff
  • I followed guidelines laid out in the Gloo Edge contribution guide
  • I opened a draft PR or added the work in progress label if my PR is not ready for review
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@jbohanon jbohanon requested a review from a team as a code owner July 21, 2023 21:41
@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/5155

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Jul 21, 2023
@jbohanon jbohanon added the work in progress signals bulldozer to keep pr open (don't auto-merge) label Jul 21, 2023
@github-actions
Copy link

github-actions bot commented Jul 21, 2023

Visit the preview URL for this PR (updated for commit 68096ed):

https://gloo-edge--pr8499-envoy-gloo-bump-for-43ptaome.web.app

(expires Thu, 03 Aug 2023 21:01:34 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

Copy link
Contributor

@nfuden nfuden left a comment

Choose a reason for hiding this comment

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

We should allow for inheritance either via a gateway or settings level default

elcasteel
elcasteel previously approved these changes Jul 27, 2023
Copy link
Contributor

@elcasteel elcasteel left a comment

Choose a reason for hiding this comment

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

LGTM!

elcasteel
elcasteel previously approved these changes Jul 27, 2023
[]*envoytransformation.TransformationTemplate_HeaderToAppend,
len(inTemplate.GetHeadersToAppend()))
headers := inTemplate.GetHeadersToAppend()
for i := range headers {
Copy link
Contributor

Choose a reason for hiding this comment

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

for i, hdr := range headers is more idiomatic and only an extra allocation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's one allocation, but it copies the slice val out to the temp on every loop iteration

nfuden
nfuden previously approved these changes Jul 27, 2023
@jbohanon jbohanon dismissed stale reviews from nfuden and elcasteel via 68096ed July 27, 2023 20:44
@jbohanon jbohanon removed the work in progress signals bulldozer to keep pr open (don't auto-merge) label Jul 28, 2023
@soloio-bulldozer soloio-bulldozer bot merged commit 9be86ed into main Jul 28, 2023
@soloio-bulldozer soloio-bulldozer bot deleted the envoy-gloo-bump-for-inja branch July 28, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants