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

4.x: SE Multi-port config #8019

Open
danielkec opened this issue Nov 16, 2023 · 4 comments
Open

4.x: SE Multi-port config #8019

danielkec opened this issue Nov 16, 2023 · 4 comments
Assignees
Labels

Comments

@danielkec
Copy link
Contributor

Environment Details

  • Helidon Version: 4.0.0
  • Helidon SE or Helidon MP: SE
  • JDK version: 21
  • OS: Ubuntu 23.10 6.5.0-10-generic

Problem Description

Configuration of additional sockets behave inconsistently, depending on the order of builder methods.

When config is provided before putSocket random port is used - additional socket config is ignored.
When config is provided after putSocket correct port is used/reported as used but socket routes doesn't work.

Steps to reproduce

Provide config before

Config config = Config.create(() -> ConfigSources
                .create("""
                                        server:
                                          port: 8080
                                          host: "localhost"
                                          sockets:
                                            - name: admin
                                              port: 8082
                                        """,
                        MediaTypes.APPLICATION_X_YAML));

        WebServer server = WebServer.builder()
                .config(config.get("server"))
                .routing(r -> r.any("/", (req, res) -> res.send("ok")))
                .putSocket("admin", s -> s.routing(r -> r.any((req, res) -> res.send("admin ok"))))
                .build()
                .start();

        System.out.println("WEB server is up! http://localhost:" + server.port());
        System.out.println("WEB server is up! http://localhost:" + server.port("admin"));
WEB server is up! http://localhost:8080
WEB server is up! http://localhost:37689
~ curl http://localhost:37689        
admin ok%   

Provide config after

        WebServer server = WebServer.builder()
                .routing(r -> r.any("/", (req, res) -> res.send("ok")))
                .putSocket("admin", s -> s.routing(r -> r.any((req, res) -> res.send("admin ok"))))
                .config(config.get("server"))
                .build()
                .start();
WEB server is up! http://localhost:8080
WEB server is up! http://localhost:8082

But this time admin port is not accessible!

~ curl http://localhost:8082
Endpoint not found%   
@danielkec
Copy link
Contributor Author

Workaround:

        WebServer server = builder
                .config(config.get("server"))
                .routing(r -> r.any("/", (req, res) -> res.send("ok")))
                .putSocket("admin", s -> s.from(builder.sockets().get("admin")).routing(r -> r
                        .any((req, res) -> res.send("admin ok"))))
                .build()
                .start();

@saladmaker
Copy link

saladmaker commented Nov 16, 2023

i'm studying the Helidon Builder. I gues the generated

WebserverConfig.Builder{
...
putSocket(String name, Consumer<ListenerConfig.Builder> consumer)
}

is not intended to update exsiting ListenerConfig, but to build new ones.

putSocket("admin", s -> s.routing(r -> r.any((req, res) -> res.send("admin ok"))))

replaces the the "admin" ListenerConfig prototype read from configuration in the underlaying WebserverConfig.BuilderBase

@saladmaker
Copy link

this does work well

        WebServer server = WebServer.builder()
                .routing(r -> r.any("/", (req, res) -> res.send("ok")))
                .config(config.get("server"))
                .routing("admin", routingBuilder -> routingBuilder.any((req, res)-> res.send("admin ok")))
                .build()
                .start();
 > curl http://localhost:8082
 > admin ok

use this method istead of putSocket

routing(String socket, Consumer<HttpRouting.Builder> consumer)

@m0mus m0mus added the triage label Dec 6, 2023
@barchetta
Copy link
Member

This is working as designed. We should consider improving javadoc for putsocket

@barchetta barchetta added docs webserver 4.x Version 4.x and removed triage labels Dec 18, 2023
@m0mus m0mus added this to Backlog Aug 12, 2024
@m0mus m0mus moved this to Normal priority in Backlog Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Normal priority
Development

No branches or pull requests

4 participants