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

Support for Traefik V2 #97

Closed
xanderflood opened this issue Mar 24, 2020 · 18 comments
Closed

Support for Traefik V2 #97

xanderflood opened this issue Mar 24, 2020 · 18 comments

Comments

@xanderflood
Copy link

It appears that this project isn't currently compatible with Traefik v2 - it makes calls to the /api/providers/{depends on the provider}/backends endpoint, which no longer exists. It aligns roughly with the /api/http/servers endpoint, the only important difference being that a single endpoint contains the servers for all providers, so the implementation needs to filter client-side rather

I need this and am planning to start work on it if I get a chance soon. My hope is that it can be achieved pretty simply:

  1. add a new boolean configuration traefik_api_v2 which defaults to false
  2. modify the _check_for_traefik_endpoint function in proxy.py to branch on this config value - if false, use the existing implementation, and if true, use /api/http/servers and look for a server whose name is {expected_name}@{provider}

I'm not a pythoner by background but this seems like a manageable job 🤞

@devnull-mr
Copy link
Contributor

I am facing the same issue.

I run an external Traefik v2.2 instance and would like to use traefik-proxy instead of the embedded configurable-http-proxy. In the current state that is not possible as the API has changed with Traefik v2.0, as mentioned above.

I am happy to test any patches before they are being merged into the master branch (and released) !

@GeorgianaElena
Copy link
Member

Hey @xanderflood and @devnull-mr 👋

It seems that the traefik version 2.2 released 2 weeks ago, adds support for the etcd and consul backends, so switching to v2 it's actually an option right now 🎉

  1. add a new boolean configuration traefik_api_v2 which defaults to false

I'm not sure if it's worth supporting both versions of the traefik api. It might a good idea to switch the entire implementation of traefik-proxy to use traefik v2, especially since it looks like it solves a performance issue with the key-value store provider versions.
@minrk, what do you think about this?

  1. modify the _check_for_traefik_endpoint function in proxy.py to branch on this config value - if false, use the existing implementation, and if true, use /api/http/servers and look for a server whose name is {expected_name}@{provider}

@xanderflood, Traefik v2 adds a number of breaking changes (not just the api endpoint). This doc https://docs.traefik.io/migration/v1-to-v2/ seems a very good resource about migrating from v1 to v2 if you still want to give it a try. ☀️

@xanderflood
Copy link
Author

Thanks a lot for the input @GeorgianaElena! Fortunately I'm not personally migrating Traefik versions, I'm just looking into deploying a new jupyterhub container to an existing Traefik v2.1 installation, so bumping to v2.2 will be easy for me as well.

Come to think of it, I never looked into the details of how the providers themselves changed. Does Traefik v2 store data in etcd in a different format from how Traefik v1 does? The underlying data models of Tv1 and Tv2 are pretty danged different, so it seems likely that is a more involved refactoring than I originally thought.

@devnull-mr
Copy link
Contributor

FYI, I started to work on this here

Only fixed the installer for now to install the latest versions of traefik/etcd/consul so the main part of adjusting the proxy to the Traefik v2 API still needs to be done.
Would appreciate some support/collaboration on this.

@xanderflood
Copy link
Author

@devnull-mr my use case mainly involves the file provider - etcd and consul are both overkill for me, and I'm not really familiar with either of them anyway. Not sure if I'll have time this week but if I do I'll open up a PR against your branch with some work on the toml-file interaction

@alexleach
Copy link
Contributor

Hello, I was also hoping to use an existing external Traefik v2 Proxy with a JupyterHub instance. I had a look at your fork @devnull-mr , and looks like no work has been done on it since April last year. Has anyone made any headway with upgrading the traefik-proxy to work with the traefik v2 API?

@devnull-mr
Copy link
Contributor

No further progress was made on my fork, that's correct. I also don't intend to continue working on it. So hopefully the main branch will start working on this at some time.

@alexleach
Copy link
Contributor

Okay, no problem, but thanks for confirming @devnull-mr .
I have had a go myself now and appear to have got a Traefik file provider working with Traefik v2.4.8. I am unable to test with etcd or consul providers though, so have barely touched those files.
Is anyone able to provide some feedback? Happy to make a PR if that would be a preferred next step?

alexleach@41a5ef3

@alexleach
Copy link
Contributor

Okay, no problem, but thanks for confirming @devnull-mr .
I have had a go myself now and appear to have got a Traefik file provider working with Traefik v2.4.8. I am unable to test with etcd or consul providers though, so have barely touched those files.
Is anyone able to provide some feedback? Happy to make a PR if that would be a preferred next step?

alexleach@41a5ef3

I received an email shortly after making my commit notifying me that, typically all the tests fail. I will try and get a significant portion to pass and make a follow-up commit to address these failures. Hopefully most can be resolved fairly easily

alexleach added a commit to alexleach/traefik-proxy that referenced this issue Jun 18, 2021
…ng initial

work done in previous commit:-

41a5ef3

Also, see issue: jupyterhub#97

 - Relevant documentation has been updated in README.md, docs/source/file.md
   (renamed from toml.md)

 - KV providers (consul and etcd) have been updated to use new Traefik KV
   paths, as per:-
   https://doc.traefik.io/traefik/reference/dynamic-configuration/kv/

 - requirements.txt now makes toml an optional dependency as well as
   ruamel.yaml. This latter module is required by jupyterhub anyway, so should
   already be present on a system running JupyterHub.

 - The (thoroughly excellent) test system has had a bit of an overhaul:-

   - There were issues with repeated launching and shutting down of external
     `consul` and `etcd` servers, causing test failures. Added code to
     gracefully shutdown consul and wait for servers to launch (within
     `tests/conftest.py`) before continuing with the tests.

   - `traefik` v2 no longer has a 'storeconfig' command. Therefore, to load a
     pre-baked configuration file into the KV stores, have had to resort to
     loading configurations with either `etcdctl txn` or `consul kv import`
     commands.

   - The external file provider now watches a directory instead of a file, so
     have added a pre-baked dynamic_config directory (and file), where the
     rules.toml file will be saved and managed by the TraefikFileProviderProxy.

   - Removed the previous traefik_{consul_config,etcd_config}.toml files,
     (which acted as traefik static configuration files), and instead applied
     the static KV configuration using the CLI.

   - Refactored some of the text fixtures to try and re-use fixtures and make
     them (hopefully) a bit easier to follow.

   - Have duplicated the file_proxy and external_file_proxy pytest fixtures to
     test both toml and yaml files. (Would have preferred to parametrize the
     existing fixtures to avoid duplicating them, but couldn't figure out how).

   - `tests/test_traefik_api_auth.py` - Have had to make the test give traefik
     more of a chance to read its dynamic config before running the test.
     Previously, in traefik v1, the api authentication would be configured in
     the static config. Now the api 'middleware' is configured in the dynamic
     config, the previous wait condition of just waiting for the port to come
     up didn't give traefik enough time to set up the API authentication on
     that entrypoint / router.

 - All tests now pass on my dev system, woohoo!
alexleach added a commit to alexleach/traefik-proxy that referenced this issue Jun 23, 2021
…elation to

issue: jupyterhub#97

Although all tests passed on my test system, the github workflow tests failed with the
etcd tests, for the following reasons:-

    - Hadn't actually tested etcd-3.4.15, the default version installed by the
      install script. I'd used the default ubuntu version, 3.2.26. I'm not sure
      if this caused issues (maybe?, see next point), but some new warnings are
      emitted about the log parameters for instance.

    - The tests mainly failed due to what I thought was a nasty race-condition
      between successive TraefikEtcdProxy test fixture calls. The `proxy()`
      fixture in `tests/test_proxy.py` doesn't appear to fully destroy the
      dependent Proxy classes between test runs. In the case of
      TraefikEtcdProxy, this leaves the etcd3/grpc client open through the end
      of one test and into the next test. Then the next TraefikEtcdProxy test
      gets a connection error.  I don't know why only one grpc client is
      allowed - is this related to the etcd version? - but regardless, the less
      than ideal solution is to manually call
      `TraefikEtcdProxy.kv_client.close()` during the teardown of the
      external_etcd* test runs. This is also manually called now by
      `TraefikEtcdProxy.stop()`, when `should_start=True`.
            (This took me days to figure out!!)

Some further modifications to the code include:-

    - Changed the KV store structure for jupyterhub-specific information. All
      jupyterhub routespecs, targets and data are now stored as:-

          jupyterhub/
                   -/routes/{escaped_routespec} = {target}
                   -/targets/{escaped_target}   = {data}

      N.B. I think this can be condensed to one single request. Atm, to get the
      {data} for a routespec, two KV get requests are required: 1. to get the
      {target}, and 2. to get the {data} using that {target}. The {target} is
      also in the traefik configuration, so it seems like unnecessary
      duplication of information and redirection.

    - Added `log_level` and `traefik_log_level` config parameters to the base
      Proxy class. The first sets the log level for the logger (and handler)
      of the Proxy class. The latter sets traefik's log level, if
      `should_start=True`.

    - General tidy up, removing excessive debug statements and commented-out
      code.
@alexleach
Copy link
Contributor

Hi all,

I've spent quite a bit of time trying to make jupyter_traefik_proxy compatible with the Traefik v2 API, and have made a couple of commits referencing this issue (links shown above). Please see the commit text for details on the code changes, but in summary, I've tried to make all tests pass across the (rather excellent and thorough) test suite. This includes tests for etcd3 and consul backends, as well as separate tests using the file provider, to cover both 'yaml' and 'toml' traefik configuration files. I have made relevant changes to the documentation where I thought necessary, as well.

The updates will obviously make some breaking changes to anyone using Traefik v1, so I would like to propose that a new branch be created in the main repo specifically for Traefik v2. I can then open a PR to the new branch, if that sounds sensible? I'm not sure if I can make a PR to a new, non-existent branch, so please could someone with the necessary privileges create a new, v2 branch that I can make a PR to?

As of April this year, Traefik is still releasing updates to its 1.7 branch, and obviously anyone using jupyterhub_traefik_proxy currently, must also be using a traefik v1 proxy. Therefore, I think it makes sense to have two branches and make a separate jupyterhub_traefik_proxy release for v2, assuming all tests pass and it passes any required quality testing.

The workflow for the latest commit has just finished, and there are still a few errors unfortunately. I'll try and get these final tests to pass in the github workflow today. The workflow test results from the latest commit have the following short summary:-

=========================== short test summary info ============================
FAILED tests/test_installer.py::test_version - AssertionError: assert False
FAILED tests/test_proxy.py::test_get_all_routes[no_auth_etcd_proxy] - TypeErr...
FAILED tests/test_proxy.py::test_get_all_routes[auth_etcd_proxy] - TypeError:...
FAILED tests/test_proxy.py::test_get_all_routes[external_etcd_proxy] - TypeEr...
FAILED tests/test_proxy.py::test_get_all_routes[auth_external_etcd_proxy] - T...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-zoe] - TypeE...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-50fia] - Typ...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-~TestJH] - T...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-has@] - Type...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-zoe] - TypeErro...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-50fia] - TypeEr...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-~TestJH] - Type...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-has@] - TypeErr...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-zoe] - Type...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-50fia] - Ty...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-~TestJH] - ...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-has@] - Typ...
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-zoe]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-50fia]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-~TestJH]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-has@]
========== 25 failed, 186 passed, 475 warnings in 3231.97s (0:53:51) ===========

Hopefully I can get these to pass by the end of the day and then if okay with the maintainers, I hope to be ready to make a PR. Please let me know if there is anything else I can do to ease the addition of this new branch, assuming this is something you'd like to incorporate?

Kind regards,
Alex

@alexleach
Copy link
Contributor

Hi all,

I've spent quite a bit of time trying to make jupyter_traefik_proxy compatible with the Traefik v2 API, and have made a couple of commits referencing this issue (links shown above). Please see the commit text for details on the code changes, but in summary, I've tried to make all tests pass across the (rather excellent and thorough) test suite. This includes tests for etcd3 and consul backends, as well as separate tests using the file provider, to cover both 'yaml' and 'toml' traefik configuration files. I have made relevant changes to the documentation where I thought necessary, as well.

The updates will obviously make some breaking changes to anyone using Traefik v1, so I would like to propose that a new branch be created in the main repo specifically for Traefik v2. I can then open a PR to the new branch, if that sounds sensible? I'm not sure if I can make a PR to a new, non-existent branch, so please could someone with the necessary privileges create a new, v2 branch that I can make a PR to?

As of April this year, Traefik is still releasing updates to its 1.7 branch, and obviously anyone using jupyterhub_traefik_proxy currently, must also be using a traefik v1 proxy. Therefore, I think it makes sense to have two branches and make a separate jupyterhub_traefik_proxy release for v2, assuming all tests pass and it passes any required quality testing.

The workflow for the latest commit has just finished, and there are still a few errors unfortunately. I'll try and get these final tests to pass in the github workflow today. The workflow test results from the latest commit have the following short summary:-

=========================== short test summary info ============================
FAILED tests/test_installer.py::test_version - AssertionError: assert False
FAILED tests/test_proxy.py::test_get_all_routes[no_auth_etcd_proxy] - TypeErr...
FAILED tests/test_proxy.py::test_get_all_routes[auth_etcd_proxy] - TypeError:...
FAILED tests/test_proxy.py::test_get_all_routes[external_etcd_proxy] - TypeEr...
FAILED tests/test_proxy.py::test_get_all_routes[auth_external_etcd_proxy] - T...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-zoe] - TypeE...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-50fia] - Typ...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-~TestJH] - T...
FAILED tests/test_proxy.py::test_check_routes[no_auth_etcd_proxy-has@] - Type...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-zoe] - TypeErro...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-50fia] - TypeEr...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-~TestJH] - Type...
FAILED tests/test_proxy.py::test_check_routes[auth_etcd_proxy-has@] - TypeErr...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-zoe] - Type...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-50fia] - Ty...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-~TestJH] - ...
FAILED tests/test_proxy.py::test_check_routes[external_etcd_proxy-has@] - Typ...
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-zoe]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-50fia]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-\u79c0\u6a39]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-~TestJH]
FAILED tests/test_proxy.py::test_check_routes[auth_external_etcd_proxy-has@]
========== 25 failed, 186 passed, 475 warnings in 3231.97s (0:53:51) ===========

Hopefully I can get these to pass by the end of the day and then if okay with the maintainers, I hope to be ready to make a PR. Please let me know if there is anything else I can do to ease the addition of this new branch, assuming this is something you'd like to incorporate?

Kind regards,
Alex

Ah, the test failures appear to stem from jupyterhub/install.py. I'd forked from 077a625, where the latest commit broke the test_warning fixture of tests/test_installer.py. Essentially, where unknown versions of traefik/etcd/consul were requested, the commit made install.py not download them, whereas the original repo issued a warning but downloaded unknown versions anyway. I've reverted the change, and this will hopefully make the next workflow tests pass. (I'd also not updated the default version of etcd, so the default version didn't have a checksum and wouldn't be downloaded. The reasons the etcd tests therefore failed was because there wasn't an etcd on the test server.)

@alexleach
Copy link
Contributor

Just to update, all tests now pass on my Traefik v2 branch, commit 508987c. I'd made a minor mistake when tidying up my code for the previous commit, which is now resolved. I'll try and create a PR to the main repo. Hope that's okay.

I was reading through the jupyterhub/zero-to-jupyterhub-k8s#1162 PR as well, where @minrk mentions ( jupyterhub/zero-to-jupyterhub-k8s#1162 (comment) ) :-

traefik uses Apache basic or digest auth and there doesn't appear to be a way to put the token into the template in the right format without requiring the user to specify them in their config file after calling htpasswd on them. This is not insurmountable, but it's pretty inconvenient. One hurdle is that these use base64-encoded digests and the sprig digest functions output hex (and base64-encoded hex is not the same as base64-encoded original bytes). The Apache Basic auth spec does define a 'plain' unencrypted format that isn't supported everywhere, and traefik is indeed one of the places that it's not supported.

I'm trying to avoid forcing the user to generate more secrets to set up one thing, but I may punt and say that you need to call htpasswd to paste it into the form traefik wants. That's pretty annoying, though. The alternative is to rely on network policies to protect the proxy (maybe not the best).

I'm quoting this because I've seen Traefik v2 allows for the API passwords to be changed in the dynamic config, either by file or by KV store. I also made a change in proxy.py (ref alexleach@faa2832#diff-8dd9ce05cdee91b9579bfc2587c6be429f4f8279513c7e2488b01f17f0f2940aR295), where the api password will get set in the dynamic config, should jupyterhub manage the traefik proxy. This should allow the traefik api password to get set while running, without any downtime.

Hoping that mentioning this may garner some further interest in the traefik v2 proxy :)

@maulikjs
Copy link

maulikjs commented Oct 6, 2021

Hey Folks
Just wondering if there has been any update on providing support for traefik v2! I am happy to help in any way I can :)

@alexleach
Copy link
Contributor

Hi @maulikjs, all has gone quiet at my end I'm afraid. I opened a couple of PRs, to try and get a traefik-v2 implementation merged here (i.e. #133 and #135), and don't know what I can next do to help really.

In PR #133 I might have gone a bit overboard and tried implementing every provider backend, e.g. consul, etcd, and a file provider, but the PR was overwhelmingly large, so was difficult to review and was kind of aborted.

PR #135 is a bit more stripped down, and focuses on getting the file provider working only. However, to keep it simple, the branch in the PR doesn't yet work with TLS, so I don't think it is particularly useful in practice. I made another branch off this (https://github.com/alexleach/traefik-proxy/tree/traefik-v2-file-tls), which has a working TLS implementation (well it works for me at least 🙂), but of course a few details may change upon further review, and if/when it is merged upstream.

I'm also happy to help if / where I can to try and keep momentum on this, as the longer it's been the more I forget!

KR,
Alex

@pragyagarg642
Copy link

Hi Team,
Is there any update on this issue for supporting traefik v2?

@maulikjs
Copy link

maulikjs commented Apr 19, 2022

Hey @alexleach ,
Just saw this, I actually switched jobs and have been busy ramping up. Thanks for the PRs, Let me go through the PR's and give them a test that can help drive further decisions.

@alexleach
Copy link
Contributor

Hi @maulikjs , yes if you could help test some of this, that would be great. I've been traveling a lot recently and am going away again next week until the end of May, so I won't have much of a chance to help with this I'm afraid, but I'll keep an eye out for any comments etc. @GeorgianaElena made some review comments that I started going through before getting tied up with other things. I'm actually using one of my branches in production, I'm sure it might help a lot of others too if it can get merged successfully 🙂

@minrk
Copy link
Member

minrk commented Feb 7, 2023

closed by #145

@minrk minrk closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants