Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: Allow custom traefik middlewares #3637

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

MarioZet23
Copy link
Contributor

@MarioZet23 MarioZet23 commented Sep 29, 2024

This is my first merge request, open to feedback
Solves #3122

Changes

  • Instead of only basicauth and redirect, all types of middlewares are now applied to the coolify router

Using the following compose-file:

services:
    http-echo:
        ...
        labels:
            - traefik.http.middlewares.customMiddleware.oneType.merge=true
            - 'traefik.http.middlewares.mybasicauth.basicauth.users=test:$2y$12$ci.4U63YX83CwkyUrjqxAucnmi2xXOIlEF6T/KdP9824f1Rf1iyNG'
            - traefik.http.middlewares.example-middleware.redirectregex.regex=^(http|https)://www\.(.+)
            - traefik.http.middlewares.example-middleware.redirectregex.replacement=${1}://${2}
            - traefik.http.middlewares.example-middleware.redirectregex.permanent=true
            - traefik.http.middlewares.anotherCustomMiddleware.anotherType

now results in the following label:

- 'traefik.http.routers.http-0-swc04wo0w0ggo44k00808c0w-http-echo.middlewares=gzip,customMiddleware,mybasicauth,example-middleware,anotherCustomMiddleware'

instead of:

- 'traefik.http.routers.http-0-swc04wo0w0ggo44k00808c0w-http-echo.middlewares=gzip,mybasicauth,example-middleware'

@MarioZet23 MarioZet23 changed the title Custom traefik middlewares feat. Allow custom traefik middlewares Oct 1, 2024
@MarioZet23 MarioZet23 changed the title feat. Allow custom traefik middlewares feat: Allow custom traefik middlewares Oct 1, 2024
@dancodes
Copy link

dancodes commented Oct 2, 2024

This closes issues 1737, 3122 and 2818 afaik

@andrasbacsai
Copy link
Member

Thank you for the PR! I have to test it, but it looks good!

@andrasbacsai andrasbacsai merged commit 5fa6501 into coollabsio:next Oct 3, 2024
1 check passed
@Paillat-dev
Copy link

Thank you for working on this. However, I've noticed that the changes don't seem to work as expected with pre-made templates, specifically the Ghost template.

Issue

After applying the update, the middlewares are not being merged for the Ghost pre-made template.

Example

In my deployable compose file for Ghost, the labels include:

labels:
  - traefik.http.routers.https-0-[redacted]-ghost.middlewares=crowdsec@file
  # ... other labels ...
  - 'traefik.http.routers.https-0-[redacted]-ghost.middlewares=gzip,redir-ghost'

We can see that the crowdsec@file middleware and the gzip,redir-ghost middlewares are defined separately, rather than being merged into a single middleware list.

Expected Behavior

Based on the changes described in the PR, we would expect all middlewares to be merged into a single list, similar to:

- 'traefik.http.routers.https-0-[redacted]-ghost.middlewares=gzip,redir-ghost,crowdsec@file'

Suggestion

It seems that the middleware merging logic might not be applied correctly to pre-made templates. Could you please investigate why the middlewares aren't being merged for the Ghost template (and potentially other pre-made templates)?

This issue suggests that there might be a difference in how labels are processed for pre-made templates compared to custom configurations. It would be helpful to ensure that the new middleware merging logic is consistently applied across all types of deployments, including pre-made templates.

@hb12devtn
Copy link

hb12devtn commented Oct 5, 2024

Hi, I experienced the same problem as Paillat-dev with Homepage service. Any hints?

@peaklabs-dev
Copy link
Member

@Paillat-dev please open a issue for your problem so we can track this better.

@Paillat-dev
Copy link

Sure will do later

@MarioZet23
Copy link
Contributor Author

Thanks for your feedback @Paillat-dev
This MR extends the logic that was already implemented for basic-auth and redirectregex middlewares, where you can define a NEW middleware in the labels, which is then applied to the router Coolify creates.
AFAIK, there is currently no logic for the router.middlewares label.
I would be willing to implement this, but it involves some design decisions I am unsure about:

  • Do we just extract the middlewares from all routers already defined in the labels? (This might break some existing setups?)
  • Do we create a new "coolify-template"-router that is merged into all the autogenerated routers?
  • Do we move this away from the traefik labels and instead introduce a coolify.middlewares label?

I think that there is nothing wrong with the pre-made templates and it's just a misunderstanding about the content of this MR, I should have been clearer. Could you try defining your crowdsec middleware inside the labels? I am unable to test it myself until next week.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants