-
Notifications
You must be signed in to change notification settings - Fork 16
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
Discovery: SQLite-based server implementation #2589
Conversation
c1c4acf
to
8108827
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is about the usecase maintainer impl but introduces SQL on the fly. The migration towards sql storage is a big one and needs a proper setup. We risk in introducing it with add-ons/patches and forget to properly design it.
Issues that come to mind:
- put the migrations in sub dirs per supported DB
- are the migration prefixes with 1, 2, 3 going to break when using 10 ("10"<"2_" stringwise)
- where are we going to put the models for the tabels, all here or in their own packages?
- migration file naming
- connectionstring documentation
- which DBs to support?
- additional config for those DBs and can that be added to later on?
SQL introduction should be done in its own PR
Extracted SQL functionality into a separate PR: #2595 |
bcae7aa
to
64d1cf4
Compare
b84d289
to
5932201
Compare
eb54df9
to
9dacfc4
Compare
Refactored with server and client having 1 store. |
note to self: check that presentation.ID is scoped to the signer DID |
First feedback processed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A PR like this takes me a full day to review to cover about 80% of the content. Adding more comment along the way or summarizing behavior of longer functions/methods (also private ones) would help me a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments are suggestions. Requesting changes because retracted_jti
is not a DID (see previous comments)
if err := s.prune(); err != nil { | ||
return err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it depends how care orgs will be allowed to access discovery services. As long as care orgs can't use use-cases that aren't configured in a service definition I suppose there is no issue.
rationale: If a node only acts as client (applies to most) and all service definitions are gone after a restart (config error), the db will only contain outdated info. The client will also no longer publish its clients to the discovery services. This could result in weird situations where orgs can find orgs managed by other vendors but not their own, etc..
additional tests additional test PR feedback PR feedback test fix remove failing test (#2618) added e2e test for OpenID4VP s2s flow (#2617) PEX: Provide ParseEnvelope to correctly parse PEX VP envelopes (#2620) Bump schneider.vip/problem from 1.8.1 to 1.9.0 (#2622) Bumps [schneider.vip/problem](https://github.com/mschneider82/problem) from 1.8.1 to 1.9.0. - [Commits](mschneider82/problem@v1.8.1...v1.9.0) --- updated-dependencies: - dependency-name: schneider.vip/problem dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump golang.org/x/crypto from 0.15.0 to 0.16.0 (#2628) Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.15.0 to 0.16.0. - [Commits](golang/crypto@v0.15.0...v0.16.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump golang.org/x/time from 0.4.0 to 0.5.0 (#2627) Bumps [golang.org/x/time](https://github.com/golang/time) from 0.4.0 to 0.5.0. - [Commits](golang/time@v0.4.0...v0.5.0) --- updated-dependencies: - dependency-name: golang.org/x/time dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Missing rfc021 e2e tests changes (#2623) Discovery: SQLite-based server implementation (#2589) Bump github.com/nuts-foundation/go-leia/v4 from 4.0.0 to 4.0.1 (#2632) Bumps [github.com/nuts-foundation/go-leia/v4](https://github.com/nuts-foundation/go-leia) from 4.0.0 to 4.0.1. - [Release notes](https://github.com/nuts-foundation/go-leia/releases) - [Commits](nuts-foundation/go-leia@v4.0.0...v4.0.1) --- updated-dependencies: - dependency-name: github.com/nuts-foundation/go-leia/v4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Root URL server config property to replace auth.publicurl (#2633) change idToDID to use did:web in iam API (#2635) Bump github.com/lestrrat-go/jwx/v2 from 2.0.17 to 2.0.18 (#2645) Bumps [github.com/lestrrat-go/jwx/v2](https://github.com/lestrrat-go/jwx) from 2.0.17 to 2.0.18. - [Release notes](https://github.com/lestrrat-go/jwx/releases) - [Changelog](https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes) - [Commits](lestrrat-go/jwx@v2.0.17...v2.0.18) --- updated-dependencies: - dependency-name: github.com/lestrrat-go/jwx/v2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump github.com/nats-io/nats-server/v2 from 2.10.5 to 2.10.6 (#2644) Bumps [github.com/nats-io/nats-server/v2](https://github.com/nats-io/nats-server) from 2.10.5 to 2.10.6. - [Release notes](https://github.com/nats-io/nats-server/releases) - [Changelog](https://github.com/nats-io/nats-server/blob/main/.goreleaser.yml) - [Commits](nats-io/nats-server@v2.10.5...v2.10.6) --- updated-dependencies: - dependency-name: github.com/nats-io/nats-server/v2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump alpine from 3.18.4 to 3.18.5 (#2636) Bumps alpine from 3.18.4 to 3.18.5. --- updated-dependencies: - dependency-name: alpine dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> more NO_CONTENT timeout godoc
Base maintainer implementation, not including:
Requires