From 0ef7a2222ef4e9b221f310ac60ca6a535d9c840d Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Wed, 23 Sep 2026 08:47:21 -0700 Subject: [PATCH] fix(mobile-infra): render push grant lifetimes as decimal in chart 0.3.2 (#7820) A valid 30-day push gateway grant lifetime could render in scientific notation (e.g. `2.592e+06`) in the chart, which the gateway binary does not accept. Always ormat the integer as decimal text, preserving the strict gateway parser. ### Testing Added regression coverage that feeds actual Helm-rendered environment values into the production config parser. Covers defaults, values files, command-line overrides, exact lifetime boundaries, and invalid input rejection. The default case failed before the fix and passes afterward. Signed-off-by: Tom Brow --- .github/workflows/push-gateway-helm-chart.yml | 2 + crates/buzz-push-gateway/src/config.rs | 24 ++++++++++ deploy/charts/buzz-push-gateway/Chart.yaml | 2 +- .../templates/deployment.yaml | 3 +- .../buzz-push-gateway/tests/grant-lifetime.sh | 46 +++++++++++++++++++ .../charts/buzz-push-gateway/tests/render.sh | 1 + docs/push-gateway-deployment.md | 10 +++- 7 files changed, 84 insertions(+), 4 deletions(-) create mode 100755 deploy/charts/buzz-push-gateway/tests/grant-lifetime.sh diff --git a/.github/workflows/push-gateway-helm-chart.yml b/.github/workflows/push-gateway-helm-chart.yml index 1aaa910021..897e23acd1 100644 --- a/.github/workflows/push-gateway-helm-chart.yml +++ b/.github/workflows/push-gateway-helm-chart.yml @@ -14,6 +14,7 @@ on: pull_request: paths: - "deploy/charts/buzz-push-gateway/**" + - "crates/buzz-push-gateway/src/config.rs" - ".github/workflows/push-gateway-helm-chart.yml" permissions: {} env: @@ -27,6 +28,7 @@ jobs: with: ref: ${{ github.event_name == 'workflow_dispatch' && inputs.ref || '' }} fetch-depth: 0 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - uses: azure/setup-helm@1a275c3b69536ee54be43f2070a358922e12c8d4 with: { version: v3.16.4 } - run: deploy/charts/buzz-push-gateway/tests/render.sh diff --git a/crates/buzz-push-gateway/src/config.rs b/crates/buzz-push-gateway/src/config.rs index 51c2164f69..74012c0b0b 100644 --- a/crates/buzz-push-gateway/src/config.rs +++ b/crates/buzz-push-gateway/src/config.rs @@ -449,6 +449,8 @@ mod tests { ("BUZZ_PUSH_DOGFOOD_APNS_ENVIRONMENT", "staging"), ("BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS", "0"), ("BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS", "31536001"), + ("BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS", "2.592e+06"), + ("BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS", "2592000.5"), ("BUZZ_PUSH_MAX_INSTALLATION_LIFETIME_SECONDS", "0"), ] { let mut env = base(); @@ -457,6 +459,28 @@ mod tests { } } + // The chart test supplies actual Helm-rendered environment values. Keep this + // separate from ordinary unit tests, which do not require Helm or fixtures. + #[test] + #[ignore = "run deploy/charts/buzz-push-gateway/tests/grant-lifetime.sh"] + fn helm_rendered_grant_lifetimes() { + let path = std::env::var("BUZZ_TEST_HELM_ENV_FILE").unwrap(); + let cases: Vec<(String, i64, HashMap)> = + serde_json::from_str(&std::fs::read_to_string(path).unwrap()).unwrap(); + assert_eq!(cases.len(), 9); + for (label, expected, rendered) in cases { + let mut env = base(); + env.extend(rendered); + let config = Config::from_map(&env).unwrap_or_else(|error| panic!("{label}: {error}")); + assert_eq!(config.max_grant_lifetime_seconds, expected, "{label}"); + assert_eq!( + env["BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS"], + expected.to_string(), + "{label} must render decimal integer text" + ); + } + } + #[test] fn cross_keyring_id_or_material_reuse_fails_startup() { for token_keys in [ diff --git a/deploy/charts/buzz-push-gateway/Chart.yaml b/deploy/charts/buzz-push-gateway/Chart.yaml index 7b630c9363..a72de0b66a 100644 --- a/deploy/charts/buzz-push-gateway/Chart.yaml +++ b/deploy/charts/buzz-push-gateway/Chart.yaml @@ -3,6 +3,6 @@ apiVersion: v2 # branches (see docs/push-gateway-deployment.md, "Gateway chart release"). name: buzz-push-gateway description: Public capability-gated APNs last-hop gateway for Buzz -version: 0.3.1 +version: 0.3.2 appVersion: "0.1.0" type: application diff --git a/deploy/charts/buzz-push-gateway/templates/deployment.yaml b/deploy/charts/buzz-push-gateway/templates/deployment.yaml index faefb1e917..5eb8293967 100644 --- a/deploy/charts/buzz-push-gateway/templates/deployment.yaml +++ b/deploy/charts/buzz-push-gateway/templates/deployment.yaml @@ -33,7 +33,8 @@ spec: - { name: BUZZ_PUSH_BIND_ADDR, value: "0.0.0.0:8080" } - { name: BUZZ_PUSH_HEALTH_ADDR, value: "0.0.0.0:8081" } - { name: BUZZ_PUSH_GATEWAY_ORIGIN, value: {{ required "gatewayOrigin is required" .Values.gatewayOrigin | quote }} } - - { name: BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS, value: {{ .Values.maxGrantLifetimeSeconds | quote }} } + {{/* Schema bounds this integer; normalize YAML numbers before quote can emit scientific notation. */}} + - { name: BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS, value: {{ .Values.maxGrantLifetimeSeconds | int64 | quote }} } - { name: BUZZ_PUSH_APP_ATTEST_ROOT_CERT_PATH, value: /run/buzz/app-attest/root.pem } - { name: BUZZ_PUSH_DOGFOOD_APP_ATTEST_APP_ID, value: {{ .Values.profiles.dogfood.appAttestAppId | quote }} } - { name: BUZZ_PUSH_DOGFOOD_APNS_TOPIC, value: {{ .Values.profiles.dogfood.apnsTopic | quote }} } diff --git a/deploy/charts/buzz-push-gateway/tests/grant-lifetime.sh b/deploy/charts/buzz-push-gateway/tests/grant-lifetime.sh new file mode 100755 index 0000000000..301a382f27 --- /dev/null +++ b/deploy/charts/buzz-push-gateway/tests/grant-lifetime.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +set -euo pipefail +fixture=$(mktemp) +trap 'rm -f "$fixture"' EXIT + +# Use real Helm renders, including YAML values (not just --set, which can +# preserve a different numeric type). No deployment or real secrets are needed. +env -u GEM_HOME -u GEM_PATH -u RUBYLIB -u RUBYOPT ruby -rjson -ryaml -ropen3 \ + - "$fixture" <<'RUBY' +chart = 'deploy/charts/buzz-push-gateway' +command = ['helm', 'template', 'push', chart, '--set', 'gatewayOrigin=https://push.example'] +cases = [] +render = lambda do |label, expected, args, values| + output, error, status = Open3.capture3(*command, *args, stdin_data: values) + raise "#{label}: Helm failed: #{error}" unless status.success? + deployment = YAML.load_stream(output).compact.find { |x| x['kind'] == 'Deployment' } + entries = deployment.fetch('spec').fetch('template').fetch('spec').fetch('containers')[0].fetch('env') + # Secret references are intentionally excluded; Config's fixture supplies + # synthetic keyrings and database configuration in memory. + literal_env = entries.select { |entry| entry.key?('value') }.to_h do |entry| + value = entry.fetch('value') + raise "#{label}: #{entry['name']} is not a string" unless value.is_a?(String) + [entry.fetch('name'), value] + end + raise "#{label}: lifetime missing" unless literal_env.key?('BUZZ_PUSH_MAX_GRANT_LIFETIME_SECONDS') + cases << [label, expected, literal_env] +end +render.call('default', 2592000, [], '') +[1, 2592000, 2592001, 31536000].each do |seconds| + render.call("values #{seconds}", seconds, ['-f', '-'], "maxGrantLifetimeSeconds: #{seconds}\n") + render.call("set #{seconds}", seconds, ['--set', "maxGrantLifetimeSeconds=#{seconds}"], '') +end + +# Integer conversion must never hide invalid inputs. Helm's schema remains +# authoritative, including rejecting strings that merely look numeric. +['0', '-1', '31536001', '2592000.5', '"2592000"', 'true'].each do |value| + _, error, status = Open3.capture3(*command, '-f', '-', stdin_data: "maxGrantLifetimeSeconds: #{value}\n") + unless !status.success? && error.include?('maxGrantLifetimeSeconds') + raise "expected schema rejection for #{value}: #{error}" + end +end +File.write(ARGV.fetch(0), JSON.generate(cases)) +RUBY + +BUZZ_TEST_HELM_ENV_FILE="$fixture" cargo test -p buzz-push-gateway --lib \ + config::tests::helm_rendered_grant_lifetimes -- --ignored --exact diff --git a/deploy/charts/buzz-push-gateway/tests/render.sh b/deploy/charts/buzz-push-gateway/tests/render.sh index 0f4f00bb65..ac41c5714c 100755 --- a/deploy/charts/buzz-push-gateway/tests/render.sh +++ b/deploy/charts/buzz-push-gateway/tests/render.sh @@ -1,5 +1,6 @@ #!/usr/bin/env bash set -euo pipefail +bash deploy/charts/buzz-push-gateway/tests/grant-lifetime.sh out=$(mktemp); production_out=$(mktemp); route_out=$(mktemp); datadog_out=$(mktemp) trap 'rm -f "$out" "$production_out" "$route_out" "$datadog_out" "${monitoring_out:-}"' EXIT gateway_origin_arg=(--set 'gatewayOrigin=https://push.example') diff --git a/docs/push-gateway-deployment.md b/docs/push-gateway-deployment.md index 0953ac314d..75af0e2c5f 100644 --- a/docs/push-gateway-deployment.md +++ b/docs/push-gateway-deployment.md @@ -289,8 +289,14 @@ Kubernetes does not restart pods when referenced Secret bytes change. AEAD or AP The gateway chart has a collision-free release lane separate from the main `buzz` chart. To publish chart version `X.Y.Z`, update `version` in `deploy/charts/buzz-push-gateway/Chart.yaml` and keep `appVersion` equal to the -gateway binary's workspace package version. Validate the chart, then open a -same-repository PR whose branch is exactly `push-chart-release/X.Y.Z`: +gateway binary's workspace package version. + +The render tests require Helm, Ruby, and the repository's Rust toolchain. They +pass rendered environment values into the gateway's configuration parser with +synthetic credentials; no running gateway or database is needed. + +Validate the chart, then open a same-repository PR whose branch is exactly +`push-chart-release/X.Y.Z`: ```bash deploy/charts/buzz-push-gateway/tests/render.sh