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

Make the limit on HTTP/2 metadata that can be handled configurable #37372

Merged
merged 6 commits into from
Jan 21, 2025

Conversation

jaydunk
Copy link
Contributor

@jaydunk jaydunk commented Nov 26, 2024

Commit Message: Make the limit on HTTP/2 metadata configurable. Defaults to the current hardcoded value of 1 MiB.
Additional Description: The value can be configured through a new field on Http2ProtocolOptions
Risk Level: Low
Testing: Adds a unit test that covers the new logic in MetadataDecoder
Docs Changes: Added to new proto field but kept hidden to be consistent with other H2 metadata configuration
Release Notes: None
Platform Specific Features: None

Copy link

Hi @jaydunk, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #37372 was opened by jaydunk.

see: more, trace.

Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #37372 was opened by jaydunk.

see: more, trace.

@jaydunk
Copy link
Contributor Author

jaydunk commented Dec 4, 2024

/retest

@antoniovleonti
Copy link
Contributor

/assign @yanavlasov

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.

/wait

api/envoy/config/core/v3/protocol.proto Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jan 8, 2025

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added stale stalebot believes this issue/PR has not been touched recently and removed stale stalebot believes this issue/PR has not been touched recently labels Jan 8, 2025
@yanavlasov
Copy link
Contributor

Requested changes from author

/wait

@mattklein123
Copy link
Member

LGTM please check CI and merge main.

/wait

Signed-off-by: Jay Dunkelberger <[email protected]>
@jaydunk
Copy link
Contributor Author

jaydunk commented Jan 21, 2025

/retest

@jaydunk
Copy link
Contributor Author

jaydunk commented Jan 21, 2025

@mattklein123 @yanavlasov Does this need one more approval? Looks like the CI is green now.

@yanavlasov yanavlasov dismissed mattklein123’s stale review January 21, 2025 17:47

Request was addressed

@yanavlasov yanavlasov merged commit 98d153b into envoyproxy:main Jan 21, 2025
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants