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

proto: add a new field called append_action in the HeaderValueOption #18246

Merged
merged 7 commits into from Sep 24, 2021

Conversation

agrawroh
Copy link
Contributor

@agrawroh agrawroh commented Sep 23, 2021

This PR adds a new field called append_action to the HeaderValueOption. This is just the .proto change being done in the preparation for #18130. We'll mark the existing append field as deprecated in #18310.

append_action extends the header matcher by describing what append action to take indicating whether:

  1. The formatted header should be added only if it's not already present;
  2. New value should be appended to the existing values if header already exists; or
  3. New value should be overwritten by discarding any existing values if header already exists;

Commit Message: add a new field called append_action in the HeaderValueOption.
Additional Description: Adds a new field called append_action in HeaderValueOption which will ultimately deprecate the old append and will extend the existing matcher functionality.
Risk Level: -
Testing: -
Docs Changes: N/A (Will be done in #18130)
Release Notes: N/A (Will be done in #18130)
Platform Specific Features: N/A

Signed-off-by: Rohit Agrawal rohit.agrawal@databricks.com

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #18246 was opened by agrawroh.

see: more, trace.

// Header name/value pair that this option applies to.
HeaderValue header = 1 [(validate.rules).message = {required: true}];

// Should the value be appended? If true (default), the value is appended to
// existing values. Otherwise it replaces any existing values.
google.protobuf.BoolValue append = 2;

// Describes the action taken to append/overwrite the given value for an existing header
// or to only add this header if it's absent. Value defaults to :ref:`APPEND_IF_EXISTS_OR_ADD<envoy_v3_api_enum_value_config.core.v3.HeaderValueOption.HeaderAppendAction.APPEND_IF_EXISTS_OR_ADD>`.
Copy link
Member

Choose a reason for hiding this comment

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

Can you mark this [#not-implemented-hide] just in case it takes a bit for the other PR to land? Thank you.

/wait

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
@agrawroh
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18246 (comment) was created by @agrawroh.

see: more, trace.

@agrawroh
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18246 (comment) was created by @agrawroh.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks, please don't forget to revert the not-implemented hide changes in your other PR.

@mattklein123 mattklein123 merged commit 01b90ef into envoyproxy:main Sep 24, 2021
@agrawroh
Copy link
Contributor Author

Thanks, please don't forget to revert the not-implemented hide changes in your other PR.

Thanks, @mattklein123! Yes, I'll revert the not-implemented hide changes and will address all the remaining comments.

soulxu pushed a commit to soulxu/envoy that referenced this pull request Oct 16, 2021
htuch pushed a commit that referenced this pull request Aug 26, 2022
The HeaderAppendAction was added in the #18246. But the implementation never done. This PR try to complete this feature.

Further work of #18246. To close #22713.

Risk Level: Low.
Testing: Unit Test.

Signed-off-by: wbpcode <wangbaiping@corp.netease.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants