We are onboarding a client <-> identity gRPC service, and in order
to distinguish between the existing keyserver <-> identity gRPC service it
would be great for the identity service reflect the scenario
it's being used in.
Details
Details
nix develop comm-dev services start # start localstack # Configure "aws" resources on localstack (cd services/terraform && ./run.sh) # start identity service and run mobile workflow (cd services/identity && cargo run -- server &) (cd keyserver && yarn dev &) (cd native && yarn dev &)
Then you should be able to create a new user, update the password,
and delete a user with the actions being reflected in dynamodb.
Output of the actions should also be appearing in identity logs.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- jonringer/rename-grpc-keyserver
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
identity.proto should probably be renamed as identity_keyserver.proto. As I would like to add the client <-> identity protobuf file as identity_client.proto, which is confusing as we refer to the keyserver protos as "identity_client" in some parts of the code base. In other words, align the naming with the "comm" defintion of terms and move away with the more general gRPC definition of terms.
Comment Actions
This honestly feels like bike-shedding… I’d love if we could start out the work on this project with a focus on derisking the big risks, rather than renaming things
Comment Actions
This honestly feels like bike-shedding… I’d love if we could start out the work on this project with a focus on derisking the big risks, rather than renaming things
Not changing it we would have IdentityService and IdentityClientService. In the renamed world it's IdentityKeyserverService and IdentityClientService.
rather than renaming things
I would like to lessen the mental burden of moving around the identity code. The following diff D7003 will add another service, and would like to distinguish between the two in a more meaningful way.
Also, this diff didn't take much time, typing everything out for arc diff probably took more time than renaming, building, and doing a quick test.
Comment Actions
the set of RPCs that devices will call is different than the set that ashoat's keyserver can call today. also, the actual messages are slightly different, as the linear issue i linked explains. we think it makes more sense to just have a separate service for devices, rather than introduce a bunch of conditional logic in the current service. of course, both services will be served by the Identity Service (this is pretty simple). eventually, when we switch to the identity service being the source of truth for user info, we can deprecate the original service (the one being renamed here)
Comment Actions
we can deprecate the original service (the one being renamed here)
We can deprecate parts of it, keyserver-specific things like VerifyUser will still remain. But pretty much everything dealing with account creation, updating, or deletion can eventually be removed. Other items such as https://linear.app/comm/issue/ENG-3260 will require keyserver-specific additions to the service as well.
I'm more confused as to why are we're separating the protos / services out at all
One thing not mentioned by @varun is that the current AUTH_TOKEN hack we use for keyserver is implemented on a per-service basis. So we can roll out a new service without "authorization headers", while maintaining the current keyserver paradigm.
Comment Actions
One thing not mentioned by @varun is that the current AUTH_TOKEN hack we use for keyserver is implemented on a per-service basis. So we can roll out a new service without "authorization headers", while maintaining the current keyserver paradigm.
Thanks for explaining, that makes sense – the two services need to be different between the auth for them is different