diff --git a/rollout/trafficrouting/istio/istio.go b/rollout/trafficrouting/istio/istio.go index c13426782b..0449c62dff 100644 --- a/rollout/trafficrouting/istio/istio.go +++ b/rollout/trafficrouting/istio/istio.go @@ -1391,8 +1391,8 @@ func (r *Reconciler) orderRoutes(istioVirtualService *unstructured.Unstructured) // splitManagedRoutesAndNonManagedRoutes This splits the routes from an istio virtual service into two slices // one slice contains all the routes that are also in the rollouts managedRoutes object and one that contains routes // that where only in the virtual service (aka routes that where manually added by user) -func splitManagedRoutesAndNonManagedRoutes(managedRoutes []v1alpha1.MangedRoutes, httpRouteI []interface{}) (httpRoutesWithinManagedRoutes []VirtualServiceHTTPRoute, httpRoutesNotWithinManagedRoutes []VirtualServiceHTTPRoute, err error) { - var httpRoutes []VirtualServiceHTTPRoute +func splitManagedRoutesAndNonManagedRoutes(managedRoutes []v1alpha1.MangedRoutes, httpRouteI []interface{}) (httpRoutesWithinManagedRoutes []map[string]interface{}, httpRoutesNotWithinManagedRoutes []map[string]interface{}, err error) { + var httpRoutes []map[string]interface{} jsonHttpRoutes, err := json.Marshal(httpRouteI) if err != nil { @@ -1406,7 +1406,10 @@ func splitManagedRoutesAndNonManagedRoutes(managedRoutes []v1alpha1.MangedRoutes for _, route := range httpRoutes { var found bool = false for _, managedRoute := range managedRoutes { - if route.Name == managedRoute.Name { + // Not checking the cast success here is ok because it covers the case when the route has no name + // when there is no name the cast return an empty string and will just not match the managed route + name, _ := route["name"].(string) + if name == managedRoute.Name { httpRoutesWithinManagedRoutes = append(httpRoutesWithinManagedRoutes, route) found = true break @@ -1423,11 +1426,11 @@ func splitManagedRoutesAndNonManagedRoutes(managedRoutes []v1alpha1.MangedRoutes // getOrderedVirtualServiceRoutes This returns an []interface{} of istio virtual routes where the routes are ordered based // on the rollouts managedRoutes field. We take the routes from the rollouts managedRoutes field order them and place them on top // of routes that are manually defined within the virtual service (aka. routes that users have defined manually) -func getOrderedVirtualServiceRoutes(httpRouteI []interface{}, managedRoutes []v1alpha1.MangedRoutes, httpRoutesWithinManagedRoutes []VirtualServiceHTTPRoute, httpRoutesNotWithinManagedRoutes []VirtualServiceHTTPRoute) ([]interface{}, error) { - var orderedManagedRoutes []VirtualServiceHTTPRoute +func getOrderedVirtualServiceRoutes(httpRouteI []interface{}, managedRoutes []v1alpha1.MangedRoutes, httpRoutesWithinManagedRoutes []map[string]interface{}, httpRoutesNotWithinManagedRoutes []map[string]interface{}) ([]interface{}, error) { + var orderedManagedRoutes []map[string]interface{} for _, route := range managedRoutes { for _, managedRoute := range httpRoutesWithinManagedRoutes { - if route.Name == managedRoute.Name { + if route.Name == managedRoute["name"] { orderedManagedRoutes = append(orderedManagedRoutes, managedRoute) } } @@ -1436,13 +1439,16 @@ func getOrderedVirtualServiceRoutes(httpRouteI []interface{}, managedRoutes []v1 orderedVirtualServiceHTTPRoutes := append(orderedManagedRoutes, httpRoutesNotWithinManagedRoutes...) var orderedInterfaceVSVCHTTPRoutes []interface{} - for _, routeTyped := range orderedVirtualServiceHTTPRoutes { + for _, routeMap := range orderedVirtualServiceHTTPRoutes { for _, route := range httpRouteI { r := route.(map[string]interface{}) - // No need to check if exist because the empty string returned on cast failure is good for this check - name, _ := r["name"].(string) - if name == routeTyped.Name { + // Not checking the cast success here is ok because it covers the case when the route has no name + name, rNameOK := r["name"].(string) + routeMapName, RMapNameOK := routeMap["name"].(string) + // The second or clause is for the case when we have a route that has no name set because this field + // is optional in istio virtual service if there is only one route + if name == routeMapName || (!rNameOK && !RMapNameOK) { orderedInterfaceVSVCHTTPRoutes = append(orderedInterfaceVSVCHTTPRoutes, route) } } diff --git a/rollout/trafficrouting/istio/istio_test.go b/rollout/trafficrouting/istio/istio_test.go index fa4330f363..b13d472944 100644 --- a/rollout/trafficrouting/istio/istio_test.go +++ b/rollout/trafficrouting/istio/istio_test.go @@ -883,6 +883,27 @@ func TestHttpReconcileHeaderRouteWithExtra(t *testing.T) { _, found = r2["corsPolicy"] assert.True(t, found) + r.RemoveManagedRoutes() + iVirtualService, err = client.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(r.rollout.Namespace).Get(context.TODO(), ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name, metav1.GetOptions{}) + assert.NoError(t, err) + + routes, found, err = unstructured.NestedSlice(iVirtualService.Object, "spec", "http") + assert.NoError(t, err) + assert.True(t, found) + + r0 = routes[0].(map[string]interface{}) + route, found = r0["route"].([]interface{}) + assert.True(t, found) + + port1 = route[0].(map[string]interface{})["destination"].(map[string]interface{})["port"].(map[string]interface{})["number"] + assert.True(t, port1 == float64(8443)) + + r2 = routes[1].(map[string]interface{}) + _, found = r2["retries"] + assert.True(t, found) + _, found = r2["corsPolicy"] + assert.True(t, found) + } func TestReconcileUpdateHeader(t *testing.T) {