From 7ca816551e497c8785c41ecb35a35c6d8f5fae13 Mon Sep 17 00:00:00 2001 From: mreiger Date: Thu, 29 Aug 2024 11:30:38 +0200 Subject: [PATCH 01/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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