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

Add manual routes support to imperative gateway #25817

Merged
merged 5 commits into from
May 6, 2024

Conversation

mshima
Copy link
Member

@mshima mshima commented Apr 11, 2024

  • add support to quotedList to jdl
  • add types to jdl operations
  • rework jdl toString
  • add routes option for imperative gateway

Related to #25715


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@mraible
Copy link
Contributor

mraible commented Apr 25, 2024

I'm guessing there's no way to halt the generation of all apps when this error occurs:

ERROR! An error occured while running jhipster:spring-cloud:gateway#reactiveByDefault
ERROR! An error occured while running jhipster:workspaces#generateApplications
ERROR! ERROR! Spring Cloud Gateway MVC support is experimental and not officially supported. To use it, run the generator with the --experimental flag.
Error: Spring Cloud Gateway MVC support is experimental and not officially supported. To use it, run the generator with the --experimental flag.
    at GatewayGenerator.reactiveByDefault (file:///Users/mraible/dev/generator-jhipster/dist/generators/spring-cloud/generators/gateway/generator.js:61:31)
    at GatewayGenerator.executeTask (file:///Users/mraible/dev/generator-jhipster/node_modules/yeoman-generator/dist/actions/lifecycle.js:244:26)
    at env.queueTask.once (file:///Users/mraible/dev/generator-jhipster/node_modules/yeoman-generator/dist/actions/lifecycle.js:218:56)
    at runLoop.add.once (file:///Users/mraible/dev/generator-jhipster/node_modules/yeoman-environment/dist/environment-base.js:395:23)
    at Immediate.<anonymous> (/Users/mraible/dev/generator-jhipster/node_modules/grouped-queue/lib/subqueue.js:48:34)
    at process.processImmediate (node:internal/timers:478:21)

I ran the following command and got distracted:

jhipster jdl app.jdl --monorepository --workspaces

When I came back to my terminal, I was confused as to why the gateway directory was empty. Then I scrolled up and saw the error. It'd be a better developer experience if it stopped after this error.

@mraible
Copy link
Contributor

mraible commented Apr 25, 2024

I tried the JDL in #25715 (comment) and was confused at first that there was no Consul Docker image to start up. Then I realized there was no service discovery so it's not needed. Should we issue a warning when there's a serviceDiscoveryType line in the JDL and it's a non-reactive gateway?

Related:

If I run npm run ci:e2e:prepare in the gateway app, only postgres starts up. I'm not sure why Keycloak is missing. If I run the same command in the blog app, it starts Keycloak and Consul.

 ✔ Container blog-consul-1                Healthy                                                                  0.0s
 ✔ Container blog-consul-config-loader-1  Healthy                                                                  0.0s
 ✔ Container blog-neo4j-1                 Created                                                                  0.0s
 ✔ Container blog-keycloak-1              Created

@mraible
Copy link
Contributor

mraible commented Apr 27, 2024

I tried this again today with the following JDL:

application {
  config {
    baseName gateway
    reactive false
    packageName org.jhipster.gateway
    applicationType gateway
    authenticationType oauth2
    buildTool gradle
    clientFramework react
    prodDatabaseType postgresql
    serviceDiscoveryType no
    testFrameworks [cypress]
    microfrontends [blog, store]
    routes ["blog:blog:8081", "store:store:8082"]
  }
}

application {
  config {
    baseName blog
    reactive false
    packageName org.jhipster.blog
    applicationType microservice
    authenticationType oauth2
    buildTool gradle
    clientFramework react
    databaseType neo4j
    enableHibernateCache false
    serverPort 8081
    serviceDiscoveryType no
    testFrameworks [cypress]
  }
  entities Blog, Post, Tag
}

application {
  config {
    baseName store
    reactive false
    packageName org.jhipster.store
    applicationType microservice
    authenticationType oauth2
    buildTool gradle
    clientFramework react
    databaseType mongodb
    enableHibernateCache false
    serverPort 8082
    serviceDiscoveryType no
    testFrameworks [cypress]
  }
  entities Product
}

entity Blog {
  name String required minlength(3)
  handle String required minlength(2)
}

entity Post {
  title String required
  content TextBlob required
  date Instant required
}

entity Tag {
  name String required minlength(2)
}

entity Product {
  title String required
  price BigDecimal required min(0)
  image ImageBlob
}

relationship ManyToOne {
  Blog{user(login)} to User with builtInEntity
  Post{blog(name)} to Blog
}

relationship ManyToMany {
  Post{tag(name)} to Tag{post}
}

paginate Post, Tag with infinite-scroll
paginate Product with pagination

deployment {
  deploymentType docker-compose
  serviceDiscoveryType consul
  appsFolders [gateway, blog, store]
  dockerRepositoryName "mraible"
}

deployment {
  deploymentType kubernetes
  appsFolders [gateway, blog, store]
  clusteredDbApps [store]
  kubernetesNamespace demo
  kubernetesUseDynamicStorage true
  kubernetesStorageClassName ""
  serviceDiscoveryType consul
  dockerRepositoryName "mraible"
}

And the following command:

jhipster jdl app.jdl --monorepository --workspaces --experimental

The gateway seems to work fine, but there is a stack trace in the logs:

Caused by: java.lang.NoClassDefFoundError: org/springframework/cloud/loadbalancer/support/LoadBalancerClientFactory
        at org.springframework.cloud.gateway.server.mvc.filter.LoadBalancerFilterFunctions.lambda$lb$5(LoadBalancerFilterFunctions.java:66)
        at org.springframework.web.servlet.function.HandlerFilterFunction.lambda$andThen$0(HandlerFilterFunction.java:58)
        at org.springframework.web.servlet.function.HandlerFilterFunction.lambda$ofRequestProcessor$3(HandlerFilterFunction.java:83)
        at org.springframework.web.servlet.function.HandlerFilterFunction.lambda$andThen$0(HandlerFilterFunction.java:58)
        at org.springframework.cloud.gateway.server.mvc.config.RouterFunctionHolderFactory.lambda$getRouterFunction$3(RouterFunctionHolderFactory.java:154)

And, after starting both the blog and store apps, the gateway can't connect to them because of a 403.

Screenshot 2024-04-27 at 2 19 07 PM

@mshima
Copy link
Member Author

mshima commented Apr 27, 2024

Indeed it works with serviceDiscovery enabled.

@mraible
Copy link
Contributor

mraible commented Apr 29, 2024

@mshima I tried changing the JDL from serviceDiscoveryType no to serviceDiscoveryType consul and regenerating everything. After starting the blog app, I can see the menu items in the gateway. However, when I click on them, there's an infinite redirect that happens.

25817.mp4

clientFramework angular
creationTimestamp 1617901618886
databaseType mongodb
jhiPrefix custom
jwtSecretKey "ZjY4MTM4YjI5YzMwZjhjYjI2OTNkNTRjMWQ5Y2Q0Y2YwOWNmZTE2NzRmYzU3NTMwM2NjOTE3MTllOTM3MWRkMzcyYTljMjVmNmQ0Y2MxOTUzODc0MDhhMTlkMDIxMzI2YzQzZDM2ZDE3MmQ3NjVkODk3OTVmYzljYTQyZDNmMTQ="
messageBroker kafka
packageName com.okta.developer.gateway
serviceDiscoveryType consul
reactive false
serviceDiscoveryType no
Copy link
Contributor

Choose a reason for hiding this comment

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

Gateway shouldn't be an option for microservice architecture, there's no point specifically routing.

I'm strongly against adding an antipattern just to please users that aren't able to achieve reactive programming from api to ms webclient.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that hard-coding routes in the gateway might not be a good idea. The reason we're considering it is that Spring Cloud Gateway MVC won't have support for service discovery until November 2024.

As far as reactive vs MVC, there does seem to be a demand for Spring Cloud Gateway MVC, that's why Spencer Gibb and the Spring Cloud team are working on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't agree.

  • it's not up to us to say you should use service discovery for every gateway/microservice use case
  • we do not forbid gateways and microservices without serviceDiscovery
  • Add manual option for service discovery #21012 is open for more than a year without any comment

IMO we have do choose one of:

Copy link
Contributor

@Tcharl Tcharl Apr 30, 2024

Choose a reason for hiding this comment

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

It's just IMHO:

Backend Gateways:

  • Act as a single point of entry of frontend request
  • Is in charge of managing permissions (users and authorities)
  • Route requests to the according microservice by querying the registry and translating exceptions and return codes
  • Retry, circuitbreaking policy, ...
    And this is simple enough to be managed by reactive paradigm to anyone (imperative would also please me as I find reactive quite hard, but still)

Otherwise, we loose one of the 12 factors and microservice paradigm. Manual route is not hipsterish and definitely not something worth the maintenance, I would personally prefer to close the #21012 as antipattern

Copy link
Contributor

Choose a reason for hiding this comment

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

ideally BTW, this paradigm should also be applied for microfrontend too ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

To add to this discussion, here's a video from Josh Long about why Spring Cloud Gateway MVC. Summary: because Java 21 with virtual threads and it's easier to understand. https://youtu.be/1ouE2QAebuE?si=-603Akrcvgn0HiMN&t=244

@mshima mshima mentioned this pull request Apr 30, 2024
6 tasks
@mshima mshima force-pushed the gateway-imperative branch from f8cebf2 to cf9d21b Compare April 30, 2024 19:49
@mshima mshima requested a review from DanielFran May 6, 2024 15:05
@DanielFran DanielFran merged commit cc67bdc into jhipster:main May 6, 2024
52 checks passed
@mshima mshima deleted the gateway-imperative branch May 18, 2024 18:42
@mraible mraible added this to the 8.5.0 milestone May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants