Webhook does not process transient errors properly

Fixed metallb#2173 Signed-off-by: shimritproj <shimritp74@gmail.com>
shimritproj · Dec 6, 2023 · 419271e · 419271e
1 parent 604376c
commit 419271e
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 2 deletions.
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -270,7 +270,7 @@ func peersFor(resources ClusterResources, BFDProfiles map[string]*BFDProfile) (m
 			if _, ok := BFDProfiles[peer.BFDProfile]; !ok {
 				return nil, TransientError{fmt.Sprintf("peer %s referencing non existing bfd profile %s", p.Name, peer.BFDProfile)}
 			}
-		}
+		}	
 		for _, ep := range res {
 			// TODO: Be smarter regarding conflicting peers. For example, two
 			// peers could have a different hold time but they'd still result

diff --git a/internal/config/validator.go b/internal/config/validator.go
@@ -28,7 +28,7 @@ type validator struct {
 	validate Validate
 }
 
-func (v *validator) Validate(resources ...client.ObjectList) error {
+func (v *validator) Validate(resources ...client.ObjectList) error { 
 	clusterResources := ClusterResources{
 		Pools:              make([]metallbv1beta1.IPAddressPool, 0),
 		Peers:              make([]metallbv1beta2.BGPPeer, 0),
@@ -38,20 +38,43 @@ func (v *validator) Validate(resources ...client.ObjectList) error {
 		LegacyAddressPools: make([]metallbv1beta1.AddressPool, 0),
 		Communities:        make([]metallbv1beta1.Community, 0),
 	}
+
 	for _, list := range resources {
 		switch list := list.(type) {
 		case *metallbv1beta1.IPAddressPoolList:
 			clusterResources.Pools = append(clusterResources.Pools, list.Items...)
 		case *metallbv1beta2.BGPPeerList:
 			clusterResources.Peers = append(clusterResources.Peers, list.Items...)
+			for i := range (clusterResources.Peers) {
+				clusterResources.Peers[i].Spec.BFDProfile = ""
+				clusterResources.Peers[i].Spec.PasswordSecret.Name = ""
+			}
 		case *metallbv1beta1.BFDProfileList:
 			clusterResources.BFDProfiles = append(clusterResources.BFDProfiles, list.Items...)
 		case *metallbv1beta1.BGPAdvertisementList:
 			clusterResources.BGPAdvs = append(clusterResources.BGPAdvs, list.Items...)
+			for i := range(clusterResources.BGPAdvs){
+				for j:= range(clusterResources.BGPAdvs[i].Spec.Communities){
+					if !existString(clusterResources.BGPAdvs[i].Spec.Communities[j],clusterResources.Communities){
+						clusterResources.BGPAdvs[i].Spec.Communities = removeElement(clusterResources.BGPAdvs[i].Spec.Communities, clusterResources.BGPAdvs[i].Spec.Communities[j])
+					}
+				}
+			}
 		case *metallbv1beta1.L2AdvertisementList:
 			clusterResources.L2Advs = append(clusterResources.L2Advs, list.Items...)
 		case *metallbv1beta1.AddressPoolList:
 			clusterResources.LegacyAddressPools = append(clusterResources.LegacyAddressPools, list.Items...)
+			for i := range (clusterResources.LegacyAddressPools) {
+				for j := range(clusterResources.LegacyAddressPools[i].Spec.BGPAdvertisements) {
+					for k := range(clusterResources.LegacyAddressPools[i].Spec.BGPAdvertisements[j].Communities){
+						if !existString(clusterResources.LegacyAddressPools[i].Spec.BGPAdvertisements[j].Communities[k], clusterResources.Communities) {
+							clusterResources.LegacyAddressPools[i].Spec.BGPAdvertisements[j].Communities = removeElement(clusterResources.LegacyAddressPools[i].Spec.BGPAdvertisements[j].Communities,
+								 clusterResources.LegacyAddressPools[i].Spec.BGPAdvertisements[j].Communities[k])
+						}
+					}
+				}
+			}
+
 		case *metallbv1beta1.CommunityList:
 			clusterResources.Communities = append(clusterResources.Communities, list.Items...)
 		case *v1.NodeList:
@@ -62,9 +85,39 @@ func (v *validator) Validate(resources ...client.ObjectList) error {
 	if errors.As(err, &TransientError{}) { // we do not want to make assumption on ordering in webhooks.
 		return nil
 	}
+
 	return err
 }
 
 func NewValidator(validate Validate) apivalidate.ClusterObjects {
 	return &validator{validate: validate}
 }
+
+func existString (c string, community []metallbv1beta1.Community ) bool {
+	for i := range community {
+		if c == community[i].Name {
+			return true
+		}
+	}
+	return false
+}
+
+func removeElement(slice []string, elementToRemove string) []string {
+    // Find the index of the element
+    index := -1
+    for i, v := range slice {
+        if v == elementToRemove {
+            index = i
+            break
+        }
+    }
+
+    // If the element is found, remove it
+    if index != -1 {
+        // Create a new slice without the element
+        return append(slice[:index], slice[index+1:]...)
+    }
+
+    // If the element is not found, return the original slice
+    return slice
+}