diff --git a/assets/components/openshift-dns/dns/configmap.yaml b/assets/components/openshift-dns/dns/configmap.yaml index 2b4194e991..8e7cdc3ae0 100644 --- a/assets/components/openshift-dns/dns/configmap.yaml +++ b/assets/components/openshift-dns/dns/configmap.yaml @@ -1,6 +1,7 @@ apiVersion: v1 data: Corefile: | + {{- .C2CCDNSBlocks }} .:5353 { bufsize 1232 errors diff --git a/cmd/generate-config/config/config-openapi-spec.json b/cmd/generate-config/config/config-openapi-spec.json index 4d5d03a2db..c2d98d1471 100755 --- a/cmd/generate-config/config/config-openapi-spec.json +++ b/cmd/generate-config/config/config-openapi-spec.json @@ -181,7 +181,28 @@ }, "clusterToCluster": { "type": "object", + "required": [ + "dns" + ], "properties": { + "dns": { + "description": "DNS cache settings for CoreDNS server blocks generated for remote clusters.", + "type": "object", + "properties": { + "cacheNegativeTTL": { + "description": "Maximum TTL (seconds) for denial (NXDOMAIN/NODATA) DNS cache entries in CoreDNS\nserver blocks generated for remote clusters. Must be \u003e= 0. Setting to 0 disables denial caching.", + "type": "integer", + "default": 10, + "minimum": 0 + }, + "cacheTTL": { + "description": "Maximum TTL (seconds) for positive DNS cache entries in CoreDNS server blocks\ngenerated for remote clusters. Must be \u003e= 0. Setting to 0 disables positive caching.", + "type": "integer", + "default": 10, + "minimum": 0 + } + } + }, "remoteClusters": { "description": "List of remote clusters to establish connectivity with.\nC2CC is disabled when this list is empty.", "type": "array", diff --git a/docs/user/howto_config.md b/docs/user/howto_config.md index 72cd84df6e..636acf1ae7 100644 --- a/docs/user/howto_config.md +++ b/docs/user/howto_config.md @@ -31,6 +31,9 @@ apiServer: cipherSuites: [] minVersion: "" clusterToCluster: + dns: + cacheNegativeTTL: 0 + cacheTTL: 0 remoteClusters: - clusterNetwork: [] domain: "" @@ -190,6 +193,9 @@ apiServer: cipherSuites: [] minVersion: VersionTLS12 clusterToCluster: + dns: + cacheNegativeTTL: 10 + cacheTTL: 10 remoteClusters: - clusterNetwork: [] domain: "" diff --git a/packaging/microshift/config.yaml b/packaging/microshift/config.yaml index 7d54331024..69ab22e68c 100644 --- a/packaging/microshift/config.yaml +++ b/packaging/microshift/config.yaml @@ -42,6 +42,14 @@ apiServer: # Defaults to VersionTLS12. minVersion: VersionTLS12 clusterToCluster: + # DNS cache settings for CoreDNS server blocks generated for remote clusters. + dns: + # Maximum TTL (seconds) for denial (NXDOMAIN/NODATA) DNS cache entries in CoreDNS + # server blocks generated for remote clusters. Must be >= 0. Setting to 0 disables denial caching. + cacheNegativeTTL: 10 + # Maximum TTL (seconds) for positive DNS cache entries in CoreDNS server blocks + # generated for remote clusters. Must be >= 0. Setting to 0 disables positive caching. + cacheTTL: 10 # List of remote clusters to establish connectivity with. # C2CC is disabled when this list is empty. remoteClusters: diff --git a/pkg/components/controllers.go b/pkg/components/controllers.go index ded367dfca..2f995fc060 100644 --- a/pkg/components/controllers.go +++ b/pkg/components/controllers.go @@ -304,8 +304,12 @@ func startDNSController(ctx context.Context, cfg *config.Config, kubeconfigPath hostsEnabled := cfg.DNS.Hosts.Status == config.HostsStatusEnabled extraParams := assets.RenderParams{ - "ClusterIP": cfg.Network.DNS, - "HostsEnabled": hostsEnabled, + "ClusterIP": cfg.Network.DNS, + "HostsEnabled": hostsEnabled, + "C2CCDNSBlocks": "", + } + if cfg.C2CC.IsEnabled() { + extraParams["C2CCDNSBlocks"] = config.RenderC2CCDNSBlocks(cfg.C2CC.Resolved, *cfg.C2CC.DNS.CacheTTL, *cfg.C2CC.DNS.CacheNegativeTTL) } if err := assets.ApplyServices(ctx, svc, renderTemplate, renderParamsFromConfig(cfg, extraParams), kubeconfigPath); err != nil { diff --git a/pkg/config/c2cc.go b/pkg/config/c2cc.go index ff77458eb0..d82eb5aa59 100644 --- a/pkg/config/c2cc.go +++ b/pkg/config/c2cc.go @@ -11,7 +11,22 @@ import ( netutils "k8s.io/utils/net" ) +type C2CCDNS struct { + // Maximum TTL (seconds) for positive DNS cache entries in CoreDNS server blocks + // generated for remote clusters. Must be >= 0. Setting to 0 disables positive caching. + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:default=10 + CacheTTL *int `json:"cacheTTL,omitempty"` + // Maximum TTL (seconds) for denial (NXDOMAIN/NODATA) DNS cache entries in CoreDNS + // server blocks generated for remote clusters. Must be >= 0. Setting to 0 disables denial caching. + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:default=10 + CacheNegativeTTL *int `json:"cacheNegativeTTL,omitempty"` +} + type C2CC struct { + // DNS cache settings for CoreDNS server blocks generated for remote clusters. + DNS C2CCDNS `json:"dns"` // List of remote clusters to establish connectivity with. // C2CC is disabled when this list is empty. RemoteClusters []RemoteCluster `json:"remoteClusters,omitempty"` @@ -39,6 +54,7 @@ type ResolvedRemoteCluster struct { ClusterNetwork []*net.IPNet ServiceNetwork []*net.IPNet Domain string + DNSIP string // 10th IP of ServiceNetwork[0], computed during validation when Domain is set } func (rc *ResolvedRemoteCluster) AllCIDRs() []*net.IPNet { @@ -164,6 +180,13 @@ func (c *C2CC) validate(cfg *Config) error { return fmt.Errorf("cluster to cluster requires OVN-Kubernetes CNI (network.cniPlugin must be \"\" or \"ovnk\", got %q)", cfg.Network.CNIPlugin) } + if c.DNS.CacheTTL != nil && *c.DNS.CacheTTL < 0 { + return fmt.Errorf("dns.cacheTTL must be >= 0, got %d", *c.DNS.CacheTTL) + } + if c.DNS.CacheNegativeTTL != nil && *c.DNS.CacheNegativeTTL < 0 { + return fmt.Errorf("dns.cacheNegativeTTL must be >= 0, got %d", *c.DNS.CacheNegativeTTL) + } + resolved, parseErrs := c.parseRemoteClusters() if len(parseErrs) > 0 { return errors.Join(parseErrs...) @@ -259,6 +282,9 @@ func validateRemoteCluster( if domainErrs := validation.IsDNS1123Subdomain(rc.Domain); len(domainErrs) > 0 { errs = append(errs, fmt.Errorf("%s.domain %q is not a valid DNS name: %s", label, rc.Domain, strings.Join(domainErrs, ", "))) } + if rc.Domain == "cluster.local" { + errs = append(errs, fmt.Errorf("%s.domain cannot be cluster.local", label)) + } if prev, ok := seenRemoteDomains[rc.Domain]; ok { errs = append(errs, fmt.Errorf("%s.domain %q duplicates remoteClusters[%d]", label, rc.Domain, prev)) } else { @@ -266,6 +292,15 @@ func validateRemoteCluster( } } + if rc.Domain != "" && len(rc.ServiceNetwork) > 0 { + dnsIP, err := getClusterDNS(rc.ServiceNetwork[0]) + if err != nil { + errs = append(errs, fmt.Errorf("%s: failed to compute DNS IP from serviceNetwork[0] %q: %w", label, rc.ServiceNetwork[0], err)) + } else { + res.DNSIP = dnsIP + } + } + errs = append(errs, validateIPFamilyConsistencyNets(res.ClusterNetwork, label+".clusterNetwork")...) errs = append(errs, validateIPFamilyConsistencyNets(res.ServiceNetwork, label+".serviceNetwork")...) errs = append(errs, validateNetworkShapeNets(res.ClusterNetwork, res.ServiceNetwork, label)...) @@ -345,3 +380,33 @@ func checkCIDRConflicts(cidr *net.IPNet, cidrStr, label string, seenCIDRs []labe func cidrsOverlap(a, b *net.IPNet) bool { return a.Contains(b.IP) || b.Contains(a.IP) } + +// RenderC2CCDNSBlocks generates CoreDNS server blocks for cross-cluster DNS. +func RenderC2CCDNSBlocks(resolved []ResolvedRemoteCluster, cacheTTL, cacheNegativeTTL int) string { + var blocks []string + for _, rc := range resolved { + if rc.Domain == "" { + continue + } + blocks = append(blocks, formatDNSBlock(rc.Domain, rc.DNSIP, cacheTTL, cacheNegativeTTL)) + } + if len(blocks) == 0 { + return "" + } + return "\n" + strings.Join(blocks, "\n") +} + +func formatDNSBlock(domain, dnsIP string, cacheTTL, cacheNegativeTTL int) string { + return fmt.Sprintf(` %s:5353 { + bufsize 1232 + errors + log . { + class error + } + rewrite stop name suffix .%s .cluster.local answer auto + forward . %s + cache %d { + denial 9984 %d + } + }`, domain, domain, dnsIP, cacheTTL, cacheNegativeTTL) +} diff --git a/pkg/config/c2cc_test.go b/pkg/config/c2cc_test.go index b215027e96..50a28984cf 100644 --- a/pkg/config/c2cc_test.go +++ b/pkg/config/c2cc_test.go @@ -2,9 +2,12 @@ package config import ( "net" + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" ) func TestC2CC_IsEnabled(t *testing.T) { @@ -55,6 +58,16 @@ func TestC2CC_StripEmptyRemoteClusters(t *testing.T) { }) } +func withDNSDefaults(c2cc C2CC) C2CC { + if c2cc.DNS.CacheTTL == nil { + c2cc.DNS.CacheTTL = ptr.To(10) + } + if c2cc.DNS.CacheNegativeTTL == nil { + c2cc.DNS.CacheNegativeTTL = ptr.To(10) + } + return c2cc +} + func mkC2CCConfig(c2cc C2CC) *Config { return &Config{ Network: Network{ @@ -65,7 +78,7 @@ func mkC2CCConfig(c2cc C2CC) *Config { Node: Node{ NodeIP: "10.100.0.1", }, - C2CC: c2cc, + C2CC: withDNSDefaults(c2cc), } } @@ -80,7 +93,7 @@ func mkDualStackC2CCConfig(c2cc C2CC) *Config { NodeIP: "10.100.0.1", NodeIPV6: "fd00::1", }, - C2CC: c2cc, + C2CC: withDNSDefaults(c2cc), } } @@ -94,7 +107,7 @@ func mkIPv6OnlyC2CCConfig(c2cc C2CC) *Config { Node: Node{ NodeIP: "fd00::1", }, - C2CC: c2cc, + C2CC: withDNSDefaults(c2cc), } } @@ -486,6 +499,43 @@ func TestC2CC_Validate(t *testing.T) { expectErr: true, errMsg: "mismatched IP families", }, + { + name: "negative cacheTTL", + cfg: mkC2CCConfig(C2CC{ + DNS: C2CCDNS{CacheTTL: ptr.To(-1), CacheNegativeTTL: ptr.To(10)}, + RemoteClusters: []RemoteCluster{{ + NextHop: "10.100.0.2", + ClusterNetwork: []string{"10.45.0.0/16"}, + ServiceNetwork: []string{"10.46.0.0/16"}, + }}, + }), + expectErr: true, + errMsg: "dns.cacheTTL must be >= 0", + }, + { + name: "negative cacheNegativeTTL", + cfg: mkC2CCConfig(C2CC{ + DNS: C2CCDNS{CacheTTL: ptr.To(10), CacheNegativeTTL: ptr.To(-5)}, + RemoteClusters: []RemoteCluster{{ + NextHop: "10.100.0.2", + ClusterNetwork: []string{"10.45.0.0/16"}, + ServiceNetwork: []string{"10.46.0.0/16"}, + }}, + }), + expectErr: true, + errMsg: "dns.cacheNegativeTTL must be >= 0", + }, + { + name: "zero cacheTTL is valid", + cfg: mkC2CCConfig(C2CC{ + DNS: C2CCDNS{CacheTTL: ptr.To(0), CacheNegativeTTL: ptr.To(0)}, + RemoteClusters: []RemoteCluster{{ + NextHop: "10.100.0.2", + ClusterNetwork: []string{"10.45.0.0/16"}, + ServiceNetwork: []string{"10.46.0.0/16"}, + }}, + }), + }, } for _, tt := range ttests { @@ -541,3 +591,134 @@ func TestC2CC_ValidateDualStack(t *testing.T) { assert.NoError(t, cfg.C2CC.validate(cfg)) }) } + +func TestC2CC_DNSIP(t *testing.T) { + stubHostIPs(t, nil) + + t.Run("DNSIP populated when domain is set", func(t *testing.T) { + cfg := mkC2CCConfig(C2CC{ + RemoteClusters: []RemoteCluster{{ + NextHop: "10.100.0.2", + ClusterNetwork: []string{"10.45.0.0/16"}, + ServiceNetwork: []string{"10.46.0.0/16"}, + Domain: "cluster-b.remote", + }}, + }) + require.NoError(t, cfg.C2CC.validate(cfg)) + assert.Equal(t, "10.46.0.10", cfg.C2CC.Resolved[0].DNSIP) + }) + + t.Run("DNSIP empty when domain is not set", func(t *testing.T) { + cfg := mkC2CCConfig(C2CC{ + RemoteClusters: []RemoteCluster{{ + NextHop: "10.100.0.2", + ClusterNetwork: []string{"10.45.0.0/16"}, + ServiceNetwork: []string{"10.46.0.0/16"}, + }}, + }) + require.NoError(t, cfg.C2CC.validate(cfg)) + assert.Empty(t, cfg.C2CC.Resolved[0].DNSIP) + }) + + t.Run("DNSIP for IPv6 service network", func(t *testing.T) { + cfg := mkIPv6OnlyC2CCConfig(C2CC{ + RemoteClusters: []RemoteCluster{{ + NextHop: "fd00::2", + ClusterNetwork: []string{"fd03::/48"}, + ServiceNetwork: []string{"fd04::/112"}, + Domain: "cluster-b.remote", + }}, + }) + require.NoError(t, cfg.C2CC.validate(cfg)) + assert.Equal(t, "fd04::a", cfg.C2CC.Resolved[0].DNSIP) + }) +} + +func parseCIDR(t *testing.T, s string) *net.IPNet { + t.Helper() + _, ipNet, err := net.ParseCIDR(s) + require.NoError(t, err) + return ipNet +} + +func TestRenderC2CCDNSBlocks(t *testing.T) { + t.Run("no domains configured", func(t *testing.T) { + resolved := []ResolvedRemoteCluster{{ + NextHop: net.ParseIP("10.100.0.2"), + ClusterNetwork: []*net.IPNet{parseCIDR(t, "10.45.0.0/16")}, + ServiceNetwork: []*net.IPNet{parseCIDR(t, "10.46.0.0/16")}, + }} + result := RenderC2CCDNSBlocks(resolved, 10, 10) + assert.Empty(t, result) + }) + + t.Run("single domain with default TTLs", func(t *testing.T) { + resolved := []ResolvedRemoteCluster{{ + NextHop: net.ParseIP("10.100.0.2"), + ClusterNetwork: []*net.IPNet{parseCIDR(t, "10.45.0.0/16")}, + ServiceNetwork: []*net.IPNet{parseCIDR(t, "10.46.0.0/16")}, + Domain: "cluster-b.remote", + DNSIP: "10.46.0.10", + }} + result := RenderC2CCDNSBlocks(resolved, 10, 10) + assert.True(t, strings.HasPrefix(result, "\n"), "result should start with newline for YAML block scalar") + assert.Contains(t, result, "cluster-b.remote:5353") + assert.Contains(t, result, "rewrite stop name suffix .cluster-b.remote .cluster.local answer auto") + assert.Contains(t, result, "forward . 10.46.0.10") + assert.Contains(t, result, "cache 10 {") + assert.Contains(t, result, "denial 9984 10") + }) + + t.Run("custom TTLs", func(t *testing.T) { + resolved := []ResolvedRemoteCluster{{ + Domain: "cluster-b.remote", + DNSIP: "10.46.0.10", + }} + result := RenderC2CCDNSBlocks(resolved, 30, 60) + assert.Contains(t, result, "cache 30 {") + assert.Contains(t, result, "denial 9984 60") + }) + + t.Run("zero TTLs", func(t *testing.T) { + resolved := []ResolvedRemoteCluster{{ + Domain: "cluster-b.remote", + DNSIP: "10.46.0.10", + }} + result := RenderC2CCDNSBlocks(resolved, 0, 0) + assert.Contains(t, result, "cache 0 {") + assert.Contains(t, result, "denial 9984 0") + }) + + t.Run("multiple domains", func(t *testing.T) { + resolved := []ResolvedRemoteCluster{ + { + Domain: "cluster-b.remote", + DNSIP: "10.46.0.10", + }, + { + Domain: "cluster-c.remote", + DNSIP: "10.56.0.10", + }, + } + result := RenderC2CCDNSBlocks(resolved, 10, 10) + assert.Contains(t, result, "cluster-b.remote:5353") + assert.Contains(t, result, "forward . 10.46.0.10") + assert.Contains(t, result, "cluster-c.remote:5353") + assert.Contains(t, result, "forward . 10.56.0.10") + }) + + t.Run("mixed domain and no-domain", func(t *testing.T) { + resolved := []ResolvedRemoteCluster{ + { + Domain: "cluster-b.remote", + DNSIP: "10.46.0.10", + }, + { + Domain: "", + }, + } + result := RenderC2CCDNSBlocks(resolved, 10, 10) + assert.Contains(t, result, "cluster-b.remote:5353") + assert.NotContains(t, result, "cluster-c") + }) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 192ee26d73..bac66b96b7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -199,7 +199,9 @@ func (c *Config) fillDefaults() error { c.GenericDevicePlugin = genericDevicePluginDefaults() c.Telemetry = telemetryDefaults() c.DNS = dnsDefaults() - c.C2CC = C2CC{} + c.C2CC = C2CC{ + DNS: C2CCDNS{CacheTTL: ptr.To(10), CacheNegativeTTL: ptr.To(10)}, + } return nil } @@ -459,7 +461,13 @@ func (c *Config) incorporateUserSettings(u *Config) { } if u.C2CC.RemoteClusters != nil { - c.C2CC = u.C2CC + c.C2CC.RemoteClusters = u.C2CC.RemoteClusters + } + if u.C2CC.DNS.CacheTTL != nil { + c.C2CC.DNS.CacheTTL = u.C2CC.DNS.CacheTTL + } + if u.C2CC.DNS.CacheNegativeTTL != nil { + c.C2CC.DNS.CacheNegativeTTL = u.C2CC.DNS.CacheNegativeTTL } } diff --git a/pkg/controllers/c2cc/controller.go b/pkg/controllers/c2cc/controller.go index ec352ba334..8091817cec 100644 --- a/pkg/controllers/c2cc/controller.go +++ b/pkg/controllers/c2cc/controller.go @@ -13,7 +13,7 @@ import ( ) const ( - reconcileInterval = 10 * time.Second + reconcileInterval = 2 * time.Second ) type C2CCRouteManager struct { @@ -72,7 +72,7 @@ func (c *C2CCRouteManager) Run(ctx context.Context, ready chan<- struct{}, stopp return fmt.Errorf("failed to init subsystems: %w", err) } - reconcileCh := make(chan string, 10) + reconcileCh := make(chan string, 50) c.ovn.subscribe(ctx, reconcileCh) diff --git a/pkg/controllers/c2cc/helpers_test.go b/pkg/controllers/c2cc/helpers_test.go index 605c6251e0..474840488c 100644 --- a/pkg/controllers/c2cc/helpers_test.go +++ b/pkg/controllers/c2cc/helpers_test.go @@ -12,6 +12,7 @@ type testRemoteConfig struct { nextHop string clusterNetwork []string serviceNetwork []string + domain string } func testRemote(nextHop string, clusterNetwork, serviceNetwork []string) testRemoteConfig { @@ -22,6 +23,15 @@ func testRemote(nextHop string, clusterNetwork, serviceNetwork []string) testRem } } +func testRemoteWithDomain(nextHop string, clusterNetwork, serviceNetwork []string, domain string) testRemoteConfig { + return testRemoteConfig{ + nextHop: nextHop, + clusterNetwork: clusterNetwork, + serviceNetwork: serviceNetwork, + domain: domain, + } +} + func testConfigWithRemotes(t *testing.T, remotes ...testRemoteConfig) *config.Config { t.Helper() @@ -32,6 +42,7 @@ func testConfigWithRemotes(t *testing.T, remotes ...testRemoteConfig) *config.Co for _, r := range remotes { resolved := config.ResolvedRemoteCluster{ NextHop: net.ParseIP(r.nextHop), + Domain: r.domain, } require.NotNil(t, resolved.NextHop, "invalid nextHop: %s", r.nextHop) diff --git a/pkg/controllers/c2cc/policy_routes.go b/pkg/controllers/c2cc/policy_routes.go index 11507dea1b..0181bd8868 100644 --- a/pkg/controllers/c2cc/policy_routes.go +++ b/pkg/controllers/c2cc/policy_routes.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "net" + "syscall" "github.com/vishvananda/netlink" "k8s.io/klog/v2" @@ -110,9 +111,13 @@ func (t *policyRouteTable) subscribe(reconcileCh chan<- string, reason string) ( if update.Table != t.table { continue } + if update.Type == syscall.RTM_DELROUTE { + klog.V(2).Infof("Detected external deletion of route in table %d: %v", t.table, update.Route) + } select { case reconcileCh <- reason: default: + klog.V(4).Infof("Reconcile channel full, dropping %s event for table %d", reason, t.table) } } }() diff --git a/test/assets/c2cc/hello-microshift.yaml b/test/assets/c2cc/hello-microshift.yaml index 6169449b78..34ffcb717f 100644 --- a/test/assets/c2cc/hello-microshift.yaml +++ b/test/assets/c2cc/hello-microshift.yaml @@ -10,10 +10,27 @@ spec: - name: hello-microshift image: quay.io/microshift/busybox:1.36 command: ["/bin/sh"] - args: ["-c", "while true; do echo -ne \"HTTP/1.0 200 OK\r\nContent-Length: 16\r\n\r\nHello MicroShift\" | nc -l -p 8080 ; done"] + args: + - -c + - | + mkdir -p /tmp/www/cgi-bin + cat > /tmp/www/cgi-bin/hello <