Page MenuHomePhabricator
Differential D12609

[Tunnelbroker] implement APNs config
ClosedPublic
Actions

Authored by kamil on Jun 28 2024, 4:25 AM.
Tags
None
Referenced Files
F3373538: D12609.diff
Tue, Nov 26, 10:15 AM
Unknown Object (File)
Tue, Nov 12, 12:13 PM
Unknown Object (File)
Tue, Nov 12, 11:40 AM
Unknown Object (File)
Tue, Nov 12, 11:30 AM
Unknown Object (File)
Tue, Nov 12, 7:16 AM
Unknown Object (File)
Tue, Nov 12, 2:43 AM
Unknown Object (File)
Oct 14 2024, 8:36 AM
Unknown Object (File)
Oct 14 2024, 7:12 AM
View All 63 Files
Subscribers
ashoat
tomek

Details

Reviewers
bartek
marcin
Commits
rCOMM48b881f7b7e3: [Tunnelbroker] implement APNs config
Summary

Config to APNs similar to keyserver, but instead of reading private key from the file we pass it inside JSON (command line or AWS KMS secrets).

Keyserver config: link.

Depends on D12608

Test Plan
  1. Add config var from commandline and log it
  2. For staging (using env::var) going to test after deploying

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.Jun 28 2024, 4:25 AM
kamil held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 28 2024, 4:25 AM
Harbormaster completed remote builds in B29987: Diff 41786.Jun 28 2024, 4:41 AM
kamil added a child revision: D12610: [Tunnelbroker] implement APNs Token generation and lifetime.Jun 28 2024, 4:46 AM
kamil published this revision for review.Jun 28 2024, 6:24 AM
kamil edited the summary of this revision. (Show Details)
bartek requested changes to this revision.Jul 1 2024, 1:58 AM

Can we either?:

  • Fully rely on clap parsing the env variable
  • Remove the env support in clap
services/tunnelbroker/src/config.rs
31 ↗(On Diff #41786)
  1. This code means you can pass either --apns-config CLI param or env variable named "ENV_APNS_CONFIG".
  2. Separately you have crate::constants::ENV_APNS_CONFIG (which has value "APNS_CONFIG") and you're parsing it below (manually without clap).

If you go with 2 only, you can remove this line

services/tunnelbroker/src/main.rs
36–42 ↗(On Diff #41786)

If you rely on clap parsing the env var, this shouldn't be necessary, #[arg(env = ENV_APNS_CONFIG)] should handle it for you

This revision now requires changes to proceed.Jul 1 2024, 1:59 AM
kamil updated this revision to Diff 41872.Jul 2 2024, 2:23 AM

address review

kamil added a comment.Jul 2 2024, 2:24 AM

Thanks, @bartek, update to rely on clap.

Tested both:

cargo run -- --apns-config '{"key":"your_key_here","keyId":"your_key_id_here","teamId":"your_team_id_here","production":true}'

and

APNS_CONFIG='{"key":"your_key_here","keyId":"your_key_id_here","teamId":"your_team_id_here","production":true}' cargo run
Harbormaster completed remote builds in B30042: Diff 41872.Jul 2 2024, 2:39 AM
bartek accepted this revision.Jul 2 2024, 3:36 AM

I'm glad it worked, thanks!

This revision is now accepted and ready to land.Jul 2 2024, 3:36 AM
kamil mentioned this in D12608: [Tunnelbroker] add notifs module and needed crates.Jul 2 2024, 4:06 AM
Closed by commit rCOMM48b881f7b7e3: [Tunnelbroker] implement APNs config (authored by kamil). · Explain WhyJul 2 2024, 8:35 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMM48b881f7b7e3: [Tunnelbroker] implement APNs config.

Revision Contents
Changeset List

PathSize
services/
tunnelbroker/
src/
6 lines
3 lines
notifs/
apns/
18 lines
2 lines

Diff 41872

View Options

services/tunnelbroker/src/config.rs

Loading...
View Options

services/tunnelbroker/src/main.rs

Loading...
View Options

services/tunnelbroker/src/notifs/apns/config.rs

Loading...
View Options

services/tunnelbroker/src/notifs/apns/mod.rs

Loading...