[services/commtest] Tunnelbroker - Adding `deviceID` validation check function
ClosedPublic
Actions

Authored by • max on Dec 19 2022, 6:47 AM.

Details

Reviewers

• jon
bartek

Commits

rCOMMd64f7d4d179c: [services/commtest] Tunnelbroker - Adding `deviceID` validation check function

Summary

This diff introduces a function that makes a call to the Tunnelbroker (get signature API) with the deviceID in the wrong format.
In this case, we are making sure that the Tunnelbroker returns an error on the wrong format deviceID provided.
This function is called during the test in D5931.

Linear task: ENG-1657

Test Plan

The commtest app is successfully built and the tests are successfully passed in the following D5931 where the function is used.

Diff Detail

Repository

rCOMM Comm

Branch

commtest-session-sign-device-id-valid

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Dec 19 2022, 6:47 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptDec 19 2022, 6:47 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald Transcript

• max added a parent revision: D5928: [services/commtest] Tunnelbroker - Adding the tonic client builder function.Dec 19 2022, 6:47 AM

• max added a child revision: D5931: [services/commtest] Tunnelbroker - Adding the API integration tests call function.Dec 19 2022, 6:52 AM

Harbormaster completed remote builds in B14312: Diff 19554.Dec 19 2022, 6:55 AM

• max mentioned this in D5926: [services/commtest] Adding of openssl, base64, anyhow to dependencies.Dec 19 2022, 7:00 AM

• max published this revision for review.Dec 19 2022, 7:17 AM

• max edited the summary of this revision. (Show Details)

• max edited the test plan for this revision. (Show Details)

• max added a reviewer: bartek. • max added 1 blocking reviewer(s): • jon.

bartek added inline comments.Dec 19 2022, 12:20 PM

services/commtest/src/tunnelbroker/tunnelbroker_utils.rs
23 ↗	(On Diff #19554)	We tend to use `anyhow::Result` to skip specifying the error type when it's the `anyhow::Error`

Would really like to see these tests get added to the CI. They are pretty expensive to run locally.

This revision is now accepted and ready to land.Dec 20 2022, 9:55 AM

• max mentioned this in rCOMM8ab6a820b421: [services/commtest] Adding of openssl, base64, anyhow to dependencies.Dec 27 2022, 4:52 AM

Change the Result to be an anyhow.

• max marked an inline comment as done.Dec 27 2022, 5:06 AM

• max added inline comments.

services/commtest/src/tunnelbroker/tunnelbroker_utils.rs
23 ↗	(On Diff #19554)	We tend to use `anyhow::Result` to skip specifying the error type when it's the `anyhow::Error` We are not using `anyhow::Result` globally in this file, so making it `use` globally doesn't make sense, but it makes sense to change the `Result` to `anyhow::Result` in this function. It looks better. Thanks, @bartek !

Harbormaster completed remote builds in B14784: Diff 20163.Dec 27 2022, 5:17 AM

In D5929#178691, @jon wrote:

Would really like to see these tests get added to the CI. They are pretty expensive to run locally.

Yes, we should start the RabbitMQ and Localstack on the CI machine to do this, I think we are ok with this now, thanks to nix. It's tracked by the ENG-2548