Page MenuHomePhabricator
Differential D3812

[Identity] Add config for Identity service
ClosedPublic
Actions

Authored by varun on Apr 21 2022, 12:07 PM.
Tags
None
Referenced Files
F3113727: D3812.id11758.diff
Thu, Oct 31, 6:31 PM
F3113720: D3812.id11804.diff
Thu, Oct 31, 6:27 PM
F3113718: D3812.id12143.diff
Thu, Oct 31, 6:27 PM
Unknown Object (File)
Fri, Oct 25, 12:05 AM
Unknown Object (File)
Fri, Oct 25, 12:05 AM
Unknown Object (File)
Fri, Oct 25, 12:05 AM
Unknown Object (File)
Fri, Oct 25, 12:04 AM
Unknown Object (File)
Fri, Oct 25, 12:04 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
• benschac

Details

Reviewers
• jim
• karol
tomek
atul
Commits
rCOMMe70770673a3b: [Identity] Add config for Identity service
Summary

Depends on D3799

In order to implement PAKE registration/login in the Identity service, we need to persist a secret key and then share that key with the gRPC methods that use it. In this diff I have created a config module to load the key from a file into memory and then shared the key with the gRPC methods by adding a Config struct to MyIdentityService.

Test Plan

Ran the service and tested that the relevant Config was available in MyIdentityService

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun created this revision.Apr 21 2022, 12:07 PM
Herald added subscribers: • abosh, • benschac, • adrian, ashoat. · View Herald TranscriptApr 21 2022, 12:07 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Apr 21 2022, 12:10 PM
Harbormaster failed remote builds in B8426: Diff 11758!
varun requested review of this revision.Apr 21 2022, 12:11 PM
varun added 1 blocking reviewer(s): • jim.Apr 21 2022, 1:43 PM
varun updated this revision to Diff 11804.Apr 22 2022, 7:29 AM

trigger new build after rebase

Harbormaster completed remote builds in B8465: Diff 11804.Apr 22 2022, 7:36 AM
varun added a child revision: D3852: [Identity] Add tracing to the Identity service.Apr 25 2022, 8:47 PM
• jim requested changes to this revision.Apr 26 2022, 9:57 AM
• jim added inline comments.
services/identity/src/config.rs
22 ↗(On Diff #11804)

This isn't a great way to do error handling -- you're throwing away the type information...

I would make a wrapper enum with derive_more.

#[derive(Debug, derive_more::Display, derive_more::From, derive_more::Error)]
pub enum Error {
  #[display(...)]
  Pake(PakeError),
  #[display(...)]
  IO(io::Error),
}
services/identity/src/main.rs
11 ↗(On Diff #11804)

Load the config first then, MyIdentityService::new(config)

15 ↗(On Diff #11804)

Why expect? You can just return an err.

This revision now requires changes to proceed.Apr 26 2022, 9:57 AM
varun updated this revision to Diff 12142.May 2 2022, 11:12 AM

Address feedback

Harbormaster completed remote builds in B8718: Diff 12142.May 2 2022, 11:18 AM
varun updated this revision to Diff 12143.May 2 2022, 11:21 AM

Replace Config with Self

Harbormaster completed remote builds in B8719: Diff 12143.May 2 2022, 11:28 AM
• jim accepted this revision.May 4 2022, 10:18 AM
This revision is now accepted and ready to land.May 4 2022, 10:18 AM
Closed by commit rCOMMe70770673a3b: [Identity] Add config for Identity service (authored by varun). · Explain WhyMay 4 2022, 1:13 PM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMMe70770673a3b: [Identity] Add config for Identity service.

Revision Contents
Changeset List

PathSize
services/
identity/
3 lines
src/
32 lines
6 lines
8 lines

Diff 12142

View Options

services/identity/Cargo.lock

Loading...
View Options

services/identity/Cargo.toml

Loading...
View Options

services/identity/src/config.rs

Loading...
View Options

services/identity/src/main.rs

Loading...
View Options

services/identity/src/service.rs

Loading...