Skip to content

feat: add ipv4/ipv6 dual stack support#4375

Merged
arkodg merged 5 commits intoenvoyproxy:mainfrom
juwon8891:dualstack
Oct 24, 2024
Merged

feat: add ipv4/ipv6 dual stack support#4375
arkodg merged 5 commits intoenvoyproxy:mainfrom
juwon8891:dualstack

Conversation

@juwon8891
Copy link
Copy Markdown
Contributor

What type of PR is this?

feat: add ipv4/ipv6 dual stack support

What this PR does / why we need it:
This PR adds IPv4/IPv6 dual stack support to Envoy Gateway. It implements test cases for routing to IPv6-only, dual-stack, and IPv4-only services using HTTPRoute resources. This enhancement is crucial for supporting modern network environments and ensuring compatibility with both IPv4 and IPv6 infrastructures.

Key changes include:

  1. New test file backend_dualstack.go for testing IPv6 and dual-stack backends
  2. New test file httproute_dualstack.go for testing HTTPRoute support with various IP versions
  3. New manifest files for defining resources used in dual-stack tests
  4. Implementation of getDnsLookupFamily function to control DNS lookup behavior

Which issue(s) this PR fixes:

Fixes #184

Additional Notes:

  • Tests are skipped in IPv4-only environments (IP_FAMILY=ipv4)
  • All tests run in IPv6 or dual-stack environments (IP_FAMILY=ipv6 or IP_FAMILY=dual)

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Sep 30, 2024

hey @juwon8891 we probably also need to add some logic in

func (t *Translator) processServiceDestinationSetting(
which selects specific endpoints based on the ipFamilyPolicy field

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we also check for the IP type in the X-Forwarded-For response header ?

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 7, 2024

Codecov Report

Attention: Patch coverage is 61.11111% with 7 lines in your changes missing coverage. Please review.

Project coverage is 65.80%. Comparing base (7188dad) to head (faa584b).

Files with missing lines Patch % Lines
internal/gatewayapi/helpers.go 36.36% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4375      +/-   ##
==========================================
+ Coverage   65.78%   65.80%   +0.02%     
==========================================
  Files         210      210              
  Lines       31516    31557      +41     
==========================================
+ Hits        20732    20766      +34     
- Misses       9588     9594       +6     
- Partials     1196     1197       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Oct 7, 2024

hey @juwon8891 lint is failing


internal/xds/translator/testdata/in/xds-ir/tcp-listener-ipfamily.yaml
  Error: 7:5 [indentation] wrong indentation: expected 6 but found 4
  Error: 11:9 [indentation] wrong indentation: expected 10 but found 8
  Error: 12:11 [indentation] wrong indentation: expected 12 but found 10
  Error: 13:32 [new-line-at-end-of-file] no new line character at the end of file

@juwon8891 juwon8891 force-pushed the dualstack branch 2 times, most recently from 263b8bf to b97d58a Compare October 8, 2024 22:21
zhaohuabing

This comment was marked as resolved.

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Oct 9, 2024

hey @juwon8891 looks like some gateway api tests got wiped out, can you add them back ?

@zhaohuabing
Copy link
Copy Markdown
Member

zhaohuabing commented Oct 14, 2024

@juwon8891 Do you have time to address the comments this week? We want to ensure dual stack support is ready to be included in the coming v1.2.

@juwon8891
Copy link
Copy Markdown
Contributor Author

Yes, I'm working on it. I'll answer as soon as possible

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

func QueryLogCountFromLoki(t *testing.T, c client.Client, keyValues map[string]string, match string) (int, error) {

can be used to verify pod ip/port

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm still working on this test part. I'll post it separately as another PR.

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Oct 22, 2024

hey @juwon8891, added some minor comments, but overall LGTM !

@zirain
Copy link
Copy Markdown
Member

zirain commented Oct 22, 2024

@juwon8891 please run make gen-check to pass the gen-check CI.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you make it a pointer? so that this PR won't be that huge.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll post it separately as another PR.

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Oct 22, 2024

@juwon8891 DCO is failing, can you sign your commits

Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com>
Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com>
Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com>
Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com>
Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com>
Copy link
Copy Markdown
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg added this to the v1.2.0-rc1 milestone Oct 24, 2024
Copy link
Copy Markdown
Contributor

@guydc guydc left a comment

Choose a reason for hiding this comment

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

LGTM!

@guydc
Copy link
Copy Markdown
Contributor

guydc commented Oct 24, 2024

/retest

@arkodg arkodg merged commit d7849d7 into envoyproxy:main Oct 24, 2024
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.

Add IPv4/IPv6 Dual Stack Support

6 participants