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

make store service configurable #8419

Merged
merged 5 commits into from Feb 26, 2024
Merged

make store service configurable #8419

merged 5 commits into from Feb 26, 2024

Conversation

butonic
Copy link
Member

@butonic butonic commented Feb 9, 2024

The store service was born out of a misunderstanding ... we need to clean that up. This PR changes the default signing key store to nats, but allows configuring and using the old store service to running existing installations without changes to the deployment (if updating the configuratin).

The change does not need a migration as signing keys are just regenerated on the fly.

I don't know if the web ui will re download the signing key if it expired. I don't think it has a way of detecting that as the browser will handle the download. Maybe they should just forget the signing key ... every 5min? so it is periodically refetched?

I can see the ocis web ui being left open for a long time, so I set the new signing key TTL default to 12h. In the past it never expired, which IMO is a pad practice.

cc @mmattel @wkloucek @kobergj @kulmann

Copy link

update-docs bot commented Feb 9, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@mmattel
Copy link
Contributor

mmattel commented Feb 9, 2024

Removing the store service is great, as it reduces the list of services not having a description about their purpose 🤣 @kobergj

@mmattel
Copy link
Contributor

mmattel commented Feb 9, 2024

Ok, means:

  • We need to add entries in the changed envvars tables (added, removed).
  • Reverse the entries made by New Service Checklist like in the makefile in ocis repo root etc.
  • Add the caching info to the ocs readme in the same way we have like in the proxy readme.
  • Clarification needed (same for PROXY_):
    OCS_PRESIGNEDURL_SIGNING_KEYS_STORE_DATABASE and
    OCS_PRESIGNEDURL_SIGNING_KEYS_STORE_TABLE
    Reason, default values --> improve description text

@butonic butonic force-pushed the kill-store-service branch 2 times, most recently from 3cf361e to 52bad71 Compare February 9, 2024 11:37
@butonic butonic marked this pull request as draft February 9, 2024 11:55
@butonic butonic force-pushed the kill-store-service branch 3 times, most recently from 35a839c to 59bc13e Compare February 9, 2024 13:51
@butonic butonic marked this pull request as ready for review February 15, 2024 16:05
@butonic butonic enabled auto-merge (squash) February 16, 2024 11:38
@butonic butonic changed the title drop store service in favor of a micro store implementation make store service configurable Feb 23, 2024
Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • How is migration handled in this scenario?

changelog/unreleased/change-presigned-key-store.md Outdated Show resolved Hide resolved
changelog/unreleased/change-presigned-key-store.md Outdated Show resolved Hide resolved
@butonic
Copy link
Member Author

butonic commented Feb 23, 2024

Migration is not needed as the keys are ephemeral and can just be regenerated.

@butonic
Copy link
Member Author

butonic commented Feb 23, 2024

I forgot to not start the store service by default. will change the default services.

@ScharfViktor
Copy link
Contributor

fail while building ocis binary make -C ocis clean generate build

go build -v -tags 'disable_crypt' -ldflags '-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=warn -s -w -X "github.com/owncloud/ocis/v2/ocis-pkg/version.String=71dd19bf4b" -X "github.com/owncloud/ocis/v2/ocis-pkg/version.Tag=" -X "github.com/owncloud/ocis/v2/ocis-pkg/version.Date=20240223"' -o bin/ocis ./cmd/ocis
../services/ocs/pkg/server/http/server.go:14:2: cannot find module providing package github.com/owncloud/ocis/v2/services/store/pkg/store: import lookup disabled by -mod=vendor
        (Go version in go.mod is at least 1.14 and vendor directory exists.)
make: *** [bin/ocis] Error 1

services/store/pkg/store/store.go Show resolved Hide resolved
services/store/pkg/store/store.go Show resolved Hide resolved
services/store/pkg/store/store.go Show resolved Hide resolved
services/store/pkg/store/store.go Show resolved Hide resolved
services/store/pkg/store/store.go Show resolved Hide resolved
services/store/pkg/store/store.go Show resolved Hide resolved
services/store/pkg/store/store.go Show resolved Hide resolved
services/store/pkg/store/store.go Show resolved Hide resolved
Copy link
Collaborator

@kobergj kobergj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

butonic and others added 3 commits February 26, 2024 11:59
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Co-authored-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Copy link

sonarcloud bot commented Feb 26, 2024

@butonic butonic merged commit 26136f8 into master Feb 26, 2024
4 checks passed
@delete-merged-branch delete-merged-branch bot deleted the kill-store-service branch February 26, 2024 15:08
ownclouders pushed a commit that referenced this pull request Feb 26, 2024
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@micbar micbar mentioned this pull request Mar 13, 2024
71 tasks
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

Successfully merging this pull request may close these issues.

None yet

5 participants