docs: Update docs for enterprise#1138
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Autopush’s enterprise-facing documentation and configuration examples, while also aligning the codebase toward the newer *_CRYPTO_KEYS setting (with backwards compatibility for *_CRYPTO_KEY in autoconnect).
Changes:
- Update docs to remove/limit Docker guidance, add metrics documentation, and refresh running/releasing instructions.
- Rename/augment crypto key configuration from
*_CRYPTO_KEYto*_CRYPTO_KEYSacross scripts/docs/config samples. - Update
autoconnectsettings to supportcrypto_keys(with an obsoletecrypto_keyfallback) and add reminder comments in settings modules.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/autokey.py | Updates generated env var names for crypto keys. |
| docs/src/running.md | Rewrites “Running” guide for non-Docker local setups and new key naming. |
| docs/src/releasing.md | Updates release process to reflect Cargo-based workflow and CD promotion. |
| docs/src/metrics.md | Adds a new metrics overview page (statsd + Prometheus reliability metrics). |
| docs/src/install.md | Minor install doc tweaks and removes legacy “dual” storage section. |
| docs/src/config_options.md | Updates config options docs (adds anchors, crypto_keys, etc.). |
| configs/autoendpoint.toml.sample | Expands sample config with required/optional sections and reliability notes. |
| configs/autoconnect.toml.sample | Expands sample config with required/optional sections and reliability notes. |
| autoendpoint/src/settings.rs | Adds a reminder comment to keep docs/sample config in sync with settings. |
| autoconnect/autoconnect-settings/src/lib.rs | Adds crypto_keys field + default; updates env-var test. |
| autoconnect/autoconnect-settings/src/app_state.rs | Implements crypto_keys/crypto_key selection and parsing. |
Comments suppressed due to low confidence (7)
scripts/autokey.py:12
autoconnectparsesCRYPTO_KEYSas a bracketed, comma-separated list and does not strip inner quotes (seeautoconnect-settings/src/app_state.rs). This script currently prints JSON-style values like["<key>"], which will leave quotes in the key and causeFernet::new()to fail at runtime. Consider printing[<key>](no inner quotes) or updating the Rust parsing logic to strip quotes/spaces likeautoendpointdoes.
def main():
key = Fernet.generate_key().decode("utf-8")
print("\n# Crypto Keys:")
print(f"AUTOCONNECT__CRYPTO_KEYS=[\"{key}\"]")
print(f"AUTOEND__CRYPTO_KEYS=[\"{key}\"]")
print("\n\n# Auth Key:")
key = Fernet.generate_key().decode("utf-8")
print(f"AUTOEND__AUTH_KEYS=[\"{key}\"]")
autoconnect/autoconnect-settings/src/app_state.rs:70
- The error/validation messages use
{ENV_PREFIX}_CRYPTO_KEY(single underscore) but the actual env vars use the__separator (e.g.AUTOCONNECT__CRYPTO_KEYS). Also, the message always saysCRYPTO_KEYeven whenCRYPTO_KEYSis the provided/invalid setting. Update these messages (and the panic text) to reference the correct variable name(s).
if settings.crypto_key.is_none() && settings.crypto_keys.is_none() {
return Err(ConfigError::Message(format!(
"Missing required configuration: {ENV_PREFIX}__CRYPTO_KEY or {ENV_PREFIX}__CRYPTO_KEYS"
)));
};
let crypto_keys = &settings
.crypto_keys
.clone()
.unwrap_or(settings.crypto_key.clone().unwrap());
if !(crypto_keys.starts_with('[') && crypto_keys.ends_with(']')) {
return Err(ConfigError::Message(format!(
"Invalid {ENV_PREFIX}_CRYPTO_KEY"
)));
}
let fernet_keys = &crypto_keys[1..crypto_keys.len() - 1];
debug!("🔐 Fernet keys: {:?}", &fernet_keys);
let fernets: Vec<Fernet> = fernet_keys
.split(',')
.map(|s| s.trim().to_string())
.map(|key| {
Fernet::new(&key).unwrap_or_else(|| panic!("Invalid {ENV_PREFIX}_CRYPTO_KEY"))
})
.collect();
autoconnect/autoconnect-settings/src/app_state.rs:69
autoconnect's crypto key parsing doesn't strip quotes/spaces before splitting, unlikeautoendpoint(which doesreplace(['"',' '], "")). As a result, values like["key"](produced byscripts/autokey.py) will include quotes and fail Fernet initialization. Consider normalizing the string (strip quotes/spaces) before validation/splitting to make env/config formats consistent across binaries.
let crypto_keys = &settings
.crypto_keys
.clone()
.unwrap_or(settings.crypto_key.clone().unwrap());
if !(crypto_keys.starts_with('[') && crypto_keys.ends_with(']')) {
return Err(ConfigError::Message(format!(
"Invalid {ENV_PREFIX}_CRYPTO_KEY"
)));
}
let fernet_keys = &crypto_keys[1..crypto_keys.len() - 1];
debug!("🔐 Fernet keys: {:?}", &fernet_keys);
let fernets: Vec<Fernet> = fernet_keys
.split(',')
.map(|s| s.trim().to_string())
.map(|key| {
Fernet::new(&key).unwrap_or_else(|| panic!("Invalid {ENV_PREFIX}_CRYPTO_KEY"))
})
docs/src/running.md:31
- This section still says you need a
CRYPTO_KEYand references generating it “with the docker image”, but the configuration/env var name has moved to*_CRYPTO_KEYSand the doc earlier says Docker isn’t intended for production. Update this paragraph to match the current key name(s) and generation method(s) being documented.
### Generate a Crypto-Key
As the `cryptography` section notes, you will need a `CRYPTO_KEY` to run
both of the Autopush daemons. To generate one with the docker image:
``` bash
docs/src/running.md:57
- Typo: “you may with to use” should be “you may wish to use”.
[Bigtable](https://docs.cloud.google.com/bigtable/docs/emulator) and [Redis](https://redis.io/docs/latest/operate/oss_and_stack/install/install-stack/docker/). This document does not detail the steps required, because they may change.
Once you have the Bigtable emulator installed, you may with to use the following bash script to start it. (Note, this presumes that you are using the GCLI interface. Please edit as needed.)
docs/src/releasing.md:81
- The release branch naming is inconsistent: steps 55-63 create/check out
release/{major}.{minor}.{patch}, but steps 11-12 still pushrelease/{major}.{minor}. Update the push commands (and any other references) to use the same branch naming convention.
If this is a new major/minor release,
`git checkout -b release/{major}.{minor}.{patch}` to create a new release
branch.
If this is a new patch release, you will first need to ensure you
have the minor release branch checked out, then:
> 1. `git checkout release/{major}.{minor}.{patch}`
> 2. `git pull` to ensure the branch is up-to-date.
> 3. `git merge master` to merge the new changes into the release
> branch.
5. Run `cargo test` locally to ensure no artifacts or other local changes that
might break tests have been introduced. (_NOTE_: you may needs to run various feature
versions based on production vs. enterprise. For production, you should run `cargo test --features=production`. For enterprise, you should run `cargo test --no-default-features --features=enterprise`.)
6. Edit `Cargo.toml` so that the version number reflects the
desired release version.
7. Run `clog --setversion {version}`, verify changes were properly
accounted for in `CHANGELOG.md`.
9. `git commit -am "chore: tag {version}"` to commit the new version and
record of changes.
10. `git tag -sm "chore: tag {version}" {version}` to create a signed
tag of the current HEAD commit for release.
11. `git push --set-upstream origin release/{major}.{minor}` to push the
commits to a new origin release branch.
12. `git push --tags origin release/{major}.{minor}` to push the tags to
the release branch.
docs/src/install.md:151
- Typo in product name: “Autopendpoint” should be “Autoendpoint” (or “AutoEndpoint”, matching the binary name elsewhere).
The `db_dsn` to access this data store with Autopendpoint would be:
`grpc://localhost:8086`
The `db_setings` contains a JSON dictionary indicating the names of the message and router families, as well as the path to the table name.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… docs/PUSH-653_min_config
| pub endpoint_port: u16, | ||
| /// The seed key to use for endpoint encryption | ||
| pub crypto_key: String, | ||
| pub crypto_key: Option<String>, // obsolete, use `crypto_keys` instead |
There was a problem hiding this comment.
It is, but we want to persist the key to allow for older installs to not break suddenly.
| .crypto_keys | ||
| .clone() | ||
| .unwrap_or_else(|| settings.crypto_key.clone().unwrap()); | ||
| // TODO: switch to make_fernet(); move make_fernet() to autopush_common. |
There was a problem hiding this comment.
Ideally, yes, but this PR is already complex.
https://mozilla-hub.atlassian.net/browse/PUSH-174 should cover the work.
| .clone() | ||
| .unwrap_or_else(|| settings.crypto_key.clone().unwrap()); | ||
| // TODO: switch to make_fernet(); move make_fernet() to autopush_common. | ||
| if !(crypto_keys.starts_with('[') && crypto_keys.ends_with(']')) { |
There was a problem hiding this comment.
Can we add a comment here, why starting with [ is invalid, what is this magic [?
There was a problem hiding this comment.
This is to be consistent with autoendpoint's usage. The [] imply a mock JSON "list" definition indicating multiple keys
There was a problem hiding this comment.
can we give s a batter name? When reading the code, it's confusing what s means.
| pub fn test_settings() -> Self { | ||
| // Provide test settings based on enabled features. | ||
| // semi-hack to satisfy clippy --all --all-features | ||
| let crypto_keys = Some(format!("[\"{}\"]", Fernet::generate_key())); |
There was a problem hiding this comment.
What is the semi-hack here? Why can't we use the usual format? And, could we not implement the Dispaly trait on Fernet so we can stringify them without format here?
There was a problem hiding this comment.
We're testing the way that the string should be provided. (e.g. as a JSON like set of potentially multiple values.)
Fernet is foreign, and while we could add a Display trait, we shouldn't.
… docs/PUSH-653_min_config
…s/autopush-rs into docs/PUSH-653_min_config
Closes: PUSH-653, PUSH-624, PUSH-664, PUSH-604