api documentation

[ch604]

Pull Request Checklist

• The Rayhunter team has recently expressed interest in reviewing a PR for this.
  • If not, this PR may be closed due our limited resources and need to prioritize how we spend them.
• Added or updated any documentation as needed to support the changes in this PR.
• Code has been linted and run through cargo fmt.
• If any new functionality has been added, unit tests were also added.
• CONTRIBUTING.md has been read.

[ch604]

the background for this was me trying to get my orbic tethered so it could send notifications without a sim card, but strolling the code found that there are several api endpoints we can use to get analysis results programatically over adb instead of making an http request. i documented all the ones i found passed to axum::Router in main.rs.

[BeigeBox]

Couldn't you make this into an OpenAPI specification? There's a ton of tools that can read that format. There's even tools to auto generate the OpenAPI Spec so that it stays up to date as the code changes.

[ch604]

word; im looking further into utopia-axum to see if it can auto-generate from the existing router. the other methods i found required more alteration to the code than simple annotation.

[untitaker]

we don't make guarantees that those endpoints stay the same across versions, API docs are all good but we need to set that expectation imo.

[cooperq]

I'm generally fine with these changes, I think its good to document the API we have. I did include a suggestion that we let people know that the API may change out from under them, but I think its generally okay for us to commit to the API remaining fairly stable.

[ch604]

i think having the docs as written is also good, at least as a point in time to foster new development. i started a branch to implement openapi via utoipa with code decorators, and got nearly finished, but im stymied by making a standalone bin for generating json. i dont think we need rayhunter to server its own docs from devices; it would be better in the online documentation. so, ill give that a while to stew and make a new pull if i can get it over the finish line.

[ch604]

openapi.json
was able to get it working to produce the attached output :) now to get it integrated with swagger into the docs on github... here is the command used to make the json file from the new bin/gen_api.rs code, which could also become part of the ci/cd deployment:

$ cargo run --bin gen_api -- $HOME/Desktop/openapi.json

Compiling rayhunter-daemon v0.10.1 (~/github/rayhunter/daemon)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.05s
Running `~/github/rayhunter/target/debug/gen_api ~/Desktop/openapi.json`

[cooperq]

Is this ready for review or still WIP?

[ch604]

just about ready to review, truing up the workflow for pages and making sure it builds correctly

[ch604]

ok, build is working as expected! i have the workflow updated to add api-docs underneath the existing pages:

https://ch604.github.io/rayhunter/api-docs/#/

there is also a node in the main docs to link there:

https://ch604.github.io/rayhunter/api-docs.html

as you can see from commits, the SUMMARY.md file does not like external URLs, so this was what i could get with mdbook.

[cooperq]

This all looks fine on its surface, though I need to understand a little more about what utopia is before I approve it. If @untitaker or @wgreenberg know what it is and feel comfortable merging they are welcome to.

[cooperq]

Okay I'm fine with this modulo my one requested change.

[cooperq]

I really like that this integrates the docs into the code so that if we change the code we will remember to change the docs.

[untitaker]

this change does not seem right. can openapi not just take the first paragraph if this becomes an aesthetic issue?

[ch604]

utoipa will take a triple-slashed comment as a description for the following uncommented element, which is far simpler for the programmer to implement than a separate decorator. A double-slashed comment will not get parsed by utoipa, and since this is a note regarding programming, this does not need to be part of the api documentation.

[untitaker]

Ah okay, i thought you omitted this message due to rendering issues. But I think it's relevant for API consumers. How about this rephrasing: A display state of "warning detected" with an informational event should never be produced, but if it is, consumers should treat it the same as "recording"

[untitaker]

is there a way to get this stuff from the axum route itself, or otherwise restore a single source of truth?

[ch604]

there is not a way to avoid using the utoipa::path decorator, since it contains a lot more necessary metadata for the code than is present already, but there is a way to deduplicate some route/path listing by replacing main.rs#L55 with OpenApiRouter() instead of Router(), and use a macro which would itself take specific defined decorators around the code and turn them into paths. for instance,

Router::new()
.route("/api/pcap/{name}", get(get_pcap))
...

becomes

OpenApiRouter::new().routes(routes!(
get_pcap,
...
))

[untitaker]

I read up on it now and afaik this will conflict with the idea of making utoipa an optional dependency. If it comes down to that, I think keeping this optional is probably more important to me, but it's up to you.

[untitaker]

Since this is pub i wonder if having apidocs as a separate crate would simplify things.

i.e. apidocs crate depends on daemon with the apidocs feature, then you can use cfg(feature = "apidocs") everywhere to disable any code that is not necessary for compiling docs (see other comments)

this way the utoipa crate can also become an optional dependency gated behind that feature and the daemon binary does not explode in size.

#[cfg_attr(feature = "apidocs", derive(utoipa::ToSchema))]

[ch604]

interesting thought, though a bit over my head for implementing at the immediate moment. i definitely dont want to inflate the binary and take up space which could be used by recordings. ill look into this option.

[untitaker]

Introducing the cargo feature for apidocs can technically be done independently of moving the binary to a different crate. If you keep the binary where it is but featuregate all the code, you'd have to invoke the tool like this: cargo run --bin gen_api --features apidocs otherwise it won't compile

[ch604]

last commit shows that the standalone html with unpkg is working just fine for swagger: https://ch604.github.io/rayhunter/api-docs/

[untitaker]

The swagger UI code now looks very good, I also looked at how unpkg works and it seems that specific version (@x.y.z) also cannot be changed out from under us at all. 👍

[ch604]

utoipa refs are now optional in the cargo.toml files, and gen_api must now be run with the "--features apidocs" flag (updated wf to do so). linted and clipped, then tested and confirmed that the base binary still builds correctly with or without the feature, and apidocs are also built and deployed properly. only shaves 5mb off of my target bin, but its not nothing :)

[untitaker]

I think the diff is good now, seems like everything is addressed. I would probably squash the commits on merge since we pivoted a lot, if you don't mind.

approved except for one change, change has been made

[ch604]

sqush'd!

[untitaker]

sorry for being unclear, GitHub has a button for squashing, you didn't have to do it. anyway I'll merge it once green

[untitaker]

ty!

[BeigeBox]

the background for this was me trying to get my orbic tethered so it could send notifications without a sim card, but strolling the code found that there are several api endpoints we can use to get analysis results programatically over adb instead of making an http request. i documented all the ones i found passed to axum::Router in main.rs.

By the way I do have the orbic able to join a wifi network to let you tether it. I’m enhancing the functionality so that it also works for mixed and conforming it doesn’t impact other devices until those are supported. Once that’s ready I’m anticipating it getting merged into a release.