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

xds: use V4_PREFERRED dnsLookupFamily by default #4745

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

zirain
Copy link
Contributor

@zirain zirain commented Nov 20, 2024

fixes: #4744

@zirain zirain requested a review from a team as a code owner November 20, 2024 19:56
@zirain
Copy link
Contributor Author

zirain commented Nov 20, 2024

cc @zetaab can you verify this on your env? hope it will work.

Copy link

codecov bot commented Nov 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.60%. Comparing base (78da42c) to head (c17c635).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4745      +/-   ##
==========================================
- Coverage   65.61%   65.60%   -0.02%     
==========================================
  Files         211      211              
  Lines       31961    31961              
==========================================
- Hits        20972    20968       -4     
- Misses       9751     9753       +2     
- Partials     1238     1240       +2     

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


🚨 Try these New Features:

arkodg
arkodg previously approved these changes Nov 20, 2024
@arkodg arkodg requested review from a team November 20, 2024 20:07
Copy link
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

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

@zirain @arkodg I think V4_PREFERRED won't work for IPv6 env where the envoy pod only has an IPv6 address.

If V4_PREFERRED is specified, the DNS resolver will first perform a lookup for addresses in the IPv4 family and fallback to a lookup for addresses in the IPv6 family.

@zirain
Copy link
Contributor Author

zirain commented Nov 21, 2024

@zirain @arkodg I think V4_PREFERRED won't work for IPv6 env where the envoy pod only has an IPv6 address.

If V4_PREFERRED is specified, the DNS resolver will first perform a lookup for addresses in the IPv4 family and fallback to a lookup for addresses in the IPv6 family.

tested passed on an IPv6 only cluster.

3b26516

@zhaohuabing
Copy link
Member

3b26516

I mean "real" IPv6 only where the pod just has an IPv6 address and has no IPv4 address.

Does the pod in this test has an IPv4 address? If it only has an IPv6 address, it shouldn't be able to establish an connection to an IPv4 server.

@zirain
Copy link
Contributor Author

zirain commented Nov 21, 2024

3b26516

I mean "real" IPv6 only where the pod just has an IPv6 address and has no IPv4 address.

Does the pod in this test has an IPv4 address? If it only has an IPv6 address, it shouldn't be able to establish an connection to an IPv4 server.

it's IPv6 only cluster, not IPv6 first.

@zhaohuabing
Copy link
Member

zhaohuabing commented Nov 21, 2024

#4744

I guess if AUTO(IPv6_prefered) didn't work for pod without IPv6 address ( like in #4744), then IPv4_prefered may not work for pod without IPv4 address as well. I may be wrong, will test it myself.

@zhaohuabing
Copy link
Member

zhaohuabing commented Nov 21, 2024

This test verfied that V4_PREFERRED doesn't work with IPv6 only.

#4745SP with a JWK configuration:

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: demo-api-jwt
spec:
  jwt:
    providers:
    - name: test
      remoteJWKS:
        uri: https://www.zhaohuabing.com/misc/jwks.json
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: backend

DNS records for IPv4 and IPv6

➜  gateway git:(use/V4_PREFERRED) dig www.zhaohuabing.com     

;; ANSWER SECTION:
www.zhaohuabing.com.	247	IN	A	104.21.60.99
www.zhaohuabing.com.	247	IN	A	172.67.195.133

➜  gateway git:(use/V4_PREFERRED) dig AAAA www.zhaohuabing.com

;; ANSWER SECTION:
www.zhaohuabing.com.	244	IN	AAAA	2606:4700:3037::ac43:c385
www.zhaohuabing.com.	244	IN	AAAA	2606:4700:3034::6815:3c63

Test with IPv6 only, failed to fetch jwks

export IP_FAMILY=ipv6; export IMAGE_PULL_POLICY=IfNotPresent; make create-cluster kube-install-image kube-deploy

Envoy log

[2024-11-21 06:37:38.446][1][warning][jwt] [source/extensions/filters/http/jwt_authn/jwks_async_fetcher.cc:115] Jwks async fetching url=https://www.zhaohuabing.com/misc/jwks.json: failed

The jwks can be downloaded via curl --ipv6

curl --ipv6 -v https://www.zhaohuabing.com/misc/jwks.json
* Host www.zhaohuabing.com:443 was resolved.
* IPv6: 2606:4700:3034::6815:3c63, 2606:4700:3037::ac43:c385
* IPv4: (none)
*   Trying [2606:4700:3034::6815:3c63]:443...
* Connected to www.zhaohuabing.com (2606:4700:3034::6815:3c63) port 443
....
{ "keys":[ {"e":"AQAB","kid":"DHFbpoIUqrY8t2zpA2qXfCmr5VO5ZEr4RzHU_-envvQ","kty":"RSA","n":"xAE7eB6qugXyCAG3yhh7pkDkT65pHymX-P7KfIupjf59vsdo91bSP9C8H07pSAGQO1MV_xFj9VswgsCg4R6otmg5PV2He95lZdHtOcU5DXIg_pbhLdKXbi66GlVeK6ABZOUW3WYtnNHD-91gVuoeJT_DwtGGcp4ignkgXfkiEm4sw-4sfb4qdt5oLbyVpmW6x9cfa7vs2WTfURiCrBoUqgBo_-4WTiULmmHSGZHOjzwa8WtrtOQGsAFjIbno85jp6MnGGGZPYZbDAa_b3y5u-YpW7ypZrvD8BgtKVjgtQgZhLAGezMt0ua3DRrWnKqTZ0BJ_EyxOGuHJrLsn00fnMQ"}]}

Test with IPv4 only, succeeded

export IP_FAMILY=ipv4; export IMAGE_PULL_POLICY=IfNotPresent; make create-cluster kube-install-image kube-deploy
[2024-11-21 06:32:39.285][1][debug][filter] [source/extensions/filters/http/common/jwks_fetcher.cc:92] onSuccess: fetch pubkey [uri = https://www.zhaohuabing.com/misc/jwks.json]: succeeded

@zirain
Copy link
Contributor Author

zirain commented Nov 21, 2024

can you passed the test with IPv4_only on a IPv6 only cluster?

zhaohuabing
zhaohuabing previously approved these changes Nov 21, 2024
Copy link
Member

@zhaohuabing zhaohuabing 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.

Sorry I didn't notice that the DNSLookupfamily will be changed based on the IPFamily setting.

It would be useful to include a few lines to the IPFamily API docs explaining how IPFamily affects the Envoy DNS resolver.

zhaohuabing
zhaohuabing previously approved these changes Nov 21, 2024
zhaohuabing
zhaohuabing previously approved these changes Nov 21, 2024
@zirain zirain requested a review from arkodg November 21, 2024 08:05
zhaohuabing
zhaohuabing previously approved these changes Nov 21, 2024
@zhaohuabing
Copy link
Member

zhaohuabing commented Nov 22, 2024

envoyProxy resource IPFamily is only meant for listener and Envoy Proxy fleet Service spec

BackendRef IPFamily determines cluster setting

Are we planning to introduce another "BackendRef IPFamily" configuration knob? Even though they have different meaning, typically the value of listener IP famaily and the Backend IP family for in-cluster services should be the same.

@arkodg arkodg merged commit 0130b77 into envoyproxy:main Nov 26, 2024
24 checks passed
@zirain zirain deleted the use/V4_PREFERRED branch November 27, 2024 00:19
zhaohuabing pushed a commit to zhaohuabing/gateway that referenced this pull request Nov 27, 2024
* use Cluster_V4_PREFERRED

Signed-off-by: zirain <[email protected]>

* release notes

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
zhaohuabing added a commit that referenced this pull request Nov 27, 2024
* fix: tcp listener is rejected when no route attached (#4681)

* fix: tcp listener is rejected when no route attached

Signed-off-by: Huabing Zhao <[email protected]>

* change cluter name

Signed-off-by: Huabing Zhao <[email protected]>

* fix listener connection limit test

Signed-off-by: Huabing Zhao <[email protected]>

* fix listener connetcp keepalive  test

Signed-off-by: Huabing Zhao <[email protected]>

* fix tcp endpoint stats test

Signed-off-by: Huabing Zhao <[email protected]>

* fix tcp-route-enable-req-resp-sizes-stats

Signed-off-by: Huabing Zhao <[email protected]>

* fix extensionpolicy-tcp-udp-http test

Signed-off-by: Huabing Zhao <[email protected]>

* fix lint

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>
(cherry picked from commit f99c36c)
Signed-off-by: Huabing Zhao <[email protected]>

* fix: remove backendrefs validation (#4705)

* remove backendrefs validation

Signed-off-by: Huabing Zhao <[email protected]>

* add tests

Signed-off-by: Huabing Zhao <[email protected]>

* add tests

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>
Co-authored-by: zirain <[email protected]>
(cherry picked from commit 5068698)
Signed-off-by: Huabing Zhao <[email protected]>

* fix: translator reports errors for existing clusters and secretes (#4707)

* fix: existing clusters and secretes

Signed-off-by: Huabing Zhao <[email protected]>

* fix cluster index for SP

Signed-off-by: Huabing Zhao <[email protected]>

* minor change

Signed-off-by: Huabing Zhao <[email protected]>

* minor change

Signed-off-by: Huabing Zhao <[email protected]>

* minor change

Signed-off-by: Huabing Zhao <[email protected]>

* minor change

Signed-off-by: Huabing Zhao <[email protected]>

* fix lint

Signed-off-by: Huabing Zhao <[email protected]>

* add comment

Signed-off-by: Huabing Zhao <[email protected]>

* remove index

Signed-off-by: Huabing Zhao <[email protected]>

* fix lint

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>

* xds: always use `::` and `IPv4Compact` for dynamic listener (#4743)

* enable IPv4Compact

Signed-off-by: zirain <[email protected]>

* fix xds test

Signed-off-by: zirain <[email protected]>

* release-notes

Signed-off-by: zirain <[email protected]>

* nit

Signed-off-by: zirain <[email protected]>

* gen

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 78da42c)
Signed-off-by: Huabing Zhao <[email protected]>

* Fix: frequent 503 errors when connecting to a Service experiencing high Pod churn (#4754)

* Revert "fix: some status updates are discarded by the status updater (#4337)"

This reverts commit 14830c7.

Signed-off-by: Huabing Zhao <[email protected]>

* store update events and process it later

Signed-off-by: Huabing Zhao <[email protected]>

* rename method

Signed-off-by: Huabing Zhao <[email protected]>

* add release note

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>

* xds: use V4_PREFERRED dnsLookupFamily by default (#4745)

* use Cluster_V4_PREFERRED

Signed-off-by: zirain <[email protected]>

* release notes

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: zirain <[email protected]>
Co-authored-by: zirain <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dnslookupfamily returns ipv6 addresses for external clusters (oidc)
3 participants