From ecd86ade23a6f38bd00b1d50bda8136c6eb7a15b Mon Sep 17 00:00:00 2001 From: Christopher Dziomba Date: Thu, 19 Sep 2024 17:09:47 +0200 Subject: [PATCH 1/7] Add config hot-replace feature --- pkg/config/config.go | 7 +++++++ pkg/frr/configure.go | 12 +++++++++++- pkg/frr/frr_test.go | 8 ++++---- pkg/reconciler/layer3.go | 2 +- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index af8bc8bb..aae329d3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -18,6 +18,8 @@ type Config struct { BPFInterfaces []string `yaml:"bpfInterfaces"` SkipVRFConfig []string `yaml:"skipVRFConfig"` ServerASN int `yaml:"serverASN"` + + Replacements []Replacement `yaml:"replacements"` } type VRFConfig struct { @@ -25,6 +27,11 @@ type VRFConfig struct { RT string `yaml:"rt"` } +type Replacement struct { + Old string `yaml:"old"` + New string `yaml:"new"` +} + func LoadConfig() (*Config, error) { config := &Config{} diff --git a/pkg/frr/configure.go b/pkg/frr/configure.go index e6902267..00132e8d 100644 --- a/pkg/frr/configure.go +++ b/pkg/frr/configure.go @@ -3,6 +3,7 @@ package frr import ( "bytes" "fmt" + "github.com/telekom/das-schiff-network-operator/pkg/config" "os" "regexp" @@ -33,7 +34,7 @@ type templateConfig struct { HostRouterID string } -func (m *Manager) Configure(in Configuration, nm *nl.Manager) (bool, error) { +func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg config.Config) (bool, error) { // Remove permit from VRF and only allow deny rules for mgmt VRFs for i := range in.VRFs { if in.VRFs[i].Name != m.mgmtVrf { @@ -66,6 +67,7 @@ func (m *Manager) Configure(in Configuration, nm *nl.Manager) (bool, error) { } targetConfig = fixRouteTargetReload(targetConfig) + targetConfig = fixCfgReplacements(targetConfig, nwopCfg.Replacements) if !bytes.Equal(currentConfig, targetConfig) { err = os.WriteFile(m.ConfigPath, targetConfig, frrPermissions) @@ -172,3 +174,11 @@ func fixRouteTargetReload(config []byte) []byte { return []byte(lines[:len(lines)-1]) }) } + +// fixCfgReplacements replaces placeholders in the configuration with the actual values. +func fixCfgReplacements(config []byte, replacements []config.Replacement) []byte { + for _, replacement := range replacements { + config = bytes.ReplaceAll(config, []byte(replacement.Old), []byte(replacement.New)) + } + return config +} diff --git a/pkg/frr/frr_test.go b/pkg/frr/frr_test.go index 88808721..b1c6f0a6 100644 --- a/pkg/frr/frr_test.go +++ b/pkg/frr/frr_test.go @@ -134,7 +134,7 @@ var _ = Describe("frr", func() { It("return error if cannot get underlay IP", func() { m := &Manager{} nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return(nil, errors.New("error listing addresses")) - _, err := m.Configure(Configuration{}, nl.NewManager(nlMock)) + _, err := m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) Expect(err).To(HaveOccurred()) }) It("return error if cannot node's name", func() { @@ -148,7 +148,7 @@ var _ = Describe("frr", func() { nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return([]netlink.Addr{ {IPNet: netlink.NewIPNet(net.IPv4(0, 0, 0, 0))}, }, nil) - _, err := m.Configure(Configuration{}, nl.NewManager(nlMock)) + _, err := m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) Expect(err).To(HaveOccurred()) if isSet { @@ -165,7 +165,7 @@ var _ = Describe("frr", func() { nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return([]netlink.Addr{ {IPNet: netlink.NewIPNet(net.IPv4(0, 0, 0, 0))}, }, nil) - _, err = m.Configure(Configuration{}, nl.NewManager(nlMock)) + _, err = m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) Expect(err).To(HaveOccurred()) if isSet { @@ -189,7 +189,7 @@ var _ = Describe("frr", func() { nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return([]netlink.Addr{ {IPNet: netlink.NewIPNet(net.IPv4(0, 0, 0, 0))}, }, nil) - _, err = m.Configure(Configuration{}, nl.NewManager(nlMock)) + _, err = m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) Expect(err).To(HaveOccurred()) if isSet { diff --git a/pkg/reconciler/layer3.go b/pkg/reconciler/layer3.go index bf50b417..9b526032 100644 --- a/pkg/reconciler/layer3.go +++ b/pkg/reconciler/layer3.go @@ -100,7 +100,7 @@ func (r *reconcile) configureFRR(vrfConfigs []frr.VRFConfiguration, reloadTwice changed, err := r.frrManager.Configure(frr.Configuration{ VRFs: vrfConfigs, ASN: r.config.ServerASN, - }, r.netlinkManager) + }, r.netlinkManager, *r.config) if err != nil { r.Logger.Error(err, "error updating FRR configuration") return fmt.Errorf("error updating FRR configuration: %w", err) From 789bcef00df081251078656dd531c5c27170ea09 Mon Sep 17 00:00:00 2001 From: Christopher Dziomba Date: Thu, 19 Sep 2024 17:15:39 +0200 Subject: [PATCH 2/7] pass by pointer instead of by value --- pkg/frr/configure.go | 6 +++--- pkg/frr/frr_test.go | 8 ++++---- pkg/reconciler/layer3.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/frr/configure.go b/pkg/frr/configure.go index 00132e8d..870304eb 100644 --- a/pkg/frr/configure.go +++ b/pkg/frr/configure.go @@ -34,7 +34,7 @@ type templateConfig struct { HostRouterID string } -func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg config.Config) (bool, error) { +func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg *config.Config) (bool, error) { // Remove permit from VRF and only allow deny rules for mgmt VRFs for i := range in.VRFs { if in.VRFs[i].Name != m.mgmtVrf { @@ -51,7 +51,7 @@ func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg config.Con } } - config, err := m.renderSubtemplates(in, nm) + frrConfig, err := m.renderSubtemplates(in, nm) if err != nil { return false, err } @@ -61,7 +61,7 @@ func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg config.Con return false, fmt.Errorf("error reading configuration file: %w", err) } - targetConfig, err := render(m.configTemplate, config) + targetConfig, err := render(m.configTemplate, frrConfig) if err != nil { return false, err } diff --git a/pkg/frr/frr_test.go b/pkg/frr/frr_test.go index b1c6f0a6..f04eb7b1 100644 --- a/pkg/frr/frr_test.go +++ b/pkg/frr/frr_test.go @@ -134,7 +134,7 @@ var _ = Describe("frr", func() { It("return error if cannot get underlay IP", func() { m := &Manager{} nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return(nil, errors.New("error listing addresses")) - _, err := m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) + _, err := m.Configure(Configuration{}, nl.NewManager(nlMock), &config.Config{}) Expect(err).To(HaveOccurred()) }) It("return error if cannot node's name", func() { @@ -148,7 +148,7 @@ var _ = Describe("frr", func() { nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return([]netlink.Addr{ {IPNet: netlink.NewIPNet(net.IPv4(0, 0, 0, 0))}, }, nil) - _, err := m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) + _, err := m.Configure(Configuration{}, nl.NewManager(nlMock), &config.Config{}) Expect(err).To(HaveOccurred()) if isSet { @@ -165,7 +165,7 @@ var _ = Describe("frr", func() { nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return([]netlink.Addr{ {IPNet: netlink.NewIPNet(net.IPv4(0, 0, 0, 0))}, }, nil) - _, err = m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) + _, err = m.Configure(Configuration{}, nl.NewManager(nlMock), &config.Config{}) Expect(err).To(HaveOccurred()) if isSet { @@ -189,7 +189,7 @@ var _ = Describe("frr", func() { nlMock.EXPECT().AddrList(gomock.Any(), gomock.Any()).Return([]netlink.Addr{ {IPNet: netlink.NewIPNet(net.IPv4(0, 0, 0, 0))}, }, nil) - _, err = m.Configure(Configuration{}, nl.NewManager(nlMock), config.Config{}) + _, err = m.Configure(Configuration{}, nl.NewManager(nlMock), &config.Config{}) Expect(err).To(HaveOccurred()) if isSet { diff --git a/pkg/reconciler/layer3.go b/pkg/reconciler/layer3.go index 9b526032..f66e91db 100644 --- a/pkg/reconciler/layer3.go +++ b/pkg/reconciler/layer3.go @@ -100,7 +100,7 @@ func (r *reconcile) configureFRR(vrfConfigs []frr.VRFConfiguration, reloadTwice changed, err := r.frrManager.Configure(frr.Configuration{ VRFs: vrfConfigs, ASN: r.config.ServerASN, - }, r.netlinkManager, *r.config) + }, r.netlinkManager, r.config) if err != nil { r.Logger.Error(err, "error updating FRR configuration") return fmt.Errorf("error updating FRR configuration: %w", err) From 0fd15aff1be785c20ebc748ea62c861553cfa2ef Mon Sep 17 00:00:00 2001 From: Christopher Dziomba Date: Thu, 19 Sep 2024 17:23:33 +0200 Subject: [PATCH 3/7] fix shadowing --- pkg/frr/configure.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/frr/configure.go b/pkg/frr/configure.go index 870304eb..cc5ff2e4 100644 --- a/pkg/frr/configure.go +++ b/pkg/frr/configure.go @@ -176,9 +176,9 @@ func fixRouteTargetReload(config []byte) []byte { } // fixCfgReplacements replaces placeholders in the configuration with the actual values. -func fixCfgReplacements(config []byte, replacements []config.Replacement) []byte { +func fixCfgReplacements(frrConfig []byte, replacements []config.Replacement) []byte { for _, replacement := range replacements { - config = bytes.ReplaceAll(config, []byte(replacement.Old), []byte(replacement.New)) + frrConfig = bytes.ReplaceAll(frrConfig, []byte(replacement.Old), []byte(replacement.New)) } - return config + return frrConfig } From 480d5199851d41a8e9dc27134e1580a86f88451c Mon Sep 17 00:00:00 2001 From: Christopher Dziomba Date: Thu, 19 Sep 2024 18:09:16 +0200 Subject: [PATCH 4/7] fix shadowing in a different func --- pkg/frr/configure.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/frr/configure.go b/pkg/frr/configure.go index cc5ff2e4..fd651dad 100644 --- a/pkg/frr/configure.go +++ b/pkg/frr/configure.go @@ -156,8 +156,8 @@ func (m *Manager) renderSubtemplates(in Configuration, nlManager *nl.Manager) (* // fixRouteTargetReload is a workaround for FRR's inability to reload route-targets if they are configured in a single line. // This function splits such lines into multiple lines, each containing a single route-target. -func fixRouteTargetReload(config []byte) []byte { - return rtLinesRe.ReplaceAllFunc(config, func(s []byte) []byte { +func fixRouteTargetReload(frrConfig []byte) []byte { + return rtLinesRe.ReplaceAllFunc(frrConfig, func(s []byte) []byte { parts := rtPartsRe.FindSubmatch(s) if parts == nil { return s From c8f1de6748a2debe0338094031f5f7ead91194b0 Mon Sep 17 00:00:00 2001 From: Christopher Dziomba Date: Thu, 19 Sep 2024 18:12:12 +0200 Subject: [PATCH 5/7] fix import ordering --- pkg/frr/configure.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/frr/configure.go b/pkg/frr/configure.go index fd651dad..734d1b2d 100644 --- a/pkg/frr/configure.go +++ b/pkg/frr/configure.go @@ -3,10 +3,10 @@ package frr import ( "bytes" "fmt" - "github.com/telekom/das-schiff-network-operator/pkg/config" "os" "regexp" + "github.com/telekom/das-schiff-network-operator/pkg/config" "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" "github.com/telekom/das-schiff-network-operator/pkg/nl" ) From 73fdf3374f4f7d39bcb8e6f75468b25415c80330 Mon Sep 17 00:00:00 2001 From: Christopher Dziomba Date: Fri, 20 Sep 2024 09:23:19 +0200 Subject: [PATCH 6/7] allow regex replace --- pkg/config/config.go | 5 +++-- pkg/frr/configure.go | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index aae329d3..c7054888 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -28,8 +28,9 @@ type VRFConfig struct { } type Replacement struct { - Old string `yaml:"old"` - New string `yaml:"new"` + Old string `yaml:"old"` + New string `yaml:"new"` + Regex bool `yaml:"regex"` } func LoadConfig() (*Config, error) { diff --git a/pkg/frr/configure.go b/pkg/frr/configure.go index 734d1b2d..fa83c581 100644 --- a/pkg/frr/configure.go +++ b/pkg/frr/configure.go @@ -178,7 +178,12 @@ func fixRouteTargetReload(frrConfig []byte) []byte { // fixCfgReplacements replaces placeholders in the configuration with the actual values. func fixCfgReplacements(frrConfig []byte, replacements []config.Replacement) []byte { for _, replacement := range replacements { - frrConfig = bytes.ReplaceAll(frrConfig, []byte(replacement.Old), []byte(replacement.New)) + if !replacement.Regex { + frrConfig = bytes.ReplaceAll(frrConfig, []byte(replacement.Old), []byte(replacement.New)) + } else { + re := regexp.MustCompile(replacement.Old) + frrConfig = re.ReplaceAll(frrConfig, []byte(replacement.New)) + } } return frrConfig } From 1f5aeccf2d153ecb3fc0e5b25563fa3a4074b565 Mon Sep 17 00:00:00 2001 From: Christopher Dziomba Date: Fri, 20 Sep 2024 13:56:27 +0200 Subject: [PATCH 7/7] rename fixCfgReplacements to applyCfgReplacements --- pkg/frr/configure.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/frr/configure.go b/pkg/frr/configure.go index fa83c581..d5d8782c 100644 --- a/pkg/frr/configure.go +++ b/pkg/frr/configure.go @@ -67,7 +67,7 @@ func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg *config.Co } targetConfig = fixRouteTargetReload(targetConfig) - targetConfig = fixCfgReplacements(targetConfig, nwopCfg.Replacements) + targetConfig = applyCfgReplacements(targetConfig, nwopCfg.Replacements) if !bytes.Equal(currentConfig, targetConfig) { err = os.WriteFile(m.ConfigPath, targetConfig, frrPermissions) @@ -175,8 +175,8 @@ func fixRouteTargetReload(frrConfig []byte) []byte { }) } -// fixCfgReplacements replaces placeholders in the configuration with the actual values. -func fixCfgReplacements(frrConfig []byte, replacements []config.Replacement) []byte { +// applyCfgReplacements replaces placeholders in the configuration with the actual values. +func applyCfgReplacements(frrConfig []byte, replacements []config.Replacement) []byte { for _, replacement := range replacements { if !replacement.Regex { frrConfig = bytes.ReplaceAll(frrConfig, []byte(replacement.Old), []byte(replacement.New))