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

[common] Dont load fixed middlewares if not found or not enabling treafik integration by default #28996

Open
2 tasks done
PrivatePuffin opened this issue Nov 8, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@PrivatePuffin
Copy link
Member

Is your feature request related to a problem?

Currently I think we try-to load fixed-middleware, even if they are not found.
This means our ingress (without disabling the integration code), is inherently not compatible with upstream treafik, which makes it less easy for non-truecharts-centered users to use TrueCharts common in their clusters.

Describe the solution you'd like

Not loading fixed-middlewares if they are not found/present and the treafik namespace is not set.

Describe alternatives you've considered

Not enabling the integration by default.
Which is a serieus option we should consider for compatibility tbh.

Additional context

No response

I've read and agree with the following

  • I've checked all open and closed issues and my request is not there.
  • I've checked all open and closed pull requests and my request is not there.
@PrivatePuffin PrivatePuffin added the enhancement New feature or request label Nov 8, 2024
@PrivatePuffin
Copy link
Member Author

@stavros-k You wrote most of this, so I've assigned it to your for feedback and convenience.

@stavros-k
Copy link
Collaborator

traefik integration is:

  1. enabled by default in the code
  2. enabled by default for main ingress in common values

Fixed middlewares are:

  1. enabled by default in the code
  2. enabled by default in the global common values

if namespace is given, we use that, otherwise we try to lookup.
If we dont find the namespace we fail.

For this we can do either:

  • if namespace key is missing and we don't find it with lookup, ignore that middleware. (if the key exists but its "" we fail)
  • Same as above, but also add a "warning" in the NOTES.txt

tbh I don't like silently ignoring.
we could leave the logic as is but make the fixedMiddlewares opt-in or to have "less" impact, make the traefik integration opt-in?
This way users that dont want to use traefik won't ever see deal with any fixed middlewares

@PrivatePuffin
Copy link
Member Author

@stavros-k I would say:

  • Traefik integration disabled by default,
  • Move fixed-middlewares to common
  • Create them when enabled on each chart separately
  • Fixed-middlewares enabled by default when traefik integration is enabled

It moves logic from traefik -> common and ensure we would never have to rely on cross-namespace lookups

@stavros-k
Copy link
Collaborator

Cool. I think first would be to make a class/spawner for middlewares.
Then do the rest of the changes

@PrivatePuffin
Copy link
Member Author

Cool. I think first would be to make a class/spawner for middlewares. Then do the rest of the changes

correct

@stavros-k
Copy link
Collaborator

Cool. I think first would be to make a class/spawner for middlewares. Then do the rest of the changes

correct

#30529

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants