Skip to content

Commit

Permalink
Restrict CIS to sync static routes created by itself (#3283)
Browse files Browse the repository at this point in the history
  • Loading branch information
arzzon authored Feb 21, 2024
1 parent 7f99ba1 commit 66e6b35
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 7 deletions.
5 changes: 5 additions & 0 deletions docs/cis-3.x/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,8 @@ Prometheus Metrics
| k8s_bigip_ctlr_configuration_warnings | Gauge | Enabled | The total number of configuration warnings by the CIS Controller | ["kind" ,"namespace", "name", "warning"] |
| k8s_bigip_ctlr_managed_bigips | Gauge | Enabled | The total number of bigips where the CIS Controller posts the declaration | - |
| k8s_bigip_ctlr_monitored_nodes | Gauge | Enabled | The total number of monitored nodes by the CIS Controller | ["nodeselector"] |


## Recommendations
* Never change the controllerIdentifier parameter in the deploy config CR for a CIS instance. ControllerIdentifier is a unique identifier for the CIS instance. CIS uses it for uniquely creating static routes configured on Big-IP Next. Changing it may render some static routes out of sync in case CIS is running in staticRoutingMode.

2 changes: 1 addition & 1 deletion pkg/controller/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -3993,7 +3993,7 @@ func (ctlr *Controller) processConfigCR(configCR *cisapiv1.DeployConfig, isDelet
bigipconfig := configCR.Spec.BigIpConfig
ctlr.handleBigipConfigUpdates(bigipconfig)
if ctlr.StaticRoutingMode && ctlr.PoolMemberType != NodePort {
err := ctlr.networkManager.SetInstanceIds(configCR.Spec.BigIpConfig)
err := ctlr.networkManager.SetInstanceIds(configCR.Spec.BigIpConfig, ctlr.ControllerIdentifier)
if err != nil {
log.Errorf("%v", err)
os.Exit(1)
Expand Down
16 changes: 12 additions & 4 deletions pkg/networkmanager/networkmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func NewNetworkManager(tm *tokenmanager.TokenManager, clusterName string) *Netwo
}

// SetInstanceIds performs an HTTP GET request to the API, extracts address and ID mappings, and stores them
func (nm *NetworkManager) SetInstanceIds(bigIpConfigs []cisapiv1.BigIpConfig) error {
func (nm *NetworkManager) SetInstanceIds(bigIpConfigs []cisapiv1.BigIpConfig, controllerID string) error {

// initialize the device map
nm.DeviceMap = make(map[string]string)
Expand Down Expand Up @@ -175,7 +175,7 @@ func (nm *NetworkManager) SetInstanceIds(bigIpConfigs []cisapiv1.BigIpConfig) er
nm.DeviceMap[address] = id
nm.L3ForwardStore.Lock()
if _, ok := nm.L3ForwardStore.InstanceStaticRoutes[id]; !ok {
staticRouteMap, err := nm.GetL3ForwardsFromInstance(id)
staticRouteMap, err := nm.GetL3ForwardsFromInstance(id, controllerID)
if err != nil {
log.Errorf("%v Error getting static routes for instance %v: %v", networkManagerPrefix, id, err)
nm.L3ForwardStore.Unlock()
Expand All @@ -195,7 +195,7 @@ func (nm *NetworkManager) SetInstanceIds(bigIpConfigs []cisapiv1.BigIpConfig) er
}

// GetL3ForwardsFromInstance performs an HTTP GET request to the API, extracts name and route information, and stores them
func (nm *NetworkManager) GetL3ForwardsFromInstance(instanceId string) (StaticRouteMap, error) {
func (nm *NetworkManager) GetL3ForwardsFromInstance(instanceId string, controllerID string) (StaticRouteMap, error) {

// Create request
req, err := http.NewRequest("GET", nm.CMTokenManager.ServerURL+InstancesURI+instanceId+L3Forwards, nil)
Expand Down Expand Up @@ -228,8 +228,16 @@ func (nm *NetworkManager) GetL3ForwardsFromInstance(instanceId string) (StaticRo
if l3ForwardsArray, ok := embedded["l3forwards"].([]interface{}); ok {
for _, l3ForwardData := range l3ForwardsArray {
if l3Forward, ok := l3ForwardData.(map[string]interface{}); ok {
id, idOk := l3Forward["id"].(string)
name, nameOk := l3Forward["payload"].(map[string]interface{})["name"].(string)
if nameOk {
routeNameArray := strings.Split(name, "/")
if len(routeNameArray) == 0 || routeNameArray[0] != controllerID {
// ControllerID is not present or not matching in the l3Forward name, so skip this L3Forward
// as it's not be created by this CIS
continue
}
}
id, idOk := l3Forward["id"].(string)

configData, configOk := l3Forward["payload"].(map[string]interface{})["config"].(map[string]interface{})
config := StaticRouteConfig{}
Expand Down
4 changes: 2 additions & 2 deletions pkg/networkmanager/networkmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ var _ = Describe("Network Manager Tests", func() {
bigIPConfig = []cisapiv1.BigIpConfig{{
BigIpAddress: BigIPAddress,
}}
networkManager.SetInstanceIds(bigIPConfig)
networkManager.SetInstanceIds(bigIPConfig, "")
go networkManager.NetworkConfigHandler()
})
AfterEach(func() {
Expand Down Expand Up @@ -442,7 +442,7 @@ var _ = Describe("Network Manager Tests", func() {
It("Initialize the network controller when l3forwards are present on server", func() {
isr, _ := networkManager.L3ForwardStore.InstanceStaticRoutes[BigIpId]
Expect(len(isr)).To(BeZero())
networkManager.SetInstanceIds(bigIPConfig)
networkManager.SetInstanceIds(bigIPConfig, "cluster-1")
isr, _ = networkManager.L3ForwardStore.InstanceStaticRoutes[BigIpId]
Expect(len(isr)).ToNot(BeZero())
// test retry timeout increment
Expand Down

0 comments on commit 66e6b35

Please sign in to comment.