owncloud / ocis-charts

:chart_with_upwards_trend: Helm Charts for ownCloud's OCIS
https://owncloud.dev/ocis/deployment/kubernetes/
Apache License 2.0
48 stars 27 forks source link

bring back WEB_OPTION_TOKEN_STORAGE_LOCAL config option #806

Closed wkloucek closed 5 days ago

wkloucek commented 6 days ago

Description

brings back the WEB_OPTION_TOKEN_STORAGE_LOCAL / .Values.services.web.config.tokenStorageLocal.enabled config that was removed for some reason.

I only know that it existet in https://github.com/owncloud/ocis-charts/blob/792b72dbb46986041ee0f16ca08001a55e073b8a/charts/ocis/templates/web/deployment.yaml#L105-L106 but couldn't find out why and when it was removed.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

wkloucek commented 6 days ago

@d7oc are you more successful than me to find the commit that removes this option?

d7oc commented 6 days ago

So option was added in https://github.com/owncloud/ocis-charts/commit/d73fe65204ebf14e888bdfcb996ee9660d4a5a5b and got removed through https://github.com/owncloud/ocis-charts/pull/507/files#diff-dee545bb8ade2b54fbd21fe63d559f68f42030a20a09de7c70e6bb6d0d55923d

I'm just not sure if we really should revert the whole merge 😉.

btw. also unsuccessfully tried git bisect. I guess it's ignoring merge commits. So I walked the tree graphically in sourcetree.

d7oc commented 6 days ago

Assumption about git bisect was correct. And there is an option for that!

> git bisect start --first-parent HEAD d73fe65204ebf14e888bdfcb996ee9660d4a5a5b --
Bisecting: 125 revisions left to test after this (roughly 7 steps)
[bdc6059995b8add084e5167480d6f95a59c053a2] Merge pull request #628 from owncloud/renovate/rook-ceph-1.x
> git bisect run grep WEB_OPTION_TOKEN_STORAGE_LOCAL charts/ocis/templates/web/deployment.yaml
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
Bisecting: 62 revisions left to test after this (roughly 6 steps)
[8b27c20298dbfb1cfaf4544ac0e6853e9f73bbed] Merge pull request #557 from owncloud/renovate/rabbitmq-14.x
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
Bisecting: 31 revisions left to test after this (roughly 5 steps)
[e377758897683de4a48fb02e07fb54ccbd71e025] - added STORAGE_USERS_GATEWAY_GRPC_ADDR for CLI tool (#534)
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
warning: unable to rmdir 'deployments/external-user-management/charts/keycloak-k8s-resources': Directory not empty
Bisecting: 15 revisions left to test after this (roughly 4 steps)
[7b57ba8695c2e3a41b969dbb7785faf9900d5405] chore(deps): update owncloud/ocis docker tag to v4.0.6 (#480)
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
            - name: WEB_OPTION_TOKEN_STORAGE_LOCAL
Bisecting: 7 revisions left to test after this (roughly 3 steps)
[a01226991d9b0c22b2afbb5dca97bd69814a25bf] Merge pull request #519 from owncloud/bultin-idp-default-passwort-30-chars
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[62592b3ad78349670abae0a32a7249e70d28c05d] Merge pull request #507 from owncloud/ocis-5.0.0
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
warning: unable to rmdir 'deployments/external-user-management/charts/keycloak-k8s-resources': Directory not empty
Bisecting: 1 revision left to test after this (roughly 1 step)
[89c7928184b650254fc1aea52526f226edb9e3a9] chore(deps): update rancher/k3s docker tag to v1.29.2 (#493)
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
            - name: WEB_OPTION_TOKEN_STORAGE_LOCAL
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[776042fe8947437f0259a9bc044739885c92eca6] switch to main branch as default branch (#501)
running 'grep' 'WEB_OPTION_TOKEN_STORAGE_LOCAL' 'charts/ocis/templates/web/deployment.yaml'
            - name: WEB_OPTION_TOKEN_STORAGE_LOCAL
62592b3ad78349670abae0a32a7249e70d28c05d is the first bad commit
commit 62592b3ad78349670abae0a32a7249e70d28c05d
Merge: 776042f ab55b5a
Author: Willy Kloucek <34452982+wkloucek@users.noreply.github.com>
Date:   Tue Mar 19 07:01:39 2024 +0100

    Merge pull request #507 from owncloud/ocis-5.0.0

    ocis 5.0.0
wkloucek commented 5 days ago

So option was added in d73fe65 and got removed through https://github.com/owncloud/ocis-charts/pull/507/files#diff-dee545bb8ade2b54fbd21fe63d559f68f42030a20a09de7c70e6bb6d0d55923d

Thanks for finding it! I have no clue why it was removed back then.

I'm just not sure if we really should revert the whole merge 😉.

Sure not :laughing:. I'm making this PR ready to bring it back.