diff --git a/confd/etc/calico/confd/conf.d/bird6_ipam.toml b/confd/etc/calico/confd/conf.d/bird6_ipam.toml index 03784a4b263..6027000d862 100644 --- a/confd/etc/calico/confd/conf.d/bird6_ipam.toml +++ b/confd/etc/calico/confd/conf.d/bird6_ipam.toml @@ -6,8 +6,6 @@ keys = [ "/bgpconfig", "/v1/ipam/v6/pool", "/bgp/v1/host//NODENAME", - "/bgp/v1/global/svc_loadbalancer_ips", - "/bgp/v1/global/program_cluster_routes", "/staticroutesv6", "/rejectcidrsv6", ] diff --git a/confd/etc/calico/confd/conf.d/bird_ipam.toml b/confd/etc/calico/confd/conf.d/bird_ipam.toml index 237c58889fc..74596438853 100644 --- a/confd/etc/calico/confd/conf.d/bird_ipam.toml +++ b/confd/etc/calico/confd/conf.d/bird_ipam.toml @@ -6,8 +6,6 @@ keys = [ "/bgpconfig", "/v1/ipam/v4/pool", "/bgp/v1/host//NODENAME", - "/bgp/v1/global/svc_loadbalancer_ips", - "/bgp/v1/global/program_cluster_routes", "/staticroutes", "/rejectcidrs", ] diff --git a/confd/etc/calico/confd/templates/bird6_ipam.cfg.template b/confd/etc/calico/confd/templates/bird6_ipam.cfg.template index 425bae83ce3..9461bd99fae 100644 --- a/confd/etc/calico/confd/templates/bird6_ipam.cfg.template +++ b/confd/etc/calico/confd/templates/bird6_ipam.cfg.template @@ -44,12 +44,11 @@ function calico_export_to_bgp_peers(bool internal_peer) { {{- end}} {{- end}} {{- $rr_cluster_id_key := printf "/bgp/v1/host/%s/rr_cluster_id" $config.NodeName}} -{{- $lb_ips := "/bgp/v1/global/svc_loadbalancer_ips"}} {{- if exists $rr_cluster_id_key}}{{$rr_cluster_id := getv $rr_cluster_id_key}} -{{- if and (not (eq $rr_cluster_id "")) (exists $lb_ips)}} +{{- if and (not (eq $rr_cluster_id "")) $config.LoadBalancerIPs}} # Configured as a RR - accept any routes within configured LB service IP ranges - {{- range split (getv $lb_ips) ","}} + {{- range $config.LoadBalancerIPs}} {{- $cidr := .}} {{- if contains $cidr ":"}} if ( net ~ {{$cidr}} ) then { accept; } diff --git a/confd/etc/calico/confd/templates/bird_ipam.cfg.template b/confd/etc/calico/confd/templates/bird_ipam.cfg.template index 22865f7657a..962c0bf283f 100644 --- a/confd/etc/calico/confd/templates/bird_ipam.cfg.template +++ b/confd/etc/calico/confd/templates/bird_ipam.cfg.template @@ -44,12 +44,11 @@ function calico_export_to_bgp_peers(bool internal_peer) { {{- end}} {{- end}} {{- $rr_cluster_id_key := printf "/bgp/v1/host/%s/rr_cluster_id" $config.NodeName}} -{{- $lb_ips := "/bgp/v1/global/svc_loadbalancer_ips"}} {{- if exists $rr_cluster_id_key}}{{$rr_cluster_id := getv $rr_cluster_id_key}} -{{- if and (not (eq $rr_cluster_id "")) (exists $lb_ips)}} +{{- if and (not (eq $rr_cluster_id "")) $config.LoadBalancerIPs}} # Configured as a RR - accept any routes within configured LB service IP ranges - {{- range split (getv $lb_ips) ","}} + {{- range $config.LoadBalancerIPs}} {{- $cidr := .}} {{- if not (contains $cidr ":")}} if ( net ~ {{$cidr}} ) then { accept; } diff --git a/confd/pkg/backends/calico/bgp_processor.go b/confd/pkg/backends/calico/bgp_processor.go index 8c8c38bb779..2af756aad0b 100644 --- a/confd/pkg/backends/calico/bgp_processor.go +++ b/confd/pkg/backends/calico/bgp_processor.go @@ -62,10 +62,11 @@ func (c *client) GetBirdBGPConfig(ipVersion int) (*types.BirdBGPConfig, error) { pc := c.getBGPProcessorContext() config := &types.BirdBGPConfig{ - NodeName: NodeName, - Peers: make([]types.BirdBGPPeer, 0), - Filters: make(map[string]string), - Communities: make([]types.CommunityRule, 0), + NodeName: NodeName, + Peers: make([]types.BirdBGPPeer, 0), + Filters: make(map[string]string), + Communities: make([]types.CommunityRule, 0), + LoadBalancerIPs: getServiceLoadBalancerIPs(pc.globalBGPConfig), } // Get basic node configuration @@ -76,7 +77,7 @@ func (c *client) GetBirdBGPConfig(ipVersion int) (*types.BirdBGPConfig, error) { logc.Debugf("Populated node configuration: node=%s, ip=%s, ipv6=%s, as=%s", config.NodeName, config.NodeIP, config.NodeIPv6, config.ASNumber) // Process all peer types - if err := c.processPeers(config, ipVersion); err != nil { + if err := c.processPeers(pc, config, ipVersion); err != nil { logc.WithError(err).Warn("Failed to process BGP peers") return nil, err } @@ -198,23 +199,17 @@ func (c *client) populateNodeConfig(pc *processorContext, config *types.BirdBGPC } // Process ignored interfaces and build complete interface string - ignoredInterfaces, err := c.getNodeOrGlobalValue(NodeName, "ignored_interfaces") - - // Build the complete interface pattern string - if err == nil && ignoredInterfaces != "" { - // Parse comma-separated list and build pattern - ifaceList := strings.Split(ignoredInterfaces, ",") - var patterns []string - for _, iface := range ifaceList { - patterns = append(patterns, fmt.Sprintf(`-"%s"`, iface)) - } - // Add standard exclusions and wildcard - patterns = append(patterns, `-"cali*"`, `-"kube-ipvs*"`, `"*"`) - config.DirectInterfaces = strings.Join(patterns, ", ") - } else { - // Default pattern with explanatory comment - config.DirectInterfaces = `-"cali*", -"kube-ipvs*", "*"` + var ignoredInterfaces []string + if pc.globalBGPConfig != nil { + ignoredInterfaces = pc.globalBGPConfig.Spec.IgnoredInterfaces } + var patterns []string + for _, iface := range ignoredInterfaces { + patterns = append(patterns, fmt.Sprintf(`-"%s"`, iface)) + } + // Add standard exclusions and wildcard include. + patterns = append(patterns, `-"cali*"`, `-"kube-ipvs*"`, `"*"`) + config.DirectInterfaces = strings.Join(patterns, ", ") // Set NormalRoutePriority from BGPConfiguration (default 1024). config.NormalRoutePriority = getNormalRoutePriority(ipVersion, pc.globalBGPConfig) @@ -247,12 +242,12 @@ func getNormalRoutePriority(ipVersion int, bgpConfig *v3.BGPConfiguration) (prio } // processPeers processes all BGP peers (mesh, global, and node-specific) -func (c *client) processPeers(config *types.BirdBGPConfig, ipVersion int) error { +func (c *client) processPeers(pc *processorContext, config *types.BirdBGPConfig, ipVersion int) error { // Get node's route reflector cluster ID nodeClusterID, _ := c.GetValue(fmt.Sprintf("/calico/bgp/v1/host/%s/rr_cluster_id", NodeName)) // Process node-to-node mesh peers - if err := c.processMeshPeers(config, nodeClusterID, ipVersion); err != nil { + if err := c.processMeshPeers(pc, config, nodeClusterID, ipVersion); err != nil { return fmt.Errorf("failed to process mesh peers: %w", err) } @@ -270,7 +265,7 @@ func (c *client) processPeers(config *types.BirdBGPConfig, ipVersion int) error } // processMeshPeers processes node-to-node mesh BGP peers -func (c *client) processMeshPeers(config *types.BirdBGPConfig, nodeClusterID string, ipVersion int) error { +func (c *client) processMeshPeers(pc *processorContext, config *types.BirdBGPConfig, nodeClusterID string, ipVersion int) error { logc := log.WithField("ipVersion", ipVersion) // If this node is a route reflector, skip mesh processing @@ -300,10 +295,6 @@ func (c *client) processMeshPeers(config *types.BirdBGPConfig, nodeClusterID str return nil } - // Get global mesh settings - meshPassword, _ := c.GetValue("/calico/bgp/v1/global/node_mesh_password") - meshRestartTime, _ := c.GetValue("/calico/bgp/v1/global/node_mesh_restart_time") - // Determine which IP address field to use ipAddrSuffix := "ip_addr_v4" if ipVersion == 6 { @@ -382,8 +373,8 @@ func (c *client) processMeshPeers(config *types.BirdBGPConfig, nodeClusterID str ASNumber: peerAS, Type: "mesh", SourceAddr: currentNodeIP, - Password: meshPassword, - GracefulRestart: meshRestartTime, + Password: c.getNodeMeshPassword(pc.globalBGPConfig), + GracefulRestart: getNodeMeshRestartTime(pc.globalBGPConfig), } // Make mesh unidirectional to avoid race conditions diff --git a/confd/pkg/backends/calico/bgp_processor_test.go b/confd/pkg/backends/calico/bgp_processor_test.go index e9463607fab..be1a53b2980 100644 --- a/confd/pkg/backends/calico/bgp_processor_test.go +++ b/confd/pkg/backends/calico/bgp_processor_test.go @@ -25,6 +25,8 @@ import ( v3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "github.com/projectcalico/calico/confd/pkg/backends/types" @@ -594,10 +596,10 @@ func TestPopulateNodeConfig_IgnoredInterfaces_Custom(t *testing.T) { cache := map[string]string{ "/calico/bgp/v1/host/test-node/ip_addr_v4": "10.0.0.1", "/calico/bgp/v1/global/as_num": "64512", - "/calico/bgp/v1/global/ignored_interfaces": "eth0,docker*", } c := newTestClient(cache, nil) + c.globalBGPConfig.Spec.IgnoredInterfaces = []string{"eth0", "docker*"} config := &types.BirdBGPConfig{ NodeName: NodeName, } @@ -613,34 +615,6 @@ func TestPopulateNodeConfig_IgnoredInterfaces_Custom(t *testing.T) { assert.Contains(t, config.DirectInterfaces, `"*"`) } -func TestPopulateNodeConfig_NodeSpecificIgnoredInterfaces(t *testing.T) { - originalNodeName := NodeName - NodeName = "test-node" - defer func() { NodeName = originalNodeName }() - - _ = os.Unsetenv("CALICO_ROUTER_ID") - - cache := map[string]string{ - "/calico/bgp/v1/host/test-node/ip_addr_v4": "10.0.0.1", - "/calico/bgp/v1/global/as_num": "64512", - "/calico/bgp/v1/global/ignored_interfaces": "global-if", // Global - "/calico/bgp/v1/host/test-node/ignored_interfaces": "node-if", // Node-specific (should win) - } - - c := newTestClient(cache, nil) - config := &types.BirdBGPConfig{ - NodeName: NodeName, - } - - err := c.populateNodeConfig(c.getBGPProcessorContext(), config, 4) - require.NoError(t, err) - - // Node-specific interface should be present - assert.Contains(t, config.DirectInterfaces, `-"node-if"`) - // Global interface should NOT be present - assert.NotContains(t, config.DirectInterfaces, `-"global-if"`) -} - func TestPopulateNodeConfig_NoBindMode(t *testing.T) { originalNodeName := NodeName NodeName = "test-node" @@ -1158,7 +1132,7 @@ func TestProcessMeshPeers_BasicMesh(t *testing.T) { ASNumber: "64512", } - err := c.processMeshPeers(config, "", 4) + err := c.processMeshPeers(c.getBGPProcessorContext(), config, "", 4) require.NoError(t, err) // Should have 2 mesh peers (node-2 and node-3, excluding ourselves) @@ -1200,7 +1174,7 @@ func TestProcessMeshPeers_IPv6(t *testing.T) { ASNumber: "64512", } - err := c.processMeshPeers(config, "", 6) + err := c.processMeshPeers(c.getBGPProcessorContext(), config, "", 6) require.NoError(t, err) assert.Len(t, config.Peers, 1) @@ -1232,7 +1206,7 @@ func TestProcessMeshPeers_MeshDisabled(t *testing.T) { ASNumber: "64512", } - err := c.processMeshPeers(config, "", 4) + err := c.processMeshPeers(c.getBGPProcessorContext(), config, "", 4) require.NoError(t, err) // No peers should be added when mesh is disabled @@ -1263,7 +1237,7 @@ func TestProcessMeshPeers_RouteReflector(t *testing.T) { } // Node is a route reflector - should skip mesh - err := c.processMeshPeers(config, "rr-cluster-1", 4) + err := c.processMeshPeers(c.getBGPProcessorContext(), config, "rr-cluster-1", 4) require.NoError(t, err) assert.Len(t, config.Peers, 0) @@ -1294,7 +1268,7 @@ func TestProcessMeshPeers_SkipRouteReflectorPeers(t *testing.T) { ASNumber: "64512", } - err := c.processMeshPeers(config, "", 4) + err := c.processMeshPeers(c.getBGPProcessorContext(), config, "", 4) require.NoError(t, err) // Should only have node-3, node-2 is a route reflector @@ -1328,7 +1302,7 @@ func TestProcessMeshPeers_PassiveMode(t *testing.T) { ASNumber: "64512", } - err := c.processMeshPeers(config, "", 4) + err := c.processMeshPeers(c.getBGPProcessorContext(), config, "", 4) require.NoError(t, err) assert.Len(t, config.Peers, 2) @@ -1356,21 +1330,29 @@ func TestProcessMeshPeers_WithPasswordAndRestartTime(t *testing.T) { meshConfigJSON, _ := json.Marshal(meshConfig) cache := map[string]string{ - "/calico/bgp/v1/global/node_mesh": string(meshConfigJSON), - "/calico/bgp/v1/global/node_mesh_password": "secret123", - "/calico/bgp/v1/global/node_mesh_restart_time": "120", - "/calico/bgp/v1/host/node-1/ip_addr_v4": "10.0.0.1", - "/calico/bgp/v1/host/node-2/ip_addr_v4": "10.0.0.2", + "/calico/bgp/v1/global/node_mesh": string(meshConfigJSON), + "/calico/bgp/v1/host/node-1/ip_addr_v4": "10.0.0.1", + "/calico/bgp/v1/host/node-2/ip_addr_v4": "10.0.0.2", } c := newTestClient(cache, nil) + c.globalBGPConfig.Spec.NodeMeshMaxRestartTime = ptr.To(metav1.Duration{Duration: 120 * time.Second}) + c.globalBGPConfig.Spec.NodeMeshPassword = &v3.BGPPassword{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "", + }, + Key: "", + }, + } + c.secretWatcher = &fakeSecret{secret: "secret123"} config := &types.BirdBGPConfig{ NodeIP: "10.0.0.1", ASNumber: "64512", } - err := c.processMeshPeers(config, "", 4) + err := c.processMeshPeers(c.getBGPProcessorContext(), config, "", 4) require.NoError(t, err) require.Len(t, config.Peers, 1) @@ -1378,6 +1360,18 @@ func TestProcessMeshPeers_WithPasswordAndRestartTime(t *testing.T) { assert.Equal(t, "120", config.Peers[0].GracefulRestart) } +type fakeSecret struct { + secret string +} + +func (s *fakeSecret) GetSecret(name, key string) (string, error) { + return s.secret, nil +} + +func (s *fakeSecret) MarkStale() {} + +func (s *fakeSecret) SweepStale() {} + func TestProcessGlobalPeers_BasicPeer(t *testing.T) { originalNodeName := NodeName NodeName = "node-1" @@ -1647,7 +1641,7 @@ func TestProcessPeers_CombinedMeshGlobalNode(t *testing.T) { } // Process all peer types - err := c.processPeers(config, 4) + err := c.processPeers(c.getBGPProcessorContext(), config, 4) require.NoError(t, err) // Should have: 1 mesh peer (node-2), 1 global peer, 1 node peer diff --git a/confd/pkg/backends/calico/client.go b/confd/pkg/backends/calico/client.go index a134d069640..7740a9499f3 100644 --- a/confd/pkg/backends/calico/client.go +++ b/confd/pkg/backends/calico/client.go @@ -74,10 +74,6 @@ var ( largeCommunity = regexp.MustCompile(`^(\d+):(\d+):(\d+)$`) ) -var sensitiveValues = map[string]any{ - "/calico/bgp/v1/global/node_mesh_password": nil, -} - // backendClientAccessor is an interface to access the backend client from the main v2 client. type backendClientAccessor interface { Backend() api.Client @@ -367,7 +363,7 @@ type client struct { k8sClient kubernetes.Interface // Subcomponent for accessing and watching secrets (that hold BGP passwords). - secretWatcher *secretWatcher + secretWatcher secretWatcherInterface // Subcomponent for accessing and watching local BGP peers. localBGPPeerWatcher *LocalBGPPeerWatcher @@ -995,7 +991,7 @@ func (c *client) OnUpdates(updates []api.Update) { } } -func (c *client) onUpdates(updates []api.Update, needUpdatePeersV1 bool) { +func (c *client) onUpdates(updates []api.Update, triggered bool) { // Update our cache from the updates. c.cacheLock.Lock() defer c.cacheLock.Unlock() @@ -1003,6 +999,13 @@ func (c *client) onUpdates(updates []api.Update, needUpdatePeersV1 bool) { // Indicate that our cache has been updated. c.incrementCacheRevision() + // If triggered (by c.recheckPeerConfig), force recomputation of the set of BGP peers, and + // of the details that go into their rendering. + needUpdatePeersV1 := triggered + if triggered { + c.keyUpdated("/calico/bgpconfig") + } + // Track whether these updates require BGP peerings to be recomputed. needUpdatePeersReasons := []string{} @@ -1164,10 +1167,8 @@ func (c *client) onUpdates(updates []api.Update, needUpdatePeersV1 bool) { log.Info("Recompute BGP peerings: " + strings.Join(needUpdatePeersReasons, "; ")) c.updatePeersV1() - // Also update BGP config passwords before removing old watched secrets - if c.globalBGPConfig != nil { - c.getNodeMeshPasswordKVPair(c.globalBGPConfig, model.GlobalBGPConfigKey{}) - } + // Re-reference the BGPConfiguration mesh password, if any. + c.getNodeMeshPassword(c.globalBGPConfig) // Clean up any secrets that are no longer of interest. if c.secretWatcher != nil { @@ -1191,29 +1192,16 @@ func (c *client) onUpdates(updates []api.Update, needUpdatePeersV1 bool) { } } - // Update external IP CIDRs. In v1 format, they are a single comma-separated - // string. If the string isn't empty, split on the comma and pass a list of strings - // to the route generator. An empty string indicates a withdrawal of that set of - // service IPs. - var externalIPs []string - if len(c.cache["/calico/bgp/v1/global/svc_external_ips"]) > 0 { - externalIPs = strings.Split(c.cache["/calico/bgp/v1/global/svc_external_ips"], ",") - } - c.onExternalIPsUpdate(externalIPs) + // Update external IP CIDRs. + c.onExternalIPsUpdate(getServiceExternalIPs(c.globalBGPConfig)) - // Same for cluster CIDRs. - var clusterIPs []string - if len(c.cache["/calico/bgp/v1/global/svc_cluster_ips"]) > 0 { - clusterIPs = strings.Split(c.cache["/calico/bgp/v1/global/svc_cluster_ips"], ",") + // Same for cluster CIDRs - except those can be overridden by an env var, so only + // process cluster IPs in BGPConfiguration if that env var is not set. + if len(os.Getenv(envAdvertiseClusterIPs)) == 0 { + c.onClusterIPsUpdate(getServiceClusterIPs(c.globalBGPConfig)) } - c.onClusterIPsUpdate(clusterIPs) - // Same for loadbalancer CIDRs. - var loadBalancerIPs []string - if len(c.cache["/calico/bgp/v1/global/svc_loadbalancer_ips"]) > 0 { - loadBalancerIPs = strings.Split(c.cache["/calico/bgp/v1/global/svc_loadbalancer_ips"], ",") - } - c.onLoadBalancerIPsUpdate(loadBalancerIPs) + c.onLoadBalancerIPsUpdate(getServiceLoadBalancerIPs(c.globalBGPConfig)) if c.rg != nil { // Trigger the route generator to recheck and advertise or withdraw @@ -1232,15 +1220,8 @@ func (c *client) updateBGPConfigCache(resName string, v3res *apiv3.BGPConfigurat c.getPrefixAdvertisementsKVPair(v3res, model.GlobalBGPConfigKey{}) c.getListenPortKVPair(v3res, model.GlobalBGPConfigKey{}, updatePeersV1, updateReasons) c.getASNumberKVPair(v3res, model.GlobalBGPConfigKey{}, updatePeersV1, updateReasons) - c.getServiceExternalIPsKVPair(v3res, model.GlobalBGPConfigKey{}, svcAdvertisement) - c.getServiceClusterIPsKVPair(v3res, model.GlobalBGPConfigKey{}, svcAdvertisement) - c.getServiceLoadBalancerIPsKVPair(v3res, model.GlobalBGPConfigKey{}, svcAdvertisement) c.getNodeToNodeMeshKVPair(v3res, model.GlobalBGPConfigKey{}) c.getLogSeverityKVPair(v3res, model.GlobalBGPConfigKey{}) - c.getNodeMeshRestartTimeKVPair(v3res, model.GlobalBGPConfigKey{}) - c.getNodeMeshPasswordKVPair(v3res, model.GlobalBGPConfigKey{}) - c.getIgnoredInterfacesKVPair(v3res, model.GlobalBGPConfigKey{}) - c.getProgramClusterRoutesKVPair(v3res, model.GlobalBGPConfigKey{}) // Update service load balancer aggregation setting if v3res != nil && v3res.Spec.ServiceLoadBalancerAggregation != nil { @@ -1250,18 +1231,15 @@ func (c *client) updateBGPConfigCache(resName string, v3res *apiv3.BGPConfigurat c.serviceLoadBalancerAggregation = apiv3.ServiceLoadBalancerAggregationEnabled } - // Check for changes in BGPConfiguration fields that impact GetBirdBGPConfig. Note, - // the following changes do not affect the _set_ of BGP peers, so do not require - // setting updatePeersV1 and updateReasons. - if getNormalRoutePriority(4, v3res) != getNormalRoutePriority(4, c.globalBGPConfig) { - c.keyUpdated("/calico/bgpconfig") - } - if getNormalRoutePriority(6, v3res) != getNormalRoutePriority(6, c.globalBGPConfig) { - c.keyUpdated("/calico/bgpconfig") - } - if getBindMode(v3res) != getBindMode(c.globalBGPConfig) { - c.keyUpdated("/calico/bgpconfig") - } + // BGPConfiguration changes should not be very frequent, but they often impact + // GetBirdBGPConfig when they do occur, so flag that templates should be processed + // again. Note, BGPConfiguration changes do not generally affect the _set_ of BGP + // peers, so do not always require setting updatePeersV1 and updateReasons. + c.keyUpdated("/calico/bgpconfig") + + // Some of the previous per-field logic above used to set svcAdvertisement + // unconditionally. It's equivalent to set it unconditionally here. + *svcAdvertisement = true // Cache the updated BGP configuration c.globalBGPConfig = v3res @@ -1406,13 +1384,8 @@ func (c *client) getASNumberKVPair(v3res *apiv3.BGPConfiguration, key any, updat *updatePeersV1 = true } -func (c *client) getServiceExternalIPsKVPair(v3res *apiv3.BGPConfiguration, key any, svcAdvertisement *bool) { - svcExternalIPKey := getBGPConfigKey("svc_external_ips", key) - - if v3res != nil && v3res.Spec.ServiceExternalIPs != nil && len(v3res.Spec.ServiceExternalIPs) != 0 { - // We wrap each Service external IP in a ServiceExternalIPBlock struct to - // achieve the desired API structure, unpack that. - ipCidrs := make([]string, 0, len(v3res.Spec.ServiceExternalIPs)) +func getServiceExternalIPs(v3res *apiv3.BGPConfiguration) (ipCidrs []string) { + if v3res != nil { for _, ipBlock := range v3res.Spec.ServiceExternalIPs { if ipBlock.CIDR == "" { // The CRD allows CIDR to be optional so we just ignore empty CIDRs. @@ -1420,18 +1393,12 @@ func (c *client) getServiceExternalIPsKVPair(v3res *apiv3.BGPConfiguration, key } ipCidrs = append(ipCidrs, ipBlock.CIDR) } - c.updateCache(api.UpdateTypeKVUpdated, getKVPair(svcExternalIPKey, strings.Join(ipCidrs, ","))) - } else { - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(svcExternalIPKey)) } - *svcAdvertisement = true + return } -func (c *client) getServiceLoadBalancerIPsKVPair(v3res *apiv3.BGPConfiguration, key any, svcAdvertisement *bool) { - svcLoadBalancerIPKey := getBGPConfigKey("svc_loadbalancer_ips", key) - - if v3res != nil && v3res.Spec.ServiceLoadBalancerIPs != nil && len(v3res.Spec.ServiceLoadBalancerIPs) != 0 { - ipCidrs := make([]string, 0, len(v3res.Spec.ServiceLoadBalancerIPs)) +func getServiceLoadBalancerIPs(v3res *apiv3.BGPConfiguration) (ipCidrs []string) { + if v3res != nil { for _, ipBlock := range v3res.Spec.ServiceLoadBalancerIPs { if ipBlock.CIDR == "" { // The CRD allows CIDR to be optional so we just ignore empty CIDRs. @@ -1439,39 +1406,21 @@ func (c *client) getServiceLoadBalancerIPsKVPair(v3res *apiv3.BGPConfiguration, } ipCidrs = append(ipCidrs, ipBlock.CIDR) } - c.updateCache(api.UpdateTypeKVUpdated, getKVPair(svcLoadBalancerIPKey, strings.Join(ipCidrs, ","))) - } else { - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(svcLoadBalancerIPKey)) } - *svcAdvertisement = true + return } -func (c *client) getServiceClusterIPsKVPair(v3res *apiv3.BGPConfiguration, key any, svcAdvertisement *bool) { - svcInternalIPKey := getBGPConfigKey("svc_cluster_ips", key) - - if len(os.Getenv(envAdvertiseClusterIPs)) != 0 { - // ClusterIPs are configurable through an environment variable. If specified, - // that variable takes precedence over datastore config, so we should ignore the update. - // Setting Spec.ServiceClusterIPs to nil, so we keep using the cache value set during startup. - log.Infof("Ignoring serviceClusterIPs update due to environment variable %s", envAdvertiseClusterIPs) - } else { - if v3res != nil && v3res.Spec.ServiceClusterIPs != nil && len(v3res.Spec.ServiceClusterIPs) != 0 { - // We wrap each Service Cluster IP in a ServiceClusterIPBlock to - // achieve the desired API structure. This unpacks that. - ipCidrs := make([]string, 0, len(v3res.Spec.ServiceClusterIPs)) - for _, ipBlock := range v3res.Spec.ServiceClusterIPs { - if ipBlock.CIDR == "" { - // The CRD allows CIDR to be optional so we just ignore empty CIDRs. - continue - } - ipCidrs = append(ipCidrs, ipBlock.CIDR) +func getServiceClusterIPs(v3res *apiv3.BGPConfiguration) (ipCidrs []string) { + if v3res != nil { + for _, ipBlock := range v3res.Spec.ServiceClusterIPs { + if ipBlock.CIDR == "" { + // The CRD allows CIDR to be optional so we just ignore empty CIDRs. + continue } - c.updateCache(api.UpdateTypeKVUpdated, getKVPair(svcInternalIPKey, strings.Join(ipCidrs, ","))) - } else { - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(svcInternalIPKey)) + ipCidrs = append(ipCidrs, ipBlock.CIDR) } - *svcAdvertisement = true } + return } func (c *client) getNodeToNodeMeshKVPair(v3res *apiv3.BGPConfiguration, key any) { @@ -1507,53 +1456,25 @@ func (c *client) getLogSeverityKVPair(v3res *apiv3.BGPConfiguration, key any) { } } -func (c *client) getNodeMeshRestartTimeKVPair(v3res *apiv3.BGPConfiguration, key any) { - meshRestartKey := getBGPConfigKey("node_mesh_restart_time", key) - +func getNodeMeshRestartTime(v3res *apiv3.BGPConfiguration) (restartTimeStr string) { if v3res != nil && v3res.Spec.NodeMeshMaxRestartTime != nil { - restartTime := *v3res.Spec.NodeMeshMaxRestartTime - c.updateCache(api.UpdateTypeKVUpdated, getKVPair(meshRestartKey, fmt.Sprintf("%v", int(math.Round(restartTime.Seconds()))))) - } else { - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(meshRestartKey)) + restartTimeStr = fmt.Sprintf("%v", int(math.Round(v3res.Spec.NodeMeshMaxRestartTime.Seconds()))) } + return } -func (c *client) getNodeMeshPasswordKVPair(v3res *apiv3.BGPConfiguration, key any) { - meshPasswordKey := getBGPConfigKey("node_mesh_password", key) - +func (c *client) getNodeMeshPassword(v3res *apiv3.BGPConfiguration) (password string) { if c.secretWatcher != nil && v3res != nil && v3res.Spec.NodeMeshPassword != nil && v3res.Spec.NodeMeshPassword.SecretKeyRef != nil { - password, err := c.secretWatcher.GetSecret( + var err error + password, err = c.secretWatcher.GetSecret( v3res.Spec.NodeMeshPassword.SecretKeyRef.Name, v3res.Spec.NodeMeshPassword.SecretKeyRef.Key, ) if err != nil { log.WithError(err).Warningf("Can't read password referenced by BGP Configuration %v in secret %s:%s", v3res.Name, v3res.Spec.NodeMeshPassword.SecretKeyRef.Name, v3res.Spec.NodeMeshPassword.SecretKeyRef.Key) - // Secret or key not available, treat as a delete. - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(meshPasswordKey)) - return } - c.updateCache(api.UpdateTypeKVUpdated, getKVPair(meshPasswordKey, password)) - } else { - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(meshPasswordKey)) - } -} - -func (c *client) getIgnoredInterfacesKVPair(v3res *apiv3.BGPConfiguration, key any) { - ignoredIfacesKey := getBGPConfigKey("ignored_interfaces", key) - if v3res != nil && v3res.Spec.IgnoredInterfaces != nil { - c.updateCache(api.UpdateTypeKVUpdated, getKVPair(ignoredIfacesKey, strings.Join(v3res.Spec.IgnoredInterfaces, ","))) - } else { - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(ignoredIfacesKey)) - } -} - -func (c *client) getProgramClusterRoutesKVPair(v3res *apiv3.BGPConfiguration, key any) { - programClusterRoutesKey := getBGPConfigKey("program_cluster_routes", key) - if v3res != nil && v3res.Spec.ProgramClusterRoutes != nil { - c.updateCache(api.UpdateTypeKVUpdated, getKVPair(programClusterRoutesKey, *v3res.Spec.ProgramClusterRoutes)) - } else { - c.updateCache(api.UpdateTypeKVDeleted, getKVPair(programClusterRoutesKey)) } + return } func getNodeName(nodeName string) string { @@ -1819,9 +1740,6 @@ func (c *client) updateCache(updateType api.UpdateType, kvp *model.KVPair) bool } logVal := c.cache[k] - if c.isSensitive(k) { - logVal = "redacted" - } log.Debugf("Cache entry updated from event type %d: %s=%s", updateType, k, logVal) if c.syncedOnce { c.keyUpdated(k) @@ -2112,12 +2030,3 @@ func filterNonSingleIPsFromCIDRs(ipCidrs []string) []string { } return nonSingleIPs } - -// Checks whether or not a key references sensitive information (like a BGP password) so that -// logging output for the field can be redacted. -func (c *client) isSensitive(path string) bool { - if _, ok := sensitiveValues[path]; ok { - return true - } - return false -} diff --git a/confd/pkg/backends/calico/ippools_test.go b/confd/pkg/backends/calico/ippools_test.go index 83b2cfa4d90..47562a69836 100644 --- a/confd/pkg/backends/calico/ippools_test.go +++ b/confd/pkg/backends/calico/ippools_test.go @@ -25,6 +25,7 @@ import ( v3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3" "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" "github.com/projectcalico/calico/confd/pkg/backends/types" "github.com/projectcalico/calico/libcalico-go/lib/backend/encap" @@ -314,10 +315,9 @@ func Test_processIPPoolsV4_FelixProgramsClusterRoutes(t *testing.T) { cache[fmt.Sprintf("/calico/bgp/v1/host/%s/network_v4", NodeName)] = "1.1.1.0/24" c := newTestClient(cache, nil) - programClusterRoutes := "Disabled" c.globalBGPConfig = &v3.BGPConfiguration{ Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: &programClusterRoutes, + ProgramClusterRoutes: ptr.To("Disabled"), }, } config := &types.BirdBGPConfig{ @@ -390,10 +390,9 @@ func Test_processIPPoolsV6_FelixProgramsClusterRoutes(t *testing.T) { cache := ippoolTestCasesToKVPairs(t, poolsTestsV6, 6) c := newTestClient(cache, nil) - programClusterRoutes := "Disabled" c.globalBGPConfig = &v3.BGPConfiguration{ Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: &programClusterRoutes, + ProgramClusterRoutes: ptr.To("Disabled"), }, } config := &types.BirdBGPConfig{ @@ -459,128 +458,3 @@ func filterExpectedStatements(statements []string, filterAction string) (filtere } return } - -// programClusterRoutesCacheKey is the confd cache key that mirrors -// BGPConfiguration.Spec.ProgramClusterRoutes into BIRD config. -const programClusterRoutesCacheKey = "/calico/bgp/v1/global/program_cluster_routes" - -func strPtr(s string) *string { return &s } - -func TestGetProgramClusterRoutesKVPair(t *testing.T) { - // Initially, set ProgramClusterRoutes to Enabled. - c := &client{cache: make(map[string]string)} - res := &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: strPtr("Enabled"), - }, - } - c.getProgramClusterRoutesKVPair(res, model.GlobalBGPConfigKey{}) - require.Equal(t, "Enabled", c.cache[programClusterRoutesCacheKey]) - - // Switch ProgramClusterRoutes to Disabled. - res = &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: strPtr("Disabled"), - }, - } - c.getProgramClusterRoutesKVPair(res, model.GlobalBGPConfigKey{}) - require.Equal(t, "Disabled", c.cache[programClusterRoutesCacheKey]) - - // Switch ProgramClusterRoutes to Enabled. - res = &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: strPtr("Enabled"), - }, - } - c.getProgramClusterRoutesKVPair(res, model.GlobalBGPConfigKey{}) - require.Equal(t, "Enabled", c.cache[programClusterRoutesCacheKey]) - - // Unsetting ProgramClusterRoutes. - res = &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: nil, - }, - } - c.getProgramClusterRoutesKVPair(res, model.GlobalBGPConfigKey{}) - require.NotContains(t, c.cache, programClusterRoutesCacheKey) -} - -func TestGetProgramClusterRoutesKVPair_NilResourceDeletesCacheEntry(t *testing.T) { - c := &client{cache: map[string]string{ - programClusterRoutesCacheKey: "Disabled", - }} - c.getProgramClusterRoutesKVPair(nil, model.GlobalBGPConfigKey{}) - require.NotContains(t, c.cache, programClusterRoutesCacheKey) -} - -// ProgramClusterRoutes is intentionally wired as global-only in -// updateBGPConfigCache (client.go): the per-node branch does not call -// getProgramClusterRoutesKVPair. This test pins that behavior — if per-node -// support is ever added, this test should be updated along with the call site. -func TestGetProgramClusterRoutesKVPair_PerNodeKeyDoesNotWriteGlobal(t *testing.T) { - c := &client{cache: make(map[string]string)} - res := &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: strPtr("Enabled"), - }, - } - c.getProgramClusterRoutesKVPair(res, model.NodeBGPConfigKey{Nodename: "nodeX"}) - require.NotContains(t, c.cache, programClusterRoutesCacheKey) -} - -func TestUpdateBGPConfigCache_ProgramClusterRoutes_UpdateThenDelete(t *testing.T) { - c := &client{cache: make(map[string]string)} - var ( - svcAdvertisement bool - updatePeersV1 bool - updateReasons []string - ) - - // First: set ProgramClusterRoutes=Disabled and confirm cache is populated - // via the full updateBGPConfigCache entrypoint (exercises wiring at - // client.go's global branch). - res := &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: strPtr("Disabled"), - }, - } - c.updateBGPConfigCache("default", res, &svcAdvertisement, &updatePeersV1, &updateReasons) - require.Equal(t, "Disabled", c.cache[programClusterRoutesCacheKey]) - - // Second: set ProgramClusterRoutes=Enabled and confirm cache is populated - // via the full updateBGPConfigCache entrypoint (exercises wiring at - // client.go's global branch). - res = &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: strPtr("Enabled"), - }, - } - c.updateBGPConfigCache("default", res, &svcAdvertisement, &updatePeersV1, &updateReasons) - require.Equal(t, "Enabled", c.cache[programClusterRoutesCacheKey]) - - // Finally: clear the field and confirm the cache entry is removed. - res = &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: nil, - }, - } - c.updateBGPConfigCache("default", res, &svcAdvertisement, &updatePeersV1, &updateReasons) - require.NotContains(t, c.cache, programClusterRoutesCacheKey) -} - -func TestUpdateBGPConfigCache_ProgramClusterRoutes_PerNodeResourceNameSkipsGlobalKey(t *testing.T) { - c := &client{cache: make(map[string]string)} - var ( - svcAdvertisement bool - updatePeersV1 bool - updateReasons []string - ) - - res := &v3.BGPConfiguration{ - Spec: v3.BGPConfigurationSpec{ - ProgramClusterRoutes: strPtr("Enabled"), - }, - } - c.updateBGPConfigCache("node.nodeX", res, &svcAdvertisement, &updatePeersV1, &updateReasons) - require.NotContains(t, c.cache, programClusterRoutesCacheKey) -} diff --git a/confd/pkg/backends/calico/routes_test.go b/confd/pkg/backends/calico/routes_test.go index 4de5b2757b3..b304b07a57d 100644 --- a/confd/pkg/backends/calico/routes_test.go +++ b/confd/pkg/backends/calico/routes_test.go @@ -13,8 +13,6 @@ import ( discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - - "github.com/projectcalico/calico/libcalico-go/lib/backend/model" ) const ( @@ -825,24 +823,6 @@ var _ = Describe("RouteGenerator", func() { }) }) -var _ = Describe("Update BGP Config Cache", func() { - c := &client{cache: make(map[string]string)} - - It("should update cache value when IgnoredInterfaces is set in BGPConfiguration", func() { - By("No value cached") - Expect(c.cache["/calico/bgp/v1/global/ignored_interfaces"]).To(BeEmpty()) - - By("After updating") - res := &apiv3.BGPConfiguration{ - Spec: apiv3.BGPConfigurationSpec{ - IgnoredInterfaces: []string{"iface-1", "iface-2"}, - }, - } - c.getIgnoredInterfacesKVPair(res, model.GlobalBGPConfigKey{}) - Expect(c.cache["/calico/bgp/v1/global/ignored_interfaces"]).To(Equal("iface-1,iface-2")) - }) -}) - var _ = Describe("Service Load Balancer Aggregation", func() { var rg *routeGenerator diff --git a/confd/pkg/backends/calico/secret_watcher.go b/confd/pkg/backends/calico/secret_watcher.go index f9ad2b49d47..2e242381c15 100644 --- a/confd/pkg/backends/calico/secret_watcher.go +++ b/confd/pkg/backends/calico/secret_watcher.go @@ -37,6 +37,12 @@ type secretWatchData struct { secret *v1.Secret } +type secretWatcherInterface interface { + GetSecret(name, key string) (string, error) + MarkStale() + SweepStale() +} + type secretWatcher struct { client *client namespace string diff --git a/confd/pkg/backends/types/bird_bgp_config.go b/confd/pkg/backends/types/bird_bgp_config.go index 6c858b4b946..3c7b9f176dd 100644 --- a/confd/pkg/backends/types/bird_bgp_config.go +++ b/confd/pkg/backends/types/bird_bgp_config.go @@ -30,6 +30,7 @@ type BirdBGPConfig struct { ListenPort string DirectInterfaces string // Complete interface pattern string for protocol direct + LoadBalancerIPs []string BGPExportFilterForDisabledIPPools []string BGPExportFilterForEnabledIPPools []string KernelFilterForIPPools []string