Skip to content

api: Add validate to required member.#20881

Merged
mattklein123 merged 1 commit intoenvoyproxy:mainfrom
vehre-x41:ensure-bin-op-operand-is-present-in-api
Apr 19, 2022
Merged

api: Add validate to required member.#20881
mattklein123 merged 1 commit intoenvoyproxy:mainfrom
vehre-x41:ensure-bin-op-operand-is-present-in-api

Conversation

@vehre-x41
Copy link
Copy Markdown
Contributor

Commit Message: api: Add validate to required member.
Additional Description:
ComparisonFilter's value now marked as required in validate to ensure valid
input to fuzz tests.
Signed-off-by: Andre Vehreschild vehre@x41-dsec.de
Risk Level: Low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
API Considerations: Add a validate rule to the value a binary operation compares against. Not ensuring that the value exists without giving a reasonable default is questionable. Therefore this patch adds a validate rule to at least flag the missing value.

ComparisonFilter's value now marked as required in validate to ensure valid
input to fuzz tests.

Signed-off-by: Andre Vehreschild <vehre@x41-dsec.de>
@repokitteh-read-only
Copy link
Copy Markdown

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: #20881 was opened by vehre-x41.

see: more, trace.

@vehre-x41
Copy link
Copy Markdown
Contributor Author

@adisuissa, @yanavlasov, @htuch Here is the patch as requested.

@adisuissa adisuissa self-assigned this Apr 19, 2022
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM.
This is a breaking change to the API!
That said looking at the ComparisonFilter type, it doesn't make sense to only have op without the value, so I think it makes sense

@vehre-x41
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #20881 (comment) was created by @vehre-x41.

see: more, trace.

@mattklein123 mattklein123 merged commit 8df3136 into envoyproxy:main Apr 19, 2022
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
ComparisonFilter's value now marked as required in validate to ensure valid
input to fuzz tests.

Signed-off-by: Andre Vehreschild <vehre@x41-dsec.de>
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.

3 participants