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.

Sign up for GitHub

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

Jump to bottom

Implementation for minTTL #6808

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Implementation for minTTL #6808

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
346988f
initial commit for implementing minTTL
hkiiita Nov 13, 2024
66e72f1
auto generated antrea yml files
hkiiita Nov 14, 2024
d92246f
Refactored minTTL confgiyration and use.
hkiiita Nov 15, 2024
2c58ad2
Merge branch 'antrea-io:main' into fqdn-cache-minttl
hkiiita Nov 15, 2024
e9e0286
update final assertion and comment for the test.
hkiiita Nov 15, 2024
dd43d6b
move minTTL validation to parent function and rename to fqdnCacheMinTTL
hkiiita Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions build/charts/antrea/conf/antrea-agent.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ kubeAPIServerOverride: {{ .Values.kubeAPIServerOverride | quote }}
# 10.96.0.10:53, [fd00:10:96::a]:53).
dnsServerOverride: {{ .Values.dnsServerOverride | quote }}

# The minTTL setting helps address the problem of applications caching DNS response IPs beyond the TTL value for the DNS record.
# It is used to enforce FQDN policy rules, ensuring that resolved IPs are included in datapath rules for as long as the application is caching them.
# This value should ideally be set to the maximum caching duration across all applications.
minTTL: {{ .Values.minTTL }}
Copy link
Contributor

Choose a reason for hiding this comment

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

the name is a bit generic, maybe we should go with fqdnCacheMinTTL?


# Comma-separated list of Cipher Suites. If omitted, the default Go Cipher Suites will be used.
# https://golang.org/pkg/crypto/tls/#pkg-constants
# Note that TLS1.3 Cipher Suites cannot be added to the list. But the apiserver will always
Expand Down
4 changes: 4 additions & 0 deletions build/charts/antrea/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ kubeAPIServerOverride: ""
# -- Address of DNS server, to override the kube-dns Service. It's used to
# resolve hostnames in a FQDN policy.
dnsServerOverride: ""
# -- The minTTL setting helps address the problem of applications caching DNS response IPs indefinitely.
# The Cluster administrators should configure this value, ideally setting it to be equal to or greater than the maximum TTL
# value of the application's DNS cache.
minTTL: 0
# -- IPv4 CIDR range used for Services. Required when AntreaProxy is disabled.
serviceCIDR: ""
# -- IPv6 CIDR range used for Services. Required when AntreaProxy is disabled.
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-aks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4234,6 +4234,11 @@ data:
# 10.96.0.10:53, [fd00:10:96::a]:53).
dnsServerOverride: ""

# The minTTL setting helps address the problem of applications caching DNS response IPs beyond the TTL value for the DNS record.
# It is used to enforce FQDN policy rules, ensuring that resolved IPs are included in datapath rules for as long as the application is caching them.
# This value should ideally be set to the maximum caching duration across all applications.
minTTL: 0

# Comma-separated list of Cipher Suites. If omitted, the default Go Cipher Suites will be used.
# https://golang.org/pkg/crypto/tls/#pkg-constants
# Note that TLS1.3 Cipher Suites cannot be added to the list. But the apiserver will always
Expand Down Expand Up @@ -5383,7 +5388,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: e2d1d8af083c88667ac4c22c87dea63e595b2f4f770190c32afb00c480440fe3
checksum/config: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5621,7 +5626,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: e2d1d8af083c88667ac4c22c87dea63e595b2f4f770190c32afb00c480440fe3
checksum/config: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-eks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4234,6 +4234,11 @@ data:
# 10.96.0.10:53, [fd00:10:96::a]:53).
dnsServerOverride: ""

# The minTTL setting helps address the problem of applications caching DNS response IPs beyond the TTL value for the DNS record.
# It is used to enforce FQDN policy rules, ensuring that resolved IPs are included in datapath rules for as long as the application is caching them.
# This value should ideally be set to the maximum caching duration across all applications.
minTTL: 0

# Comma-separated list of Cipher Suites. If omitted, the default Go Cipher Suites will be used.
# https://golang.org/pkg/crypto/tls/#pkg-constants
# Note that TLS1.3 Cipher Suites cannot be added to the list. But the apiserver will always
Expand Down Expand Up @@ -5383,7 +5388,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: e2d1d8af083c88667ac4c22c87dea63e595b2f4f770190c32afb00c480440fe3
checksum/config: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5622,7 +5627,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: e2d1d8af083c88667ac4c22c87dea63e595b2f4f770190c32afb00c480440fe3
checksum/config: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-gke.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4234,6 +4234,11 @@ data:
# 10.96.0.10:53, [fd00:10:96::a]:53).
dnsServerOverride: ""

# The minTTL setting helps address the problem of applications caching DNS response IPs beyond the TTL value for the DNS record.
# It is used to enforce FQDN policy rules, ensuring that resolved IPs are included in datapath rules for as long as the application is caching them.
# This value should ideally be set to the maximum caching duration across all applications.
minTTL: 0

# Comma-separated list of Cipher Suites. If omitted, the default Go Cipher Suites will be used.
# https://golang.org/pkg/crypto/tls/#pkg-constants
# Note that TLS1.3 Cipher Suites cannot be added to the list. But the apiserver will always
Expand Down Expand Up @@ -5383,7 +5388,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 7e42a403d388e2ed556d9b41f4af83917eadd0863d4e2bef67353f5adb2ef6c3
checksum/config: 6c71e376d2b2ea5a377700084d271a88925337887b10a4432d68b7d269911d90
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5619,7 +5624,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 7e42a403d388e2ed556d9b41f4af83917eadd0863d4e2bef67353f5adb2ef6c3
checksum/config: 6c71e376d2b2ea5a377700084d271a88925337887b10a4432d68b7d269911d90
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-ipsec.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4247,6 +4247,11 @@ data:
# 10.96.0.10:53, [fd00:10:96::a]:53).
dnsServerOverride: ""

# The minTTL setting helps address the problem of applications caching DNS response IPs beyond the TTL value for the DNS record.
# It is used to enforce FQDN policy rules, ensuring that resolved IPs are included in datapath rules for as long as the application is caching them.
# This value should ideally be set to the maximum caching duration across all applications.
minTTL: 0

# Comma-separated list of Cipher Suites. If omitted, the default Go Cipher Suites will be used.
# https://golang.org/pkg/crypto/tls/#pkg-constants
# Note that TLS1.3 Cipher Suites cannot be added to the list. But the apiserver will always
Expand Down Expand Up @@ -5396,7 +5401,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 7d8b0a065c3db85e34e127fdf38b820b32712657900e3f8fe2703d4310c40632
checksum/config: d4ce97447743fcc1980c3e46b6ac2ee821a64817bee4671d6ce838813643dd24
checksum/ipsec-secret: d0eb9c52d0cd4311b6d252a951126bf9bea27ec05590bed8a394f0f792dcb2a4
labels:
app: antrea
Expand Down Expand Up @@ -5678,7 +5683,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 7d8b0a065c3db85e34e127fdf38b820b32712657900e3f8fe2703d4310c40632
checksum/config: d4ce97447743fcc1980c3e46b6ac2ee821a64817bee4671d6ce838813643dd24
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea.yml
View file
Open in desktop
antoninbas marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -4234,6 +4234,11 @@ data:
# 10.96.0.10:53, [fd00:10:96::a]:53).
dnsServerOverride: ""

# The minTTL setting helps address the problem of applications caching DNS response IPs beyond the TTL value for the DNS record.
# It is used to enforce FQDN policy rules, ensuring that resolved IPs are included in datapath rules for as long as the application is caching them.
# This value should ideally be set to the maximum caching duration across all applications.
minTTL: 0

# Comma-separated list of Cipher Suites. If omitted, the default Go Cipher Suites will be used.
# https://golang.org/pkg/crypto/tls/#pkg-constants
# Note that TLS1.3 Cipher Suites cannot be added to the list. But the apiserver will always
Expand Down Expand Up @@ -5383,7 +5388,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 2b4d82bcb825d50926115bad2125097f85aed424bfc49147444314cad8b7826a
checksum/config: d6ebdff39564e13df1eabb4bcf0894359a6b03bd8b8b528168a94b3e7a0ba319
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5619,7 +5624,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 2b4d82bcb825d50926115bad2125097f85aed424bfc49147444314cad8b7826a
checksum/config: d6ebdff39564e13df1eabb4bcf0894359a6b03bd8b8b528168a94b3e7a0ba319
labels:
app: antrea
component: antrea-controller
Expand Down
1 change: 1 addition & 0 deletions cmd/antrea-agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ func run(o *Options) error {
nodeConfig,
podNetworkWait,
l7Reconciler,
o.config.MinTTL,
)
if err != nil {
return fmt.Errorf("error creating new NetworkPolicy controller: %v", err)
Expand Down
3 changes: 3 additions & 0 deletions cmd/antrea-agent/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,9 @@ func (o *Options) validateK8sNodeOptions() error {
o.dnsServerOverride = hostPort
}

// Ensure that the minTTL is not negative.
o.config.MinTTL = max(o.config.MinTTL, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be better to return an error if o.config.MinTTL < 0 IMO

while this code is in validateK8sNodeOptions, I am not sure this configuration parameter is specific to the "K8sNode" case. Maybe the check should be in the parent function? cc @tnqn


if err := o.validateSecondaryNetworkConfig(); err != nil {
return fmt.Errorf("failed to validate secondary network config: %v", err)
}
Expand Down
8 changes: 5 additions & 3 deletions pkg/agent/controller/networkpolicy/fqdn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ type fqdnController struct {
ofClient openflow.Client
// dnsServerAddr stores the coreDNS server address, or the user provided DNS server address.
dnsServerAddr string
minTTL uint32

// dirtyRuleHandler is a callback that is run upon finding a rule out-of-sync.
dirtyRuleHandler func(string)
Expand Down Expand Up @@ -160,7 +161,7 @@ type fqdnController struct {
clock clock.Clock
}

func newFQDNController(client openflow.Client, allocator *idAllocator, dnsServerOverride string, dirtyRuleHandler func(string), v4Enabled, v6Enabled bool, gwPort uint32, clock clock.WithTicker) (*fqdnController, error) {
func newFQDNController(client openflow.Client, allocator *idAllocator, dnsServerOverride string, dirtyRuleHandler func(string), v4Enabled, v6Enabled bool, gwPort uint32, clock clock.WithTicker, minTTL int) (*fqdnController, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if minTTL should be uint32 here. This way we know it is a positive number. The conversion from int to uint32 can happen in cmd/antrea-agent.

controller := &fqdnController{
ofClient: client,
dirtyRuleHandler: dirtyRuleHandler,
Expand All @@ -182,6 +183,7 @@ func newFQDNController(client openflow.Client, allocator *idAllocator, dnsServer
ipv6Enabled: v6Enabled,
gwPort: gwPort,
clock: clock,
minTTL: uint32(minTTL),
}
if controller.ofClient != nil {
if err := controller.ofClient.NewDNSPacketInConjunction(dnsInterceptRuleID); err != nil {
Expand Down Expand Up @@ -643,15 +645,15 @@ func (f *fqdnController) parseDNSResponse(msg *dns.Msg) (string, map[string]ipWi
if f.ipv4Enabled {
responseIPs[r.A.String()] = ipWithExpiration{
ip: r.A,
expirationTime: currentTime.Add(time.Duration(r.Header().Ttl) * time.Second),
expirationTime: currentTime.Add(time.Duration(max(f.minTTL, r.Header().Ttl)) * time.Second),
}

}
case *dns.AAAA:
if f.ipv6Enabled {
responseIPs[r.AAAA.String()] = ipWithExpiration{
ip: r.AAAA,
expirationTime: currentTime.Add(time.Duration(r.Header().Ttl) * time.Second),
expirationTime: currentTime.Add(time.Duration(max(f.minTTL, r.Header().Ttl)) * time.Second),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions pkg/agent/controller/networkpolicy/fqdn_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func newMockFQDNController(t *testing.T, controller *gomock.Controller, dnsServe
false,
config.DefaultHostGatewayOFPort,
clockToInject,
0,
)
require.NoError(t, err)
return f, mockOFClient
Expand Down
4 changes: 2 additions & 2 deletions pkg/agent/controller/networkpolicy/networkpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func NewNetworkPolicyController(antreaClientGetter client.AntreaClientProvider,
gwPort, tunPort uint32,
nodeConfig *config.NodeConfig,
podNetworkWait *utilwait.Group,
l7Reconciler *l7engine.Reconciler) (*Controller, error) {
l7Reconciler *l7engine.Reconciler, minTTL int) (*Controller, error) {
idAllocator := newIDAllocator(asyncRuleDeleteInterval, dnsInterceptRuleID)
c := &Controller{
antreaClientProvider: antreaClientGetter,
Expand Down Expand Up @@ -227,7 +227,7 @@ func NewNetworkPolicyController(antreaClientGetter client.AntreaClientProvider,

var err error
if antreaPolicyEnabled {
if c.fqdnController, err = newFQDNController(ofClient, idAllocator, dnsServerOverride, c.enqueueRule, v4Enabled, v6Enabled, gwPort, clock.RealClock{}); err != nil {
if c.fqdnController, err = newFQDNController(ofClient, idAllocator, dnsServerOverride, c.enqueueRule, v4Enabled, v6Enabled, gwPort, clock.RealClock{}, minTTL); err != nil {
return nil, err
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/agent/controller/networkpolicy/networkpolicy_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ func newTestController() (*Controller, *fake.Clientset, *mockReconciler) {
config.DefaultTunOFPort,
&config.NodeConfig{},
wait.NewGroup(),
l7reconciler)
l7reconciler,
0)
reconciler := newMockReconciler()
controller.podReconciler = reconciler
controller.auditLogger = nil
Expand Down
4 changes: 4 additions & 0 deletions pkg/config/agent/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ type AgentConfig struct {
// Defaults to "". It must be a host string or a host:port pair of the DNS server (e.g. 10.96.0.10,
// 10.96.0.10:53, [fd00:10:96::a]:53).
DNSServerOverride string `yaml:"dnsServerOverride,omitempty"`
// The minTTL setting helps address the problem of applications caching DNS response IPs indefinitely.
// The Cluster administrators should configure this value, ideally setting it to be equal to or greater than the maximum TTL
// value of the application's DNS cache.
MinTTL int `yaml:"minTTL,omitempty"`
// Cipher suites to use.
TLSCipherSuites string `yaml:"tlsCipherSuites,omitempty"`
// TLS min version.
Expand Down
15 changes: 9 additions & 6 deletions test/e2e/antreapolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5270,8 +5270,12 @@ func testAntreaClusterNetworkPolicyStats(t *testing.T, data *TestData) {
k8sUtils.Cleanup(namespaces)
}

// TestFQDNCacheMinTTL tests stable FQDN access for applications with cached DNS resolutions
// when FQDN NetworkPolicy are in use and the FQDN-to-IP resolution changes frequently.
// TestFQDNCacheMinTTL ensures stable FQDN access for applications that cache DNS resolutions,
// even when FQDN-to-IP mappings change frequently, and FQDN-based NetworkPolicies are in use.
// It validates the functionality of the new minTTL configuration, which is used for scenarios
// where applications may cache DNS responses beyond the TTL defined in original DNS response.
// The minTTL value enforces that resolved IPs remain in datapath rules for as long as
// applications might cache them, thereby preventing intermittent network connectivity issues to the FQDN concerned.
func TestFQDNCacheMinTTL(t *testing.T) {
const (
testFQDN = "fqdn-test-pod.lfx.test"
Expand Down Expand Up @@ -5368,14 +5372,13 @@ func TestFQDNCacheMinTTL(t *testing.T) {
require.NoError(t, data.setPodAnnotation(data.testNamespace, "custom-dns-server", "test.antrea.io/random-value",
randSeq(8)), "failed to update custom DNS Pod annotation.")

// finally verify that Curling the previously cached IP fails after DNS update.
// finally verify that Curling the previously cached IP does not fail after DNS update.
// The wait time here should be slightly longer than the reload value specified in the custom DNS configuration.
// TODO: This assertion currently verifies the issue described in https://github.com/antrea-io/antrea/issues/6229.
// It will need to be updated once minTTL support is implemented.
// TODO: This assertion verifies the fix to the issue described in https://github.com/antrea-io/antrea/issues/6229.
t.Logf("Trying to curl the existing cached IP of the domain: %s", fqdnIP)
assert.EventuallyWithT(t, func(t *assert.CollectT) {
_, err := curlFQDN(fqdnIP)
assert.Error(t, err)
assert.NoError(t, err)
}, 10*time.Second, 1*time.Second)
}

Expand Down
Loading