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 1 commit
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
4 changes: 2 additions & 2 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,10 +306,10 @@ 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.
# The fqdnCacheMinTTL 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 }}
fqdnCacheMinTTL: {{ .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.

Suggested change
fqdnCacheMinTTL: {{ .Values.minTTL }}
fqdnCacheMinTTL: {{ .Values.fqdnCacheMinTTL }}

This is why the manifests are not generated correctly (fqdnCacheMinTTL: instead of fqdnCacheMinTTL: 0)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry , my bad. Will correct that.


# Comma-separated list of Cipher Suites. If omitted, the default Go Cipher Suites will be used.
# https://golang.org/pkg/crypto/tls/#pkg-constants
Expand Down
2 changes: 1 addition & 1 deletion build/charts/antrea/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ 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
fqdnCacheMinTTL: 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
8 changes: 4 additions & 4 deletions build/yamls/antrea-aks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4234,10 +4234,10 @@ 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.
# The fqdnCacheMinTTL 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
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
Expand Down Expand Up @@ -5388,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: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
checksum/config: 8b260e981a71f970ab28471bcf056893615089492f917f16ee3b8d749ed6d348
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5626,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: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
checksum/config: 8b260e981a71f970ab28471bcf056893615089492f917f16ee3b8d749ed6d348
labels:
app: antrea
component: antrea-controller
Expand Down
8 changes: 4 additions & 4 deletions build/yamls/antrea-eks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4234,10 +4234,10 @@ 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.
# The fqdnCacheMinTTL 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
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
Expand Down Expand Up @@ -5388,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: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
checksum/config: 8b260e981a71f970ab28471bcf056893615089492f917f16ee3b8d749ed6d348
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5627,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: c235e6afdb719cee68a98bec85a0b40682b2d85b51bf4196d1556f1478ce2633
checksum/config: 8b260e981a71f970ab28471bcf056893615089492f917f16ee3b8d749ed6d348
labels:
app: antrea
component: antrea-controller
Expand Down
8 changes: 4 additions & 4 deletions build/yamls/antrea-gke.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4234,10 +4234,10 @@ 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.
# The fqdnCacheMinTTL 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
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
Expand Down Expand Up @@ -5388,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: 6c71e376d2b2ea5a377700084d271a88925337887b10a4432d68b7d269911d90
checksum/config: 96a86cbe034da4285e15a136b3c05b954b12d148eb54aeaf1a3ad543fb2588c2
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5624,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: 6c71e376d2b2ea5a377700084d271a88925337887b10a4432d68b7d269911d90
checksum/config: 96a86cbe034da4285e15a136b3c05b954b12d148eb54aeaf1a3ad543fb2588c2
labels:
app: antrea
component: antrea-controller
Expand Down
8 changes: 4 additions & 4 deletions build/yamls/antrea-ipsec.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4247,10 +4247,10 @@ 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.
# The fqdnCacheMinTTL 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
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
Expand Down Expand Up @@ -5401,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: d4ce97447743fcc1980c3e46b6ac2ee821a64817bee4671d6ce838813643dd24
checksum/config: 5deeee1fbf11902f265061f60855c1720e19fb0521692c7d22e130f880947c78
checksum/ipsec-secret: d0eb9c52d0cd4311b6d252a951126bf9bea27ec05590bed8a394f0f792dcb2a4
labels:
app: antrea
Expand Down Expand Up @@ -5683,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: d4ce97447743fcc1980c3e46b6ac2ee821a64817bee4671d6ce838813643dd24
checksum/config: 5deeee1fbf11902f265061f60855c1720e19fb0521692c7d22e130f880947c78
labels:
app: antrea
component: antrea-controller
Expand Down
8 changes: 4 additions & 4 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,10 +4234,10 @@ 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.
# The fqdnCacheMinTTL 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
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
Expand Down Expand Up @@ -5388,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: d6ebdff39564e13df1eabb4bcf0894359a6b03bd8b8b528168a94b3e7a0ba319
checksum/config: dbebe7ad81b43b8a9e102971e323ac5ab89137efac9d4f5140c256f454ec5d66
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -5624,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: d6ebdff39564e13df1eabb4bcf0894359a6b03bd8b8b528168a94b3e7a0ba319
checksum/config: dbebe7ad81b43b8a9e102971e323ac5ab89137efac9d4f5140c256f454ec5d66
labels:
app: antrea
component: antrea-controller
Expand Down
2 changes: 1 addition & 1 deletion cmd/antrea-agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ func run(o *Options) error {
nodeConfig,
podNetworkWait,
l7Reconciler,
o.config.MinTTL,
uint32(o.config.FqdnCacheMinTTL),
)
if err != nil {
return fmt.Errorf("error creating new NetworkPolicy controller: %v", err)
Expand Down
15 changes: 9 additions & 6 deletions cmd/antrea-agent/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,19 @@ func (o *Options) validate(args []string) error {
return fmt.Errorf("nodeType %s requires feature gate ExternalNode to be enabled", o.config.NodeType)
}

if o.config.NodeType == config.ExternalNode.String() {
// validate FqdnCacheMinTTL
if o.config.FqdnCacheMinTTL < 0 {
return fmt.Errorf("fqdnCacheMinTTL set to an invalid value, its must be a positive integer")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("fqdnCacheMinTTL set to an invalid value, its must be a positive integer")
return fmt.Errorf("fqdnCacheMinTTL must be greater than or equal to 0")

}

switch o.config.NodeType {
case config.ExternalNode.String():
o.nodeType = config.ExternalNode
return o.validateExternalNodeOptions()
} else if o.config.NodeType == config.K8sNode.String() {
case config.K8sNode.String():
o.nodeType = config.K8sNode
return o.validateK8sNodeOptions()
} else {
default:
return fmt.Errorf("unsupported nodeType %s", o.config.NodeType)
Comment on lines +163 to 171
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not a bad change, but I would avoid doing it in this PR as it is unrelated

}
}
Expand Down Expand Up @@ -605,9 +611,6 @@ func (o *Options) validateK8sNodeOptions() error {
o.dnsServerOverride = hostPort
}

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

if err := o.validateSecondaryNetworkConfig(); err != nil {
return fmt.Errorf("failed to validate secondary network config: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/agent/controller/networkpolicy/fqdn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,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, minTTL int) (*fqdnController, error) {
func newFQDNController(client openflow.Client, allocator *idAllocator, dnsServerOverride string, dirtyRuleHandler func(string), v4Enabled, v6Enabled bool, gwPort uint32, clock clock.WithTicker, fqdnCacheMinTTL uint32) (*fqdnController, error) {
controller := &fqdnController{
ofClient: client,
dirtyRuleHandler: dirtyRuleHandler,
Expand All @@ -183,7 +183,7 @@ func newFQDNController(client openflow.Client, allocator *idAllocator, dnsServer
ipv6Enabled: v6Enabled,
gwPort: gwPort,
clock: clock,
minTTL: uint32(minTTL),
minTTL: fqdnCacheMinTTL,
}
if controller.ofClient != nil {
if err := controller.ofClient.NewDNSPacketInConjunction(dnsInterceptRuleID); err != nil {
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, minTTL int) (*Controller, error) {
l7Reconciler *l7engine.Reconciler, fqdnCacheMinTTL uint32) (*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{}, minTTL); err != nil {
if c.fqdnController, err = newFQDNController(ofClient, idAllocator, dnsServerOverride, c.enqueueRule, v4Enabled, v6Enabled, gwPort, clock.RealClock{}, fqdnCacheMinTTL); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/config/agent/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ type AgentConfig struct {
// 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"`
FqdnCacheMinTTL int `yaml:"fqdnCacheMinTTL,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the field name here should be FQDNCacheMinTTL per our conventions

// Cipher suites to use.
TLSCipherSuites string `yaml:"tlsCipherSuites,omitempty"`
// TLS min version.
Expand Down
Loading