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

Switch to caddy webserver #268

Merged
merged 14 commits into from
Nov 4, 2024
Merged

Switch to caddy webserver #268

merged 14 commits into from
Nov 4, 2024

Conversation

edevosc2c
Copy link
Member

@edevosc2c edevosc2c commented Jan 16, 2024

Introduction

Traefik has been the source of multiple problems, #214 in particular, and is not a friendly web server to work with.

This pull request propose to switch from Traefik to Caddy webserver: https://caddyserver.com/

Caddy by itself is mature and has existed since 2015, the same year when Traefik launched.

Also, Caddy is so much simpler to work with, take a look at the documentation: https://caddyserver.com/docs/quick-starts/caddyfile

Benefits of this pull request

Overall: Way simpler docker-compose.yml and fewer services to run!

  • Way simpler syntax and fewer lines. 55 lines (caddy) VS 70 lines (Traefik) spread all around the docker-compose for the same results.
    From 160 lines to 60 lines in the docker-compose.override.yml file, overall easier maintainability.
    Caddy is so much simpler to configure than Traefik, take a look at their tutorial: https://caddyserver.com/docs/quick-starts/caddyfile
  • Automatic self-signed TLS certificate. No need for "traefik-me-certificate-downloader" or another component anymore.
    This drops the automatic traefik.me certificate component and uses a self-signed TLS certificate.
  • It won't be needed to name the directory "docker" because this switches to a static configuration file.
    Fixes Directory name needs to be named "docker" #214
  • Automatically trust the self-signed certificate using one simple command: caddy trust. It is documented through this PR.
    Fixes Migrate away from georchestra-127-0-1-1.traefik.me in the traefik docker compose file #219
  • No need for the "static" component anymore, as Caddy supports serving local files using file_server.
  • /favicon.ico, /crossdomain.xml and /robots.txt works out of the box!
  • One place to change the domain for all the docker composition, change the domain in the file .envs-common and this gets propagated to every service including the web server.
  • Caddy gives the best TLS security, and is compliant with PCI DSS requirements.
    On ssllabs.com you get an A grade!

What this pull request does not change

  • The same automated Let's Encrypt generation like Traefik. May be even better because the implementation is very well written: https://caddyserver.com/docs/automatic-https
    And there is no need to add complicated stuff in the Traefik for Let's Encrypt to be activated, Caddy will try Let's Encrypt out of the box.
  • Same functionalities that Traefik offered.

(potential) downsides of this pull request

  • A new web server. It shouldn't be an issue because people that uses the docker composition don't care about the web server as long as it works.
  • Since it uses a self-signed TLS certificate, the user has to accept the "security risk".
    But the user can easily trust the self-signed TLS certificate using caddy trust! It is documented in this PR.

TODO (missing)

Fixes

@f-necas
Copy link
Contributor

f-necas commented Jan 16, 2024

Could it be possible to use http and not https with it ? Is it configurable ?

@jeanpommier
Copy link
Member

It will be nice to have a solution to the SSL certificate issue, nice proposal. I'll try it ASAP (can't today).

I'm wondering if this one would justify a communication on the mailing list. A priori, this shouldn't matter a great deal to change the reverse-proxy technology. But maybe some people have the docker compo in production and would have some unforeseen impact.

@fvanderbiest
Copy link
Member

Thanks for the proposal Emilien.
Had not expected this and didn't know Caddy at all ;-)

"Caddy shines in its simplicity, automatic HTTPS, and intuitive configuration syntax, making it great for beginners and small to medium-sized projects. On the other hand, Traefik excels in its dynamic configuration capabilities, extensive plugin ecosystem, and strong community support, making it more suitable for larger and complex containerized environments." from https://stackshare.io/stackups/caddy-vs-traefik

Also saw a YT video showing Traefik outperformed Caddy in a test.

Apart from that, I'm a bit sad to see traefik go away.
Loved it !

@jeanpommier
Copy link
Member

Also saw a YT video showing Traefik outperformed Caddy in a test.

I'm not sure we care about performance, for the docker-compose. It's not the advised production setup anyway

Apart from that, I'm a bit sad to see traefik go away. Loved it !

Will you really be missing the configuration ? I quite like Traefik, once configured, but hate the config part. More seriously, I had gotten used to it and I'm rather bothered by having to learn yet another similar tool. But if it can help solve the SSL certificate issue for local deployments, I'm in.

@jeanpommier
Copy link
Member

But... I'm thinking, if the aim is to generate self-signed certificate on startup, it should be fairly easy to replace the traefik-me-certificate-downloader service by a service that would auto-generate the certificates. And keep traefik. What do you think ?

I also believe it should be possible to simplify our configuration for traefik. It should be possible to get something simpler for the routing at least, I don't like the way we list all possible services, it doesn't match with Traefik phylosophy)

@edevosc2c
Copy link
Member Author

I want to remind everyone that I'm using a project built on top of caddy called https://github.com/lucaslorentz/caddy-docker-proxy. This project has existed for almost 6 years now and is here to stay: lucaslorentz/caddy-docker-proxy/releases?page=4.

But this is because I have wanted to keep the same ability to configure the web server from docker labels.

It may not be what we really want, maybe a simple file would be much better. We would just use the containers names for configuring where to send the requests to, that would work too.

I'm wondering about that because docker labels are not necessary the ideal way to configure a web server and can make the configuration more difficult to work with.


But... I'm thinking, if the aim is to generate self-signed certificate on startup, it should be fairly easy to replace the traefik-me-certificate-downloader service by a service that would auto-generate the certificates. And keep traefik. What do you think ?

Sure, but "one off" containers in docker has always been the source of confusion or problems, it's not supported by Docker.

You need to hack a logic to check that everything went well, otherwise the container will keep repeating the same actions once the full docker composition is restarted. Sometimes this hack doesn't work well, so the container keep doing the same actions over and over anyway.

I also believe it should be possible to simplify our configuration for traefik. It should be possible to get something simpler for the routing at least, I don't like the way we list all possible services, it doesn't match with Traefik phylosophy)

You can't really change how traefik was made, in my opinion the configuration is awful to work with.

@jeanpommier
Copy link
Member

But this is because I have wanted to keep the same ability to configure the web server from docker labels.

It may not be what we really want, maybe a simple file would be much better. We would just use the containers names for configuring where to send the requests to, that would work too.

I'm wondering about that because docker labels are not necessary the ideal way to configure a web server and can make the configuration more difficult to work with.

Indeed. In the end, how many services are configured behind the proxy ? Only security-proxy and static ?
(What is the purpose of the static container BTW ? It looks like it has added a lot of extra complexity in the reverse-proxy config)

We are not really using the dynamic service discovery thing, actually, are we ? Static config could probably do the job

@edevosc2c
Copy link
Member Author

edevosc2c commented May 24, 2024

I had some time to tackle this PR again! I have taken into account the feedback.

What's new?

Additional finding about Caddy

  • By switching to Caddy and without doing anything, you get the best TLS security, and you are fully compliant with PCI DSS requirements.
    On ssllabs.com you get an A grade! If your client asks you to get compliant with the latest requirements for the TLS security, you can say to him that the job is already done!

Summary

Please give me some feedback about these new changes. I'm confident that it's these were the remaining changes to greatly improve the docker composition of geOrchestra.

I was wondering if I should write a migration guide? A basic one for people that already have the docker composition for geOrchestra 23 with Traefik. Or if we don't care because we consider that this docker composition is for development only.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@fvanderbiest
Copy link
Member

Thanks for this great contribution Emilien.
It is indeed a clear improvement over the previous option.

@edevosc2c edevosc2c added this to the 24.0 milestone Jun 5, 2024
Copy link
Member

@jeanpommier jeanpommier left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks Emilien. The config is indeed much leaner.

CAn you please also remove the 2 traefik*.yml files in resources/ ?

@f-necas
Copy link
Contributor

f-necas commented Jun 7, 2024

Second test :

FF + Gateway : Some config needs to be updated, still can't login by now

Git diff

Sorry for whitespaces

Subject: [PATCH] feat: add data api to docker
---
Index: config/console/console.properties
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>ISO-8859-1
===================================================================
diff --git a/config/console/console.properties b/config/console/console.properties
--- a/config/console/console.properties	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/console/console.properties	(date 1717771250977)
@@ -311,7 +311,7 @@
 #saslEnabled=false
 
 # Activates extractor data
-# if set to true, enable layer extraction in analytics part 
+# if set to true, enable layer extraction in analytics part
 # default: false
 # extractorappEnabled=false
 
@@ -330,4 +330,4 @@
 # Activates area of competence
 # if set to true, enable competence area in the account form and in organization page
 # default: false
-# competenceAreaEnabled=false
\ No newline at end of file
+# competenceAreaEnabled=false
Index: config/gateway/application.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/config/gateway/application.yaml b/config/gateway/application.yaml
--- a/config/gateway/application.yaml	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/gateway/application.yaml	(date 1717773358202)
@@ -24,4 +24,4 @@
         - PreserveHostHeader
       filter:
         secure-headers:
-          referrer-policy: strict-origin
\ No newline at end of file
+          referrer-policy: strict-origin
Index: config/gateway/gateway.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/config/gateway/gateway.yaml b/config/gateway/gateway.yaml
--- a/config/gateway/gateway.yaml	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/gateway/gateway.yaml	(date 1717773212110)
@@ -7,7 +7,7 @@
 georchestra:
   gateway:
     # Redirection URL after a successful logout : Defaults to /?logout
-    # logoutUrl : "/?logout" 
+    # logoutUrl : "/?logout"
     default-headers:
       # Default security headers to append to proxied requests
       proxy: true
@@ -24,11 +24,6 @@
       - "/proxy/?url=*"
       anonymous: true
     services:
-      header:
-        target: ${georchestra.gateway.services.header.target}
-        access-rules:
-        - intercept-url: /header/**
-          anonymous: true
       datafeeder:
         target: ${georchestra.gateway.services.datafeeder.target}
         headers:
@@ -37,13 +32,13 @@
         access-rules:
         - intercept-url: /datafeeder/**
           anonymous: false
-          allowed-roles: SUPERUSER,DATAFEEDER
+          allowed-roles: SUPERUSER,IMPORT
       import:
         target: ${georchestra.gateway.services.import.target}
         access-rules:
         - intercept-url: /import/**
           anonymous: false
-          allowed-roles: SUPERUSER,DATAFEEDER
+          allowed-roles: SUPERUSER,IMPORT
       console:
         target: ${georchestra.gateway.services.console.target}
         access-rules:
Index: config/gateway/routes.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/config/gateway/routes.yaml b/config/gateway/routes.yaml
--- a/config/gateway/routes.yaml	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/gateway/routes.yaml	(date 1717771625812)
@@ -10,7 +10,7 @@
         predicates:
         - Path=/
         filters:
-        - RedirectTo=308, /header
+        - RedirectTo=308, /datahub
       - id: header
         uri: ${georchestra.gateway.services.header.target}
         predicates:
Index: config/gateway/security.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/config/gateway/security.yaml b/config/gateway/security.yaml
--- a/config/gateway/security.yaml	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/gateway/security.yaml	(date 1717773059943)
@@ -24,6 +24,12 @@
           orgs:
             rdn: ${ldapOrgsRdn:ou=orgs}
             protectedRoles: ADMINISTRATOR, GN_.*, ORGADMIN, REFERENT, USER, SUPERUSER
+
+spring:
+  webflux:
+    static-path-pattern: /login/**
+
+
 # uncomment for oauth 2.0
 #spring:
 #  security:
Index: resources/caddy/etc/Caddyfile
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/resources/caddy/etc/Caddyfile b/resources/caddy/etc/Caddyfile
--- a/resources/caddy/etc/Caddyfile	(revision f11be9ea2edd1cf06c6d2760e912b0035cd5c96a)
+++ b/resources/caddy/etc/Caddyfile	(date 1717770377133)
@@ -43,7 +43,7 @@
     }
 
     handle {
-        reverse_proxy proxy:8080
+        reverse_proxy gateway:8080
         header {
             Access-Control-Allow-Origin *
             Access-Control-Allow-Methods "GET, POST, PUT, PATCH, DELETE, OPTIONS"
Index: config/datahub/conf/default.toml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/config/datahub/conf/default.toml b/config/datahub/conf/default.toml
--- a/config/datahub/conf/default.toml	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/datahub/conf/default.toml	(date 1717771250989)
@@ -69,7 +69,7 @@
 # Optional; specify a GeoJSON object to be used as filter: all records contained inside the geometry will be boosted on top,
 # all records which do not intersect with the geometry will be shown with lower priority; can be specified as URL or inline
 # Note: if the GeoJSON object contains multiple features, only the geometry of the first one will be kept!
-filter_geometry_url = "/console/account/areaofcompetence"
+# filter_geometry_url = "/console/account/areaofcompetence"
 # filter_geometry_data = '{ "coordinates": [...], "type": "Polygon" }'
 
 # The advanced search filters available to the user can be customized with this setting.
Index: docker-compose.yml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/docker-compose.yml b/docker-compose.yml
--- a/docker-compose.yml	(revision f11be9ea2edd1cf06c6d2760e912b0035cd5c96a)
+++ b/docker-compose.yml	(date 1717770101546)
@@ -14,6 +14,7 @@
   datafeeder_postgis_data:
   esdata:
   georchestra_datadir:
+  rabbitmq_data:
 
 secrets:
   slapd_password:
@@ -79,30 +80,19 @@
       - ldap_config:/etc/ldap
     restart: always
 
-  proxy:
-    image: georchestra/security-proxy:latest
-    healthcheck:
-      test: ["CMD-SHELL", "curl -s -f http://localhost:8080/_static/bootstrap_3.0.0/css/bootstrap-theme.min.css >/dev/null || exit 1"]
-      interval: 30s
-      timeout: 10s
-      retries: 10
+  gateway:
+    image: georchestra/gateway:latest
     depends_on:
-      ldap:
-        condition: service_healthy
-      database:
-        condition: service_healthy
+      - database
     volumes:
       - georchestra_datadir:/etc/georchestra
     environment:
-      - JAVA_OPTIONS=-Dorg.eclipse.jetty.annotations.AnnotationParser.LEVEL=OFF
-      - XMS=256M
-      - XMX=1G
+      - JAVA_TOOL_OPTIONS=-Dgeorchestra.datadir=/etc/georchestra
     env_file:
       - .envs-common
       - .envs-ldap
       - .envs-hosts
       - .envs-database-georchestra
-    restart: always
 
   cas:
     image: georchestra/cas:latest
@@ -186,6 +176,8 @@
         condition: service_healthy
       database:
         condition: service_healthy
+      rabbitmq:
+        condition: service_healthy
     volumes:
       - georchestra_datadir:/etc/georchestra
     environment:
@@ -195,6 +187,7 @@
     env_file:
       - .envs-common
       - .envs-ldap
+      - .envs-rabbitmq
       - .envs-database-georchestra
       - .envs-hosts
     restart: always
@@ -401,5 +394,20 @@
     volumes:
       - georchestra_datadir:/etc/georchestra
     restart: always
+
+  rabbitmq:
+    image: docker.io/bitnami/rabbitmq:3.12
+    healthcheck:
+      test: rabbitmq-diagnostics -q ping && rabbitmq-diagnostics -q check_local_alarms
+      interval: 60s
+      timeout: 30s
+      retries: 3
+    env_file:
+      - .envs-rabbitmq
+    environment:
+      - RABBITMQ_LOGS=-
+    volumes:
+      - 'rabbitmq_data:/bitnami/rabbitmq/mnesia'
+    restart: always
 
-      
+
Index: config/cas/config/cas.properties
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>ISO-8859-1
===================================================================
diff --git a/config/cas/config/cas.properties b/config/cas/config/cas.properties
--- a/config/cas/config/cas.properties	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/cas/config/cas.properties	(date 1717769175467)
@@ -6,7 +6,7 @@
 cas.theme.param-name=georchestra
 cas.theme.default-theme-name=georchestra
 
-cas.service-registry.initFromJson=false
+cas.service-registry.core.init-from-json=false
 cas.service-registry.json.location=file:/etc/georchestra/cas/services
 #uncomment if getting 302 redirects on cas.{css,js} behind nginx/apache
 #server.forward-headers-strategy=FRAMEWORK
@@ -47,7 +47,7 @@
 
 cas.authn.ldap[0].type=DIRECT
 cas.authn.ldap[0].dn-format=uid=%s,ou=users,dc=georchestra,dc=org
-cas.authn.oidc.jwks.jwks-file=file:///tmp/keystore.jwksdown
+cas.authn.oidc.jwks.file-system.jwks-file=file:///tmp/keystore.jwksdown
 
 cas.authn.saml-idp.core.entity-id=https://${FQDN}/idp
-cas.authn.saml-idp.metadata.location=file:///tmp/
+cas.authn.saml-idp.metadata.file-system.location=file:///tmp/
Index: config/default.properties
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>ISO-8859-1
===================================================================
diff --git a/config/default.properties b/config/default.properties
--- a/config/default.properties	(revision 05119872450ec875ccfc6f58fcd8da65c1d3e37d)
+++ b/config/default.properties	(date 1717770607745)
@@ -83,13 +83,13 @@
 # rabbitmq server domain name
 rabbitmqHost=${RABBITMQ_HOST}
 
-# rabbitmq user 
+# rabbitmq user
 rabbitmqUser=${RABBITMQ_USERNAME}
 
-# rabbitmq password 
+# rabbitmq password
 rabbitmqPassword=${RABBITMQ_PASSWORD}
 
-# rabbitmq port 
+# rabbitmq port
 rabbitmqPort=${RABBITMQ_PORT}
 
 ### LDAP properties
@@ -144,10 +144,10 @@
 # Listening port of the SMTP server
 smtpPort=${SMTPPORT}
 
-# Activates analytics 
+# Activates analytics
 # WARNING : When using the geOrchestra gateway, analytics should be disabled
-# if set to true, the analytics app will be enabled (https://github.com/georchestra/georchestra/tree/master/analytics) 
+# if set to true, the analytics app will be enabled (https://github.com/georchestra/georchestra/tree/master/analytics)
 # console will add all links to analytics and also execute XHR requests
 # header will diplay analytics app
 # default: true
-# analyticsEnabled=true
+analyticsEnabled=false
Login response from gateway
✗ curl --request POST --url http://localhost:8080/login \ 
  --data username=testadmin \
  --data password=testadmin -v
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> POST /login HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 37
> Content-Type: application/x-www-form-urlencoded
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 302 Found
< Location: /
< Cache-Control: no-cache, no-store, max-age=0, must-revalidate
< Pragma: no-cache
< Expires: 0
< X-Content-Type-Options: nosniff
< X-Frame-Options: DENY
< X-XSS-Protection: 1 ; mode=block
< Referrer-Policy: no-referrer
< Set-Cookie: SESSION=0bddcc89-5113-4bfc-ba8a-594b44315234; Path=/; HttpOnly; SameSite=Lax
< content-length: 0
< 
* Connection #0 to host localhost left intact

@jeanpommier
Copy link
Member

Hi, any news on this one ? Switching off traefik seems quite desirable. I believe the main issue is Chrome support for caddy's self-signed certificate ?

@jeanpommier
Copy link
Member

I tested it again. Didn't have any issue with the certificate generated by caddy, looks like Chrome had no issue with it. But apparently needs a fix, in the config: https://georchestra-127-0-1-1.traefik.me/ is giving me a 503 error and so are any path to the apps (e.g. https://georchestra-127-0-1-1.traefik.me/console/)

@edevosc2c
Copy link
Member Author

Hello jean, I haven't had the time to look into improving or at least checking if it fully works. I expect to do so before 2025.

@jeanpommier
Copy link
Member

jeanpommier commented Sep 27, 2024

Hello jean, I haven't had the time to look into improving or at least checking if it fully works. I expect to do so before 2025.

Hi Emilien,
Yes, I know that feeling ;o)
If I get some spare time I'll try to have a look too. It would be a waste to leave it unmerged

@edevosc2c
Copy link
Member Author

edevosc2c commented Sep 27, 2024

OK I have wanted to spend a little time on this PR in this Friday afternoon.

So I have just validated that caddy trust works on Google Chrome.

image

I have fixed the logging feature by removing the final slash "redirect" because this is now handled in the gateway since georchestra/georchestra-gateway#109

There is just "import" container (datafeeder) that doesn't seem to redirect correctly. Well need to fix that into the docker image, for now there is a single workaround for that.

This PR is now ready to be merged! I didn't see any other issues.

Copy link
Contributor

@jeanmi151 jeanmi151 left a comment

Choose a reason for hiding this comment

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

Tested and working except for browsers installed through snap which have their own certificate trust store.
Could create security risk if the root cert is leaked.

@edevosc2c
Copy link
Member Author

traefik.me seems down. maybe we should switch to https://nip.io?
image

README.md Outdated Show resolved Hide resolved
@edevosc2c edevosc2c merged commit 254e7f4 into georchestra:master Nov 4, 2024
@edevosc2c edevosc2c deleted the caddy branch November 4, 2024 16:26
edevosc2c added a commit that referenced this pull request Nov 4, 2024
* switch to caddy webserver

* switching to classic caddy service

* adapt .gitignore

* add return line end of file Caddyfile

* fix wording in README.md

* remove traefik.yml ressources

* switch to gateway + add caddy binary to gitignore

* remove redirection because now handled in gateway + redirect just import

* add more comments

* remove -> ignore

* can keep cas path uncommented since it doesn't hurt anything

* migrate from traefik.me to nip.io

* add note about snap and security caddy trust

* add warning icon for fnecas
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants