diff --git a/docs/RELEASE-NOTES.rst b/docs/RELEASE-NOTES.rst index 07f327634..b13afe34a 100644 --- a/docs/RELEASE-NOTES.rst +++ b/docs/RELEASE-NOTES.rst @@ -18,6 +18,7 @@ Bug Fixes * `Issue 3371 https://github.com/F5Networks/k8s-bigip-ctlr/issues/3371`_: CIS added irules cannot have "event disable all" * `Issue 3402 https://github.com/F5Networks/k8s-bigip-ctlr/issues/3402`_: TLS iRule fails after recent browser updates * Fix for EDNS pool member empty issue. +* `Issue 3414 https://github.com/F5Networks/k8s-bigip-ctlr/issues/3414`_: CIS is deleting referenced SSL Profile it did not create 2.16.1 diff --git a/docs/config_examples/customResourceDefinitions/incubator/customresourcedefinitions.yml b/docs/config_examples/customResourceDefinitions/incubator/customresourcedefinitions.yml index dc792a98f..be3769167 100644 --- a/docs/config_examples/customResourceDefinitions/incubator/customresourcedefinitions.yml +++ b/docs/config_examples/customResourceDefinitions/incubator/customresourcedefinitions.yml @@ -399,6 +399,9 @@ spec: type: integer minimum: 1 maximum: 65535 + x-kubernetes-validations: + - rule: "self.partition != Common" + message: "Partition cannot be set as Common" status: type: object properties: @@ -757,6 +760,9 @@ spec: - virtualServerPort - pool - mode + x-kubernetes-validations: + - rule: "self.partition != Common" + message: "Partition cannot be set as Common" status: type: object properties: @@ -974,6 +980,9 @@ spec: type: string type: object type: object + x-kubernetes-validations: + - rule: "self.partition != Common" + message: "Partition cannot be set as Common" status: type: object properties: diff --git a/pkg/controller/backend.go b/pkg/controller/backend.go index 3c017d65c..b531b8fb6 100644 --- a/pkg/controller/backend.go +++ b/pkg/controller/backend.go @@ -309,7 +309,9 @@ func (agent *Agent) agentWorker() { agent.postTenantsDeclaration(decl, rsConfig, priorityTenants) } // Updating the remaining tenants - agent.postTenantsDeclaration(decl, rsConfig, updatedTenants) + if len(updatedTenants) > 0 { + agent.postTenantsDeclaration(decl, rsConfig, updatedTenants) + } agent.declUpdate.Unlock() } diff --git a/pkg/controller/constants.go b/pkg/controller/constants.go index 3eeef4b1f..1a11b6251 100644 --- a/pkg/controller/constants.go +++ b/pkg/controller/constants.go @@ -42,4 +42,6 @@ const ( CALICO_K8S = "calico-k8s" CALICO_API_BLOCK_AFFINITIES = "/apis/crd.projectcalico.org/v1/blockaffinities" CALICONodeIPAnnotation = "projectcalico.org/IPv4Address" + + CommonPartition = "Common" ) diff --git a/pkg/controller/nativeResourceWorker.go b/pkg/controller/nativeResourceWorker.go index a782c6be9..c1913a078 100644 --- a/pkg/controller/nativeResourceWorker.go +++ b/pkg/controller/nativeResourceWorker.go @@ -1040,7 +1040,12 @@ func (ctlr *Controller) processRouteConfigFromGlobalCM(es extendedSpec, isDelete } var partition string if len(ergc.BigIpPartition) > 0 { - partition = ergc.BigIpPartition + if ergc.BigIpPartition != CommonPartition { + partition = ergc.BigIpPartition + } else { + return fmt.Errorf("partition Common is not allowed in RouteGroup, "+ + "skipping RouteGroup: %s processing", routeGroup), false + } } else { partition = ctlr.Partition } diff --git a/pkg/controller/nativeResourceWorker_test.go b/pkg/controller/nativeResourceWorker_test.go index 09e71c782..5b9791041 100644 --- a/pkg/controller/nativeResourceWorker_test.go +++ b/pkg/controller/nativeResourceWorker_test.go @@ -1421,6 +1421,44 @@ extendedRouteSpec: Expect(ok).To(BeTrue()) }) + It("Extended Route Spec with Common partition in RouteGroups and defaultRouteGroup of baseRouteSpec", func() { + // Verify Common partition is not allowed for defaultRouteGroup of baseRouteSpec + data["extendedSpec"] = ` +baseRouteSpec: + defaultRouteGroup: + bigIpPartition: Common +extendedRouteSpec: + - namespace: default + vserverAddr: 10.8.3.11 + vserverName: nextgenroutes + allowOverride: false + - namespace: new + vserverAddr: 10.8.3.12 + allowOverride: false + bigIpPartition: Common +` + err, ok := mockCtlr.processConfigMap(cm, false) + Expect(err).NotTo(BeNil(), "Common partition in defaultRouteGroup of baseRouteSpec should not be allowed") + Expect(ok).To(BeFalse(), "Common partition in defaultRouteGroup of baseRouteSpec should not be allowed") + + // Verify Common partition is not allowed for RouteGroup + data["extendedSpec"] = ` +extendedRouteSpec: + - namespace: default + vserverAddr: 10.8.3.11 + vserverName: nextgenroutes + allowOverride: false + - namespace: new + vserverAddr: 10.8.3.12 + allowOverride: false + bigIpPartition: Common +` + err, ok = mockCtlr.processConfigMap(cm, false) + Expect(err).NotTo(BeNil(), "Common partition in RouteGroups should not be allowed") + Expect(ok).To(BeFalse(), "Common partition in RouteGroups should not be allowed") + + }) + It("Extended Route Spec Allow local", func() { data["extendedSpec"] = ` extendedRouteSpec: diff --git a/pkg/controller/validate.go b/pkg/controller/validate.go index f89b58df6..bb6841359 100644 --- a/pkg/controller/validate.go +++ b/pkg/controller/validate.go @@ -42,6 +42,13 @@ func (ctlr *Controller) checkValidVirtualServer( log.Infof("VirtualServer %s is invalid", vsName) return false } + + // Check if Partition is set as Common + if vsResource.Spec.Partition == CommonPartition { + log.Errorf("VirtualServer %s cannot be created in Common partition", vsName) + return false + } + // Check if HTTPTraffic is set for insecure VS if vsResource.Spec.TLSProfileName == "" && vsResource.Spec.HTTPTraffic != "" { log.Errorf("HTTPTraffic not allowed to be set for insecure VirtualServer: %v", vsName) @@ -101,6 +108,12 @@ func (ctlr *Controller) checkValidTransportServer( return false } + // Check if Partition is set as Common + if tsResource.Spec.Partition == CommonPartition { + log.Errorf("TransportServer %s cannot be created in Common partition", vsName) + return false + } + bindAddr := tsResource.Spec.VirtualServerAddress if ctlr.ipamCli == nil { @@ -157,6 +170,12 @@ func (ctlr *Controller) checkValidIngressLink( return false } + // Check if Partition is set as Common + if il.Spec.Partition == CommonPartition { + log.Errorf("IngressLink %s cannot be created in Common partition", ilName) + return false + } + bindAddr := il.Spec.VirtualServerAddress if ctlr.ipamCli == nil { diff --git a/pkg/controller/worker.go b/pkg/controller/worker.go index ec67d1a34..97c5792b5 100644 --- a/pkg/controller/worker.go +++ b/pkg/controller/worker.go @@ -1532,7 +1532,7 @@ func (ctlr *Controller) getAssociatedVirtualServers( } if _, ok := uniquePaths[pool.Path]; ok { // path already exists for the same host - log.Debugf("Discarding the VirtualServer %v/%v due to duplicate path", + log.Warningf("Discarding the VirtualServer %v/%v due to duplicate path", vrt.ObjectMeta.Namespace, vrt.ObjectMeta.Name) isUnique = false break @@ -4239,6 +4239,11 @@ func (ctlr *Controller) processConfigMap(cm *v1.ConfigMap, isDelete bool) (error if err != nil { return fmt.Errorf("invalid extended route spec in configmap: %v/%v error: %v", cm.Namespace, cm.Name, err), false } + // Check if Partition is set to Common, which is not allowed + if es.DefaultRouteGroupConfig.BigIpPartition == CommonPartition { + return fmt.Errorf("invalid partition %v provided in defaultRouteGroup of configmap: %v/%v", + CommonPartition, cm.Namespace, cm.Name), false + } // clusterConfigUpdated, oldClusterRatio and oldClusterAdminState are used for tracking cluster ratio and cluster Admin state updates clusterConfigUpdated := false oldClusterRatio := make(map[string]int) diff --git a/pkg/controller/worker_test.go b/pkg/controller/worker_test.go index f783fa464..68b329ea6 100644 --- a/pkg/controller/worker_test.go +++ b/pkg/controller/worker_test.go @@ -790,6 +790,12 @@ var _ = Describe("Worker Tests", func() { Expect(address).To(Equal("192.168.1.1"), "Should not return empty virtual address") Expect(err).To(BeNil(), "error should be nil") }) + + It("Verifies Common partition is not allowed in VS", func() { + vrt3.Spec.Partition = CommonPartition + Expect(mockCtlr.checkValidVirtualServer(vrt3)).To(BeFalse(), "VS with Common partition "+ + "should not be allowed") + }) }) }) Describe("Endpoints", func() { @@ -2517,6 +2523,9 @@ var _ = Describe("Worker Tests", func() { Expect(len(mockCtlr.resources.ltmConfig[mockCtlr.Partition].ResourceMap)).To(Equal(2), "Invalid TS count") Expect(len(mockCtlr.resources.ltmConfig["dev2"].ResourceMap)).To(Equal(1), "Invalid TS count") + // Verify TS with Common partition is not allowed + ts.Spec.Partition = CommonPartition + Expect(mockCtlr.checkValidTransportServer(ts)).To(BeFalse(), "TS with Common partition is not allowed") }) }) @@ -2847,6 +2856,9 @@ var _ = Describe("Worker Tests", func() { Expect(len(mockCtlr.resources.ltmConfig)).To(Equal(1), "Invalid Partition count") Expect(len(mockCtlr.resources.ltmConfig[mockCtlr.Partition].ResourceMap)).To(Equal(2), "Invalid IL count") + // Verify IL with Common partition is not allowed + ingressLink1.Spec.Partition = CommonPartition + Expect(mockCtlr.checkValidIngressLink(ingressLink1)).To(BeFalse(), "IL with Common partition is not allowed") }) }) })