search

envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.87k stars 4.78k forks source link

Sanity checks on dummy configs for integration tests #20275

Open RenjieTang opened 2 years ago

RenjieTang commented 2 years ago

This issue was discovered in #20259 which used MockClusterInfo for HTTP integration tests.

However the default value of the connect timeout would cause Quiche to complain and crash at QuicConnection::SetNetworkTimeouts.

I think default value like this doesn't make much sense. I wonder if more of such cases exist in Envoy's integration test setups.

RyanTheOptimist commented 2 years ago

cc: @alyssawilk @danzh2010

danzh2010 commented 2 years ago

Thanks for bringing up this issue! I think this is indeed a config sanitization issue rather than test config being non-sense. we need to guard Envoy from bad config input.

alyssawilk commented 2 years ago

+1, we shouldn't accept config which causes ENVOY_BUG later down the line.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

danzh2010 commented 2 years ago

needs a non-stale tag