From 7ca816551e497c8785c41ecb35a35c6d8f5fae13 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 11:30:38 +0200 Subject: [PATCH 01/71] First stab at changing dnscach datastructure and cache update handling --- Makefile | 2 +- api/v1/clusterwidenetworkpolicy_types.go | 17 ++- api/v1/zz_generated.deepcopy.go | 8 +- ...l-stack.io_clusterwidenetworkpolicies.yaml | 8 +- pkg/dns/dnscache.go | 121 ++++++++---------- 5 files changed, 77 insertions(+), 79 deletions(-) diff --git a/Makefile b/Makefile index 958d01c8..fd01579f 100644 --- a/Makefile +++ b/Makefile @@ -66,8 +66,8 @@ vet: # Generate code generate: controller-gen mockgen manifests - go generate ./... $(CONTROLLER_GEN) object paths="./..." + go generate ./... .PHONY: controller-gen controller-gen: $(CONTROLLER_GEN) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index 8864db65..bc7c381b 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -153,13 +153,20 @@ type FQDNSelector struct { // IPSet stores set name association to IP addresses type IPSet struct { - FQDN string `json:"fqdn,omitempty"` - SetName string `json:"setName,omitempty"` - IPs []string `json:"ips,omitempty"` - ExpirationTime metav1.Time `json:"expirationTime,omitempty"` - Version IPVersion `json:"version,omitempty"` + FQDN string `json:"fqdn,omitempty"` + SetName string `json:"setName,omitempty"` + IPs map[string]metav1.Time `json:"ips,omitempty"` + Version IPVersion `json:"version,omitempty"` } +// type IPSet struct { +// FQDN string `json:"fqdn,omitempty"` +// SetName string `json:"setName,omitempty"` +// IPs []string `json:"ips,omitempty"` +// ExpirationTime metav1.Time `json:"expirationTime,omitempty"` +// Version IPVersion `json:"version,omitempty"` +// } + func (l *ClusterwideNetworkPolicyList) GetFQDNs() []FQDNSelector { s := []FQDNSelector{} for _, i := range l.Items { diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index c9863969..86663767 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -6,6 +6,7 @@ package v1 import ( networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -154,10 +155,11 @@ func (in *IPSet) DeepCopyInto(out *IPSet) { *out = *in if in.IPs != nil { in, out := &in.IPs, &out.IPs - *out = make([]string, len(*in)) - copy(*out, *in) + *out = make(map[string]metav1.Time, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } } - in.ExpirationTime.DeepCopyInto(&out.ExpirationTime) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPSet. diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index ea74df44..c03ce628 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -245,15 +245,13 @@ spec: items: description: IPSet stores set name association to IP addresses properties: - expirationTime: - format: date-time - type: string fqdn: type: string ips: - items: + additionalProperties: + format: date-time type: string - type: array + type: object setName: type: string version: diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index aa5be334..18f35dcc 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -4,10 +4,7 @@ import ( "crypto/md5" //nolint:gosec "encoding/hex" "fmt" - "math" - "net" "regexp" - "sort" "strings" "sync" "time" @@ -42,31 +39,22 @@ type RenderIPSet struct { } type ipEntry struct { - ips []string - expirationTime time.Time - setName string + // ips is a map of the ip address and its expiration time which is the time of the DNS lookup + the TTL + ips map[string]time.Time + setName string } -func newIPEntry(setName string, expirationTime time.Time) *ipEntry { +func newIPEntry(setName string) *ipEntry { return &ipEntry{ - expirationTime: expirationTime, - setName: setName, + setName: setName, } } -func (e *ipEntry) update(setName string, ips []net.IP, expirationTime time.Time, dtype nftables.SetDatatype) error { - newIPs, deletedIPs := e.getNewAndDeletedIPs(ips) - if !e.expirationTime.After(time.Now()) { - e.expirationTime = expirationTime - } +func (e *ipEntry) update(setName string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { + deletedIPs := e.expireIPs() + newIPs := e.addAndUpdateIPs(rrs, lookupTime) if newIPs != nil || deletedIPs != nil { - e.ips = make([]string, len(ips)) - for i, ip := range ips { - e.ips[i] = ip.String() - } - sort.Strings(e.ips) - if err := updateNftSet(newIPs, deletedIPs, setName, dtype); err != nil { return fmt.Errorf("failed to update nft set: %w", err) } @@ -75,27 +63,31 @@ func (e *ipEntry) update(setName string, ips []net.IP, expirationTime time.Time, return nil } -func (e *ipEntry) getNewAndDeletedIPs(ips []net.IP) (newIPs, deletedIPs []nftables.SetElement) { - currentIps := make(map[string]bool, len(e.ips)) - for _, ip := range e.ips { - currentIps[ip] = false - } - - for _, ip := range ips { - s := ip.String() - if _, ok := currentIps[s]; ok { - currentIps[s] = true - } else { - newIPs = append(newIPs, nftables.SetElement{Key: ip}) +func (e *ipEntry) expireIPs() (deletedIPs []nftables.SetElement) { + for ip, expirationTime := range e.ips { + if expirationTime.Before(time.Now()) { + deletedIPs = append(deletedIPs, nftables.SetElement{Key: []byte(ip)}) + delete(e.ips, ip) } } + return +} - for ip, exists := range currentIps { - if !exists { - deletedIPs = append(deletedIPs, nftables.SetElement{Key: net.ParseIP(ip)}) +func (e *ipEntry) addAndUpdateIPs(rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { + for _, rr := range rrs { + var s string + switch rr.(type) { + case *dnsgo.A: + s = rr.String() + case *dnsgo.AAAA: + s = rr.String() } - } + if _, ok := e.ips[s]; ok { + newIPs = append(newIPs, nftables.SetElement{Key: []byte(s)}) + } + e.ips[s] = lookupTime.Add(time.Duration(rr.Header().Ttl)) + } return } @@ -197,9 +189,10 @@ func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { } ipe := &ipEntry{ - ips: s.IPs, - expirationTime: s.ExpirationTime.Time, - setName: s.SetName, + setName: s.SetName, + } + for ip, expirationTime := range s.IPs { + ipe.ips[ip] = expirationTime.Time } switch s.Version { case firewallv1.IPv4: @@ -311,10 +304,8 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq return true, fmt.Errorf("too many hops, fqdn chain: %s", strings.Join(fqdns, ",")) } - ipv4 := []net.IP{} - ipv6 := []net.IP{} - minIPv4TTL := uint32(math.MaxUint32) - minIPv6TTL := uint32(math.MaxUint32) + ipv4 := []dnsgo.RR{} + ipv6 := []dnsgo.RR{} found := false for _, ans := range msg.Answer { @@ -326,17 +317,11 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq switch rr := ans.(type) { case *dnsgo.A: - ipv4 = append(ipv4, rr.A) - if minIPv4TTL > rr.Hdr.Ttl { - minIPv4TTL = rr.Hdr.Ttl - } + ipv4 = append(ipv4, rr) found = true c.log.V(4).Info("DEBUG dnscache Update function A record found", "IPs", ipv4) case *dnsgo.AAAA: - ipv6 = append(ipv6, rr.AAAA) - if minIPv6TTL > rr.Hdr.Ttl { - minIPv6TTL = rr.Hdr.Ttl - } + ipv6 = append(ipv6, rr) found = true c.log.V(4).Info("DEBUG dnscache Update function AAAA record found", "IPs", ipv6) case *dnsgo.CNAME: @@ -362,12 +347,12 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq for _, fqdn := range fqdns { c.log.V(4).Info("DEBUG dnscache Update function Updating DNS cache for", "fqdn", fqdn, "ipv4", ipv4, "ipv6", ipv6) if c.ipv4Enabled && len(ipv4) > 0 { - if err := c.updateIPEntry(fqdn, ipv4, lookupTime.Add(time.Duration(minIPv4TTL)), nftables.TypeIPAddr); err != nil { + if err := c.updateIPEntry(fqdn, ipv4, lookupTime, nftables.TypeIPAddr); err != nil { return false, fmt.Errorf("failed to update IPv4 addresses: %w", err) } } if c.ipv6Enabled && len(ipv6) > 0 { - if err := c.updateIPEntry(fqdn, ipv6, lookupTime.Add(time.Duration(minIPv6TTL)), nftables.TypeIP6Addr); err != nil { + if err := c.updateIPEntry(fqdn, ipv6, lookupTime, nftables.TypeIP6Addr); err != nil { return false, fmt.Errorf("failed to update IPv6 addresses: %w", err) } } @@ -376,10 +361,10 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq return found, nil } -func (c *DNSCache) updateIPEntry(qname string, ips []net.IP, expirationTime time.Time, dtype nftables.SetDatatype) error { +func (c *DNSCache) updateIPEntry(qname string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { scopedLog := c.log.WithValues( "fqdn", qname, - "ip_len", len(ips), + "ip_len", len(rrs), "dtype", dtype.Name, ) @@ -396,21 +381,21 @@ func (c *DNSCache) updateIPEntry(qname string, ips []net.IP, expirationTime time case nftables.TypeIPAddr: if entry.ipv4 == nil { setName := c.createSetName(qname, dtype.Name, 0) - ipe = newIPEntry(setName, expirationTime) + ipe = newIPEntry(setName) entry.ipv4 = ipe } ipe = entry.ipv4 case nftables.TypeIP6Addr: if entry.ipv6 == nil { setName := c.createSetName(qname, dtype.Name, 0) - ipe = newIPEntry(setName, expirationTime) + ipe = newIPEntry(setName) entry.ipv6 = ipe } ipe = entry.ipv6 } setName := ipe.setName - if err := ipe.update(setName, ips, expirationTime, dtype); err != nil { + if err := ipe.update(setName, rrs, lookupTime, dtype); err != nil { return fmt.Errorf("failed to update ipEntry: %w", err) } c.fqdnToEntry[qname] = entry @@ -478,19 +463,25 @@ func updateNftSet( } func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *ipEntry) firewallv1.IPSet { - return firewallv1.IPSet{ - FQDN: fqdn, - SetName: entry.setName, - IPs: entry.ips, - ExpirationTime: metav1.Time{Time: entry.expirationTime}, - Version: version, + ips := firewallv1.IPSet{ + FQDN: fqdn, + SetName: entry.setName, + Version: version, } + for ip, expirationTime := range entry.ips { + ips.IPs[ip] = metav1.Time{Time: expirationTime} + } + return ips } func createRenderIPSetFromIPEntry(version IPVersion, entry *ipEntry) RenderIPSet { + var ips []string + for ip, _ := range entry.ips { + ips = append(ips, ip) + } return RenderIPSet{ SetName: entry.setName, - IPs: entry.ips, + IPs: ips, Version: version, } } From 478a9905e7fcb84178913658d737ab200fd389bb Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 12:06:21 +0200 Subject: [PATCH 02/71] Fix linter errors --- controllers/clusterwidenetworkpolicy_controller.go | 4 ++-- pkg/dns/dnsproxy.go | 2 +- pkg/nftables/firewall_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 797d5a3d..9a169693 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -154,9 +154,9 @@ func (r *ClusterwideNetworkPolicyReconciler) manageDNSProxy( // If proxy is ON, update DNS address(if it's set in spec) if r.DnsProxy != nil && f.Spec.DNSServerAddress != "" { - port := 53 + port := uint(53) if f.Spec.DNSPort != nil { - port = int(*f.Spec.DNSPort) + port = *f.Spec.DNSPort } addr := fmt.Sprintf("%s:%d", f.Spec.DNSServerAddress, port) if err = r.DnsProxy.UpdateDNSServerAddr(addr); err != nil { diff --git a/pkg/dns/dnsproxy.go b/pkg/dns/dnsproxy.go index 7e5a13a3..66749e7d 100644 --- a/pkg/dns/dnsproxy.go +++ b/pkg/dns/dnsproxy.go @@ -148,7 +148,7 @@ func bindToPort(host string, port uint, log logr.Logger) (*net.UDPConn, *net.TCP var listener net.Listener var conn net.PacketConn - bindAddr := net.JoinHostPort(host, strconv.Itoa(int(port))) + bindAddr := net.JoinHostPort(host, strconv.FormatUint(uint64(port), 10)) defer func() { if err != nil { diff --git a/pkg/nftables/firewall_test.go b/pkg/nftables/firewall_test.go index 36d7f129..d18729ae 100644 --- a/pkg/nftables/firewall_test.go +++ b/pkg/nftables/firewall_test.go @@ -19,7 +19,7 @@ func init() { // or to run only the integration test go test Integration func TestFirewallValidateRulesIntegration(t *testing.T) { if os.Getegid() != 0 { - t.Skipf(color.YellowString("skipping integration test because not root")) + t.Skipf(color.YellowString("skipping integration test because not root")) //nolint:govet } type fields struct { From bafef1acaa34b15762727e623a721c29e26c36f8 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 12:09:58 +0200 Subject: [PATCH 03/71] Fix remaining linter error --- pkg/nftables/firewall_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/nftables/firewall_test.go b/pkg/nftables/firewall_test.go index d18729ae..ae80a332 100644 --- a/pkg/nftables/firewall_test.go +++ b/pkg/nftables/firewall_test.go @@ -19,7 +19,7 @@ func init() { // or to run only the integration test go test Integration func TestFirewallValidateRulesIntegration(t *testing.T) { if os.Getegid() != 0 { - t.Skipf(color.YellowString("skipping integration test because not root")) //nolint:govet + t.Skipf(color.YellowString("skipping integration test because not root")) //nolint:govet,staticcheck } type fields struct { From 2d8f679e939e94e2ef5d1b0c1d36b719d17bf690 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 16:20:11 +0200 Subject: [PATCH 04/71] Forgot to initialize map --- pkg/dns/dnscache.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 18f35dcc..2a502621 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -47,6 +47,7 @@ type ipEntry struct { func newIPEntry(setName string) *ipEntry { return &ipEntry{ setName: setName, + ips: map[string]time.Time{}, } } From 8df351c8aba8fbe9f883130d6e1af50021baf13a Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 16:39:16 +0200 Subject: [PATCH 05/71] Another uninitialized map --- pkg/dns/dnscache.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 2a502621..231d5e8f 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -467,6 +467,7 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *ip ips := firewallv1.IPSet{ FQDN: fqdn, SetName: entry.setName, + IPs: map[string]metav1.Time{}, Version: version, } for ip, expirationTime := range entry.ips { From c05d4f16043f5be69cf97cf68395aed495f696dd Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 17:07:07 +0200 Subject: [PATCH 06/71] Fix nftables generation: Only add IP port of RR to cache --- pkg/dns/dnscache.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 231d5e8f..8d3df07e 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -77,11 +77,11 @@ func (e *ipEntry) expireIPs() (deletedIPs []nftables.SetElement) { func (e *ipEntry) addAndUpdateIPs(rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { for _, rr := range rrs { var s string - switch rr.(type) { + switch r := rr.(type) { case *dnsgo.A: - s = rr.String() + s = r.A.String() case *dnsgo.AAAA: - s = rr.String() + s = r.AAAA.String() } if _, ok := e.ips[s]; ok { newIPs = append(newIPs, nftables.SetElement{Key: []byte(s)}) From 42d089edd776b0e0a7ddfe292e77b8c7ca6b6b76 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 18:41:29 +0200 Subject: [PATCH 07/71] Try to fix expiration date --- pkg/dns/dnscache.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 8d3df07e..c9b585e5 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -86,7 +86,7 @@ func (e *ipEntry) addAndUpdateIPs(rrs []dnsgo.RR, lookupTime time.Time) (newIPs if _, ok := e.ips[s]; ok { newIPs = append(newIPs, nftables.SetElement{Key: []byte(s)}) } - e.ips[s] = lookupTime.Add(time.Duration(rr.Header().Ttl)) + e.ips[s] = lookupTime.Add(time.Duration(rr.Header().Ttl * uint32(time.Second))) } return @@ -396,6 +396,7 @@ func (c *DNSCache) updateIPEntry(qname string, rrs []dnsgo.RR, lookupTime time.T } setName := ipe.setName + scopedLog.WithValues("set", setName, "lookupTime", lookupTime, "rrs", rrs).Info("updating ip entry") if err := ipe.update(setName, rrs, lookupTime, dtype); err != nil { return fmt.Errorf("failed to update ipEntry: %w", err) } From 7b87dc03ceb6ac6fd71cbad5306114e0634a2576 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 19:40:56 +0200 Subject: [PATCH 08/71] Second attempt to fix expiration time --- pkg/dns/dnscache.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index c9b585e5..4ba4cd80 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -51,9 +51,9 @@ func newIPEntry(setName string) *ipEntry { } } -func (e *ipEntry) update(setName string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { +func (e *ipEntry) update(log logr.Logger, setName string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { deletedIPs := e.expireIPs() - newIPs := e.addAndUpdateIPs(rrs, lookupTime) + newIPs := e.addAndUpdateIPs(log, rrs, lookupTime) if newIPs != nil || deletedIPs != nil { if err := updateNftSet(newIPs, deletedIPs, setName, dtype); err != nil { @@ -74,7 +74,7 @@ func (e *ipEntry) expireIPs() (deletedIPs []nftables.SetElement) { return } -func (e *ipEntry) addAndUpdateIPs(rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { +func (e *ipEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { for _, rr := range rrs { var s string switch r := rr.(type) { @@ -86,7 +86,8 @@ func (e *ipEntry) addAndUpdateIPs(rrs []dnsgo.RR, lookupTime time.Time) (newIPs if _, ok := e.ips[s]; ok { newIPs = append(newIPs, nftables.SetElement{Key: []byte(s)}) } - e.ips[s] = lookupTime.Add(time.Duration(rr.Header().Ttl * uint32(time.Second))) + log.WithValues("ip", s, "rr header ttl", rr.Header().Ttl, "expiration time", lookupTime.Add(time.Duration(rr.Header().Ttl)*time.Second)) + e.ips[s] = lookupTime.Add(time.Duration(rr.Header().Ttl) * time.Second) } return @@ -397,7 +398,7 @@ func (c *DNSCache) updateIPEntry(qname string, rrs []dnsgo.RR, lookupTime time.T setName := ipe.setName scopedLog.WithValues("set", setName, "lookupTime", lookupTime, "rrs", rrs).Info("updating ip entry") - if err := ipe.update(setName, rrs, lookupTime, dtype); err != nil { + if err := ipe.update(scopedLog, setName, rrs, lookupTime, dtype); err != nil { return fmt.Errorf("failed to update ipEntry: %w", err) } c.fqdnToEntry[qname] = entry From a814da6ba37f9b063c7adc24bb22fd94174dc7e9 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 30 Aug 2024 11:31:53 +0200 Subject: [PATCH 09/71] Try keep the CRD stable --- api/v1/clusterwidenetworkpolicy_types.go | 25 ++++++++++--------- api/v1/zz_generated.deepcopy.go | 8 +++--- ...l-stack.io_clusterwidenetworkpolicies.yaml | 8 +++--- pkg/dns/dnscache.go | 18 +++++++++---- 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index bc7c381b..37da70eb 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -152,21 +152,22 @@ type FQDNSelector struct { } // IPSet stores set name association to IP addresses -type IPSet struct { - FQDN string `json:"fqdn,omitempty"` - SetName string `json:"setName,omitempty"` - IPs map[string]metav1.Time `json:"ips,omitempty"` - Version IPVersion `json:"version,omitempty"` -} - // type IPSet struct { -// FQDN string `json:"fqdn,omitempty"` -// SetName string `json:"setName,omitempty"` -// IPs []string `json:"ips,omitempty"` -// ExpirationTime metav1.Time `json:"expirationTime,omitempty"` -// Version IPVersion `json:"version,omitempty"` +// FQDN string `json:"fqdn,omitempty"` +// SetName string `json:"setName,omitempty"` +// IPs map[string]metav1.Time `json:"ips,omitempty"` +// Version IPVersion `json:"version,omitempty"` // } +// IPSet stores set name association to IP addresses +type IPSet struct { + FQDN string `json:"fqdn,omitempty"` + SetName string `json:"setName,omitempty"` + IPs []string `json:"ips,omitempty"` + ExpirationTime metav1.Time `json:"expirationTime,omitempty"` + Version IPVersion `json:"version,omitempty"` +} + func (l *ClusterwideNetworkPolicyList) GetFQDNs() []FQDNSelector { s := []FQDNSelector{} for _, i := range l.Items { diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 86663767..c9863969 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -6,7 +6,6 @@ package v1 import ( networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -155,11 +154,10 @@ func (in *IPSet) DeepCopyInto(out *IPSet) { *out = *in if in.IPs != nil { in, out := &in.IPs, &out.IPs - *out = make(map[string]metav1.Time, len(*in)) - for key, val := range *in { - (*out)[key] = *val.DeepCopy() - } + *out = make([]string, len(*in)) + copy(*out, *in) } + in.ExpirationTime.DeepCopyInto(&out.ExpirationTime) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPSet. diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index c03ce628..ea74df44 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -245,13 +245,15 @@ spec: items: description: IPSet stores set name association to IP addresses properties: + expirationTime: + format: date-time + type: string fqdn: type: string ips: - additionalProperties: - format: date-time + items: type: string - type: object + type: array setName: type: string version: diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 4ba4cd80..0eed00b4 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -13,7 +13,7 @@ import ( "github.com/go-logr/logr" "github.com/google/nftables" dnsgo "github.com/miekg/dns" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + // metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" ) @@ -193,8 +193,13 @@ func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { ipe := &ipEntry{ setName: s.SetName, } - for ip, expirationTime := range s.IPs { - ipe.ips[ip] = expirationTime.Time + for _, ip := range s.IPs { + ipa, _, _ := strings.Cut(ip, ",") + expirationTime := time.Now() + if _, ets, found := strings.Cut(ip, ": "); found { + expirationTime.UnmarshalText([]byte(ets)) + } + ipe.ips[ipa] = expirationTime } switch s.Version { case firewallv1.IPv4: @@ -469,11 +474,14 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *ip ips := firewallv1.IPSet{ FQDN: fqdn, SetName: entry.setName, - IPs: map[string]metav1.Time{}, + IPs: []string{}, Version: version, } for ip, expirationTime := range entry.ips { - ips.IPs[ip] = metav1.Time{Time: expirationTime} + if et, err := expirationTime.MarshalText(); err == nil { + ip = ip + ", expiration time: " + string(et) + } + ips.IPs = append(ips.IPs, ip) } return ips } From 4c406fd0bcbf345d16d3ac049a938a39a1da79d1 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 30 Aug 2024 11:58:47 +0200 Subject: [PATCH 10/71] Add forgotten error handling --- pkg/dns/dnscache.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 0eed00b4..ed204c11 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -197,7 +197,9 @@ func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { ipa, _, _ := strings.Cut(ip, ",") expirationTime := time.Now() if _, ets, found := strings.Cut(ip, ": "); found { - expirationTime.UnmarshalText([]byte(ets)) + if err := expirationTime.UnmarshalText([]byte(ets)); err != nil { + expirationTime = time.Now() + } } ipe.ips[ipa] = expirationTime } From dddc7afaed2879fb88edaf35628d657f7754be5e Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 30 Aug 2024 18:30:49 +0200 Subject: [PATCH 11/71] Try and fix bug where only one rule's FQDNState would be shown --- pkg/nftables/networkpolicy.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/nftables/networkpolicy.go b/pkg/nftables/networkpolicy.go index cb46d0d7..26a044f2 100644 --- a/pkg/nftables/networkpolicy.go +++ b/pkg/nftables/networkpolicy.go @@ -78,6 +78,7 @@ func clusterwideNetworkPolicyEgressRules( np firewallv1.ClusterwideNetworkPolicy, logAcceptedConnections bool, ) (rules nftablesRules, updated firewallv1.ClusterwideNetworkPolicy) { + var fqdnState firewallv1.FQDNState for _, e := range np.Spec.Egress { tcpPorts, udpPorts := calculatePorts(e.Ports) ruleBases := []ruleBase{} @@ -95,9 +96,9 @@ func clusterwideNetworkPolicyEgressRules( ruleBases = append(ruleBases, ruleBase{base: rb}) } else if len(e.ToFQDNs) > 0 && cache.IsInitialized() { // Generate allow rules based on DNS selectors - rbs, u := clusterwideNetworkPolicyEgressToFQDNRules(cache, e) - np.Status.FQDNState = u + rbs, u := clusterwideNetworkPolicyEgressToFQDNRules(cache, fqdnState, e) ruleBases = append(ruleBases, rbs...) + fqdnState = u } comment := fmt.Sprintf("accept traffic for np %s", np.ObjectMeta.Name) @@ -111,6 +112,7 @@ func clusterwideNetworkPolicyEgressRules( } } + np.Status.FQDNState = fqdnState return uniqueSorted(rules), np } @@ -125,9 +127,9 @@ func clusterwideNetworkPolicyEgressToRules(e firewallv1.EgressRule) (allow, exce func clusterwideNetworkPolicyEgressToFQDNRules( cache FQDNCache, + fqdnState firewallv1.FQDNState, e firewallv1.EgressRule, ) (rules []ruleBase, updatedState firewallv1.FQDNState) { - fqdnState := firewallv1.FQDNState{} for _, fqdn := range e.ToFQDNs { fqdnName := fqdn.MatchName From c67aa1cedb615a2acafddbf10f88bbe6bff8148a Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 30 Aug 2024 18:46:15 +0200 Subject: [PATCH 12/71] Fix forgotten initialisation of map --- pkg/nftables/networkpolicy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/nftables/networkpolicy.go b/pkg/nftables/networkpolicy.go index 26a044f2..b29206bb 100644 --- a/pkg/nftables/networkpolicy.go +++ b/pkg/nftables/networkpolicy.go @@ -130,6 +130,9 @@ func clusterwideNetworkPolicyEgressToFQDNRules( fqdnState firewallv1.FQDNState, e firewallv1.EgressRule, ) (rules []ruleBase, updatedState firewallv1.FQDNState) { + if fqdnState == nil { + fqdnState = firewallv1.FQDNState{} + } for _, fqdn := range e.ToFQDNs { fqdnName := fqdn.MatchName From 53ecc4b4d36a7b9cbb56f6568ef2c9b52b02a7c4 Mon Sep 17 00:00:00 2001 From: mreiger Date: Tue, 3 Sep 2024 19:40:39 +0200 Subject: [PATCH 13/71] get shoot client to dns cache for handling state configmap --- pkg/dns/dnscache.go | 9 ++++++++- pkg/dns/dnsproxy.go | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index ed204c11..3dcdf655 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -13,6 +13,8 @@ import ( "github.com/go-logr/logr" "github.com/google/nftables" dnsgo "github.com/miekg/dns" + "sigs.k8s.io/controller-runtime/pkg/client" + // metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" @@ -29,6 +31,9 @@ const ( // How many DNS redirections (CNAME/DNAME) are followed, to break up redirection loops. maxDNSRedirects = 10 + + // Configmap that holds the FQDN state + fqdnStateConfigmapNameSuffix = "fqdnstate" ) // RenderIPSet stores set info for rendering @@ -105,16 +110,18 @@ type DNSCache struct { fqdnToEntry map[string]cacheEntry setNames map[string]struct{} dnsServerAddr string + shootClient client.Client ipv4Enabled bool ipv6Enabled bool } -func newDNSCache(dns string, ipv4Enabled, ipv6Enabled bool, log logr.Logger) *DNSCache { +func newDNSCache(dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) *DNSCache { return &DNSCache{ log: log, fqdnToEntry: map[string]cacheEntry{}, setNames: map[string]struct{}{}, dnsServerAddr: dns, + shootClient: shootClient, ipv4Enabled: ipv4Enabled, ipv6Enabled: ipv6Enabled, } diff --git a/pkg/dns/dnsproxy.go b/pkg/dns/dnsproxy.go index ea8dcc71..fdec37b8 100644 --- a/pkg/dns/dnsproxy.go +++ b/pkg/dns/dnsproxy.go @@ -7,6 +7,7 @@ import ( "strconv" "github.com/metal-stack/metal-networker/pkg/netconf" + "sigs.k8s.io/controller-runtime/pkg/client" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" @@ -36,11 +37,11 @@ type DNSProxy struct { handler DNSHandler } -func NewDNSProxy(dns string, port *uint, log logr.Logger) (*DNSProxy, error) { +func NewDNSProxy(dns string, port *uint, shootClient client.Client, log logr.Logger) (*DNSProxy, error) { if dns == "" { dns = defaultDNSServerAddr } - cache := newDNSCache(dns, true, false, log.WithName("DNS cache")) + cache := newDNSCache(dns, true, false, shootClient, log.WithName("DNS cache")) handler := NewDNSProxyHandler(log, cache) host, err := getHost() From fabc726cdf9a26d7a3aecef4a9d9f139b229546f Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 27 Feb 2025 10:05:26 +0100 Subject: [PATCH 14/71] Implement writing to state configmap --- .../clusterwidenetworkpolicy_controller.go | 2 +- pkg/dns/dnscache.go | 158 ++++++++++++------ pkg/dns/dnscache_test.go | 4 +- pkg/dns/dnsproxy.go | 7 +- 4 files changed, 116 insertions(+), 55 deletions(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 9a169693..7b3a2c6d 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -142,7 +142,7 @@ func (r *ClusterwideNetworkPolicyReconciler) manageDNSProxy( if enableDNS && r.DnsProxy == nil { r.Log.Info("DNS Proxy is initialized") - if r.DnsProxy, err = dns.NewDNSProxy(f.Spec.DNSServerAddress, f.Spec.DNSPort, ctrl.Log.WithName("DNS proxy")); err != nil { + if r.DnsProxy, err = dns.NewDNSProxy(ctx, f.Spec.DNSServerAddress, f.Spec.DNSPort, r.ShootClient, ctrl.Log.WithName("DNS proxy")); err != nil { return fmt.Errorf("failed to init DNS proxy: %w", err) } go r.DnsProxy.Run(ctx) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 3dcdf655..d62a8ede 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -1,8 +1,10 @@ package dns import ( + "context" "crypto/md5" //nolint:gosec "encoding/hex" + "encoding/json" "fmt" "regexp" "strings" @@ -13,9 +15,10 @@ import ( "github.com/go-logr/logr" "github.com/google/nftables" dnsgo "github.com/miekg/dns" + v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" - // metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" ) @@ -33,7 +36,8 @@ const ( maxDNSRedirects = 10 // Configmap that holds the FQDN state - fqdnStateConfigmapNameSuffix = "fqdnstate" + fqdnStateConfigmapName = "fqdnstate" + fqdnStateNamespace = "firewall" ) // RenderIPSet stores set info for rendering @@ -106,30 +110,76 @@ type cacheEntry struct { type DNSCache struct { sync.RWMutex - log logr.Logger - fqdnToEntry map[string]cacheEntry - setNames map[string]struct{} - dnsServerAddr string - shootClient client.Client - ipv4Enabled bool - ipv6Enabled bool + log logr.Logger + fqdnToEntry map[string]cacheEntry + setNames map[string]struct{} + dnsServerAddr string + shootClient client.Client + ctx context.Context + stateConfigmap *v1.ConfigMap + ipv4Enabled bool + ipv6Enabled bool } -func newDNSCache(dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) *DNSCache { - return &DNSCache{ +func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) (*DNSCache, error) { + c := DNSCache{ log: log, fqdnToEntry: map[string]cacheEntry{}, setNames: map[string]struct{}{}, dnsServerAddr: dns, shootClient: shootClient, - ipv4Enabled: ipv4Enabled, - ipv6Enabled: ipv6Enabled, + stateConfigmap: &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: fqdnStateConfigmapName, + Namespace: fqdnStateNamespace, + }, + }, + ipv4Enabled: ipv4Enabled, + ipv6Enabled: ipv6Enabled, + } + if err := shootClient.Get(ctx, client.ObjectKey{Namespace: fqdnStateNamespace, Name: fqdnStateConfigmapName}, c.stateConfigmap); err != nil { + if err := shootClient.Create(ctx, c.stateConfigmap, nil); err != nil { + return nil, err + } + } + if c.stateConfigmap.Data == nil { + return &c, nil + + } + if c.stateConfigmap.Data["state"] == "" { + return &c, nil + + } + err := json.Unmarshal([]byte(c.stateConfigmap.Data["state"]), &c.fqdnToEntry) + if err != nil { + return nil, err } + return &c, nil +} + +// writeStateToConfigmap writes the whole DNS cache to the state configmap +func (c *DNSCache) writeStateToConfigmap() error { + if err := c.shootClient.Get(c.ctx, client.ObjectKey{Namespace: fqdnStateNamespace, Name: fqdnStateConfigmapName}, c.stateConfigmap); err != nil { // make sure the configmap exists + if err := c.shootClient.Create(c.ctx, c.stateConfigmap, nil); err != nil { + return err + } + } + + s, err := json.Marshal(c.fqdnToEntry) + if err != nil { + return err + } + c.stateConfigmap.Data["state"] = string(s) + + if err := c.shootClient.Update(c.ctx, c.stateConfigmap); err != nil { + return err + } + return nil } // getSetsForFQDN returns sets for FQDN selector func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firewallv1.IPSet) (result []firewallv1.IPSet) { - c.restoreSets(fqdnSets) + // c.restoreSets(fqdnSets) sets := map[string]firewallv1.IPSet{} if fqdn.MatchName != "" { @@ -186,42 +236,42 @@ func (c *DNSCache) updateDNSServerAddr(addr string) { } // restoreSets add missing sets from FQDNSelector.Sets -func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { - for _, s := range fqdnSets { - // Add cache entries from fqdn.Sets if missing - c.Lock() - if _, ok := c.setNames[s.SetName]; !ok { - c.setNames[s.SetName] = struct{}{} - entry, exists := c.fqdnToEntry[s.FQDN] - if !exists { - entry = cacheEntry{} - } - - ipe := &ipEntry{ - setName: s.SetName, - } - for _, ip := range s.IPs { - ipa, _, _ := strings.Cut(ip, ",") - expirationTime := time.Now() - if _, ets, found := strings.Cut(ip, ": "); found { - if err := expirationTime.UnmarshalText([]byte(ets)); err != nil { - expirationTime = time.Now() - } - } - ipe.ips[ipa] = expirationTime - } - switch s.Version { - case firewallv1.IPv4: - entry.ipv4 = ipe - case firewallv1.IPv6: - entry.ipv6 = ipe - } - - c.fqdnToEntry[s.FQDN] = entry - } - c.Unlock() - } -} +// func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { +// for _, s := range fqdnSets { +// // Add cache entries from fqdn.Sets if missing +// c.Lock() +// if _, ok := c.setNames[s.SetName]; !ok { +// c.setNames[s.SetName] = struct{}{} +// entry, exists := c.fqdnToEntry[s.FQDN] +// if !exists { +// entry = cacheEntry{} +// } +// +// ipe := &ipEntry{ +// setName: s.SetName, +// } +// for _, ip := range s.IPs { +// ipa, _, _ := strings.Cut(ip, ",") +// expirationTime := time.Now() +// if _, ets, found := strings.Cut(ip, ": "); found { +// if err := expirationTime.UnmarshalText([]byte(ets)); err != nil { +// expirationTime = time.Now() +// } +// } +// ipe.ips[ipa] = expirationTime +// } +// switch s.Version { +// case firewallv1.IPv4: +// entry.ipv4 = ipe +// case firewallv1.IPv6: +// entry.ipv6 = ipe +// } +// +// c.fqdnToEntry[s.FQDN] = entry +// } +// c.Unlock() +// } +// } // getSetNameForFQDN returns FQDN set data func (c *DNSCache) getSetNameForFQDN(fqdn string) (result []firewallv1.IPSet) { @@ -360,6 +410,8 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq } } + ipEntriesUpdated := false + for _, fqdn := range fqdns { c.log.V(4).Info("DEBUG dnscache Update function Updating DNS cache for", "fqdn", fqdn, "ipv4", ipv4, "ipv6", ipv6) if c.ipv4Enabled && len(ipv4) > 0 { @@ -374,6 +426,12 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq } } + if ipEntriesUpdated { + if err := c.writeStateToConfigmap(); err != nil { + c.log.V(4).Info("DEBUG could not write updated DNS cache to state configmap", "configmap", fqdnStateConfigmapName, "namespace", fqdnStateNamespace) + } + } + return found, nil } diff --git a/pkg/dns/dnscache_test.go b/pkg/dns/dnscache_test.go index ade37e6c..1074fdb8 100644 --- a/pkg/dns/dnscache_test.go +++ b/pkg/dns/dnscache_test.go @@ -91,7 +91,7 @@ func Test_GetSetsForFQDN(t *testing.T) { MatchPattern: "ww*.freechess.org", }, }, - { + /* { name: "restore sets", fqdnToEntry: map[string]cacheEntry{}, fqdnSelector: firewallv1.FQDNSelector{}, @@ -99,7 +99,7 @@ func Test_GetSetsForFQDN(t *testing.T) { FQDN: "test-fqdn", SetName: "test-set", }}, - }, + }, FIXME see how we can test the new state configmap approach */ } for _, tt := range tests { diff --git a/pkg/dns/dnsproxy.go b/pkg/dns/dnsproxy.go index fdec37b8..b0c21d3a 100644 --- a/pkg/dns/dnsproxy.go +++ b/pkg/dns/dnsproxy.go @@ -37,11 +37,14 @@ type DNSProxy struct { handler DNSHandler } -func NewDNSProxy(dns string, port *uint, shootClient client.Client, log logr.Logger) (*DNSProxy, error) { +func NewDNSProxy(ctx context.Context, dns string, port *uint, shootClient client.Client, log logr.Logger) (*DNSProxy, error) { if dns == "" { dns = defaultDNSServerAddr } - cache := newDNSCache(dns, true, false, shootClient, log.WithName("DNS cache")) + cache, err := newDNSCache(ctx, dns, true, false, shootClient, log.WithName("DNS cache")) + if err != nil { + return nil, err + } handler := NewDNSProxyHandler(log, cache) host, err := getHost() From 8215f603822419647513b62dde8f7fad4087a63c Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 27 Feb 2025 10:15:47 +0100 Subject: [PATCH 15/71] pacify linter --- pkg/dns/dnscache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index d62a8ede..4044f339 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -198,7 +198,7 @@ func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firew result = append(result, s) } - c.log.WithValues("fqdn", fqdn, "sets", result).Info("sets for FQDN") + c.log.WithValues("fqdn", fqdn, "fqdnSets", fqdnSets, "sets", result).Info("sets for FQDN") return } From 559f37d7b704192c3e4b2100de295b0cc1eafa26 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 27 Feb 2025 10:18:47 +0100 Subject: [PATCH 16/71] update setup-gcloud --- .github/workflows/build.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index dfd17d2c..d1b15301 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -37,7 +37,7 @@ jobs: credentials_json: '${{ secrets.GCP_SA_KEY }}' - name: Set up Cloud SDK - uses: google-github-actions/setup-gcloud@v0 + uses: google-github-actions/setup-gcloud@v2 - name: Set up Go 1.23 uses: actions/setup-go@v5 From 9b36afbc2bb20aeeaa2572537bd6416183d29a0f Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 28 Feb 2025 11:38:39 +0100 Subject: [PATCH 17/71] Try and initialize the configmap correctly --- pkg/dns/dnscache.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 4044f339..ada4c589 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -128,15 +128,19 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, setNames: map[string]struct{}{}, dnsServerAddr: dns, shootClient: shootClient, - stateConfigmap: &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: fqdnStateConfigmapName, - Namespace: fqdnStateNamespace, - }, + ipv4Enabled: ipv4Enabled, + ipv6Enabled: ipv6Enabled, + } + + scm := v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: fqdnStateConfigmapName, + Namespace: fqdnStateNamespace, }, - ipv4Enabled: ipv4Enabled, - ipv6Enabled: ipv6Enabled, } + + c.stateConfigmap = &scm + if err := shootClient.Get(ctx, client.ObjectKey{Namespace: fqdnStateNamespace, Name: fqdnStateConfigmapName}, c.stateConfigmap); err != nil { if err := shootClient.Create(ctx, c.stateConfigmap, nil); err != nil { return nil, err From ea9bfb2c594d458655d70afabd7cdaafa8255015 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 28 Feb 2025 12:57:53 +0100 Subject: [PATCH 18/71] Try different approach for initialising dns cache from configmap --- pkg/dns/dnscache.go | 66 +++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index ada4c589..e286bdc3 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -16,6 +16,7 @@ import ( "github.com/google/nftables" dnsgo "github.com/miekg/dns" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -110,15 +111,14 @@ type cacheEntry struct { type DNSCache struct { sync.RWMutex - log logr.Logger - fqdnToEntry map[string]cacheEntry - setNames map[string]struct{} - dnsServerAddr string - shootClient client.Client - ctx context.Context - stateConfigmap *v1.ConfigMap - ipv4Enabled bool - ipv6Enabled bool + log logr.Logger + fqdnToEntry map[string]cacheEntry + setNames map[string]struct{} + dnsServerAddr string + shootClient client.Client + ctx context.Context + ipv4Enabled bool + ipv6Enabled bool } func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) (*DNSCache, error) { @@ -132,29 +132,20 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, ipv6Enabled: ipv6Enabled, } - scm := v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: fqdnStateConfigmapName, - Namespace: fqdnStateNamespace, - }, - } + nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} + scm := v1.ConfigMap{} - c.stateConfigmap = &scm - - if err := shootClient.Get(ctx, client.ObjectKey{Namespace: fqdnStateNamespace, Name: fqdnStateConfigmapName}, c.stateConfigmap); err != nil { - if err := shootClient.Create(ctx, c.stateConfigmap, nil); err != nil { - return nil, err - } + if err := shootClient.Get(ctx, nn, &scm); err != nil { } - if c.stateConfigmap.Data == nil { + if scm.Data == nil { return &c, nil } - if c.stateConfigmap.Data["state"] == "" { + if scm.Data["state"] == "" { return &c, nil } - err := json.Unmarshal([]byte(c.stateConfigmap.Data["state"]), &c.fqdnToEntry) + err := json.Unmarshal([]byte(scm.Data["state"]), &c.fqdnToEntry) if err != nil { return nil, err } @@ -163,19 +154,30 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, // writeStateToConfigmap writes the whole DNS cache to the state configmap func (c *DNSCache) writeStateToConfigmap() error { - if err := c.shootClient.Get(c.ctx, client.ObjectKey{Namespace: fqdnStateNamespace, Name: fqdnStateConfigmapName}, c.stateConfigmap); err != nil { // make sure the configmap exists - if err := c.shootClient.Create(c.ctx, c.stateConfigmap, nil); err != nil { - return err - } - } - s, err := json.Marshal(c.fqdnToEntry) if err != nil { return err } - c.stateConfigmap.Data["state"] = string(s) - if err := c.shootClient.Update(c.ctx, c.stateConfigmap); err != nil { + nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} + scm := v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: fqdnStateConfigmapName, + Namespace: fqdnStateNamespace, + }, + Data: map[string]string{ + "state": string(s), + }, + } + + if err := c.shootClient.Get(c.ctx, nn, &v1.ConfigMap{}); err != nil { + if err := c.shootClient.Create(c.ctx, &scm, nil); err != nil { + return err + } + return nil + } + + if err := c.shootClient.Update(c.ctx, &scm); err != nil { return err } return nil From 2064bfaa54a1ede36b48662452952a35ce597fc2 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 28 Feb 2025 13:03:32 +0100 Subject: [PATCH 19/71] properly check for existing configmap --- pkg/dns/dnscache.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index e286bdc3..476ac190 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -16,6 +16,7 @@ import ( "github.com/google/nftables" dnsgo "github.com/miekg/dns" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -135,7 +136,9 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} scm := v1.ConfigMap{} - if err := shootClient.Get(ctx, nn, &scm); err != nil { + err := shootClient.Get(ctx, nn, &scm) + if err != nil && !apierrors.IsNotFound(err) { + return nil, err } if scm.Data == nil { return &c, nil @@ -145,7 +148,7 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, return &c, nil } - err := json.Unmarshal([]byte(scm.Data["state"]), &c.fqdnToEntry) + err = json.Unmarshal([]byte(scm.Data["state"]), &c.fqdnToEntry) if err != nil { return nil, err } From e14c8de336259b0f712653f98c1eaefff175dd60 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 28 Feb 2025 13:27:46 +0100 Subject: [PATCH 20/71] Actually try to write dns cache to configmap --- pkg/dns/dnscache.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 476ac190..945e2732 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -427,11 +427,13 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq if err := c.updateIPEntry(fqdn, ipv4, lookupTime, nftables.TypeIPAddr); err != nil { return false, fmt.Errorf("failed to update IPv4 addresses: %w", err) } + ipEntriesUpdated = true } if c.ipv6Enabled && len(ipv6) > 0 { if err := c.updateIPEntry(fqdn, ipv6, lookupTime, nftables.TypeIP6Addr); err != nil { return false, fmt.Errorf("failed to update IPv6 addresses: %w", err) } + ipEntriesUpdated = true } } From e65d6824bcdc65382693efd41d519a97a10c8500 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 28 Feb 2025 18:04:49 +0100 Subject: [PATCH 21/71] Do not try to write nil cache to configmap; debug output --- pkg/dns/dnscache.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 945e2732..47ea74a2 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -161,6 +161,10 @@ func (c *DNSCache) writeStateToConfigmap() error { if err != nil { return err } + if s == nil { + return nil + } + c.log.V(4).Info("DEBUG writing cache to configmap", "fqdnToEntry", s) nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} scm := v1.ConfigMap{ @@ -172,17 +176,22 @@ func (c *DNSCache) writeStateToConfigmap() error { "state": string(s), }, } + c.log.V(4).Info("DEBUG created configmap", "scm", scm) if err := c.shootClient.Get(c.ctx, nn, &v1.ConfigMap{}); err != nil { + c.log.V(4).Info("DEBUG configmap not found, trying to create") if err := c.shootClient.Create(c.ctx, &scm, nil); err != nil { return err } + c.log.V(4).Info("DEBUG configmap created", "scm", scm) return nil } + c.log.V(4).Info("DEBUG configmap found, trying to update") if err := c.shootClient.Update(c.ctx, &scm); err != nil { return err } + c.log.V(4).Info("DEBUG configmap created", "scm", scm) return nil } From febbaf66dc540ba7c889ae3d6cb20849f65be97e Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 28 Feb 2025 20:05:38 +0100 Subject: [PATCH 22/71] Try and turn on debug logging --- main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 4029390d..462d4b9b 100644 --- a/main.go +++ b/main.go @@ -6,6 +6,7 @@ import ( "fmt" "log/slog" "os" + "runtime/debug" "time" "github.com/metal-stack/v" @@ -91,7 +92,7 @@ func main() { return } - jsonHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{}) + jsonHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug}) l := slog.New(jsonHandler) ctrl.SetLogger(logr.FromSlogHandler(jsonHandler)) From 6069a0520a4b6ab051fc177a00a280ae4df8a371 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 28 Feb 2025 20:08:12 +0100 Subject: [PATCH 23/71] remove accidental package definition --- main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/main.go b/main.go index 462d4b9b..19e77488 100644 --- a/main.go +++ b/main.go @@ -6,7 +6,6 @@ import ( "fmt" "log/slog" "os" - "runtime/debug" "time" "github.com/metal-stack/v" From 70ac71c3ee28c1c9e32d9721ef3fd04444b7c787 Mon Sep 17 00:00:00 2001 From: mreiger Date: Mon, 3 Mar 2025 16:48:48 +0100 Subject: [PATCH 24/71] Try to fix JSON marshaling --- pkg/dns/dnscache.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 47ea74a2..b3b82d4a 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -51,8 +51,8 @@ type RenderIPSet struct { type ipEntry struct { // ips is a map of the ip address and its expiration time which is the time of the DNS lookup + the TTL - ips map[string]time.Time - setName string + ips map[string]time.Time `json:"ips,omitempty"` + setName string `json:"setName,omitempty"` } func newIPEntry(setName string) *ipEntry { @@ -105,8 +105,8 @@ func (e *ipEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime ti } type cacheEntry struct { - ipv4 *ipEntry - ipv6 *ipEntry + ipv4 *ipEntry `json:"ipv4,omitempty"` + ipv6 *ipEntry `json:"ipv4,omitempty"` } type DNSCache struct { From bffdffb812d7725e92dc525d5384b95a0f4b206c Mon Sep 17 00:00:00 2001 From: mreiger Date: Mon, 3 Mar 2025 17:26:35 +0100 Subject: [PATCH 25/71] Export fields so the json library can read them --- pkg/dns/dnscache.go | 100 +++++++++++++++++++-------------------- pkg/dns/dnscache_test.go | 56 +++++++++++----------- 2 files changed, 78 insertions(+), 78 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index b3b82d4a..6252cce5 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -49,20 +49,20 @@ type RenderIPSet struct { Version IPVersion `json:"version,omitempty"` } -type ipEntry struct { +type IPEntry struct { // ips is a map of the ip address and its expiration time which is the time of the DNS lookup + the TTL - ips map[string]time.Time `json:"ips,omitempty"` - setName string `json:"setName,omitempty"` + IPs map[string]time.Time `json:"ips,omitempty"` + SetName string `json:"setName,omitempty"` } -func newIPEntry(setName string) *ipEntry { - return &ipEntry{ - setName: setName, - ips: map[string]time.Time{}, +func newIPEntry(setName string) *IPEntry { + return &IPEntry{ + SetName: setName, + IPs: map[string]time.Time{}, } } -func (e *ipEntry) update(log logr.Logger, setName string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { +func (e *IPEntry) update(log logr.Logger, setName string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { deletedIPs := e.expireIPs() newIPs := e.addAndUpdateIPs(log, rrs, lookupTime) @@ -75,17 +75,17 @@ func (e *ipEntry) update(log logr.Logger, setName string, rrs []dnsgo.RR, lookup return nil } -func (e *ipEntry) expireIPs() (deletedIPs []nftables.SetElement) { - for ip, expirationTime := range e.ips { +func (e *IPEntry) expireIPs() (deletedIPs []nftables.SetElement) { + for ip, expirationTime := range e.IPs { if expirationTime.Before(time.Now()) { deletedIPs = append(deletedIPs, nftables.SetElement{Key: []byte(ip)}) - delete(e.ips, ip) + delete(e.IPs, ip) } } return } -func (e *ipEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { +func (e *IPEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { for _, rr := range rrs { var s string switch r := rr.(type) { @@ -94,26 +94,26 @@ func (e *ipEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime ti case *dnsgo.AAAA: s = r.AAAA.String() } - if _, ok := e.ips[s]; ok { + if _, ok := e.IPs[s]; ok { newIPs = append(newIPs, nftables.SetElement{Key: []byte(s)}) } log.WithValues("ip", s, "rr header ttl", rr.Header().Ttl, "expiration time", lookupTime.Add(time.Duration(rr.Header().Ttl)*time.Second)) - e.ips[s] = lookupTime.Add(time.Duration(rr.Header().Ttl) * time.Second) + e.IPs[s] = lookupTime.Add(time.Duration(rr.Header().Ttl) * time.Second) } return } -type cacheEntry struct { - ipv4 *ipEntry `json:"ipv4,omitempty"` - ipv6 *ipEntry `json:"ipv4,omitempty"` +type CacheEntry struct { + IPv4 *IPEntry `json:"ipv4,omitempty"` + IPv6 *IPEntry `json:"ipv6,omitempty"` } type DNSCache struct { sync.RWMutex log logr.Logger - fqdnToEntry map[string]cacheEntry + fqdnToEntry map[string]CacheEntry setNames map[string]struct{} dnsServerAddr string shootClient client.Client @@ -125,7 +125,7 @@ type DNSCache struct { func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) (*DNSCache, error) { c := DNSCache{ log: log, - fqdnToEntry: map[string]cacheEntry{}, + fqdnToEntry: map[string]CacheEntry{}, setNames: map[string]struct{}{}, dnsServerAddr: dns, shootClient: shootClient, @@ -237,11 +237,11 @@ func (c *DNSCache) getSetsForRendering(fqdns []firewallv1.FQDNSelector) (result } } if matched { - if e.ipv4 != nil { - result = append(result, createRenderIPSetFromIPEntry(IPv4, e.ipv4)) + if e.IPv4 != nil { + result = append(result, createRenderIPSetFromIPEntry(IPv4, e.IPv4)) } - if e.ipv6 != nil { - result = append(result, createRenderIPSetFromIPEntry(IPv6, e.ipv6)) + if e.IPv6 != nil { + result = append(result, createRenderIPSetFromIPEntry(IPv6, e.IPv6)) } } } @@ -262,10 +262,10 @@ func (c *DNSCache) updateDNSServerAddr(addr string) { // c.setNames[s.SetName] = struct{}{} // entry, exists := c.fqdnToEntry[s.FQDN] // if !exists { -// entry = cacheEntry{} +// entry = CacheEntry{} // } // -// ipe := &ipEntry{ +// ipe := &IPEntry{ // setName: s.SetName, // } // for _, ip := range s.IPs { @@ -276,7 +276,7 @@ func (c *DNSCache) updateDNSServerAddr(addr string) { // expirationTime = time.Now() // } // } -// ipe.ips[ipa] = expirationTime +// ipe.IPs[ipa] = expirationTime // } // switch s.Version { // case firewallv1.IPv4: @@ -312,11 +312,11 @@ func (c *DNSCache) getSetNameForFQDN(fqdn string) (result []firewallv1.IPSet) { } defer c.RUnlock() - if entry.ipv4 != nil { - result = append(result, createIPSetFromIPEntry(fqdn, firewallv1.IPv4, entry.ipv4)) + if entry.IPv4 != nil { + result = append(result, createIPSetFromIPEntry(fqdn, firewallv1.IPv4, entry.IPv4)) } - if entry.ipv6 != nil { - result = append(result, createIPSetFromIPEntry(fqdn, firewallv1.IPv6, entry.ipv6)) + if entry.IPv6 != nil { + result = append(result, createIPSetFromIPEntry(fqdn, firewallv1.IPv6, entry.IPv6)) } return } @@ -359,11 +359,11 @@ func (c *DNSCache) getSetNameForRegex(regex string) (sets []firewallv1.IPSet) { continue } - if e.ipv4 != nil { - sets = append(sets, createIPSetFromIPEntry(n, firewallv1.IPv4, e.ipv4)) + if e.IPv4 != nil { + sets = append(sets, createIPSetFromIPEntry(n, firewallv1.IPv4, e.IPv4)) } - if e.ipv6 != nil { - sets = append(sets, createIPSetFromIPEntry(n, firewallv1.IPv6, e.ipv6)) + if e.IPv6 != nil { + sets = append(sets, createIPSetFromIPEntry(n, firewallv1.IPv6, e.IPv6)) } } @@ -467,31 +467,31 @@ func (c *DNSCache) updateIPEntry(qname string, rrs []dnsgo.RR, lookupTime time.T entry, exists := c.fqdnToEntry[qname] if !exists { - entry = cacheEntry{} + entry = CacheEntry{} } - var ipe *ipEntry + var ipe *IPEntry switch dtype { case nftables.TypeIPAddr: - if entry.ipv4 == nil { + if entry.IPv4 == nil { setName := c.createSetName(qname, dtype.Name, 0) ipe = newIPEntry(setName) - entry.ipv4 = ipe + entry.IPv4 = ipe } - ipe = entry.ipv4 + ipe = entry.IPv4 case nftables.TypeIP6Addr: - if entry.ipv6 == nil { + if entry.IPv6 == nil { setName := c.createSetName(qname, dtype.Name, 0) ipe = newIPEntry(setName) - entry.ipv6 = ipe + entry.IPv6 = ipe } - ipe = entry.ipv6 + ipe = entry.IPv6 } - setName := ipe.setName + setName := ipe.SetName scopedLog.WithValues("set", setName, "lookupTime", lookupTime, "rrs", rrs).Info("updating ip entry") if err := ipe.update(scopedLog, setName, rrs, lookupTime, dtype); err != nil { - return fmt.Errorf("failed to update ipEntry: %w", err) + return fmt.Errorf("failed to update IPEntry: %w", err) } c.fqdnToEntry[qname] = entry @@ -557,14 +557,14 @@ func updateNftSet( return nil } -func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *ipEntry) firewallv1.IPSet { +func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IPEntry) firewallv1.IPSet { ips := firewallv1.IPSet{ FQDN: fqdn, - SetName: entry.setName, + SetName: entry.SetName, IPs: []string{}, Version: version, } - for ip, expirationTime := range entry.ips { + for ip, expirationTime := range entry.IPs { if et, err := expirationTime.MarshalText(); err == nil { ip = ip + ", expiration time: " + string(et) } @@ -573,13 +573,13 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *ip return ips } -func createRenderIPSetFromIPEntry(version IPVersion, entry *ipEntry) RenderIPSet { +func createRenderIPSetFromIPEntry(version IPVersion, entry *IPEntry) RenderIPSet { var ips []string - for ip, _ := range entry.ips { + for ip, _ := range entry.IPs { ips = append(ips, ip) } return RenderIPSet{ - SetName: entry.setName, + SetName: entry.SetName, IPs: ips, Version: version, } diff --git a/pkg/dns/dnscache_test.go b/pkg/dns/dnscache_test.go index 1074fdb8..ef5bfe81 100644 --- a/pkg/dns/dnscache_test.go +++ b/pkg/dns/dnscache_test.go @@ -11,20 +11,20 @@ import ( func Test_GetSetsForFQDN(t *testing.T) { tests := []struct { name string - fqdnToEntry map[string]cacheEntry + fqdnToEntry map[string]CacheEntry expectedSets []string fqdnSelector firewallv1.FQDNSelector cachedSets []firewallv1.IPSet }{ { name: "get result for matchName", - fqdnToEntry: map[string]cacheEntry{ + fqdnToEntry: map[string]CacheEntry{ "test.com.": { - ipv4: &ipEntry{ - setName: "testv4", + IPv4: &IPEntry{ + SetName: "testv4", }, - ipv6: &ipEntry{ - setName: "testv6", + IPv6: &IPEntry{ + SetName: "testv6", }, }, }, @@ -35,37 +35,37 @@ func Test_GetSetsForFQDN(t *testing.T) { }, { name: "get result for matchPattern", - fqdnToEntry: map[string]cacheEntry{ + fqdnToEntry: map[string]CacheEntry{ "test.com.": { - ipv4: &ipEntry{ - setName: "testv4", + IPv4: &IPEntry{ + SetName: "testv4", }, - ipv6: &ipEntry{ - setName: "testv6", + IPv6: &IPEntry{ + SetName: "testv6", }, }, "test.io.": { - ipv4: &ipEntry{ - setName: "testiov4", + IPv4: &IPEntry{ + SetName: "testiov4", }, - ipv6: &ipEntry{ - setName: "testiov6", + IPv6: &IPEntry{ + SetName: "testiov6", }, }, "example.com.": { - ipv4: &ipEntry{ - setName: "examplev4", + IPv4: &IPEntry{ + SetName: "examplev4", }, - ipv6: &ipEntry{ - setName: "examplev6", + IPv6: &IPEntry{ + SetName: "examplev6", }, }, "second.example.com.": { - ipv4: &ipEntry{ - setName: "2examplev4", + IPv4: &IPEntry{ + SetName: "2examplev4", }, - ipv6: &ipEntry{ - setName: "2examplev6", + IPv6: &IPEntry{ + SetName: "2examplev6", }, }, }, @@ -76,13 +76,13 @@ func Test_GetSetsForFQDN(t *testing.T) { }, { name: "pattern from integration testing", - fqdnToEntry: map[string]cacheEntry{ + fqdnToEntry: map[string]CacheEntry{ "www.freechess.org.": { - ipv4: &ipEntry{ - setName: "testv4", + IPv4: &IPEntry{ + SetName: "testv4", }, - ipv6: &ipEntry{ - setName: "testv6", + IPv6: &IPEntry{ + SetName: "testv6", }, }, }, From 9f16a985b93bbd182292ca517b90e7aa95881d8d Mon Sep 17 00:00:00 2001 From: mreiger Date: Mon, 3 Mar 2025 18:27:36 +0100 Subject: [PATCH 26/71] Try to fix configmap creation --- pkg/dns/dnscache.go | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 6252cce5..cde29eb5 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -40,6 +40,7 @@ const ( // Configmap that holds the FQDN state fqdnStateConfigmapName = "fqdnstate" fqdnStateNamespace = "firewall" + fqdnStateConfigmapKey = "state" ) // RenderIPSet stores set info for rendering @@ -144,7 +145,7 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, return &c, nil } - if scm.Data["state"] == "" { + if scm.Data[fqdnStateConfigmapKey] == "" { return &c, nil } @@ -172,13 +173,23 @@ func (c *DNSCache) writeStateToConfigmap() error { Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace, }, - Data: map[string]string{ - "state": string(s), - }, } - c.log.V(4).Info("DEBUG created configmap", "scm", scm) - if err := c.shootClient.Get(c.ctx, nn, &v1.ConfigMap{}); err != nil { + found := true + err = c.shootClient.Get(c.ctx, nn, &scm) + if err != nil { + if !apierrors.IsNotFound(err) { + return err + } + c.log.V(4).Info("DEBUG configmap not found", "NamespacedName", nn) + found = false + } + + scm.Data[fqdnStateConfigmapKey] = string(s) + + c.log.V(4).Info("DEBUG configmap to write", "scm", scm) + + if !found { c.log.V(4).Info("DEBUG configmap not found, trying to create") if err := c.shootClient.Create(c.ctx, &scm, nil); err != nil { return err @@ -191,7 +202,7 @@ func (c *DNSCache) writeStateToConfigmap() error { if err := c.shootClient.Update(c.ctx, &scm); err != nil { return err } - c.log.V(4).Info("DEBUG configmap created", "scm", scm) + c.log.V(4).Info("DEBUG configmap updated", "scm", scm) return nil } From c9ebc624586a4d3b523a5821f979cdf396af41f7 Mon Sep 17 00:00:00 2001 From: mreiger Date: Mon, 3 Mar 2025 18:59:07 +0100 Subject: [PATCH 27/71] Avoid writing to nil map --- pkg/dns/dnscache.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index cde29eb5..ccc264e7 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -185,7 +185,9 @@ func (c *DNSCache) writeStateToConfigmap() error { found = false } - scm.Data[fqdnStateConfigmapKey] = string(s) + scm.Data = map[string]string{ + fqdnStateConfigmapKey: string(s), + } c.log.V(4).Info("DEBUG configmap to write", "scm", scm) From f1ab16cc27a0a35791558cf3179f7e4ec7c4e128 Mon Sep 17 00:00:00 2001 From: mreiger Date: Mon, 3 Mar 2025 19:57:31 +0100 Subject: [PATCH 28/71] Trying to fix configmap creation --- pkg/dns/dnscache.go | 49 +++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index ccc264e7..3f88a0b7 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "reflect" "regexp" "strings" "sync" @@ -168,43 +169,39 @@ func (c *DNSCache) writeStateToConfigmap() error { c.log.V(4).Info("DEBUG writing cache to configmap", "fqdnToEntry", s) nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} - scm := v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: fqdnStateConfigmapName, - Namespace: fqdnStateNamespace, - }, + meta := metav1.ObjectMeta{ + Name: fqdnStateConfigmapName, + Namespace: fqdnStateNamespace, } - found := true - err = c.shootClient.Get(c.ctx, nn, &scm) - if err != nil { - if !apierrors.IsNotFound(err) { - return err - } - c.log.V(4).Info("DEBUG configmap not found", "NamespacedName", nn) - found = false + var currentCm v1.ConfigMap + err = c.shootClient.Get(c.ctx, nn, ¤tCm) + if err != nil && !apierrors.IsNotFound(err) { + return err } - scm.Data = map[string]string{ - fqdnStateConfigmapKey: string(s), + scm := v1.ConfigMap{ + ObjectMeta: meta, + Data: map[string]string{ + fqdnStateConfigmapKey: string(s), + }, } - c.log.V(4).Info("DEBUG configmap to write", "scm", scm) - - if !found { - c.log.V(4).Info("DEBUG configmap not found, trying to create") - if err := c.shootClient.Create(c.ctx, &scm, nil); err != nil { + if apierrors.IsNotFound(err) { + c.log.V(4).Info("DEBUG configmap not found, trying to create configmap", "NamespacedName", nn, "configmap to create", scm) + err = c.shootClient.Create(c.ctx, &scm) + if err != nil { return err } - c.log.V(4).Info("DEBUG configmap created", "scm", scm) - return nil } - c.log.V(4).Info("DEBUG configmap found, trying to update") - if err := c.shootClient.Update(c.ctx, &scm); err != nil { - return err + if !reflect.DeepEqual(currentCm.Data, scm.Data) { + currentCm.Data = scm.Data + err = c.shootClient.Update(c.ctx, ¤tCm) + if err != nil { + return err + } } - c.log.V(4).Info("DEBUG configmap updated", "scm", scm) return nil } From 730269148963017b1fc6d82f3cf48155915e2d47 Mon Sep 17 00:00:00 2001 From: mreiger Date: Tue, 4 Mar 2025 17:25:07 +0100 Subject: [PATCH 29/71] Try pointers; more debug --- pkg/dns/dnscache.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 3f88a0b7..e3117a84 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -136,21 +136,24 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, } nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} - scm := v1.ConfigMap{} + scm := &v1.ConfigMap{} - err := shootClient.Get(ctx, nn, &scm) + err := shootClient.Get(ctx, nn, scm) if err != nil && !apierrors.IsNotFound(err) { + c.log.V(4).Info("DEBUG error reading fqndstate configmap") return nil, err } if scm.Data == nil { + c.log.V(4).Info("DEBUG cm contains no data", "cm", scm) return &c, nil } if scm.Data[fqdnStateConfigmapKey] == "" { + c.log.V(4).Info("DEBUG cm does not contain the right key", "cm", scm, "key", fqdnStateConfigmapKey) return &c, nil } - err = json.Unmarshal([]byte(scm.Data["state"]), &c.fqdnToEntry) + err = json.Unmarshal([]byte(scm.Data[fqdnStateConfigmapKey]), &c.fqdnToEntry) if err != nil { return nil, err } @@ -174,13 +177,13 @@ func (c *DNSCache) writeStateToConfigmap() error { Namespace: fqdnStateNamespace, } - var currentCm v1.ConfigMap - err = c.shootClient.Get(c.ctx, nn, ¤tCm) + c.log.V(4).Info("DEBUG lookint for configmap", "namespacedname", nn) + var currentCm *v1.ConfigMap + err = c.shootClient.Get(c.ctx, nn, currentCm) if err != nil && !apierrors.IsNotFound(err) { return err } - - scm := v1.ConfigMap{ + scm := &v1.ConfigMap{ ObjectMeta: meta, Data: map[string]string{ fqdnStateConfigmapKey: string(s), @@ -189,15 +192,16 @@ func (c *DNSCache) writeStateToConfigmap() error { if apierrors.IsNotFound(err) { c.log.V(4).Info("DEBUG configmap not found, trying to create configmap", "NamespacedName", nn, "configmap to create", scm) - err = c.shootClient.Create(c.ctx, &scm) + err = c.shootClient.Create(c.ctx, scm) if err != nil { return err } } + c.log.V(4).Info("DEBUG trying to updatecm", "current cm", currentCm, "cm", scm) if !reflect.DeepEqual(currentCm.Data, scm.Data) { currentCm.Data = scm.Data - err = c.shootClient.Update(c.ctx, ¤tCm) + err = c.shootClient.Update(c.ctx, currentCm) if err != nil { return err } From 72714f1a9e01aad8612513c1d3822e7cc9583f2d Mon Sep 17 00:00:00 2001 From: mreiger Date: Tue, 4 Mar 2025 17:48:35 +0100 Subject: [PATCH 30/71] more pointer fixes? --- pkg/dns/dnscache.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index e3117a84..3c0a2fe2 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -177,8 +177,8 @@ func (c *DNSCache) writeStateToConfigmap() error { Namespace: fqdnStateNamespace, } - c.log.V(4).Info("DEBUG lookint for configmap", "namespacedname", nn) - var currentCm *v1.ConfigMap + c.log.V(4).Info("DEBUG looking for configmap", "namespacedname", nn) + currentCm := &v1.ConfigMap{} err = c.shootClient.Get(c.ctx, nn, currentCm) if err != nil && !apierrors.IsNotFound(err) { return err @@ -462,7 +462,7 @@ func (c *DNSCache) Update(lookupTime time.Time, qname string, msg *dnsgo.Msg, fq if ipEntriesUpdated { if err := c.writeStateToConfigmap(); err != nil { - c.log.V(4).Info("DEBUG could not write updated DNS cache to state configmap", "configmap", fqdnStateConfigmapName, "namespace", fqdnStateNamespace) + c.log.V(4).Info("DEBUG could not write updated DNS cache to state configmap", "configmap", fqdnStateConfigmapName, "namespace", fqdnStateNamespace, "error", err) } } From 20ba22baccf754266e7e708211caed4a0cd4bcd5 Mon Sep 17 00:00:00 2001 From: mreiger Date: Tue, 4 Mar 2025 18:41:10 +0100 Subject: [PATCH 31/71] try for better configmap initialisation --- pkg/dns/dnscache.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 3c0a2fe2..71f83b8c 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -177,19 +177,19 @@ func (c *DNSCache) writeStateToConfigmap() error { Namespace: fqdnStateNamespace, } - c.log.V(4).Info("DEBUG looking for configmap", "namespacedname", nn) currentCm := &v1.ConfigMap{} - err = c.shootClient.Get(c.ctx, nn, currentCm) - if err != nil && !apierrors.IsNotFound(err) { - return err - } + cmData := map[string]string{} + cmData[fqdnStateConfigmapKey] = string(s) scm := &v1.ConfigMap{ ObjectMeta: meta, - Data: map[string]string{ - fqdnStateConfigmapKey: string(s), - }, + Data: cmData, } + c.log.V(4).Info("DEBUG looking for configmap", "namespacedname", nn) + err = c.shootClient.Get(c.ctx, nn, currentCm) + if err != nil && !apierrors.IsNotFound(err) { + return err + } if apierrors.IsNotFound(err) { c.log.V(4).Info("DEBUG configmap not found, trying to create configmap", "NamespacedName", nn, "configmap to create", scm) err = c.shootClient.Create(c.ctx, scm) From 3cce33a3d4e7ccfce5ff9307104fda5880b29528 Mon Sep 17 00:00:00 2001 From: mreiger Date: Wed, 5 Mar 2025 16:25:59 +0100 Subject: [PATCH 32/71] Try and debug configmap handling with a simple static configmap --- pkg/dns/dnscache.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 71f83b8c..447a530b 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -171,6 +171,45 @@ func (c *DNSCache) writeStateToConfigmap() error { } c.log.V(4).Info("DEBUG writing cache to configmap", "fqdnToEntry", s) + // debugging: Try to read, create, update simple configmap. + dnn := types.NamespacedName{Name: "dcm", Namespace: fqdnStateNamespace} + cdcm := v1.ConfigMap{} + + dcm := v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dcm", + Namespace: fqdnStateNamespace, + }, + Data: map[string]string{ + "testkey": "testvalue", + }, + } + + c.log.V(4).Info("DEBUG looking for debug configmap", "namespacedname", dnn) + err = c.shootClient.Get(c.ctx, dnn, &cdcm) + if err != nil && !apierrors.IsNotFound(err) { + c.log.V(4).Info("DEBUG error reading debug configmap", "namespacedname", dnn, "error", err) + return err + } + + if apierrors.IsNotFound(err) { + c.log.V(4).Info("DEBUG debug configmap not found, trying to create", "namespacedname", dnn) + err = c.shootClient.Create(c.ctx, &dcm) + if err != nil { + c.log.V(4).Info("DEBUG error creating debug configmap", "configmap", dcm, "error", err) + return err + } + } else { + c.log.V(4).Info("DEBUG debug configmap found, trying to update", "current configmap", cdcm, "configmap", dcm) + err = c.shootClient.Update(c.ctx, &dcm) + if err != nil { + c.log.V(4).Info("DEBUG error updating debug configmap", "configmap", dcm, "error", err) + return err + } + } + + // end debugging code + nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} meta := metav1.ObjectMeta{ Name: fqdnStateConfigmapName, From 4f069ccd67b1a43c2f0e044987d85a702ade4268 Mon Sep 17 00:00:00 2001 From: mreiger Date: Wed, 5 Mar 2025 16:45:58 +0100 Subject: [PATCH 33/71] forgot to initialize context --- pkg/dns/dnscache.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 447a530b..c980280b 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -131,6 +131,7 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, setNames: map[string]struct{}{}, dnsServerAddr: dns, shootClient: shootClient, + ctx: ctx, ipv4Enabled: ipv4Enabled, ipv6Enabled: ipv6Enabled, } From 65374b5e79dc9a916e3798e6ea833caac0a01928 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 6 Mar 2025 19:20:52 +0100 Subject: [PATCH 34/71] Marshal state to yaml for better readability --- pkg/dns/dnscache.go | 65 +++++++++------------------------------------ 1 file changed, 12 insertions(+), 53 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index c980280b..ab885a0a 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -4,7 +4,6 @@ import ( "context" "crypto/md5" //nolint:gosec "encoding/hex" - "encoding/json" "fmt" "reflect" "regexp" @@ -18,10 +17,10 @@ import ( dnsgo "github.com/miekg/dns" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" ) @@ -145,7 +144,7 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, return nil, err } if scm.Data == nil { - c.log.V(4).Info("DEBUG cm contains no data", "cm", scm) + c.log.V(4).Info("DEBUG cm not found or contains no data", "cm", scm) return &c, nil } @@ -154,7 +153,8 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, return &c, nil } - err = json.Unmarshal([]byte(scm.Data[fqdnStateConfigmapKey]), &c.fqdnToEntry) + c.log.V(4).Info("DEBUG state stored in cm, trying to unmarshal", fqdnStateConfigmapKey, scm.Data[fqdnStateConfigmapKey]) + err = yaml.UnmarshalStrict([]byte(scm.Data[fqdnStateConfigmapKey]), &c.fqdnToEntry) if err != nil { return nil, err } @@ -163,7 +163,7 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, // writeStateToConfigmap writes the whole DNS cache to the state configmap func (c *DNSCache) writeStateToConfigmap() error { - s, err := json.Marshal(c.fqdnToEntry) + s, err := yaml.Marshal(c.fqdnToEntry) if err != nil { return err } @@ -172,59 +172,18 @@ func (c *DNSCache) writeStateToConfigmap() error { } c.log.V(4).Info("DEBUG writing cache to configmap", "fqdnToEntry", s) - // debugging: Try to read, create, update simple configmap. - dnn := types.NamespacedName{Name: "dcm", Namespace: fqdnStateNamespace} - cdcm := v1.ConfigMap{} - - dcm := v1.ConfigMap{ + currentCm := &v1.ConfigMap{} + nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} + scm := &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "dcm", + Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace, }, Data: map[string]string{ - "testkey": "testvalue", + fqdnStateConfigmapKey: string(s), }, } - c.log.V(4).Info("DEBUG looking for debug configmap", "namespacedname", dnn) - err = c.shootClient.Get(c.ctx, dnn, &cdcm) - if err != nil && !apierrors.IsNotFound(err) { - c.log.V(4).Info("DEBUG error reading debug configmap", "namespacedname", dnn, "error", err) - return err - } - - if apierrors.IsNotFound(err) { - c.log.V(4).Info("DEBUG debug configmap not found, trying to create", "namespacedname", dnn) - err = c.shootClient.Create(c.ctx, &dcm) - if err != nil { - c.log.V(4).Info("DEBUG error creating debug configmap", "configmap", dcm, "error", err) - return err - } - } else { - c.log.V(4).Info("DEBUG debug configmap found, trying to update", "current configmap", cdcm, "configmap", dcm) - err = c.shootClient.Update(c.ctx, &dcm) - if err != nil { - c.log.V(4).Info("DEBUG error updating debug configmap", "configmap", dcm, "error", err) - return err - } - } - - // end debugging code - - nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} - meta := metav1.ObjectMeta{ - Name: fqdnStateConfigmapName, - Namespace: fqdnStateNamespace, - } - - currentCm := &v1.ConfigMap{} - cmData := map[string]string{} - cmData[fqdnStateConfigmapKey] = string(s) - scm := &v1.ConfigMap{ - ObjectMeta: meta, - Data: cmData, - } - c.log.V(4).Info("DEBUG looking for configmap", "namespacedname", nn) err = c.shootClient.Get(c.ctx, nn, currentCm) if err != nil && !apierrors.IsNotFound(err) { @@ -238,7 +197,7 @@ func (c *DNSCache) writeStateToConfigmap() error { } } - c.log.V(4).Info("DEBUG trying to updatecm", "current cm", currentCm, "cm", scm) + c.log.V(4).Info("DEBUG trying to update cm", "current cm", currentCm, "cm", scm) if !reflect.DeepEqual(currentCm.Data, scm.Data) { currentCm.Data = scm.Data err = c.shootClient.Update(c.ctx, currentCm) From ffe200ce5e5078d12cd6845ca30788e84c92e972 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 6 Mar 2025 19:36:03 +0100 Subject: [PATCH 35/71] Discard unparseable state configmap --- pkg/dns/dnscache.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index ab885a0a..f065fce6 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -156,7 +156,8 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, c.log.V(4).Info("DEBUG state stored in cm, trying to unmarshal", fqdnStateConfigmapKey, scm.Data[fqdnStateConfigmapKey]) err = yaml.UnmarshalStrict([]byte(scm.Data[fqdnStateConfigmapKey]), &c.fqdnToEntry) if err != nil { - return nil, err + c.log.V(4).Info("DEBUG could not unmarshal state from configmap, discarding content.", "error", err) + c.fqdnToEntry = map[string]CacheEntry{} } return &c, nil } From f80ae66bb4d32b63976e5b0c3f69080e52bf0e80 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 7 Mar 2025 11:26:40 +0100 Subject: [PATCH 36/71] Use UTC timezone for expiration times --- pkg/dns/dnscache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index f065fce6..5e934157 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -355,7 +355,7 @@ func (c *DNSCache) loadDataFromDNSServer(fqdns []string) error { return fmt.Errorf("failed to get DNS data about fqdn %s: %w", fqdns[0], err) } c.log.V(4).Info("DEBUG dnscache loadDataFromDNSServer function calling Update function", "answer", in, "fqdns", fqdns) - if _, err = c.Update(time.Now(), qname, in, fqdns); err != nil { + if _, err = c.Update(time.Now().UTC(), qname, in, fqdns); err != nil { return fmt.Errorf("failed to update DNS data for fqdn %s: %w", fqdns[0], err) } } From 23ffccb5231c8d26f999f1370674c6a7f9d49617 Mon Sep 17 00:00:00 2001 From: mreiger Date: Fri, 7 Mar 2025 13:04:18 +0100 Subject: [PATCH 37/71] sanitize logging --- main.go | 9 +++++++-- pkg/dns/dnscache.go | 12 ++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index 19e77488..d7da7467 100644 --- a/main.go +++ b/main.go @@ -91,7 +91,13 @@ func main() { return } - jsonHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug}) + var sll slog.Level + var err error + err = sll.UnmarshalText([]byte(logLevel)) + if err != nil { + sll = slog.LevelInfo + } + jsonHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: sll}) l := slog.New(jsonHandler) ctrl.SetLogger(logr.FromSlogHandler(jsonHandler)) @@ -105,7 +111,6 @@ func main() { // FIXME validation and controller start should be refactored into own func which returns error // instead Fatalw or Error and panic here. - var err error if firewallName == "" { firewallName, err = os.Hostname() if err != nil { diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 5e934157..1d49eb4d 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -140,23 +140,23 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, err := shootClient.Get(ctx, nn, scm) if err != nil && !apierrors.IsNotFound(err) { - c.log.V(4).Info("DEBUG error reading fqndstate configmap") + c.log.Error(err, "error reading fqndstate configmap") return nil, err } if scm.Data == nil { - c.log.V(4).Info("DEBUG cm not found or contains no data", "cm", scm) + c.log.V(4).Info("DEBUG fqdnstate cm not found or contains no data", "cm", scm) return &c, nil } if scm.Data[fqdnStateConfigmapKey] == "" { - c.log.V(4).Info("DEBUG cm does not contain the right key", "cm", scm, "key", fqdnStateConfigmapKey) + c.log.V(4).Info("DEBUG fqdnstate cm does not contain the right key", "cm", scm, "key", fqdnStateConfigmapKey) return &c, nil } - c.log.V(4).Info("DEBUG state stored in cm, trying to unmarshal", fqdnStateConfigmapKey, scm.Data[fqdnStateConfigmapKey]) + c.log.V(4).Info("DEBUG state stored in fqdnstate cm, trying to unmarshal", fqdnStateConfigmapKey, scm.Data[fqdnStateConfigmapKey]) err = yaml.UnmarshalStrict([]byte(scm.Data[fqdnStateConfigmapKey]), &c.fqdnToEntry) if err != nil { - c.log.V(4).Info("DEBUG could not unmarshal state from configmap, discarding content.", "error", err) + c.log.Info("could not unmarshal state from fqdnstate configmap, ignoring content.", "error", err) c.fqdnToEntry = map[string]CacheEntry{} } return &c, nil @@ -171,7 +171,7 @@ func (c *DNSCache) writeStateToConfigmap() error { if s == nil { return nil } - c.log.V(4).Info("DEBUG writing cache to configmap", "fqdnToEntry", s) + c.log.V(4).Info("DEBUG writing cache to configmap", "fqdnToEntry", string(s)) currentCm := &v1.ConfigMap{} nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} From 9948b4b54d2e17b00cb542c3765a746fb924418f Mon Sep 17 00:00:00 2001 From: mreiger Date: Mon, 10 Mar 2025 15:37:18 +0100 Subject: [PATCH 38/71] Debug output to figure out error messages when writing cwnp stati --- controllers/clusterwidenetworkpolicy_controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 7b3a2c6d..ffe82014 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -113,8 +113,10 @@ func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, _ ct } if updated { + r.Log.V(4).Info("DEBUG CWNP reconcile, cwnps updated, trying to update cwnp stati.", "cwnps", cwnps) for _, i := range cwnps.Items { o := i + r.Log.V(4).Info("DEBUG CWNP reconcile, trying to update cwnp status.", "cwnp", o) if err := r.updateCWNPState(ctx, o, firewallv1.PolicyDeploymentStateDeployed, ""); err != nil { return ctrl.Result{}, err } @@ -244,6 +246,7 @@ func (r *ClusterwideNetworkPolicyReconciler) allowedCWNPs(ctx context.Context, c func (r *ClusterwideNetworkPolicyReconciler) updateCWNPState(ctx context.Context, cwnp firewallv1.ClusterwideNetworkPolicy, state firewallv1.PolicyDeploymentState, msg string) error { cwnp.Status.Message = msg cwnp.Status.State = state + r.Log.V(4).Info("DEBUG CWNP reconcile, function updateCWNPState, trying to update status.", "cwnp", cwnp) if err := r.ShootClient.Status().Update(ctx, &cwnp); err != nil { return fmt.Errorf("failed to update status of CWNP %q to %q: %w", cwnp.Name, state, err) From 90c8a2cc80fd9dc715d43d26814dcc1074ae21a9 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Mon, 23 Jun 2025 13:56:15 +0200 Subject: [PATCH 39/71] add more debug logging --- controllers/clusterwidenetworkpolicy_controller.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index ffe82014..bc15f1fa 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -69,7 +69,8 @@ func (r *ClusterwideNetworkPolicyReconciler) SetupWithManager(mgr ctrl.Manager) // +kubebuilder:rbac:groups=metal-stack.io,resources=clusterwidenetworkpolicies,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metal-stack.io,resources=clusterwidenetworkpolicies/status,verbs=get;update;patch -func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { +func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + r.Log.V(4).Info("DEBUG CWNP Reconcile", "request", req, "time", time.Now()) var cwnps firewallv1.ClusterwideNetworkPolicyList if err := r.ShootClient.List(ctx, &cwnps, client.InNamespace(firewallv1.ClusterwideNetworkPolicyNamespace)); err != nil { return ctrl.Result{}, err @@ -249,7 +250,7 @@ func (r *ClusterwideNetworkPolicyReconciler) updateCWNPState(ctx context.Context r.Log.V(4).Info("DEBUG CWNP reconcile, function updateCWNPState, trying to update status.", "cwnp", cwnp) if err := r.ShootClient.Status().Update(ctx, &cwnp); err != nil { - return fmt.Errorf("failed to update status of CWNP %q to %q: %w", cwnp.Name, state, err) + return fmt.Errorf("failed to update status of CWNP %q to %q with message %s: %w", cwnp.Name, state, msg, err) } return nil } From 8073a9c410ca796bff5b905cf9b95cc5c39719d5 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 11:29:26 +0200 Subject: [PATCH 40/71] remove logs --- controllers/clusterwidenetworkpolicy_controller.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index bc15f1fa..9ab829b8 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -70,7 +70,6 @@ func (r *ClusterwideNetworkPolicyReconciler) SetupWithManager(mgr ctrl.Manager) // +kubebuilder:rbac:groups=metal-stack.io,resources=clusterwidenetworkpolicies/status,verbs=get;update;patch func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log.V(4).Info("DEBUG CWNP Reconcile", "request", req, "time", time.Now()) var cwnps firewallv1.ClusterwideNetworkPolicyList if err := r.ShootClient.List(ctx, &cwnps, client.InNamespace(firewallv1.ClusterwideNetworkPolicyNamespace)); err != nil { return ctrl.Result{}, err @@ -220,7 +219,6 @@ func (r *ClusterwideNetworkPolicyReconciler) allowedCWNPs(ctx context.Context, c } for _, cwnp := range cwnps { - cwnp := cwnp oke, err := r.validateCWNPEgressTargetPrefix(cwnp, egressSet) if err != nil { return nil, err @@ -250,7 +248,7 @@ func (r *ClusterwideNetworkPolicyReconciler) updateCWNPState(ctx context.Context r.Log.V(4).Info("DEBUG CWNP reconcile, function updateCWNPState, trying to update status.", "cwnp", cwnp) if err := r.ShootClient.Status().Update(ctx, &cwnp); err != nil { - return fmt.Errorf("failed to update status of CWNP %q to %q with message %s: %w", cwnp.Name, state, msg, err) + return fmt.Errorf("failed to update status of CWNP %q to %q: %w", cwnp.Name, state, err) } return nil } From 633501b15445c072005528668765cc261b79b8f3 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 11:40:36 +0200 Subject: [PATCH 41/71] use map to represent ips with expiration time --- api/v1/clusterwidenetworkpolicy_types.go | 19 ++++++------------- api/v1/zz_generated.deepcopy.go | 8 +++++--- ...l-stack.io_clusterwidenetworkpolicies.yaml | 8 +++----- pkg/dns/dnscache.go | 7 +++---- 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index f5362514..2fec05a4 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -151,23 +151,16 @@ type FQDNSelector struct { MatchPattern string `json:"matchPattern,omitempty"` } -// IPSet stores set name association to IP addresses -// type IPSet struct { -// FQDN string `json:"fqdn,omitempty"` -// SetName string `json:"setName,omitempty"` -// IPs map[string]metav1.Time `json:"ips,omitempty"` -// Version IPVersion `json:"version,omitempty"` -// } - // IPSet stores set name association to IP addresses type IPSet struct { - FQDN string `json:"fqdn,omitempty"` - SetName string `json:"setName,omitempty"` - IPs []string `json:"ips,omitempty"` - ExpirationTime metav1.Time `json:"expirationTime,omitempty"` - Version IPVersion `json:"version,omitempty"` + FQDN string `json:"fqdn,omitempty"` + SetName string `json:"setName,omitempty"` + IPs IPs `json:"ips,omitempty"` + Version IPVersion `json:"version,omitempty"` } +type IPs map[string]metav1.Time + func (l *ClusterwideNetworkPolicyList) GetFQDNs() []FQDNSelector { s := []FQDNSelector{} for _, i := range l.Items { diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index c9863969..86663767 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -6,6 +6,7 @@ package v1 import ( networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -154,10 +155,11 @@ func (in *IPSet) DeepCopyInto(out *IPSet) { *out = *in if in.IPs != nil { in, out := &in.IPs, &out.IPs - *out = make([]string, len(*in)) - copy(*out, *in) + *out = make(map[string]metav1.Time, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } } - in.ExpirationTime.DeepCopyInto(&out.ExpirationTime) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPSet. diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index c92a908e..07ca0419 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -243,15 +243,13 @@ spec: items: description: IPSet stores set name association to IP addresses properties: - expirationTime: - format: date-time - type: string fqdn: type: string ips: - items: + additionalProperties: + format: date-time type: string - type: array + type: object setName: type: string version: diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 1d49eb4d..40cc6127 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -575,14 +575,13 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IP ips := firewallv1.IPSet{ FQDN: fqdn, SetName: entry.SetName, - IPs: []string{}, + IPs: make(firewallv1.IPs), Version: version, } for ip, expirationTime := range entry.IPs { - if et, err := expirationTime.MarshalText(); err == nil { - ip = ip + ", expiration time: " + string(et) + ips.IPs[ip] = metav1.Time{ + Time: expirationTime, } - ips.IPs = append(ips.IPs, ip) } return ips } From 2abc897685a7521adf1943332af635f3d2ebbbba Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 11:40:50 +0200 Subject: [PATCH 42/71] linter hints and warnings --- pkg/dns/dns_proxy_handler.go | 2 +- pkg/dns/dnscache.go | 2 +- pkg/nftables/firewall.go | 2 -- pkg/nftables/networkpolicy.go | 5 +++-- pkg/nftables/service.go | 6 +++--- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/dns/dns_proxy_handler.go b/pkg/dns/dns_proxy_handler.go index 62fdfb01..133562a8 100644 --- a/pkg/dns/dns_proxy_handler.go +++ b/pkg/dns/dns_proxy_handler.go @@ -141,7 +141,7 @@ func getUpdateCacheFunc(log logr.Logger, cache *DNSCache) func(lookupTime time.T if response.Response && response.Rcode == dnsgo.RcodeSuccess { scopedLog := log.WithValues(reqIdLogField, response.Id) var qname string - if response.Question != nil && len(response.Question) > 0 { + if len(response.Question) > 0 { qname = strings.ToLower(response.Question[0].Name) } log.V(4).Info("DEBUG dnsproxyhandler function getUpdateCacheFunc updating DNS cache", "queried name", qname, "dns response", response) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 40cc6127..163ee07f 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -588,7 +588,7 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IP func createRenderIPSetFromIPEntry(version IPVersion, entry *IPEntry) RenderIPSet { var ips []string - for ip, _ := range entry.IPs { + for ip := range entry.IPs { ips = append(ips, ip) } return RenderIPSet{ diff --git a/pkg/nftables/firewall.go b/pkg/nftables/firewall.go index d4097104..89d020b8 100644 --- a/pkg/nftables/firewall.go +++ b/pkg/nftables/firewall.go @@ -197,12 +197,10 @@ func getConfiguredIPs(networkID string) []string { } var ips []string for _, nw := range c.Networks { - nw := nw if nw.Networkid == nil || *nw.Networkid != networkID { continue } for _, ip := range nw.Ips { - ip := ip ips = append(ips, ip) } } diff --git a/pkg/nftables/networkpolicy.go b/pkg/nftables/networkpolicy.go index b29206bb..2edb9439 100644 --- a/pkg/nftables/networkpolicy.go +++ b/pkg/nftables/networkpolicy.go @@ -158,9 +158,10 @@ func calculatePorts(ports []networkingv1.NetworkPolicyPort) (tcpPorts, udpPorts if p.EndPort != nil { portStr = fmt.Sprintf("%s-%d", p.Port, *p.EndPort) } - if proto == "tcp" { + switch proto { + case "tcp": tcpPorts = append(tcpPorts, portStr) - } else if proto == "udp" { + case "udp": udpPorts = append(udpPorts, portStr) } } diff --git a/pkg/nftables/service.go b/pkg/nftables/service.go index 1b46fe7f..437aa64c 100644 --- a/pkg/nftables/service.go +++ b/pkg/nftables/service.go @@ -44,11 +44,11 @@ func serviceRules(svc corev1.Service, allowed *netipx.IPSet, logAcceptedConnecti tcpPorts := []string{} udpPorts := []string{} for _, p := range svc.Spec.Ports { - p := p proto := proto(&p.Protocol) - if proto == "tcp" { + switch proto { + case "tcp": tcpPorts = append(tcpPorts, fmt.Sprint(p.Port)) - } else if proto == "udp" { + case "udp": udpPorts = append(udpPorts, fmt.Sprint(p.Port)) } } From 06428b6587a29241ba389cddf7eb281c27d35b04 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 13:23:06 +0200 Subject: [PATCH 43/71] add restoreSets func --- pkg/dns/dnscache.go | 71 ++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 163ee07f..3108faf2 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -211,7 +211,7 @@ func (c *DNSCache) writeStateToConfigmap() error { // getSetsForFQDN returns sets for FQDN selector func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firewallv1.IPSet) (result []firewallv1.IPSet) { - // c.restoreSets(fqdnSets) + c.restoreSets(fqdnSets) sets := map[string]firewallv1.IPSet{} if fqdn.MatchName != "" { @@ -268,42 +268,39 @@ func (c *DNSCache) updateDNSServerAddr(addr string) { } // restoreSets add missing sets from FQDNSelector.Sets -// func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { -// for _, s := range fqdnSets { -// // Add cache entries from fqdn.Sets if missing -// c.Lock() -// if _, ok := c.setNames[s.SetName]; !ok { -// c.setNames[s.SetName] = struct{}{} -// entry, exists := c.fqdnToEntry[s.FQDN] -// if !exists { -// entry = CacheEntry{} -// } -// -// ipe := &IPEntry{ -// setName: s.SetName, -// } -// for _, ip := range s.IPs { -// ipa, _, _ := strings.Cut(ip, ",") -// expirationTime := time.Now() -// if _, ets, found := strings.Cut(ip, ": "); found { -// if err := expirationTime.UnmarshalText([]byte(ets)); err != nil { -// expirationTime = time.Now() -// } -// } -// ipe.IPs[ipa] = expirationTime -// } -// switch s.Version { -// case firewallv1.IPv4: -// entry.ipv4 = ipe -// case firewallv1.IPv6: -// entry.ipv6 = ipe -// } -// -// c.fqdnToEntry[s.FQDN] = entry -// } -// c.Unlock() -// } -// } +func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { + for _, s := range fqdnSets { + // Add cache entries from fqdn.Sets if missing + c.Lock() + if _, ok := c.setNames[s.SetName]; !ok { + c.setNames[s.SetName] = struct{}{} + entry, exists := c.fqdnToEntry[s.FQDN] + if !exists { + entry = CacheEntry{} + } + + ipe := &IPEntry{ + SetName: s.SetName, + } + + ips := make(map[string]time.Time) + for ip, exp := range s.IPs { + ips[ip] = exp.Time + } + ipe.IPs = ips + + switch s.Version { + case firewallv1.IPv4: + entry.IPv4 = ipe + case firewallv1.IPv6: + entry.IPv6 = ipe + } + + c.fqdnToEntry[s.FQDN] = entry + } + c.Unlock() + } +} // getSetNameForFQDN returns FQDN set data func (c *DNSCache) getSetNameForFQDN(fqdn string) (result []firewallv1.IPSet) { From 4563806598c7aff622a9382066626552f2f110fb Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 14:12:02 +0200 Subject: [PATCH 44/71] make --- api/v1/zz_generated.deepcopy.go | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 86663767..6395c55d 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -6,7 +6,6 @@ package v1 import ( networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -155,7 +154,7 @@ func (in *IPSet) DeepCopyInto(out *IPSet) { *out = *in if in.IPs != nil { in, out := &in.IPs, &out.IPs - *out = make(map[string]metav1.Time, len(*in)) + *out = make(IPs, len(*in)) for key, val := range *in { (*out)[key] = *val.DeepCopy() } @@ -172,6 +171,27 @@ func (in *IPSet) DeepCopy() *IPSet { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in IPs) DeepCopyInto(out *IPs) { + { + in := &in + *out = make(IPs, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPs. +func (in IPs) DeepCopy() IPs { + if in == nil { + return nil + } + out := new(IPs) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IngressRule) DeepCopyInto(out *IngressRule) { *out = *in From 42f4be10e33baff957f7a7da20c62bd8ea94f34a Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 14:25:33 +0200 Subject: [PATCH 45/71] Revert "use map to represent ips with expiration time" This reverts commit 633501b15445c072005528668765cc261b79b8f3. --- api/v1/clusterwidenetworkpolicy_types.go | 19 +++++++++++++------ ...l-stack.io_clusterwidenetworkpolicies.yaml | 8 +++++--- pkg/dns/dnscache.go | 7 ++++--- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index 2fec05a4..f5362514 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -151,16 +151,23 @@ type FQDNSelector struct { MatchPattern string `json:"matchPattern,omitempty"` } +// IPSet stores set name association to IP addresses +// type IPSet struct { +// FQDN string `json:"fqdn,omitempty"` +// SetName string `json:"setName,omitempty"` +// IPs map[string]metav1.Time `json:"ips,omitempty"` +// Version IPVersion `json:"version,omitempty"` +// } + // IPSet stores set name association to IP addresses type IPSet struct { - FQDN string `json:"fqdn,omitempty"` - SetName string `json:"setName,omitempty"` - IPs IPs `json:"ips,omitempty"` - Version IPVersion `json:"version,omitempty"` + FQDN string `json:"fqdn,omitempty"` + SetName string `json:"setName,omitempty"` + IPs []string `json:"ips,omitempty"` + ExpirationTime metav1.Time `json:"expirationTime,omitempty"` + Version IPVersion `json:"version,omitempty"` } -type IPs map[string]metav1.Time - func (l *ClusterwideNetworkPolicyList) GetFQDNs() []FQDNSelector { s := []FQDNSelector{} for _, i := range l.Items { diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index 07ca0419..c92a908e 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -243,13 +243,15 @@ spec: items: description: IPSet stores set name association to IP addresses properties: + expirationTime: + format: date-time + type: string fqdn: type: string ips: - additionalProperties: - format: date-time + items: type: string - type: object + type: array setName: type: string version: diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 3108faf2..0e6db6e2 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -572,13 +572,14 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IP ips := firewallv1.IPSet{ FQDN: fqdn, SetName: entry.SetName, - IPs: make(firewallv1.IPs), + IPs: []string{}, Version: version, } for ip, expirationTime := range entry.IPs { - ips.IPs[ip] = metav1.Time{ - Time: expirationTime, + if et, err := expirationTime.MarshalText(); err == nil { + ip = ip + ", expiration time: " + string(et) } + ips.IPs = append(ips.IPs, ip) } return ips } From c4ea25c0d8648e703a0e42bcdae23de4016beb86 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 15:03:33 +0200 Subject: [PATCH 46/71] go back to []string instead of map for ip set expiration --- api/v1/clusterwidenetworkpolicy_types.go | 8 ------- api/v1/zz_generated.deepcopy.go | 28 +++--------------------- pkg/dns/dnscache.go | 13 +++++++---- 3 files changed, 12 insertions(+), 37 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index f5362514..7a7b32c5 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -151,14 +151,6 @@ type FQDNSelector struct { MatchPattern string `json:"matchPattern,omitempty"` } -// IPSet stores set name association to IP addresses -// type IPSet struct { -// FQDN string `json:"fqdn,omitempty"` -// SetName string `json:"setName,omitempty"` -// IPs map[string]metav1.Time `json:"ips,omitempty"` -// Version IPVersion `json:"version,omitempty"` -// } - // IPSet stores set name association to IP addresses type IPSet struct { FQDN string `json:"fqdn,omitempty"` diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 6395c55d..c9863969 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -154,11 +154,10 @@ func (in *IPSet) DeepCopyInto(out *IPSet) { *out = *in if in.IPs != nil { in, out := &in.IPs, &out.IPs - *out = make(IPs, len(*in)) - for key, val := range *in { - (*out)[key] = *val.DeepCopy() - } + *out = make([]string, len(*in)) + copy(*out, *in) } + in.ExpirationTime.DeepCopyInto(&out.ExpirationTime) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPSet. @@ -171,27 +170,6 @@ func (in *IPSet) DeepCopy() *IPSet { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in IPs) DeepCopyInto(out *IPs) { - { - in := &in - *out = make(IPs, len(*in)) - for key, val := range *in { - (*out)[key] = *val.DeepCopy() - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPs. -func (in IPs) DeepCopy() IPs { - if in == nil { - return nil - } - out := new(IPs) - in.DeepCopyInto(out) - return *out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IngressRule) DeepCopyInto(out *IngressRule) { *out = *in diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 0e6db6e2..92108031 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -283,11 +283,16 @@ func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { SetName: s.SetName, } - ips := make(map[string]time.Time) - for ip, exp := range s.IPs { - ips[ip] = exp.Time + for _, ip := range s.IPs { + ipa, _, _ := strings.Cut(ip, ",") + expirationTime := time.Now() + if _, ets, found := strings.Cut(ip, ": "); found { + if err := expirationTime.UnmarshalText([]byte(ets)); err != nil { + expirationTime = time.Now() + } + } + ipe.IPs[ipa] = expirationTime } - ipe.IPs = ips switch s.Version { case firewallv1.IPv4: From dce28bc2e6a38e5595ffd66b0cae1b561d0f749d Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 15:16:39 +0200 Subject: [PATCH 47/71] revert condition in add and update ips --- pkg/dns/dnscache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 92108031..64ca098f 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -95,7 +95,7 @@ func (e *IPEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime ti case *dnsgo.AAAA: s = r.AAAA.String() } - if _, ok := e.IPs[s]; ok { + if _, ok := e.IPs[s]; !ok { newIPs = append(newIPs, nftables.SetElement{Key: []byte(s)}) } log.WithValues("ip", s, "rr header ttl", rr.Header().Ttl, "expiration time", lookupTime.Add(time.Duration(rr.Header().Ttl)*time.Second)) From e04373fe2eac4a18655f4a793408385b5697870d Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 24 Jun 2025 15:19:23 +0200 Subject: [PATCH 48/71] remove restore sets --- pkg/dns/dnscache.go | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 64ca098f..ae9b0e56 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -211,8 +211,6 @@ func (c *DNSCache) writeStateToConfigmap() error { // getSetsForFQDN returns sets for FQDN selector func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firewallv1.IPSet) (result []firewallv1.IPSet) { - c.restoreSets(fqdnSets) - sets := map[string]firewallv1.IPSet{} if fqdn.MatchName != "" { for _, s := range c.getSetNameForFQDN(fqdn.GetMatchName()) { @@ -267,46 +265,6 @@ func (c *DNSCache) updateDNSServerAddr(addr string) { c.dnsServerAddr = addr } -// restoreSets add missing sets from FQDNSelector.Sets -func (c *DNSCache) restoreSets(fqdnSets []firewallv1.IPSet) { - for _, s := range fqdnSets { - // Add cache entries from fqdn.Sets if missing - c.Lock() - if _, ok := c.setNames[s.SetName]; !ok { - c.setNames[s.SetName] = struct{}{} - entry, exists := c.fqdnToEntry[s.FQDN] - if !exists { - entry = CacheEntry{} - } - - ipe := &IPEntry{ - SetName: s.SetName, - } - - for _, ip := range s.IPs { - ipa, _, _ := strings.Cut(ip, ",") - expirationTime := time.Now() - if _, ets, found := strings.Cut(ip, ": "); found { - if err := expirationTime.UnmarshalText([]byte(ets)); err != nil { - expirationTime = time.Now() - } - } - ipe.IPs[ipa] = expirationTime - } - - switch s.Version { - case firewallv1.IPv4: - entry.IPv4 = ipe - case firewallv1.IPv6: - entry.IPv6 = ipe - } - - c.fqdnToEntry[s.FQDN] = entry - } - c.Unlock() - } -} - // getSetNameForFQDN returns FQDN set data func (c *DNSCache) getSetNameForFQDN(fqdn string) (result []firewallv1.IPSet) { c.RLock() From ad23be331ad8e6322d0476cdc953a62b361479d7 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 25 Jun 2025 08:58:02 +0200 Subject: [PATCH 49/71] no reconcile trigger on status update --- controllers/clusterwidenetworkpolicy_controller.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 9ab829b8..61ad64e3 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -18,10 +18,12 @@ import ( "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/source" firewallv2 "github.com/metal-stack/firewall-controller-manager/api/v2" @@ -57,7 +59,7 @@ func (r *ClusterwideNetworkPolicyReconciler) SetupWithManager(mgr ctrl.Manager) } return ctrl.NewControllerManagedBy(mgr). - For(&firewallv1.ClusterwideNetworkPolicy{}). + For(&firewallv1.ClusterwideNetworkPolicy{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Watches(&corev1.Service{}, &handler.EnqueueRequestForObject{}). WatchesRawSource(&source.Channel{Source: scheduleChan}, &handler.EnqueueRequestForObject{}). Complete(r) From b50bbd91960e6981971db8365f60ab9701204f0b Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 1 Jul 2025 13:19:11 +0200 Subject: [PATCH 50/71] sort ips for fqdn state alphabetically to avoid unnecessary resourceVersion increases --- pkg/dns/dnscache.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index ae9b0e56..231f128f 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -7,6 +7,7 @@ import ( "fmt" "reflect" "regexp" + "sort" "strings" "sync" "time" @@ -544,6 +545,7 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IP } ips.IPs = append(ips.IPs, ip) } + sort.Strings(ips.IPs) return ips } From 45cea8eba8a541ca929da470a4c7520dd14e30ac Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 1 Jul 2025 13:20:28 +0200 Subject: [PATCH 51/71] remove debug logs --- controllers/clusterwidenetworkpolicy_controller.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 61ad64e3..85e06046 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -115,10 +115,8 @@ func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, req } if updated { - r.Log.V(4).Info("DEBUG CWNP reconcile, cwnps updated, trying to update cwnp stati.", "cwnps", cwnps) for _, i := range cwnps.Items { o := i - r.Log.V(4).Info("DEBUG CWNP reconcile, trying to update cwnp status.", "cwnp", o) if err := r.updateCWNPState(ctx, o, firewallv1.PolicyDeploymentStateDeployed, ""); err != nil { return ctrl.Result{}, err } @@ -247,7 +245,6 @@ func (r *ClusterwideNetworkPolicyReconciler) allowedCWNPs(ctx context.Context, c func (r *ClusterwideNetworkPolicyReconciler) updateCWNPState(ctx context.Context, cwnp firewallv1.ClusterwideNetworkPolicy, state firewallv1.PolicyDeploymentState, msg string) error { cwnp.Status.Message = msg cwnp.Status.State = state - r.Log.V(4).Info("DEBUG CWNP reconcile, function updateCWNPState, trying to update status.", "cwnp", cwnp) if err := r.ShootClient.Status().Update(ctx, &cwnp); err != nil { return fmt.Errorf("failed to update status of CWNP %q to %q: %w", cwnp.Name, state, err) From 19a10aa0efba0f933f0cfcf8d223dc1b0cad3cf7 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 1 Jul 2025 13:28:24 +0200 Subject: [PATCH 52/71] remove unused --- controllers/clusterwidenetworkpolicy_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 85e06046..18685521 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -71,7 +71,7 @@ func (r *ClusterwideNetworkPolicyReconciler) SetupWithManager(mgr ctrl.Manager) // +kubebuilder:rbac:groups=metal-stack.io,resources=clusterwidenetworkpolicies,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metal-stack.io,resources=clusterwidenetworkpolicies/status,verbs=get;update;patch -func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { var cwnps firewallv1.ClusterwideNetworkPolicyList if err := r.ShootClient.List(ctx, &cwnps, client.InNamespace(firewallv1.ClusterwideNetworkPolicyNamespace)); err != nil { return ctrl.Result{}, err From 66c7c3e382af6c4533d15c821f8f45adae3cb171 Mon Sep 17 00:00:00 2001 From: Ilja Rotar <77339620+iljarotar@users.noreply.github.com> Date: Tue, 12 Aug 2025 09:57:26 +0200 Subject: [PATCH 53/71] Update main.go Co-authored-by: Stefan Majer --- main.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index d7da7467..8e3c7abd 100644 --- a/main.go +++ b/main.go @@ -91,8 +91,10 @@ func main() { return } - var sll slog.Level - var err error + var ( + sll slog.Level + err error + ) err = sll.UnmarshalText([]byte(logLevel)) if err != nil { sll = slog.LevelInfo From b3ab138f2e60a55888ae81775f318ab8b9e12488 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 12 Aug 2025 11:08:42 +0200 Subject: [PATCH 54/71] style --- main.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/main.go b/main.go index 8e3c7abd..56eb7b30 100644 --- a/main.go +++ b/main.go @@ -91,11 +91,8 @@ func main() { return } - var ( - sll slog.Level - err error - ) - err = sll.UnmarshalText([]byte(logLevel)) + var sll slog.Level + err := sll.UnmarshalText([]byte(logLevel)) if err != nil { sll = slog.LevelInfo } From 1dd88ba94c79d51031a287fc95ccd558179dab79 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 12 Aug 2025 15:55:21 +0200 Subject: [PATCH 55/71] sort ip sets by name --- pkg/dns/dnscache.go | 7 +++++++ pkg/nftables/networkpolicy.go | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 231f128f..533033ed 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -7,6 +7,7 @@ import ( "fmt" "reflect" "regexp" + "slices" "sort" "strings" "sync" @@ -228,6 +229,9 @@ func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firew for _, s := range sets { result = append(result, s) } + slices.SortStableFunc(result, func(a, b firewallv1.IPSet) int { + return strings.Compare(a.SetName, b.SetName) + }) c.log.WithValues("fqdn", fqdn, "fqdnSets", fqdnSets, "sets", result).Info("sets for FQDN") return @@ -258,6 +262,9 @@ func (c *DNSCache) getSetsForRendering(fqdns []firewallv1.FQDNSelector) (result } } } + slices.SortStableFunc(result, func(a, b RenderIPSet) int { + return strings.Compare(a.SetName, b.SetName) + }) return } diff --git a/pkg/nftables/networkpolicy.go b/pkg/nftables/networkpolicy.go index 2edb9439..c91e0510 100644 --- a/pkg/nftables/networkpolicy.go +++ b/pkg/nftables/networkpolicy.go @@ -95,7 +95,6 @@ func clusterwideNetworkPolicyEgressRules( } ruleBases = append(ruleBases, ruleBase{base: rb}) } else if len(e.ToFQDNs) > 0 && cache.IsInitialized() { - // Generate allow rules based on DNS selectors rbs, u := clusterwideNetworkPolicyEgressToFQDNRules(cache, fqdnState, e) ruleBases = append(ruleBases, rbs...) fqdnState = u From c26d5b7af33af6762613686a5184581e56d59432 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 12 Aug 2025 16:27:26 +0200 Subject: [PATCH 56/71] also sort nftables ingress and egress rules --- pkg/nftables/rendering.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/nftables/rendering.go b/pkg/nftables/rendering.go index c637d765..db722350 100644 --- a/pkg/nftables/rendering.go +++ b/pkg/nftables/rendering.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "sort" "strings" "text/template" @@ -37,6 +38,8 @@ func newFirewallRenderingData(f *Firewall) (*firewallRenderingData, error) { egress = append(egress, e...) f.clusterwideNetworkPolicies.Items[ind] = u } + sort.Strings(ingress) + sort.Strings(egress) var serviceAllowedSet *netipx.IPSet if len(f.firewall.Spec.AllowedNetworks.Ingress) > 0 { From 3b38b2395b1c711b03b2425d3c5af334f85bd87f Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 13 Aug 2025 08:29:49 +0200 Subject: [PATCH 57/71] sort render ip set ips as well --- pkg/dns/dnscache.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 533033ed..1ef115aa 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -561,6 +561,7 @@ func createRenderIPSetFromIPEntry(version IPVersion, entry *IPEntry) RenderIPSet for ip := range entry.IPs { ips = append(ips, ip) } + sort.Strings(ips) return RenderIPSet{ SetName: entry.SetName, IPs: ips, From 86a79a2fee90a6329f942b2d583f2c59b147aa36 Mon Sep 17 00:00:00 2001 From: mreiger Date: Wed, 13 Aug 2025 12:42:52 +0200 Subject: [PATCH 58/71] Try splitting multi-line accept log rules only after assembling all the rules to make sorting work --- pkg/nftables/networkpolicy_test.go | 15 +++++---------- pkg/nftables/rendering.go | 4 ++++ pkg/nftables/service_test.go | 12 ++++-------- pkg/nftables/util.go | 10 +++++++--- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/pkg/nftables/networkpolicy_test.go b/pkg/nftables/networkpolicy_test.go index 295cfd6e..a3174c45 100644 --- a/pkg/nftables/networkpolicy_test.go +++ b/pkg/nftables/networkpolicy_test.go @@ -99,14 +99,11 @@ func TestClusterwideNetworkPolicyRules(t *testing.T) { `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`, }, ingressAL: nftablesRules{ - `ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } counter accept comment "accept traffic for k8s network policy tcp"`, + `ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } counter accept comment "accept traffic for k8s network policy tcp"`, }, egressAL: nftablesRules{ - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53, 443-448 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53, 443-448 } counter accept comment "accept traffic for np tcp"`, - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`, + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53, 443-448 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53, 443-448 } counter accept comment "accept traffic for np tcp"`, + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`, }, }, }, @@ -184,10 +181,8 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`, }, egressAL: nftablesRules{ - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } counter accept comment "accept traffic for np tcp"`, - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`, + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } counter accept comment "accept traffic for np tcp"`, + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`, }, }, }, diff --git a/pkg/nftables/rendering.go b/pkg/nftables/rendering.go index db722350..1d85bcff 100644 --- a/pkg/nftables/rendering.go +++ b/pkg/nftables/rendering.go @@ -75,6 +75,10 @@ func newFirewallRenderingData(f *Firewall) (*firewallRenderingData, error) { } egress = append(egress, rules...) } + + ingress = splitRules(ingress) + egress = splitRules(egress) + return &firewallRenderingData{ AdditionalDNSAddrs: dnsAddrs, PrivateVrfID: uint(*f.primaryPrivateNet.Vrf), // nolint:gosec diff --git a/pkg/nftables/service_test.go b/pkg/nftables/service_test.go index 9dcae4e5..466eccbe 100644 --- a/pkg/nftables/service_test.go +++ b/pkg/nftables/service_test.go @@ -60,8 +60,7 @@ func TestServiceRules(t *testing.T) { `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } ip daddr { 185.0.0.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, ingressAL: nftablesRules{ - `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } ip daddr { 185.0.0.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } ip daddr { 185.0.0.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } ip daddr { 185.0.0.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } ip daddr { 185.0.0.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, }, }, @@ -131,8 +130,7 @@ func TestServiceRules(t *testing.T) { `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, ingressAL: nftablesRules{ - `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, }, }, @@ -170,8 +168,7 @@ func TestServiceRules(t *testing.T) { `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, ingressAL: nftablesRules{ - `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, }, }, @@ -209,8 +206,7 @@ func TestServiceRules(t *testing.T) { `ip daddr { 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, ingressAL: nftablesRules{ - `ip daddr { 185.0.1.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, - `ip daddr { 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + `ip daddr { 185.0.1.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second` + "\n" + `ip daddr { 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, }, }, }, diff --git a/pkg/nftables/util.go b/pkg/nftables/util.go index 1e8e6f15..4bbb1de5 100644 --- a/pkg/nftables/util.go +++ b/pkg/nftables/util.go @@ -16,11 +16,15 @@ func uniqueSorted(elements []string) []string { for _, e := range elements { t[e] = true } - rawRules := []string{} + rules := []string{} for k := range t { - rawRules = append(rawRules, k) + rules = append(rules, k) } - sort.Strings(rawRules) + sort.Strings(rules) + return rules +} + +func splitRules(rawRules []string) []string { rules := []string{} for _, r := range rawRules { // split multiline log\naccept rules for pretty nftables file formatting rules = append(rules, strings.Split(r, "\n")...) From d4d9c71a077d47a30fc38c0912926ac7001c44c4 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 13 Aug 2025 16:29:51 +0200 Subject: [PATCH 59/71] adjust tests --- pkg/dns/dnscache.go | 4 +- pkg/dns/dnscache_test.go | 127 +++++++++++++++++---------- pkg/dns/dnsproxy.go | 4 +- pkg/nftables/firewall.go | 2 +- pkg/nftables/mocks/mock_fqdncache.go | 4 +- pkg/nftables/networkpolicy.go | 2 +- 6 files changed, 87 insertions(+), 56 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 1ef115aa..277efc84 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -212,7 +212,7 @@ func (c *DNSCache) writeStateToConfigmap() error { } // getSetsForFQDN returns sets for FQDN selector -func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firewallv1.IPSet) (result []firewallv1.IPSet) { +func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector) (result []firewallv1.IPSet) { sets := map[string]firewallv1.IPSet{} if fqdn.MatchName != "" { for _, s := range c.getSetNameForFQDN(fqdn.GetMatchName()) { @@ -233,7 +233,7 @@ func (c *DNSCache) getSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firew return strings.Compare(a.SetName, b.SetName) }) - c.log.WithValues("fqdn", fqdn, "fqdnSets", fqdnSets, "sets", result).Info("sets for FQDN") + c.log.WithValues("fqdn", fqdn, "sets", result).Info("sets for FQDN") return } diff --git a/pkg/dns/dnscache_test.go b/pkg/dns/dnscache_test.go index ef5bfe81..8536fa37 100644 --- a/pkg/dns/dnscache_test.go +++ b/pkg/dns/dnscache_test.go @@ -4,17 +4,17 @@ import ( "testing" "github.com/go-logr/logr" + "github.com/google/go-cmp/cmp" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" ) func Test_GetSetsForFQDN(t *testing.T) { tests := []struct { - name string - fqdnToEntry map[string]CacheEntry - expectedSets []string - fqdnSelector firewallv1.FQDNSelector - cachedSets []firewallv1.IPSet + name string + fqdnToEntry map[string]CacheEntry + want []firewallv1.IPSet + fqdn firewallv1.FQDNSelector }{ { name: "get result for matchName", @@ -28,8 +28,21 @@ func Test_GetSetsForFQDN(t *testing.T) { }, }, }, - expectedSets: []string{"testv4", "testv6"}, - fqdnSelector: firewallv1.FQDNSelector{ + want: []firewallv1.IPSet{ + { + SetName: "testv4", + FQDN: "test.com.", + IPs: []string{}, + Version: "ip", + }, + { + SetName: "testv6", + FQDN: "test.com.", + IPs: []string{}, + Version: "ip6", + }, + }, + fqdn: firewallv1.FQDNSelector{ MatchName: "test.com", }, }, @@ -69,8 +82,45 @@ func Test_GetSetsForFQDN(t *testing.T) { }, }, }, - expectedSets: []string{"testv4", "testv6", "examplev4", "examplev6", "2examplev4", "2examplev6"}, - fqdnSelector: firewallv1.FQDNSelector{ + want: []firewallv1.IPSet{ + { + SetName: "2examplev4", + FQDN: "second.example.com.", + IPs: []string{}, + Version: "ip", + }, + { + SetName: "2examplev6", + FQDN: "second.example.com.", + IPs: []string{}, + Version: "ip6", + }, + { + SetName: "examplev4", + FQDN: "example.com.", + IPs: []string{}, + Version: "ip", + }, + { + SetName: "examplev6", + FQDN: "example.com.", + IPs: []string{}, + Version: "ip6", + }, + { + SetName: "testv4", + FQDN: "test.com.", + IPs: []string{}, + Version: "ip", + }, + { + SetName: "testv6", + FQDN: "test.com.", + IPs: []string{}, + Version: "ip6", + }, + }, + fqdn: firewallv1.FQDNSelector{ MatchPattern: "*.com", }, }, @@ -86,58 +136,39 @@ func Test_GetSetsForFQDN(t *testing.T) { }, }, }, - expectedSets: []string{"testv4", "testv6"}, - fqdnSelector: firewallv1.FQDNSelector{ + want: []firewallv1.IPSet{ + { + SetName: "testv4", + FQDN: "www.freechess.org.", + IPs: []string{}, + Version: "ip", + }, + { + SetName: "testv6", + FQDN: "www.freechess.org.", + IPs: []string{}, + Version: "ip6", + }, + }, + fqdn: firewallv1.FQDNSelector{ MatchPattern: "ww*.freechess.org", }, }, - /* { - name: "restore sets", - fqdnToEntry: map[string]cacheEntry{}, - fqdnSelector: firewallv1.FQDNSelector{}, - cachedSets: []firewallv1.IPSet{{ - FQDN: "test-fqdn", - SetName: "test-set", - }}, - }, FIXME see how we can test the new state configmap approach */ } for _, tt := range tests { - tc := tt - t.Run(tc.name, func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { cache := DNSCache{ log: logr.Discard(), - fqdnToEntry: tc.fqdnToEntry, + fqdnToEntry: tt.fqdnToEntry, setNames: make(map[string]struct{}), ipv4Enabled: true, ipv6Enabled: true, } - result := cache.getSetsForFQDN(tc.fqdnSelector, tc.cachedSets) - - set := make(map[string]bool, len(tc.expectedSets)) - for _, s := range tc.expectedSets { - set[s] = false - } - for _, r := range result { - if _, found := set[r.SetName]; !found { - t.Errorf("set name %s wasn't expected", r.SetName) - } - set[r.SetName] = true - } - for s, b := range set { - if !b { - t.Errorf("set name %s didn't occurred in result", s) - } - } - // Check if cache was updated - for _, s := range tc.cachedSets { - if _, ok := cache.setNames[s.SetName]; !ok { - t.Errorf("set name %s wasn't added to cache", s.SetName) - } - if _, ok := cache.fqdnToEntry[s.FQDN]; !ok { - t.Errorf("FQDN %s wasn't added to cache", s.FQDN) - } + got := cache.getSetsForFQDN(tt.fqdn) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("DNSCache.getSetsForFQDN diff = %s", diff) } }) } diff --git a/pkg/dns/dnsproxy.go b/pkg/dns/dnsproxy.go index b0c21d3a..8ca70ea7 100644 --- a/pkg/dns/dnsproxy.go +++ b/pkg/dns/dnsproxy.go @@ -120,8 +120,8 @@ func (p *DNSProxy) GetSetsForRendering(fqdns []firewallv1.FQDNSelector) (result return p.cache.getSetsForRendering(fqdns) } -func (p *DNSProxy) GetSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firewallv1.IPSet) (result []firewallv1.IPSet) { - return p.cache.getSetsForFQDN(fqdn, fqdnSets) +func (p *DNSProxy) GetSetsForFQDN(fqdn firewallv1.FQDNSelector) (result []firewallv1.IPSet) { + return p.cache.getSetsForFQDN(fqdn) } func (p *DNSProxy) IsInitialized() bool { diff --git a/pkg/nftables/firewall.go b/pkg/nftables/firewall.go index 89d020b8..9413e953 100644 --- a/pkg/nftables/firewall.go +++ b/pkg/nftables/firewall.go @@ -41,7 +41,7 @@ var templates embed.FS //go:generate ../../bin/mockgen -destination=./mocks/mock_fqdncache.go -package=mocks . FQDNCache type FQDNCache interface { GetSetsForRendering(fqdns []firewallv1.FQDNSelector) (result []dns.RenderIPSet) - GetSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firewallv1.IPSet) (result []firewallv1.IPSet) + GetSetsForFQDN(fqdn firewallv1.FQDNSelector) (result []firewallv1.IPSet) IsInitialized() bool CacheAddr() (string, error) } diff --git a/pkg/nftables/mocks/mock_fqdncache.go b/pkg/nftables/mocks/mock_fqdncache.go index fbd5bc3a..8d4416b9 100644 --- a/pkg/nftables/mocks/mock_fqdncache.go +++ b/pkg/nftables/mocks/mock_fqdncache.go @@ -57,9 +57,9 @@ func (mr *MockFQDNCacheMockRecorder) CacheAddr() *gomock.Call { } // GetSetsForFQDN mocks base method. -func (m *MockFQDNCache) GetSetsForFQDN(fqdn v1.FQDNSelector, fqdnSets []v1.IPSet) []v1.IPSet { +func (m *MockFQDNCache) GetSetsForFQDN(fqdn v1.FQDNSelector) []v1.IPSet { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSetsForFQDN", fqdn, fqdnSets) + ret := m.ctrl.Call(m, "GetSetsForFQDN", fqdn) ret0, _ := ret[0].([]v1.IPSet) return ret0 } diff --git a/pkg/nftables/networkpolicy.go b/pkg/nftables/networkpolicy.go index c91e0510..fa8b7649 100644 --- a/pkg/nftables/networkpolicy.go +++ b/pkg/nftables/networkpolicy.go @@ -139,7 +139,7 @@ func clusterwideNetworkPolicyEgressToFQDNRules( fqdnName = fqdn.MatchPattern } - fqdnState[fqdnName] = cache.GetSetsForFQDN(fqdn, fqdnState[fqdnName]) + fqdnState[fqdnName] = cache.GetSetsForFQDN(fqdn) for _, set := range fqdnState[fqdnName] { rb := []string{"ip saddr == @cluster_prefixes"} rb = append(rb, fmt.Sprintf(string(set.Version)+" daddr @%s", set.SetName)) From bac53b1a913dadf9503db6806ce69bf4b7064f6d Mon Sep 17 00:00:00 2001 From: mreiger Date: Wed, 13 Aug 2025 17:19:44 +0200 Subject: [PATCH 60/71] Fixes to tests --- pkg/nftables/mocks/mock_fqdncache.go | 4 ++-- pkg/nftables/networkpolicy_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/nftables/mocks/mock_fqdncache.go b/pkg/nftables/mocks/mock_fqdncache.go index 8d4416b9..d1f26694 100644 --- a/pkg/nftables/mocks/mock_fqdncache.go +++ b/pkg/nftables/mocks/mock_fqdncache.go @@ -65,9 +65,9 @@ func (m *MockFQDNCache) GetSetsForFQDN(fqdn v1.FQDNSelector) []v1.IPSet { } // GetSetsForFQDN indicates an expected call of GetSetsForFQDN. -func (mr *MockFQDNCacheMockRecorder) GetSetsForFQDN(fqdn, fqdnSets any) *gomock.Call { +func (mr *MockFQDNCacheMockRecorder) GetSetsForFQDN(fqdn any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSetsForFQDN", reflect.TypeOf((*MockFQDNCache)(nil).GetSetsForFQDN), fqdn, fqdnSets) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSetsForFQDN", reflect.TypeOf((*MockFQDNCache)(nil).GetSetsForFQDN), fqdn) } // GetSetsForRendering mocks base method. diff --git a/pkg/nftables/networkpolicy_test.go b/pkg/nftables/networkpolicy_test.go index a3174c45..6580d7b5 100644 --- a/pkg/nftables/networkpolicy_test.go +++ b/pkg/nftables/networkpolicy_test.go @@ -221,11 +221,11 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { Return(true) cache. EXPECT(). - GetSetsForFQDN(gomock.Any(), gomock.Any()). + GetSetsForFQDN(gomock.Any()). Return([]firewallv1.IPSet{{SetName: "test", Version: firewallv1.IPv4}}) cache. EXPECT(). - GetSetsForFQDN(gomock.Any(), gomock.Any()). + GetSetsForFQDN(gomock.Any()). Return([]firewallv1.IPSet{{SetName: "test2", Version: firewallv1.IPv6}}) }, want: want{ From 64ad5327c01356707f7a49d609fbd5e64d46c993 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Fri, 22 Aug 2025 12:43:32 +0200 Subject: [PATCH 61/71] linter --- pkg/nftables/firewall.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/nftables/firewall.go b/pkg/nftables/firewall.go index 5f488251..463a616f 100644 --- a/pkg/nftables/firewall.go +++ b/pkg/nftables/firewall.go @@ -202,9 +202,7 @@ func getConfiguredIPs(networkID string) []string { if nw.Networkid == nil || *nw.Networkid != networkID { continue } - for _, ip := range nw.Ips { - ips = append(ips, ip) - } + ips = append(ips, nw.Ips...) } return ips } From 34aeedda2dea6d5e77ef1f82bf5bc86c5a5ceda7 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Mon, 25 Aug 2025 10:44:17 +0200 Subject: [PATCH 62/71] review findings --- api/v1/clusterwidenetworkpolicy_types.go | 19 ++++--- api/v1/zz_generated.deepcopy.go | 8 +++ ...l-stack.io_clusterwidenetworkpolicies.yaml | 11 ++++ pkg/dns/dnscache.go | 18 +++--- pkg/dns/dnscache_test.go | 55 ++++++++++++++++++- 5 files changed, 90 insertions(+), 21 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index 7a7b32c5..65f814ae 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -31,17 +31,17 @@ const ( // +kubebuilder:printcolumn:name="Message",type="string",JSONPath=".status.message" type ClusterwideNetworkPolicy struct { metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` + metav1.ObjectMeta `json:"metadata"` - Spec PolicySpec `json:"spec,omitempty"` - Status PolicyStatus `json:"status,omitempty"` + Spec PolicySpec `json:"spec"` + Status PolicyStatus `json:"status"` } // ClusterwideNetworkPolicyList contains a list of ClusterwideNetworkPolicy // +kubebuilder:object:root=true type ClusterwideNetworkPolicyList struct { metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` + metav1.ListMeta `json:"metadata"` Items []ClusterwideNetworkPolicy `json:"items"` } @@ -153,11 +153,12 @@ type FQDNSelector struct { // IPSet stores set name association to IP addresses type IPSet struct { - FQDN string `json:"fqdn,omitempty"` - SetName string `json:"setName,omitempty"` - IPs []string `json:"ips,omitempty"` - ExpirationTime metav1.Time `json:"expirationTime,omitempty"` - Version IPVersion `json:"version,omitempty"` + FQDN string `json:"fqdn,omitempty"` + SetName string `json:"setName,omitempty"` + IPs []string `json:"ips,omitempty"` + ExpirationTime metav1.Time `json:"expirationTime"` + IPExpirationTimes map[string]metav1.Time `json:"ipExpirationTimes,omitempty"` + Version IPVersion `json:"version,omitempty"` } func (l *ClusterwideNetworkPolicyList) GetFQDNs() []FQDNSelector { diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index c9863969..24698576 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -6,6 +6,7 @@ package v1 import ( networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -158,6 +159,13 @@ func (in *IPSet) DeepCopyInto(out *IPSet) { copy(*out, *in) } in.ExpirationTime.DeepCopyInto(&out.ExpirationTime) + if in.IPExpirationTimes != nil { + in, out := &in.IPExpirationTimes, &out.IPExpirationTimes + *out = make(map[string]metav1.Time, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPSet. diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index 9d91f1ad..24747d10 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -248,6 +248,11 @@ spec: type: string fqdn: type: string + ipExpirationTimes: + additionalProperties: + format: date-time + type: string + type: object ips: items: type: string @@ -256,6 +261,8 @@ spec: type: string version: type: string + required: + - expirationTime type: object type: array description: |- @@ -269,6 +276,10 @@ spec: description: State of the CWNP, can be either deployed or ignored type: string type: object + required: + - metadata + - spec + - status type: object served: true storage: true diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 277efc84..c4543ce9 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -151,9 +151,9 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, } if scm.Data[fqdnStateConfigmapKey] == "" { - c.log.V(4).Info("DEBUG fqdnstate cm does not contain the right key", "cm", scm, "key", fqdnStateConfigmapKey) + c.log.Error(fmt.Errorf("error reading fqdnstate configmap, ignoring content"), "fqdnstate configmap does not contain the right key", "configmap", scm, "key", fqdnStateConfigmapKey) + c.fqdnToEntry = map[string]CacheEntry{} return &c, nil - } c.log.V(4).Info("DEBUG state stored in fqdnstate cm, trying to unmarshal", fqdnStateConfigmapKey, scm.Data[fqdnStateConfigmapKey]) err = yaml.UnmarshalStrict([]byte(scm.Data[fqdnStateConfigmapKey]), &c.fqdnToEntry) @@ -541,18 +541,14 @@ func updateNftSet( func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IPEntry) firewallv1.IPSet { ips := firewallv1.IPSet{ - FQDN: fqdn, - SetName: entry.SetName, - IPs: []string{}, - Version: version, + FQDN: fqdn, + SetName: entry.SetName, + IPExpirationTimes: make(map[string]metav1.Time), + Version: version, } for ip, expirationTime := range entry.IPs { - if et, err := expirationTime.MarshalText(); err == nil { - ip = ip + ", expiration time: " + string(et) - } - ips.IPs = append(ips.IPs, ip) + ips.IPExpirationTimes[ip] = metav1.NewTime(expirationTime) } - sort.Strings(ips.IPs) return ips } diff --git a/pkg/dns/dnscache_test.go b/pkg/dns/dnscache_test.go index 8536fa37..fad130e8 100644 --- a/pkg/dns/dnscache_test.go +++ b/pkg/dns/dnscache_test.go @@ -2,11 +2,12 @@ package dns import ( "testing" + "time" "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" - firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func Test_GetSetsForFQDN(t *testing.T) { @@ -173,3 +174,55 @@ func Test_GetSetsForFQDN(t *testing.T) { }) } } + +func Test_createIPSetFromIPEntry(t *testing.T) { + tests := []struct { + name string + fqdn string + version firewallv1.IPVersion + entry *IPEntry + want firewallv1.IPSet + }{ + { + name: "empty ip entry", + fqdn: "www.freechess.org", + version: "ip", + entry: &IPEntry{ + SetName: "test", + }, + want: firewallv1.IPSet{ + FQDN: "www.freechess.org", + SetName: "test", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip", + }, + }, + { + name: "entry contains ips", + fqdn: "www.freechess.org", + version: "ip", + entry: &IPEntry{ + SetName: "test", + IPs: map[string]time.Time{ + "1.2.3.4": time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), + }, + }, + want: firewallv1.IPSet{ + FQDN: "www.freechess.org", + SetName: "test", + IPExpirationTimes: map[string]v1.Time{ + "1.2.3.4": v1.NewTime(time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC)), + }, + Version: "ip", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := createIPSetFromIPEntry(tt.fqdn, tt.version, tt.entry) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("createIPSetFromIPEntry() diff = %s", diff) + } + }) + } +} From 594308d8d9eece379fd7e8d1fb7f496b48d79f65 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Mon, 25 Aug 2025 10:54:49 +0200 Subject: [PATCH 63/71] fix test --- pkg/dns/dnscache_test.go | 80 ++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/pkg/dns/dnscache_test.go b/pkg/dns/dnscache_test.go index fad130e8..cf88084c 100644 --- a/pkg/dns/dnscache_test.go +++ b/pkg/dns/dnscache_test.go @@ -31,16 +31,16 @@ func Test_GetSetsForFQDN(t *testing.T) { }, want: []firewallv1.IPSet{ { - SetName: "testv4", - FQDN: "test.com.", - IPs: []string{}, - Version: "ip", + SetName: "testv4", + FQDN: "test.com.", + Version: "ip", + IPExpirationTimes: map[string]v1.Time{}, }, { - SetName: "testv6", - FQDN: "test.com.", - IPs: []string{}, - Version: "ip6", + SetName: "testv6", + FQDN: "test.com.", + Version: "ip6", + IPExpirationTimes: map[string]v1.Time{}, }, }, fqdn: firewallv1.FQDNSelector{ @@ -85,40 +85,40 @@ func Test_GetSetsForFQDN(t *testing.T) { }, want: []firewallv1.IPSet{ { - SetName: "2examplev4", - FQDN: "second.example.com.", - IPs: []string{}, - Version: "ip", + SetName: "2examplev4", + FQDN: "second.example.com.", + Version: "ip", + IPExpirationTimes: map[string]v1.Time{}, }, { - SetName: "2examplev6", - FQDN: "second.example.com.", - IPs: []string{}, - Version: "ip6", + SetName: "2examplev6", + FQDN: "second.example.com.", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip6", }, { - SetName: "examplev4", - FQDN: "example.com.", - IPs: []string{}, - Version: "ip", + SetName: "examplev4", + FQDN: "example.com.", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip", }, { - SetName: "examplev6", - FQDN: "example.com.", - IPs: []string{}, - Version: "ip6", + SetName: "examplev6", + FQDN: "example.com.", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip6", }, { - SetName: "testv4", - FQDN: "test.com.", - IPs: []string{}, - Version: "ip", + SetName: "testv4", + FQDN: "test.com.", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip", }, { - SetName: "testv6", - FQDN: "test.com.", - IPs: []string{}, - Version: "ip6", + SetName: "testv6", + FQDN: "test.com.", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip6", }, }, fqdn: firewallv1.FQDNSelector{ @@ -139,16 +139,16 @@ func Test_GetSetsForFQDN(t *testing.T) { }, want: []firewallv1.IPSet{ { - SetName: "testv4", - FQDN: "www.freechess.org.", - IPs: []string{}, - Version: "ip", + SetName: "testv4", + FQDN: "www.freechess.org.", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip", }, { - SetName: "testv6", - FQDN: "www.freechess.org.", - IPs: []string{}, - Version: "ip6", + SetName: "testv6", + FQDN: "www.freechess.org.", + IPExpirationTimes: map[string]v1.Time{}, + Version: "ip6", }, }, fqdn: firewallv1.FQDNSelector{ From e16d7ea43a892001e3c0260a9d3b77df17583e89 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Mon, 25 Aug 2025 17:07:49 +0200 Subject: [PATCH 64/71] crd --- api/v1/clusterwidenetworkpolicy_types.go | 8 ++++---- .../bases/metal-stack.io_clusterwidenetworkpolicies.yaml | 4 ---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index 65f814ae..589e9972 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -31,17 +31,17 @@ const ( // +kubebuilder:printcolumn:name="Message",type="string",JSONPath=".status.message" type ClusterwideNetworkPolicy struct { metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata"` + metav1.ObjectMeta `json:"metadata,omitempty"` - Spec PolicySpec `json:"spec"` - Status PolicyStatus `json:"status"` + Spec PolicySpec `json:"spec,omitempty"` + Status PolicyStatus `json:"status,omitempty"` } // ClusterwideNetworkPolicyList contains a list of ClusterwideNetworkPolicy // +kubebuilder:object:root=true type ClusterwideNetworkPolicyList struct { metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata"` + metav1.ListMeta `json:"metadata,omitempty"` Items []ClusterwideNetworkPolicy `json:"items"` } diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index 24747d10..9963d036 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -276,10 +276,6 @@ spec: description: State of the CWNP, can be either deployed or ignored type: string type: object - required: - - metadata - - spec - - status type: object served: true storage: true From 7bacada7538f059df37f2fe617f0ff0dfc6a52ed Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Mon, 25 Aug 2025 17:09:11 +0200 Subject: [PATCH 65/71] crd --- api/v1/clusterwidenetworkpolicy_types.go | 2 +- config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index 589e9972..2b8b9a7d 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -156,7 +156,7 @@ type IPSet struct { FQDN string `json:"fqdn,omitempty"` SetName string `json:"setName,omitempty"` IPs []string `json:"ips,omitempty"` - ExpirationTime metav1.Time `json:"expirationTime"` + ExpirationTime metav1.Time `json:"expirationTime,omitempty"` IPExpirationTimes map[string]metav1.Time `json:"ipExpirationTimes,omitempty"` Version IPVersion `json:"version,omitempty"` } diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index 9963d036..68a23548 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -261,8 +261,6 @@ spec: type: string version: type: string - required: - - expirationTime type: object type: array description: |- From 222434109178db6d818ae5ac63fa13ea4b9df7d8 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 26 Aug 2025 13:11:44 +0200 Subject: [PATCH 66/71] add old ips too --- pkg/dns/dnscache.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index c4543ce9..11cac875 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -544,11 +544,14 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IP FQDN: fqdn, SetName: entry.SetName, IPExpirationTimes: make(map[string]metav1.Time), + IPs: make([]string, 0), Version: version, } for ip, expirationTime := range entry.IPs { ips.IPExpirationTimes[ip] = metav1.NewTime(expirationTime) + ips.IPs = append(ips.IPs, ip) } + sort.Strings(ips.IPs) return ips } From cfc494a359ac22afcfe9f10e5ba471a618ef658f Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 26 Aug 2025 13:16:25 +0200 Subject: [PATCH 67/71] revert --- pkg/dns/dnscache.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 11cac875..c4543ce9 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -544,14 +544,11 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IP FQDN: fqdn, SetName: entry.SetName, IPExpirationTimes: make(map[string]metav1.Time), - IPs: make([]string, 0), Version: version, } for ip, expirationTime := range entry.IPs { ips.IPExpirationTimes[ip] = metav1.NewTime(expirationTime) - ips.IPs = append(ips.IPs, ip) } - sort.Strings(ips.IPs) return ips } From 206e5aaa4817ca90da9e7fbb836f5723bac7e968 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Thu, 4 Sep 2025 12:40:25 +0200 Subject: [PATCH 68/71] add comments to type definition --- api/v1/clusterwidenetworkpolicy_types.go | 16 +++++++++++----- ...etal-stack.io_clusterwidenetworkpolicies.yaml | 6 ++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index 2b8b9a7d..e8b9d3fa 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -153,12 +153,18 @@ type FQDNSelector struct { // IPSet stores set name association to IP addresses type IPSet struct { - FQDN string `json:"fqdn,omitempty"` - SetName string `json:"setName,omitempty"` - IPs []string `json:"ips,omitempty"` - ExpirationTime metav1.Time `json:"expirationTime,omitempty"` + // FQDN which this IP set is for. + FQDN string `json:"fqdn,omitempty"` + // A hash value merely used for reference. + SetName string `json:"setName,omitempty"` + // Deprecated: use `IPExpirationTimes` instead. + IPs []string `json:"ips,omitempty"` + // Deprecated: use `IPExpirationTimes` instead. + ExpirationTime metav1.Time `json:"expirationTime,omitempty"` + // Maps IP addresses to their expiration times. IPExpirationTimes map[string]metav1.Time `json:"ipExpirationTimes,omitempty"` - Version IPVersion `json:"version,omitempty"` + // Whether this is a IPv4 or a IPv6 set. + Version IPVersion `json:"version,omitempty"` } func (l *ClusterwideNetworkPolicyList) GetFQDNs() []FQDNSelector { diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index 68a23548..f9d60992 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -244,22 +244,28 @@ spec: description: IPSet stores set name association to IP addresses properties: expirationTime: + description: 'Deprecated: use `IPExpirationTimes` instead.' format: date-time type: string fqdn: + description: FQDN which this IP set is for. type: string ipExpirationTimes: additionalProperties: format: date-time type: string + description: Maps IP addresses to their expiration times. type: object ips: + description: 'Deprecated: use `IPExpirationTimes` instead.' items: type: string type: array setName: + description: ' A hash value merely used for reference.' type: string version: + description: Whether this is a IPv4 or a IPv6 set. type: string type: object type: array From f351f12bd9cd26cf978e2d4e39322dddf32ca045 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Fri, 5 Sep 2025 11:09:19 +0200 Subject: [PATCH 69/71] review findings --- .../clusterwidenetworkpolicy_controller.go | 2 +- main.go | 3 +- pkg/dns/dnscache.go | 103 +++++++++++------- pkg/dns/dnscache_test.go | 38 +++---- pkg/dns/dnsproxy.go | 4 +- pkg/nftables/util_test.go | 30 ++++- 6 files changed, 114 insertions(+), 66 deletions(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 18685521..f163b5d0 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -144,7 +144,7 @@ func (r *ClusterwideNetworkPolicyReconciler) manageDNSProxy( if enableDNS && r.DnsProxy == nil { r.Log.Info("DNS Proxy is initialized") - if r.DnsProxy, err = dns.NewDNSProxy(ctx, f.Spec.DNSServerAddress, f.Spec.DNSPort, r.ShootClient, ctrl.Log.WithName("DNS proxy")); err != nil { + if r.DnsProxy, err = dns.NewDNSProxy(f.Spec.DNSServerAddress, f.Spec.DNSPort, r.ShootClient, ctrl.Log.WithName("DNS proxy")); err != nil { return fmt.Errorf("failed to init DNS proxy: %w", err) } go r.DnsProxy.Run(ctx) diff --git a/main.go b/main.go index 56eb7b30..6b690fac 100644 --- a/main.go +++ b/main.go @@ -94,7 +94,8 @@ func main() { var sll slog.Level err := sll.UnmarshalText([]byte(logLevel)) if err != nil { - sll = slog.LevelInfo + setupLog.Error(err, "failed to unmarshal log level") + os.Exit(1) } jsonHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: sll}) l := slog.New(jsonHandler) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index c4543ce9..d15ede74 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -21,6 +21,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" @@ -41,7 +42,7 @@ const ( // Configmap that holds the FQDN state fqdnStateConfigmapName = "fqdnstate" - fqdnStateNamespace = "firewall" + fqdnStateNamespace = firewallv1.ClusterwideNetworkPolicyNamespace fqdnStateConfigmapKey = "state" ) @@ -52,20 +53,20 @@ type RenderIPSet struct { Version IPVersion `json:"version,omitempty"` } -type IPEntry struct { +type iPEntry struct { // ips is a map of the ip address and its expiration time which is the time of the DNS lookup + the TTL IPs map[string]time.Time `json:"ips,omitempty"` SetName string `json:"setName,omitempty"` } -func newIPEntry(setName string) *IPEntry { - return &IPEntry{ +func newIPEntry(setName string) *iPEntry { + return &iPEntry{ SetName: setName, IPs: map[string]time.Time{}, } } -func (e *IPEntry) update(log logr.Logger, setName string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { +func (e *iPEntry) update(log logr.Logger, setName string, rrs []dnsgo.RR, lookupTime time.Time, dtype nftables.SetDatatype) error { deletedIPs := e.expireIPs() newIPs := e.addAndUpdateIPs(log, rrs, lookupTime) @@ -78,7 +79,7 @@ func (e *IPEntry) update(log logr.Logger, setName string, rrs []dnsgo.RR, lookup return nil } -func (e *IPEntry) expireIPs() (deletedIPs []nftables.SetElement) { +func (e *iPEntry) expireIPs() (deletedIPs []nftables.SetElement) { for ip, expirationTime := range e.IPs { if expirationTime.Before(time.Now()) { deletedIPs = append(deletedIPs, nftables.SetElement{Key: []byte(ip)}) @@ -88,7 +89,7 @@ func (e *IPEntry) expireIPs() (deletedIPs []nftables.SetElement) { return } -func (e *IPEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { +func (e *iPEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime time.Time) (newIPs []nftables.SetElement) { for _, rr := range rrs { var s string switch r := rr.(type) { @@ -107,16 +108,16 @@ func (e *IPEntry) addAndUpdateIPs(log logr.Logger, rrs []dnsgo.RR, lookupTime ti return } -type CacheEntry struct { - IPv4 *IPEntry `json:"ipv4,omitempty"` - IPv6 *IPEntry `json:"ipv6,omitempty"` +type cacheEntry struct { + IPv4 *iPEntry `json:"ipv4,omitempty"` + IPv6 *iPEntry `json:"ipv6,omitempty"` } type DNSCache struct { sync.RWMutex log logr.Logger - fqdnToEntry map[string]CacheEntry + fqdnToEntry map[string]cacheEntry setNames map[string]struct{} dnsServerAddr string shootClient client.Client @@ -125,14 +126,14 @@ type DNSCache struct { ipv6Enabled bool } -func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) (*DNSCache, error) { +func newDNSCache(dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) (*DNSCache, error) { c := DNSCache{ log: log, - fqdnToEntry: map[string]CacheEntry{}, + fqdnToEntry: map[string]cacheEntry{}, setNames: map[string]struct{}{}, dnsServerAddr: dns, shootClient: shootClient, - ctx: ctx, + ctx: ctrl.SetupSignalHandler(), ipv4Enabled: ipv4Enabled, ipv6Enabled: ipv6Enabled, } @@ -140,6 +141,11 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} scm := &v1.ConfigMap{} + ctx, cancel := context.WithTimeout(c.ctx, 100*time.Millisecond) + defer func() { + cancel() + }() + err := shootClient.Get(ctx, nn, scm) if err != nil && !apierrors.IsNotFound(err) { c.log.Error(err, "error reading fqndstate configmap") @@ -152,14 +158,12 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, } if scm.Data[fqdnStateConfigmapKey] == "" { c.log.Error(fmt.Errorf("error reading fqdnstate configmap, ignoring content"), "fqdnstate configmap does not contain the right key", "configmap", scm, "key", fqdnStateConfigmapKey) - c.fqdnToEntry = map[string]CacheEntry{} return &c, nil } c.log.V(4).Info("DEBUG state stored in fqdnstate cm, trying to unmarshal", fqdnStateConfigmapKey, scm.Data[fqdnStateConfigmapKey]) err = yaml.UnmarshalStrict([]byte(scm.Data[fqdnStateConfigmapKey]), &c.fqdnToEntry) if err != nil { c.log.Info("could not unmarshal state from fqdnstate configmap, ignoring content.", "error", err) - c.fqdnToEntry = map[string]CacheEntry{} } return &c, nil } @@ -170,44 +174,59 @@ func (c *DNSCache) writeStateToConfigmap() error { if err != nil { return err } - if s == nil { - return nil - } - c.log.V(4).Info("DEBUG writing cache to configmap", "fqdnToEntry", string(s)) - currentCm := &v1.ConfigMap{} - nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} - scm := &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: fqdnStateConfigmapName, - Namespace: fqdnStateNamespace, - }, - Data: map[string]string{ + var ( + cm = &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: fqdnStateConfigmapName, + Namespace: fqdnStateNamespace, + }, + } + + data = map[string]string{ fqdnStateConfigmapKey: string(s), - }, - } + } + + debugLog = c.log.V(4).WithValues("namespace", cm.Namespace, "name", cm.Name) + ) + + debugLog.Info("DEBUG writing cache to configmap", "fqdnToEntry", string(s)) + + debugLog.Info("DEBUG looking for configmap") - c.log.V(4).Info("DEBUG looking for configmap", "namespacedname", nn) - err = c.shootClient.Get(c.ctx, nn, currentCm) + err = c.shootClient.Get(c.ctx, client.ObjectKeyFromObject(cm), cm) if err != nil && !apierrors.IsNotFound(err) { return err } + if apierrors.IsNotFound(err) { - c.log.V(4).Info("DEBUG configmap not found, trying to create configmap", "NamespacedName", nn, "configmap to create", scm) - err = c.shootClient.Create(c.ctx, scm) + debugLog.Info("DEBUG configmap not found, trying to create configmap") + + cm.Data = data + + err = c.shootClient.Create(c.ctx, cm) if err != nil { return err } + + return nil } - c.log.V(4).Info("DEBUG trying to update cm", "current cm", currentCm, "cm", scm) - if !reflect.DeepEqual(currentCm.Data, scm.Data) { - currentCm.Data = scm.Data - err = c.shootClient.Update(c.ctx, currentCm) + if !reflect.DeepEqual(cm.Data, data) { + cm.Data = data + + err = c.shootClient.Update(c.ctx, cm) if err != nil { return err } + + debugLog.Info("DEBUG updated cm", "old data", cm.Data, "new data", data) + + return nil } + + debugLog.Info("DEBUG no need to update cm, already up to date") + return nil } @@ -449,10 +468,10 @@ func (c *DNSCache) updateIPEntry(qname string, rrs []dnsgo.RR, lookupTime time.T entry, exists := c.fqdnToEntry[qname] if !exists { - entry = CacheEntry{} + entry = cacheEntry{} } - var ipe *IPEntry + var ipe *iPEntry switch dtype { case nftables.TypeIPAddr: if entry.IPv4 == nil { @@ -539,7 +558,7 @@ func updateNftSet( return nil } -func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IPEntry) firewallv1.IPSet { +func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *iPEntry) firewallv1.IPSet { ips := firewallv1.IPSet{ FQDN: fqdn, SetName: entry.SetName, @@ -552,7 +571,7 @@ func createIPSetFromIPEntry(fqdn string, version firewallv1.IPVersion, entry *IP return ips } -func createRenderIPSetFromIPEntry(version IPVersion, entry *IPEntry) RenderIPSet { +func createRenderIPSetFromIPEntry(version IPVersion, entry *iPEntry) RenderIPSet { var ips []string for ip := range entry.IPs { ips = append(ips, ip) diff --git a/pkg/dns/dnscache_test.go b/pkg/dns/dnscache_test.go index cf88084c..92c2b1fb 100644 --- a/pkg/dns/dnscache_test.go +++ b/pkg/dns/dnscache_test.go @@ -13,18 +13,18 @@ import ( func Test_GetSetsForFQDN(t *testing.T) { tests := []struct { name string - fqdnToEntry map[string]CacheEntry + fqdnToEntry map[string]cacheEntry want []firewallv1.IPSet fqdn firewallv1.FQDNSelector }{ { name: "get result for matchName", - fqdnToEntry: map[string]CacheEntry{ + fqdnToEntry: map[string]cacheEntry{ "test.com.": { - IPv4: &IPEntry{ + IPv4: &iPEntry{ SetName: "testv4", }, - IPv6: &IPEntry{ + IPv6: &iPEntry{ SetName: "testv6", }, }, @@ -49,36 +49,36 @@ func Test_GetSetsForFQDN(t *testing.T) { }, { name: "get result for matchPattern", - fqdnToEntry: map[string]CacheEntry{ + fqdnToEntry: map[string]cacheEntry{ "test.com.": { - IPv4: &IPEntry{ + IPv4: &iPEntry{ SetName: "testv4", }, - IPv6: &IPEntry{ + IPv6: &iPEntry{ SetName: "testv6", }, }, "test.io.": { - IPv4: &IPEntry{ + IPv4: &iPEntry{ SetName: "testiov4", }, - IPv6: &IPEntry{ + IPv6: &iPEntry{ SetName: "testiov6", }, }, "example.com.": { - IPv4: &IPEntry{ + IPv4: &iPEntry{ SetName: "examplev4", }, - IPv6: &IPEntry{ + IPv6: &iPEntry{ SetName: "examplev6", }, }, "second.example.com.": { - IPv4: &IPEntry{ + IPv4: &iPEntry{ SetName: "2examplev4", }, - IPv6: &IPEntry{ + IPv6: &iPEntry{ SetName: "2examplev6", }, }, @@ -127,12 +127,12 @@ func Test_GetSetsForFQDN(t *testing.T) { }, { name: "pattern from integration testing", - fqdnToEntry: map[string]CacheEntry{ + fqdnToEntry: map[string]cacheEntry{ "www.freechess.org.": { - IPv4: &IPEntry{ + IPv4: &iPEntry{ SetName: "testv4", }, - IPv6: &IPEntry{ + IPv6: &iPEntry{ SetName: "testv6", }, }, @@ -180,14 +180,14 @@ func Test_createIPSetFromIPEntry(t *testing.T) { name string fqdn string version firewallv1.IPVersion - entry *IPEntry + entry *iPEntry want firewallv1.IPSet }{ { name: "empty ip entry", fqdn: "www.freechess.org", version: "ip", - entry: &IPEntry{ + entry: &iPEntry{ SetName: "test", }, want: firewallv1.IPSet{ @@ -201,7 +201,7 @@ func Test_createIPSetFromIPEntry(t *testing.T) { name: "entry contains ips", fqdn: "www.freechess.org", version: "ip", - entry: &IPEntry{ + entry: &iPEntry{ SetName: "test", IPs: map[string]time.Time{ "1.2.3.4": time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), diff --git a/pkg/dns/dnsproxy.go b/pkg/dns/dnsproxy.go index f7338ded..8f181c47 100644 --- a/pkg/dns/dnsproxy.go +++ b/pkg/dns/dnsproxy.go @@ -37,11 +37,11 @@ type DNSProxy struct { handler DNSHandler } -func NewDNSProxy(ctx context.Context, dns string, port *uint, shootClient client.Client, log logr.Logger) (*DNSProxy, error) { +func NewDNSProxy(dns string, port *uint, shootClient client.Client, log logr.Logger) (*DNSProxy, error) { if dns == "" { dns = defaultDNSServerAddr } - cache, err := newDNSCache(ctx, dns, true, false, shootClient, log.WithName("DNS cache")) + cache, err := newDNSCache(dns, true, false, shootClient, log.WithName("DNS cache")) if err != nil { return nil, err } diff --git a/pkg/nftables/util_test.go b/pkg/nftables/util_test.go index ba88186b..995ba146 100644 --- a/pkg/nftables/util_test.go +++ b/pkg/nftables/util_test.go @@ -3,6 +3,8 @@ package nftables import ( "os" "testing" + + "github.com/google/go-cmp/cmp" ) func Test_equal(t *testing.T) { @@ -148,7 +150,6 @@ table ip firewall { }, } for _, tt := range tests { - tt := tt s, err := os.CreateTemp("/tmp", "source") if err != nil { t.Fail() @@ -173,3 +174,30 @@ table ip firewall { }) } } + +func Test_splitRules(t *testing.T) { + tests := []struct { + name string + rawRules []string + want []string + }{ + { + name: "split multiline string into separate strings", + rawRules: []string{ + "ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } log prefix \"nftables-firewall-accepted: \" limit rate 10/second\nip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } counter accept comment \"accept traffic for k8s network policy tcp\"", + }, + want: []string{ + "ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } log prefix \"nftables-firewall-accepted: \" limit rate 10/second", + "ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } counter accept comment \"accept traffic for k8s network policy tcp\"", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := splitRules(tt.rawRules) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("splitRules() diff = %s", diff) + } + }) + } +} From 32bf8a63ffc3b66604c900426044aa4a05be2f3f Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Fri, 12 Sep 2025 15:42:08 +0200 Subject: [PATCH 70/71] fix context stuff --- .../clusterwidenetworkpolicy_controller.go | 9 +++-- controllers/firewall_controller.go | 1 + main.go | 1 + pkg/dns/dnscache.go | 5 +-- pkg/dns/dnsproxy.go | 38 +++++++++++-------- 5 files changed, 32 insertions(+), 22 deletions(-) diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index f163b5d0..f8392fa8 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -40,6 +40,7 @@ type ClusterwideNetworkPolicyReconciler struct { SeedNamespace string Log logr.Logger + Ctx context.Context Recorder record.EventRecorder Interval time.Duration @@ -106,7 +107,7 @@ func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, _ ct cwnps.Items = validCwnps nftablesFirewall := nftables.NewFirewall(f, &cwnps, &services, r.DnsProxy, r.Log, r.Recorder) - if err := r.manageDNSProxy(ctx, f, cwnps, nftablesFirewall); err != nil { + if err := r.manageDNSProxy(f, cwnps, nftablesFirewall); err != nil { return ctrl.Result{}, err } updated, err := nftablesFirewall.Reconcile() @@ -129,7 +130,7 @@ func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, _ ct // manageDNSProxy start DNS proxy if toFQDN rules are present // if rules were deleted it will stop running DNS proxy func (r *ClusterwideNetworkPolicyReconciler) manageDNSProxy( - ctx context.Context, f *firewallv2.Firewall, cwnps firewallv1.ClusterwideNetworkPolicyList, nftablesFirewall *nftables.Firewall, + f *firewallv2.Firewall, cwnps firewallv1.ClusterwideNetworkPolicyList, nftablesFirewall *nftables.Firewall, ) (err error) { // Skipping is needed for testing if r.SkipDNS { @@ -144,10 +145,10 @@ func (r *ClusterwideNetworkPolicyReconciler) manageDNSProxy( if enableDNS && r.DnsProxy == nil { r.Log.Info("DNS Proxy is initialized") - if r.DnsProxy, err = dns.NewDNSProxy(f.Spec.DNSServerAddress, f.Spec.DNSPort, r.ShootClient, ctrl.Log.WithName("DNS proxy")); err != nil { + if r.DnsProxy, err = dns.NewDNSProxy(r.Ctx, f.Spec.DNSServerAddress, f.Spec.DNSPort, r.ShootClient, ctrl.Log.WithName("DNS proxy")); err != nil { return fmt.Errorf("failed to init DNS proxy: %w", err) } - go r.DnsProxy.Run(ctx) + go r.DnsProxy.Run() } else if !enableDNS && r.DnsProxy != nil { r.Log.Info("DNS Proxy is stopped") r.DnsProxy.Stop() diff --git a/controllers/firewall_controller.go b/controllers/firewall_controller.go index b56caccb..96b5ae79 100644 --- a/controllers/firewall_controller.go +++ b/controllers/firewall_controller.go @@ -39,6 +39,7 @@ type FirewallReconciler struct { Recorder record.EventRecorder Log logr.Logger + Ctx context.Context Scheme *runtime.Scheme Updater *updater.Updater diff --git a/main.go b/main.go index 6b690fac..43152de7 100644 --- a/main.go +++ b/main.go @@ -268,6 +268,7 @@ func main() { SeedClient: seedMgr.GetClient(), ShootClient: shootMgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("ClusterwideNetworkPolicy"), + Ctx: ctx, Recorder: shootMgr.GetEventRecorderFor("FirewallController"), FirewallName: firewallName, SeedNamespace: seedNamespace, diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index d15ede74..9327f2f7 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -21,7 +21,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" @@ -126,14 +125,14 @@ type DNSCache struct { ipv6Enabled bool } -func newDNSCache(dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) (*DNSCache, error) { +func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, shootClient client.Client, log logr.Logger) (*DNSCache, error) { c := DNSCache{ log: log, fqdnToEntry: map[string]cacheEntry{}, setNames: map[string]struct{}{}, dnsServerAddr: dns, shootClient: shootClient, - ctx: ctrl.SetupSignalHandler(), + ctx: ctx, ipv4Enabled: ipv4Enabled, ipv6Enabled: ipv6Enabled, } diff --git a/pkg/dns/dnsproxy.go b/pkg/dns/dnsproxy.go index 8f181c47..b9a16212 100644 --- a/pkg/dns/dnsproxy.go +++ b/pkg/dns/dnsproxy.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "strconv" + "time" "github.com/metal-stack/metal-networker/pkg/netconf" "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,9 +28,10 @@ type DNSHandler interface { } type DNSProxy struct { - log logr.Logger - cache *DNSCache - stopCh chan struct{} + log logr.Logger + ctx context.Context + cancelFunc context.CancelFunc + cache *DNSCache udpServer *dnsgo.Server tcpServer *dnsgo.Server @@ -37,15 +39,10 @@ type DNSProxy struct { handler DNSHandler } -func NewDNSProxy(dns string, port *uint, shootClient client.Client, log logr.Logger) (*DNSProxy, error) { +func NewDNSProxy(ctx context.Context, dns string, port *uint, shootClient client.Client, log logr.Logger) (*DNSProxy, error) { if dns == "" { dns = defaultDNSServerAddr } - cache, err := newDNSCache(dns, true, false, shootClient, log.WithName("DNS cache")) - if err != nil { - return nil, err - } - handler := NewDNSProxyHandler(log, cache) host, err := getHost() if err != nil { @@ -61,13 +58,22 @@ func NewDNSProxy(dns string, port *uint, shootClient client.Client, log logr.Log return nil, fmt.Errorf("failed to bind to port: %w", err) } + backgroundCtx, cancel := context.WithCancel(ctx) + cache, err := newDNSCache(backgroundCtx, dns, true, false, shootClient, log.WithName("DNS cache")) + if err != nil { + cancel() + return nil, err + } + handler := NewDNSProxyHandler(log, cache) + udpServer := &dnsgo.Server{PacketConn: udpConn, Addr: udpConn.LocalAddr().String(), Net: "udp", Handler: handler} tcpServer := &dnsgo.Server{Listener: tcpListener, Addr: udpConn.LocalAddr().String(), Net: "tcp", Handler: handler} return &DNSProxy{ - log: log, - cache: cache, - stopCh: make(chan struct{}), + log: log, + ctx: backgroundCtx, + cancelFunc: cancel, + cache: cache, udpServer: udpServer, tcpServer: tcpServer, @@ -77,7 +83,7 @@ func NewDNSProxy(dns string, port *uint, shootClient client.Client, log logr.Log } // Run starts TCP/UDP servers -func (p *DNSProxy) Run(ctx context.Context) { +func (p *DNSProxy) Run() { go func() { p.log.Info("starting UDP server") if err := p.udpServer.ActivateAndServe(); err != nil { @@ -92,7 +98,9 @@ func (p *DNSProxy) Run(ctx context.Context) { } }() - <-p.stopCh + <-p.ctx.Done() + ctx, cancel := context.WithTimeout(p.ctx, time.Second*5) + defer cancel() if err := p.udpServer.ShutdownContext(ctx); err != nil { p.log.Error(err, "failed to shut down UDP server") @@ -104,7 +112,7 @@ func (p *DNSProxy) Run(ctx context.Context) { // Stop starts TCP/UDP servers func (p *DNSProxy) Stop() { - close(p.stopCh) + p.cancelFunc() } func (p *DNSProxy) UpdateDNSServerAddr(addr string) error { From 72d0471a9652429a4eced4a4bc97dcac778f9850 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Fri, 12 Sep 2025 16:05:13 +0200 Subject: [PATCH 71/71] use ctx with timeout --- pkg/dns/dnscache.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go index 9327f2f7..4e922f0d 100644 --- a/pkg/dns/dnscache.go +++ b/pkg/dns/dnscache.go @@ -140,12 +140,12 @@ func newDNSCache(ctx context.Context, dns string, ipv4Enabled, ipv6Enabled bool, nn := types.NamespacedName{Name: fqdnStateConfigmapName, Namespace: fqdnStateNamespace} scm := &v1.ConfigMap{} - ctx, cancel := context.WithTimeout(c.ctx, 100*time.Millisecond) + ctxWithTimeout, cancel := context.WithTimeout(c.ctx, 5*time.Second) defer func() { cancel() }() - err := shootClient.Get(ctx, nn, scm) + err := shootClient.Get(ctxWithTimeout, nn, scm) if err != nil && !apierrors.IsNotFound(err) { c.log.Error(err, "error reading fqndstate configmap") return nil, err @@ -193,7 +193,12 @@ func (c *DNSCache) writeStateToConfigmap() error { debugLog.Info("DEBUG looking for configmap") - err = c.shootClient.Get(c.ctx, client.ObjectKeyFromObject(cm), cm) + ctxWithTimeout, cancel := context.WithTimeout(c.ctx, 5*time.Second) + defer func() { + cancel() + }() + + err = c.shootClient.Get(ctxWithTimeout, client.ObjectKeyFromObject(cm), cm) if err != nil && !apierrors.IsNotFound(err) { return err } @@ -203,7 +208,7 @@ func (c *DNSCache) writeStateToConfigmap() error { cm.Data = data - err = c.shootClient.Create(c.ctx, cm) + err = c.shootClient.Create(ctxWithTimeout, cm) if err != nil { return err } @@ -214,7 +219,7 @@ func (c *DNSCache) writeStateToConfigmap() error { if !reflect.DeepEqual(cm.Data, data) { cm.Data = data - err = c.shootClient.Update(c.ctx, cm) + err = c.shootClient.Update(ctxWithTimeout, cm) if err != nil { return err }