Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ipv4/ipv6 dual stack support #4375

Merged
merged 5 commits into from
Oct 24, 2024
Merged

Conversation

juwon8891
Copy link
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
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

gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns}

t.Run("HTTPRoute to IPv6 only service", func(t *testing.T) {
routeNN := types.NamespacedName{Name: "ipv6-only-route", Namespace: ns}
Copy link
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 ?

Copy link

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
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
internal/ir/xds.go Outdated Show resolved Hide resolved
zhaohuabing

This comment was marked as resolved.

internal/ir/xds.go Outdated Show resolved Hide resolved
@arkodg
Copy link
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
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
Contributor Author

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

Namespace: ns,
}

http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse)
Copy link
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
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
Contributor

arkodg commented Oct 22, 2024

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

@zirain
Copy link
Member

zirain commented Oct 22, 2024

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

@@ -224,8 +224,20 @@ type CoreListenerDetails struct {
ExtensionRefs []*UnstructuredRef `json:"extensionRefs,omitempty" yaml:"extensionRefs,omitempty"`
// Metadata is used to enrich envoy resource metadata with user and provider-specific information
Metadata *ResourceMetadata `json:"metadata,omitempty" yaml:"metadata,omitempty"`
// IPFamily specifies the IP address family for the gateway.
// It can be IPv4, IPv6, or Dual.
IPFamily IPFamily `json:"ipFamily,omitempty" yaml:"ipFamily,omitempty"`
Copy link
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
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
Contributor

arkodg commented Oct 22, 2024

@juwon8891 DCO is failing, can you sign your commits

Signed-off-by: Juwon Hwang (Kevin) <[email protected]>
Signed-off-by: Juwon Hwang (Kevin) <[email protected]>
Signed-off-by: Juwon Hwang (Kevin) <[email protected]>
Signed-off-by: Juwon Hwang (Kevin) <[email protected]>
Signed-off-by: Juwon Hwang (Kevin) <[email protected]>
Copy link
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
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
Contributor

guydc commented Oct 24, 2024

/retest

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