mesosphere / traefik-forward-auth

214 stars 46 forks source link

chore: fix master and v3 branches #54

Closed dkoshkin closed 2 years ago

dkoshkin commented 2 years ago

Fixes https://jira.d2iq.com/browse/COPS-7145

https://github.com/mesosphere/traefik-forward-auth/releases/tag/v3.0.0 was released with https://github.com/mesosphere/traefik-forward-auth/pull/41 feature. But the v3 branch was cut before that work went in. Then there was a big refactor that was done in https://github.com/mesosphere/traefik-forward-auth/commit/7e8ab37 and merged to v3 branch but the PR to master was never merged in. Then there were some more work that went into v3 and most of it did not make to master.

This PR is to get master back to expected state that includes both Jared's clusterstorage feature and the changes from the v3 branch. After this is merged we can create a new release/v3 branch from this tip that can then be used for v3 development.


Jason and I cherry-picked each commit from v3. The first 4 had the most merge conflicts. When reviewing pay special attention to 39539aa1f682010131e5d12413cae2072a6084d8 as that one made some additional changes from the cherry-pick commit. The original commit added a sessionCookie which is no longer needed with userinfo introduced in the clusterstorage PR.

image

dkoshkin commented 2 years ago

Shoutout to @jkoelker for the work here 👏 !

jkoelker commented 2 years ago

LGTM, I'll refrain from approving, since I helped with the code side, but I went through the remaining commits and it looks good to me.

dkoshkin commented 2 years ago

Using https://github.com/mesosphere/kubernetes-base-addons/tree/dkoshkin-tfa-clusterstorage

Tested manually by setting custom values on the addon:

        - name: traefik-forward-auth
          enabled: true
          values: |
            clusterStorage:
              enabled: true
              namespace: kubeaddons

Verified the login still worked and saw this:

➜  dkkonvoy-tfa-cops k view-secret -n kubeaddons tfa-claims-nlmrm
Choosing key: userInfo
{"username":"hungry_banach","email":"hungry_banach","groups":[]}

image

dkoshkin commented 2 years ago

Also ran Konvoy e2e tests (specifically should login with ...)