diff --git a/internal/controller/reconcile-captenant_test.go b/internal/controller/reconcile-captenant_test.go index f4c43f03..f04c75c7 100644 --- a/internal/controller/reconcile-captenant_test.go +++ b/internal/controller/reconcile-captenant_test.go @@ -555,7 +555,7 @@ func TestCAPTenantProvisioningCompletedWithSessionAffinityEnabled(t *testing.T) context.TODO(), t, QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}}, TestData{ - description: "captenant provisioning operation completed (creates virtual service and destination rule) with session affinity enabled", + description: "captenant provisioning operation completed - creates virtual service with session affinity enabled", initialResources: []string{ "testdata/common/domain-ready.yaml", "testdata/common/cluster-domain-ready.yaml", @@ -573,7 +573,7 @@ func TestCAPTenantProvisioningCompletedWithSessionAffinityEnabledAndVsheaders(t context.TODO(), t, QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}}, TestData{ - description: "captenant provisioning operation completed (creates virtual service and destination rule) with session affinity enabled and virtual service headers", + description: "captenant provisioning operation completed - creates virtual service with session affinity enabled and virtual service headers", initialResources: []string{ "testdata/common/domain-ready.yaml", "testdata/common/cluster-domain-ready.yaml", @@ -591,7 +591,7 @@ func TestCAPTenantProvisioningCompletedWithSessionAffinityEnabledCustomLogout(t context.TODO(), t, QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}}, TestData{ - description: "captenant provisioning operation completed (creates virtual service and destination rule) with session affinity enabled using custom logout routes", + description: "captenant provisioning operation completed - creates virtual service with session affinity enabled using custom logout routes", initialResources: []string{ "testdata/common/domain-ready.yaml", "testdata/common/cluster-domain-ready.yaml", @@ -609,7 +609,7 @@ func TestCAPTenantUpgradeOperationCompletedWithSessionAffinityEnabled(t *testing context.TODO(), t, QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}}, TestData{ - description: "captenant upgrade operation completed expecting virtual service, destination rule adjustments with session affinity enabled", + description: "captenant upgrade operation completed - expecting virtual service adjustments with session affinity enabled", initialResources: []string{ "testdata/common/domain-ready.yaml", "testdata/common/cluster-domain-ready.yaml", @@ -629,11 +629,11 @@ func TestCAPTenantUpgradedThirdTimeWithSessionAffinityEnabled(t *testing.T) { context.TODO(), t, QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}}, TestData{ - description: "captenant upgraded third time - expecting virtual service, destination rule adjustments by removing config corresponding to v1 and by adding config for v3", + description: "captenant upgraded third time - expecting virtual service adjustments by adding config for v3 with session affinity enabled and virtual service headers", initialResources: []string{ "testdata/common/domain-ready.yaml", "testdata/common/cluster-domain-ready.yaml", - "testdata/common/capapplication-session-affinity.yaml", + "testdata/common/capapplication-session-affinity-vs-headers.yaml", "testdata/common/capapplicationversion-v1.yaml", "testdata/common/capapplicationversion-v2.yaml", "testdata/common/capapplicationversion-v3.yaml", @@ -649,7 +649,7 @@ func TestCAPTenantUpgradeOperationCompletedWithSessionAffinityEnabledAndPrevious context.TODO(), t, QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}}, TestData{ - description: "captenant upgraded - expecting virtual service, destination rule adjustments after removing previous cav v1", + description: "captenant upgraded - expecting virtual service adjustments after removing previous cav v1", initialResources: []string{ "testdata/common/domain-ready.yaml", "testdata/common/cluster-domain-ready.yaml", @@ -667,7 +667,7 @@ func TestCAPTenantUpgradeOperationCompletedWithSessionAffinitySwitchedFromEnable context.TODO(), t, QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}}, TestData{ - description: "captenant upgraded - expecting virtual service, destination rule adjustments after switching session affinity from enabled to disabled", + description: "captenant upgraded - expecting virtual service adjustments after switching session affinity from enabled to disabled", initialResources: []string{ "testdata/common/domain-ready.yaml", "testdata/common/cluster-domain-ready.yaml", diff --git a/internal/controller/reconcile-networking.go b/internal/controller/reconcile-networking.go index d8b2c745..ff578a5d 100644 --- a/internal/controller/reconcile-networking.go +++ b/internal/controller/reconcile-networking.go @@ -264,25 +264,23 @@ func (c *Controller) getUpdatedTenantVirtualServiceObject(cat *v1alpha1.CAPTenan } func (c *Controller) getVirtualServiceHttpRoutes(cat *v1alpha1.CAPTenant, currentCavName string, headers *networkingv1.Headers) ([]*networkingv1.HTTPRoute, error) { - var ( - httpRoutes []*networkingv1.HTTPRoute - prevCav *v1alpha1.CAPApplicationVersion - prevDest *networkingv1.Destination - err error - ) - - // Lookup previous CAV (if any) - if len(cat.Status.PreviousCAPApplicationVersions) > 0 { - prevCavName := cat.Status.PreviousCAPApplicationVersions[len(cat.Status.PreviousCAPApplicationVersions)-1] - prevCav, err = c.crdInformerFactory.Sme().V1alpha1().CAPApplicationVersions().Lister().CAPApplicationVersions(cat.Namespace).Get(prevCavName) + type prevCavInfo struct { + cav *v1alpha1.CAPApplicationVersion + dest *networkingv1.Destination + } - if err == nil { // only if found - if prevDest, err = c.getVirtualServiceHttpRouteDestination(prevCavName, cat.Namespace); err != nil { - return nil, err - } - } else if !errors.IsNotFound(err) { - return nil, err + // Get all previous CAVs (skip any that are missing or have no router port info) + var prevCavs []prevCavInfo + for _, prevCavName := range cat.Status.PreviousCAPApplicationVersions { + prevCav, err := c.crdInformerFactory.Sme().V1alpha1().CAPApplicationVersions().Lister().CAPApplicationVersions(cat.Namespace).Get(prevCavName) + if err != nil { + continue } + prevDest, err := c.getVirtualServiceHttpRouteDestination(prevCavName, cat.Namespace) + if err != nil { + continue + } + prevCavs = append(prevCavs, prevCavInfo{cav: prevCav, dest: prevDest}) } // Lookup current CAV destination @@ -290,25 +288,24 @@ func (c *Controller) getVirtualServiceHttpRoutes(cat *v1alpha1.CAPTenant, curren if err != nil { return nil, err } - - // Retrieve current CAV for logout endpointannotations currentCav, err := c.crdInformerFactory.Sme().V1alpha1().CAPApplicationVersions().Lister().CAPApplicationVersions(cat.Namespace).Get(currentCavName) if err != nil { return nil, err } - // --- Add routes --- - // Logoff/logout routes - if prevDest != nil { - httpRoutes = append(httpRoutes, buildVirtualServiceLogOffHttpRoute(prevCav.Name, prevCav.Annotations[AnnotationLogoutEndpoint], prevDest, headers)) + var httpRoutes []*networkingv1.HTTPRoute + + // Logoff routes: all prev CAVs, then current + for _, p := range prevCavs { + httpRoutes = append(httpRoutes, buildVirtualServiceLogOffHttpRoute(p.cav.Name, p.cav.Annotations[AnnotationLogoutEndpoint], p.dest, headers)) } httpRoutes = append(httpRoutes, buildVirtualServiceLogOffHttpRoute(currentCavName, currentCav.Annotations[AnnotationLogoutEndpoint], currentDest, headers)) - // Cookie routes - if prevDest != nil { - httpRoutes = append(httpRoutes, buildVirtualServiceCookieHttpRoute(prevCav.Name, prevDest)) + // Cookie routes: all prev CAVs, then current + for _, p := range prevCavs { + httpRoutes = append(httpRoutes, buildVirtualServiceCookieHttpRoute(p.cav.Name, p.dest, headers)) } - httpRoutes = append(httpRoutes, buildVirtualServiceCookieHttpRoute(currentCavName, currentDest)) + httpRoutes = append(httpRoutes, buildVirtualServiceCookieHttpRoute(currentCavName, currentDest, headers)) // Default route to current CAV httpRoutes = append(httpRoutes, buildVirtualServiceDefaultHttpRoute(currentCavName, currentDest, headers)) @@ -334,7 +331,7 @@ func buildVirtualServiceDefaultHttpRoute(cavName string, dest *networkingv1.Dest Destination: dest, Weight: 100, }}, - Headers: enhanceHeadersWithCookie(headers, sessionCookie(cavName), "add"), + Headers: enhanceHeadersWithCookie(headers, sessionCookie(cavName), "add"), // Use "add" instead of "set" to avoid overwriting any existing Set-Cookie header } } @@ -362,7 +359,7 @@ func buildVirtualServiceLogOffHttpRoute(cavName, logoutEndpoint string, dest *ne } } -func buildVirtualServiceCookieHttpRoute(cavName string, dest *networkingv1.Destination) *networkingv1.HTTPRoute { +func buildVirtualServiceCookieHttpRoute(cavName string, dest *networkingv1.Destination, headers *networkingv1.Headers) *networkingv1.HTTPRoute { return &networkingv1.HTTPRoute{ Match: []*networkingv1.HTTPMatchRequest{{ Headers: map[string]*networkingv1.StringMatch{ @@ -373,15 +370,19 @@ func buildVirtualServiceCookieHttpRoute(cavName string, dest *networkingv1.Desti Destination: dest, Weight: 100, }}, + Headers: headers, } } func enhanceHeadersWithCookie(headers *networkingv1.Headers, cookie string, op string) *networkingv1.Headers { var h *networkingv1.Headers - if headers != nil && headers.Response != nil { + if headers != nil { h = headers.DeepCopy() } else { - h = &networkingv1.Headers{Response: &networkingv1.Headers_HeaderOperations{}} + h = &networkingv1.Headers{} + } + if h.Response == nil { + h.Response = &networkingv1.Headers_HeaderOperations{} } switch op { diff --git a/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-headers.yaml b/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-headers.yaml index 3562e1ba..76d15ae1 100644 --- a/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-headers.yaml +++ b/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-headers.yaml @@ -40,7 +40,7 @@ apiVersion: networking.istio.io/v1 kind: VirtualService metadata: annotations: - sme.sap.com/resource-hash: 96085be3c39f1f2de3042fbf71bac43f256e139aada33ee4f2d435b8c1e6e7cd + sme.sap.com/resource-hash: 4eb1315e07bab42e3cf46fa37cfe6e95192514030fed12777d67d003cf1b13d1 sme.sap.com/owner-identifier: default.test-cap-01-provider labels: sme.sap.com/owner-generation: "0" @@ -83,7 +83,15 @@ spec: port: number: 5000 weight: 100 - - match: + - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id + response: + set: + foo: bar + match: - headers: Cookie: regex: (^|.*; )CAPOP_CAV=test-cap-01-cav-v1($|; .*) diff --git a/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-upgrade-to-cav-v3.expected.yaml b/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-upgrade-to-cav-v3.expected.yaml index 31e2106c..f201f559 100644 --- a/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-upgrade-to-cav-v3.expected.yaml +++ b/internal/controller/testdata/captenant/cat-with-session-affinity-dr-vs-upgrade-to-cav-v3.expected.yaml @@ -46,7 +46,7 @@ apiVersion: networking.istio.io/v1 kind: VirtualService metadata: annotations: - sme.sap.com/resource-hash: 4ffc680079e1711f962e91fc30cde7cf23e9d6ab260a0ed69de693f0ac2043ea + sme.sap.com/resource-hash: 9e5298d1e6d0e14414cc702b9635773e6fc90a0d5c4dc9e297dad5ff8ae9140c sme.sap.com/owner-identifier: default.test-cap-01-provider labels: sme.sap.com/owner-generation: "2" @@ -69,9 +69,35 @@ spec: - my-provider.foo.bar.local http: - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id + response: + set: + Set-Cookie: CAPOP_CAV=test-cap-01-cav-v1;Path=/;HttpOnly;Secure;Max-Age=0 + foo: bar + match: + - headers: + Cookie: + regex: (^|.*; )CAPOP_CAV=test-cap-01-cav-v1($|; .*) + uri: + regex: ^|.*(logout|logoff).* + route: + - destination: + host: test-cap-01-cav-v1-app-router-svc.default.svc.cluster.local + port: + number: 5000 + weight: 100 + - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id response: set: Set-Cookie: CAPOP_CAV=test-cap-01-cav-v2;Path=/;HttpOnly;Secure;Max-Age=0 + foo: bar match: - headers: Cookie: @@ -85,9 +111,14 @@ spec: number: 5000 weight: 100 - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id response: set: Set-Cookie: CAPOP_CAV=test-cap-01-cav-v3;Path=/;HttpOnly;Secure;Max-Age=0 + foo: bar match: - headers: Cookie: @@ -100,7 +131,33 @@ spec: port: number: 5000 weight: 100 - - match: + - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id + response: + set: + foo: bar + match: + - headers: + Cookie: + regex: (^|.*; )CAPOP_CAV=test-cap-01-cav-v1($|; .*) + route: + - destination: + host: test-cap-01-cav-v1-app-router-svc.default.svc.cluster.local + port: + number: 5000 + weight: 100 + - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id + response: + set: + foo: bar + match: - headers: Cookie: regex: (^|.*; )CAPOP_CAV=test-cap-01-cav-v2($|; .*) @@ -110,7 +167,15 @@ spec: port: number: 5000 weight: 100 - - match: + - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id + response: + set: + foo: bar + match: - headers: Cookie: regex: (^|.*; )CAPOP_CAV=test-cap-01-cav-v3($|; .*) @@ -121,7 +186,13 @@ spec: number: 5000 weight: 100 - headers: + request: + set: + x-downstream-peer-subject: "%DOWNSTREAM_PEER_SUBJECT%" + x-tenant-id: invalid-tenant-id response: + set: + foo: bar add: Set-Cookie: CAPOP_CAV=test-cap-01-cav-v3;Path=/;HttpOnly;Secure route: