preserve host header + set forward-headers-strategy FRAMEWORK

edevosc2c commented 5 months ago

This has been discussed to set forward-headers-strategy to FRAMEWORK in order to pass through the x-forwarded headers sent by the reverse proxy. A deep explanation is available here: https://stackoverflow.com/a/68318932
And also not touching the host header sent by the reverse proxy using the parameter PreserveHostHeader

edevosc2c commented 5 months ago

Just launched the tests but still the same error as in the other PR:

| [INFO] Running org.georchestra.gateway.autoconfigure.accounts.RabbitmqEventsAutoConfigurationTest
| 14:47:10.639 [main] WARN  o.s.a.r.c.CachingConnectionFactory - ***
| Automatic Recovery was Enabled in the provided connection factory;
| while Spring AMQP is generally compatible with this feature, there
| are some corner cases where problems arise. Spring AMQP
| prefers to use its own recovery mechanisms; when this option is true, you may receive
| 'AutoRecoverConnectionNotCurrentlyOpenException's until the connection is recovered.
| It has therefore been disabled; if you really wish to enable it, use
| 'getRabbitConnectionFactory().setAutomaticRecoveryEnabled(true)',
| but this is discouraged.
| 14:47:10.705 [main] INFO  o.s.a.r.c.CachingConnectionFactory - Attempting to connect to: test.rabbit:3333
| 14:47:11.037 [main] INFO  o.s.a.r.l.SimpleMessageListenerContainer - Broker not available; cannot force queue declarations during start: java.net.UnknownHostException: test.rabbit: Name or service not known
| 14:47:11.048 [org.springframework.amqp.rabbit.config.ListenerContainerFactoryBean#0-1] INFO  o.s.a.r.c.CachingConnectionFactory - Attempting to connect to: test.rabbit:3333
| 14:47:11.051 [org.springframework.amqp.rabbit.config.ListenerContainerFactoryBean#0-2] INFO  o.s.a.r.l.SimpleMessageListenerContainer - Waiting for workers to finish.
| 14:47:11.051 [org.springframework.amqp.rabbit.config.ListenerContainerFactoryBean#0-2] INFO  o.s.a.r.l.SimpleMessageListenerContainer - Successfully waited for workers to finish.
| [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.701 s - in org.georchestra.gateway.autoconfigure.accounts.RabbitmqEventsAutoConfigurationTest
| [INFO] Running org.georchestra.gateway.accounts.admin.ldap.GeorchestraLdapAccessConfigurationTest
| [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.054 s - in org.georchestra.gateway.accounts.admin.ldap.GeorchestraLdapAccessConfigurationTest
| [INFO] 
| [INFO] Results:
| [INFO] 
| [ERROR] Failures: 
| [ERROR]   GeorchestraGatewayApplicationTests.errorCustomizerReturnsServiceUnavailableInsteadOfServerError:117 Status expected:<503> but was:<500>
| [INFO] 
| [ERROR] Tests run: 99, Failures: 1, Errors: 0, Skipped: 0
| [INFO] 
| [INFO] ------------------------------------------------------------------------
| [INFO] BUILD FAILURE
| [INFO] ------------------------------------------------------------------------
| [INFO] Total time:  15.848 s
| [INFO] Finished at: 2024-03-13T14:47:11Z
| [INFO] ------------------------------------------------------------------------
| [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.22.2:test (default-test) on project georchestra-gateway: There are test failures.
| [ERROR] 
| [ERROR] Please refer to /home/edevos/Documents/synced/clients/georchestra/georchestra-gateway/gateway/target/surefire-reports for the individual test results.
| [ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
| [ERROR] -> [Help 1]
| [ERROR] 
| [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
| [ERROR] Re-run Maven using the -X switch to enable full debug logging.
| [ERROR] 
| [ERROR] For more information about the errors and possible solutions, please read the following articles:
| [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
| make: *** [Makefile:7: test] Error 1
[Build and publish docker image/build]   ❌  Failure - Main Build gateway

It's the PreserveHostHeader that is causing the issue.

f-necas commented 4 months ago

I think we should get commits from #105 to get CI working.

pmauduit commented 4 months ago

I think we should get commits from #105 to get CI working.

but @groldan said it was a patch on top of a patch, and I tend to agree with him, we should not have the need for them.

pmauduit commented 3 months ago

as discussed in the meeting, let's go with this approach and add the host header in all tests

meaning "merging" this PR with #105 / getting commits from #105 in this one

f-necas commented 3 months ago

Force-push : rebased on master

georchestra / georchestra-gateway

preserve host header + set forward-headers-strategy FRAMEWORK #109