Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static routing definition for runtime apps, avoiding route refresh #179

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

tgeens
Copy link
Contributor

@tgeens tgeens commented Sep 6, 2023

No description provided.

@tgeens tgeens force-pushed the refresh-routes branch 2 times, most recently from fa80ae0 to ac17581 Compare September 6, 2023 13:35
@tgeens tgeens force-pushed the refresh-routes branch 2 times, most recently from 2454e09 to ec58211 Compare September 6, 2023 13:53
@tgeens tgeens marked this pull request as ready for review September 6, 2023 13:53
@tgeens tgeens requested a review from a team as a code owner September 6, 2023 13:53
@tgeens
Copy link
Contributor Author

tgeens commented Sep 6, 2023

This branch is currently live/deployed on sandbox

@Bean
RouteLocator runtimeAppRouteLocator(RouteLocatorBuilder builder, RuntimeRequestResolver requestResolver) {
return builder.routes()
.route(r -> r
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this be a plain gateway configuration file? Something like

That would make it easier to add additional routes that do not forward to a specific application, and makes it easier to configure additional filters when that is necessary

spring:
  cloud:
    gateway:
      routes:
        - id: apps
          uri: cg://ignored
          predicates:
            - ResolvesToApplication
          filters:
            - PreserveHostHeader

In that case, we would need to explicitly define a ResolvesToApplication gateway predicate, but that's all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a static configuration via properties, yes.

  1. Given that spring.cloud.gateway.routes is a list, it is imo too easy to overwrite a route-definition by accident
  2. makes runtime integration tests more finicky - now the behaviour is enabled with a single flag contentgrid.gateway.runtime-platform.enabled: true

This can be revisited in the future.

@tgeens tgeens merged commit 5565483 into main Sep 7, 2023
4 checks passed
@tgeens tgeens deleted the refresh-routes branch September 7, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants