Skip to content

Manual backport 1.15.x of Avoid panic applying TProxy Envoy extensions#17539

Merged
zalimeni merged 1 commit intorelease/1.15.xfrom
zalimeni/net-3900-fix-tproxy-extension-panic--1.15.x
Jun 1, 2023
Merged

Manual backport 1.15.x of Avoid panic applying TProxy Envoy extensions#17539
zalimeni merged 1 commit intorelease/1.15.xfrom
zalimeni/net-3900-fix-tproxy-extension-panic--1.15.x

Conversation

@zalimeni
Copy link
Member

@zalimeni zalimeni commented Jun 1, 2023

backport of commit 8c30455

Description

Manual backport of #17537.

Resolves automated backport PR #17542.

@zalimeni zalimeni added theme/envoy/xds Related to Envoy support pr/no-changelog PR does not need a corresponding .changelog entry pr/no-docs PR does not include docs and should not trigger reminder for cherrypicking them. pr/no-backport labels Jun 1, 2023
@zalimeni zalimeni force-pushed the zalimeni/net-3900-fix-tproxy-extension-panic--1.15.x branch from 3876ef3 to 2cec60e Compare June 1, 2023 01:56
@@ -112,89 +112,12 @@ func (b *BasicEnvoyExtender) Extend(resources *xdscommon.IndexedResources, confi
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Changes in this file are equivalent to the original PR but slightly different due to refactors that did not require backporting

@zalimeni zalimeni marked this pull request as ready for review June 1, 2023 02:05
@zalimeni zalimeni requested review from cthain and hashi-derek June 1, 2023 02:05
Copy link
Contributor

@cthain cthain left a comment

Choose a reason for hiding this comment

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

👍

@zalimeni zalimeni force-pushed the zalimeni/net-3900-fix-tproxy-extension-panic--1.15.x branch from 2cec60e to acb24c0 Compare June 1, 2023 15:28
@zalimeni zalimeni force-pushed the zalimeni/net-3900-fix-tproxy-extension-panic--1.15.x branch from acb24c0 to be7cb47 Compare June 1, 2023 16:18
Comment on lines 114 to 120

func (b *BasicEnvoyExtender) patchListener(config *RuntimeConfig, l *envoy_listener_v3.Listener) (proto.Message, bool, error) {
func (b *BasicEnvoyExtender) patchSupportedListenerFilterChains(config *RuntimeConfig, l *envoy_listener_v3.Listener) (proto.Message, bool, error) {
switch config.Kind {
case api.ServiceKindTerminatingGateway:
return b.patchTerminatingGatewayListener(config, l)
case api.ServiceKindConnectProxy:
return b.patchConnectProxyListener(config, l)
case api.ServiceKindTerminatingGateway, api.ServiceKindConnectProxy:
return b.patchListenerFilterChains(config, l)
}
return l, false, nil
}
Copy link
Member Author

@zalimeni zalimeni Jun 1, 2023

Choose a reason for hiding this comment

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

After double checking the original change that introduced this switch and the matching one above, I restored it to align with that and reduce changes here (I'd originally thought it was more copy-pasta). This could prevent a future bug if we were to support more proxy types for extensions in general without aligning that support with listeners specifically.

No other changes since the last review. I've also re-tested by hand w/ this change.

@zalimeni zalimeni merged commit aca09d2 into release/1.15.x Jun 1, 2023
@zalimeni zalimeni deleted the zalimeni/net-3900-fix-tproxy-extension-panic--1.15.x branch June 1, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry pr/no-docs PR does not include docs and should not trigger reminder for cherrypicking them. theme/envoy/xds Related to Envoy support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants