Skip to content

Pluralizes Listener Ports of Proxy Infra IR#183

Merged
danehans merged 1 commit intoenvoyproxy:mainfrom
danehans:infra_ir_update
Aug 1, 2022
Merged

Pluralizes Listener Ports of Proxy Infra IR#183
danehans merged 1 commit intoenvoyproxy:mainfrom
danehans:infra_ir_update

Conversation

@danehans
Copy link
Copy Markdown
Contributor

@danehans danehans commented Aug 1, 2022

Previously, a listener could only have a single port. This PR pluralizes infra.proxy.listeners[].ports so a listener can have multiple ports.

Signed-off-by: danehans daneyonhansen@gmail.com

@danehans danehans requested a review from a team as a code owner August 1, 2022 17:02
@lizan
Copy link
Copy Markdown
Member

lizan commented Aug 1, 2022

We might want to pluralize the combination of address and port, it is a common use case in IPv6/v4 dual stack etc.

Signed-off-by: danehans <daneyonhansen@gmail.com>
@danehans
Copy link
Copy Markdown
Contributor Author

danehans commented Aug 1, 2022

@lizan there are some nuances in Kube to support IPv6. In an effort to focus on the v0.2.0 goals, I prefer to concentrate the PR on IPv4. I created #184 to track the need to support IPv6.

@danehans danehans requested a review from arkodg August 1, 2022 17:49
@lizan
Copy link
Copy Markdown
Member

lizan commented Aug 1, 2022

I'm not saying we should do IPv6 support right now, but if we pluralize ports only now it will be messy when we pluralize address. Since this is just IR not an API so it is up to you.

@danehans
Copy link
Copy Markdown
Contributor Author

danehans commented Aug 1, 2022

@lizan understood. I think it's best not to pluralize addresses right now b/c I'm unsure if that's the right approach to supporting dual-stack. Let's make that decision as part of #184.

@danehans danehans merged commit c83b7b0 into envoyproxy:main Aug 1, 2022
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.

4 participants