feat/huawei
needs attentionac51eee · fullPR #274reviewed 2026-07-08 01:58 UTC4H · 12M · 9L · 5I- Purpose
- Port the Huawei FusionSolar integration (90 devices) into the v2 metrics engine using the session-auth shape established by Growatt
- Goal
- Full session-auth stack: auth Lambda + auth-cache DDB (Growatt pattern) + metrics/connection/exploration workers, CDK stacks, and live Site-SFN validation
- Sub-goals
- SG-1: API probe (5m availability, anchoring, secret keys) + auth lambda + auth-cache + workers + unit tests
- SG-2: CDK stacks + main.ts + coordinator grants; synth clean
- SG-3: Preview data plane — D2 legacy-credential copy + D3 partial-fleet validation (3 sites)
- SG-4: Live Site-SFN validation (3854 pts, 5m native, 0 skips) + Tinybird + legacy parity ≤1.8%
- SG-N: 9 framework learnings folded back to scope.md and integrations CLAUDE.md
- What
- New huawei integration package (4 Lambda handlers, auth-cache, FusionSolar client, translation layer, 58+ unit tests), 2 CDK stacks (auth-cache + lambda), IAM scoped to {env}/huawei/*, coordinator grants, manifest for huawei-portal, 12 other brand manifests added alongside, seed catalog + ARN entries, scripts for D2 legacy-secret copy and D3 registry migration
- Why
- Growatt was the first session-auth brand; Huawei (90 devices) is the second. This validates the Growatt template generalizes and establishes 9 learnings for the remaining 10 vendor ports.
- Areas
- services/metrics/integrations/huawei+1879−0infra/cdk+662−3packages/integration-manifests+828−2packages/database+257−14scripts/metrics+1522−0.branch+478−167domains+21−5
- Blast
- 65 files, +7377/-197 lines. Core blast radius: new vendor package only (no existing handler modified). Coordinator stack updated with new grants (additive). seed-metrics-catalog + seed-integration-arns extended with 12 new rows (additive).
Findings · 32
correctness5
monthsCovering emits spurious month on toMs midnight boundary
services/metrics/integrations/huawei/src/translation/points-to-batu.ts:219
Uses `m <= tm` which includes the month containing toMs. When toMs is an exclusive window end exactly on station-local midnight (e.g. toMs = 2026-08-01T06:00Z for America/Mexico_City), an extra getKpiStationDay call for August is emitted. seriesToPayload clips correctly so no wrong data persists, but under backfill workload on month boundaries this burns one slot of the 15/min frequency budget per station.
Rate-limit error (failCode 407) misclassified as UpstreamAuthFailed
services/metrics/integrations/huawei/src/handlers/metrics.lambda.ts:308
When the auth lambda is rate-limited during login (failCode 407 → throws 'Huawei login failed (rate-limited): ...'), the metrics worker catch checks `message.includes('login failed')` and returns `UpstreamAuthFailed` (statusCode 401) instead of `UpstreamRateLimited` (statusCode 429). SFN retry policy treats these differently — rate limits need exponential backoff, not immediate credential checks. Same bug in explore.lambda.ts:90.
Connection worker: forceRefresh retry not covered by credential-verdict catch path
services/metrics/integrations/huawei/src/handlers/connection.lambda.ts:81
The inner try/catch wraps only the initial invokeAuth. The forceRefresh retry at line 81 is inside the outer try but outside the inner catch. If the fresh login throws (e.g. rate-limit storm), the outer catch returns InvocationFailure/UpstreamUnavailable (502) rather than credentialValid=false. Edge case (requires: stale token + fresh login also rate-limited), but produces incorrect connection status semantics when it fires.
probeToken shares frequency-limited endpoint with connection worker (documented constraint)
services/metrics/integrations/huawei/src/engine/fusionsolar-client.ts:156
auth-cache probe (probeToken → getStationRealKpi) and connection worker's realKpi call both hit the same ~1/5-min per-user rate-limited endpoint. CLAUDE.md documents 'a connection probe spends TWO realKpi calls'. Not a bug — documented design constraint. Worth a code comment near probeToken to prevent future parallelization.
putSession has no conditional write — concurrent cold-start lambdas silently overwrite tokens
services/metrics/integrations/huawei/src/lib/auth-cache.ts:63
PutCommand has no ConditionExpression. Two concurrent auth lambdas on a cache miss each do fresh login; the second write overwrites the first. Both tokens remain valid (Huawei doesn't invalidate old tokens on new login), so neither invocation errors. Matches Growatt precedent — intentional design.
security7
IAM policy grants read access to portal secrets that should be revoked
infra/cdk/src/stacks/services/metrics/integrations/huawei/iam.ts:60
Policy allows `secretsmanager:GetSecretValue` on `{env}/huawei/*`, including `/portal/` human web-login variants (the CLAUDE.md notes these should be revoked after D2 --emit-db). The auth lambda validates payload shape and throws on portal secrets, so exploitation is limited — but least-privilege is violated. A narrower glob `{env}/huawei/oid=*` would exclude portal paths entirely.
Auth lambda ARN has no resource-based policy (any account principal with lambda:Invoke can call it)
infra/cdk/src/stacks/services/metrics/integrations/huawei/lambda.stack.ts:137
The three worker lambdas have explicit IAM grants on the auth ARN, but no `fn.addPermission` call restricts invocations to only those three callers. An IAM principal with generic lambda:InvokeFunction on '*' can invoke the auth lambda with arbitrary secretConfigArn. Same systemic gap in the Growatt stack.
DDB auth-cache table uses AWS-owned key, not CMK; no PITR
infra/cdk/src/stacks/services/metrics/integrations/huawei/auth-cache.stack.ts:62
Table storing XSRF session tokens (credential-equivalent) uses default AWS-owned encryption — key access not auditable per-call via CloudTrail KMS. No PITR means no forensic baseline on tampering. Both gaps are systemic across Growatt and CFE auth-cache stacks.
Error messages include full secret ARN (logs oid-to-org mapping)
services/metrics/integrations/huawei/src/handlers/auth.lambda.ts:129
Errors thrown at lines 129 and 139 include the full secretArn (`{env}/huawei/oid={oid}`), which ends up in CloudWatch Logs. The ARN itself is not a secret but reveals the oid-to-org mapping. Prefer logging the last path segment only.
forceRefresh has no per-caller rate limit — potential Huawei login-rate exhaustion
services/metrics/integrations/huawei/src/handlers/auth.lambda.ts:69
Any caller with lambda:InvokeFunction can pass forceRefresh:true, delete the DDB cache entry, and trigger a fresh Huawei login. Repeated calls exhaust Huawei's login rate limit (failCode 407), locking out the org credential. Primarily mitigated by adding a resource-based policy (see finding above); no additional fix needed if that is addressed.
unvalidated sourceEnv interpolated into RegExp in legacy migration script
scripts/metrics/lib/legacy-secrets-core.ts:57
`parseLegacySecretName` builds `new RegExp(^${sourceEnv}/...)`. targetEnv is validated against a whitelist, sourceEnv is not. A crafted value like 'prod|dev' could alter which secrets match. This is an internal ops script requiring AWS admin credentials, so exploitability is minimal — but apply the same whitelist check to sourceEnv.
Lambda return value includes xsrfToken — workers must not log auth result wholesale
services/metrics/integrations/huawei/src/handlers/auth.lambda.ts:104
Auth lambda logs tokenLen only (correct). Workers log {fromCache, forceRefresh} from the auth result (not the token itself — confirmed). No current leak. Documenting: any future structured log of the auth result object should explicitly exclude xsrfToken.
conventions5
AuthCacheTable construct ID missing integration name
infra/cdk/src/stacks/services/metrics/integrations/huawei/lambda.stack.ts:81
The auth-cache table import construct is named `'AuthCacheTable'`. INTEGRATION_STANDARDS.md §2 requires `Integration{IntegrationName}{ResourceType}{Role}` — should be `'IntegrationHuaweiDynamoDbAuthCache'`. The auth-cache stack itself uses the correct name; the mismatch makes the synthesized template harder to audit.
Manifest solar_generation.externalVariableId is vendor field name, not HuaweiChannel key
packages/integration-manifests/src/manifests/huawei-portal.ts:53
Manifest sets `solar_generation.externalVariableId = 'productPower'` (the vendor's API field name), while HuaweiChannel = 'solar_generation' | 'grid_import' | 'grid_export'. Live SFN validation succeeded (3854 pts, 0 skips) because resolveChannels() defaults to DEFAULT_CHANNEL='solar_generation' when params.sources is absent — so routing is currently safe. However the semantic mismatch is confusing: future code that reads the manifest's externalVariableId and passes it to the worker's sources[] will trigger TranslationFailed. Consider aligning the manifest to use 'solar_generation', with a comment documenting that productPower is the vendor API field handled in the translation layer.
Worker bundling missing DDB SDK as external; potential bundle bloat
infra/cdk/src/stacks/services/metrics/integrations/huawei/lambda.stack.ts:87
workerBundling marks only `@aws-sdk/client-lambda` as external. Workers don't call Secrets Manager directly, but auth-cache.ts imports DDB SDK; INTEGRATION_STANDARDS.md §7 lists DDB SDKs as common externals. Not a runtime error but increases worker zip sizes unnecessarily.
Deploy-order comment in main.ts omits Huawei chain
infra/cdk/src/app/main.ts:299
Comment at line 299 documents linear deploy order mentioning only Growatt. Now that Huawei is wired, the comment should document the pattern generically (or include Huawei chain) so the next integration author has the right mental model.
isSessionUsable extracted in huawei but inlined in growatt — should backport
services/metrics/integrations/huawei/src/lib/auth-cache.ts:43
Huawei extracts isSessionUsable() as a tested utility. Growatt inlines the same TTL/shape gate directly in the auth handler. Worth backporting to Growatt for consistency — the probe-before-reuse logic benefits from unit testing.
tests7
Auth lambda handler — 0% test coverage
services/metrics/integrations/huawei/src/handlers/auth.lambda.ts
The auth handler orchestrates credential fetch, credential hashing, DDB cache read, probe-before-reuse, cache invalidation, fresh login, and DDB write — none of these paths are tested. Only the pure `isSessionUsable` TTL gate has coverage. A regression in probe-before-reuse would cause silent credential reuse after server-side invalidation.
Explore lambda handler — 0% test coverage
services/metrics/integrations/huawei/src/handlers/explore.lambda.ts
The explore handler (paginated station fetch, mid-loop auth-expired → forceRefresh retry, stationsToAssets translation, guard paths) has zero tests. Symmetric to the metrics handler, which has good coverage.
Manifest grid-channel externalVariableId values not asserted
packages/integration-manifests/src/__tests__/device-integrations.test.ts:138
The test spot-checks `solar_generation.externalVariableId === 'productPower'` but does not assert grid channel IDs (`grid_import`, `grid_export`). These are self-referential values unusual across brands; if accidentally changed to vendor names, the worker silently rejects all grid payloads via the `isHuaweiChannel()` guard.
devTypeId=1 commercial inverter data path not exercised at handler level
services/metrics/integrations/huawei/src/__tests__/metrics-handler.test.ts:198
Handler test for 'meterless station' uses devTypeId=1 but mocks return no data. No test confirms that a devTypeId=1 inverter returning active_power data produces correct kW→W scaling through the batching loop. The points-to-batu unit test covers sumInverterPowerW, but the handler path is untested.
>100-device batching path (DEV_HISTORY_MAX_IDS) not tested at handler level
services/metrics/integrations/huawei/src/__tests__/metrics-handler.test.ts
Explicit batching loop for stations with >100 inverter IDs is untested. No test supplies >100 device IDs to exercise the slice. An off-by-one or double-count in the multi-call path would be invisible.
devTypeId=17 unverified meter scenario not exercised at handler level
services/metrics/integrations/huawei/src/__tests__/metrics-handler.test.ts
classifyDevices unit test correctly gates devTypeId=17 to unverifiedMeters. But the handler-level consequence (grid channels skip with an informative message rather than a generic 'no power sensor') is untested. A regression here produces silent skips with an uninformative error.
fusionsolar-client.ts failCode mapping path untested (entirely mocked)
services/metrics/integrations/huawei/src/engine/fusionsolar-client.ts
The HTTP client is fully mocked at every test boundary. unwrap()'s failCode handling (305→auth-expired, 407→rate-limited, other→vendor-rejected), non-JSON body, and HTTP 429 paths are only implicitly covered. A unit test for unwrap() with a fetch mock would give confidence the failCode mapping is correct. Acceptable given it mirrors Growatt precedent.
improvement8
invokeAuth + result envelope helpers copy-pasted across 5 handlers
services/metrics/integrations/huawei/src/handlers/explore.lambda.ts:119
invokeAuth (with its null-check, FunctionError decode) and the success/failure envelope helpers are identical across huawei/metrics, huawei/connection, huawei/explore, growatt/metrics, growatt/explore. With 10 more vendor ports coming, extract to a vendor-local `lib/invoke-auth.ts` and `lib/invocation-result.ts`. This is where auth-failure injection bugs tend to live — one fix should reach all handlers.
auth-cache.ts near-identical between huawei and growatt (only token field differs)
services/metrics/integrations/huawei/src/lib/auth-cache.ts
Growatt and huawei auth-cache modules share the same DDB client init, tableName() guard, getSession/putSession/invalidateSession trio, and same DDB commands. Only the session token field name differs (xsrfToken vs cookieHeader). Four remaining session-auth vendors will duplicate this. A generic createAuthCache<S extends {credentialHash:string; expiresAt:number}>(tableName) factory would eliminate duplication once a shared integration package exists.
CDK lambda.stack.ts is copy-paste boilerplate; will repeat 8 more times
infra/cdk/src/stacks/services/metrics/integrations/huawei/lambda.stack.ts
The huawei and growatt lambda stacks share the same structure (SSM-import auth-cache → create auth fn → SSM-output → 3× worker NodejsFunction blocks). A VendorSessionLambdaGroup CDK Construct accepting vendorConfig would collapse each future stack to ~20 lines.
auth-cache.stack.ts is token-for-token identical between huawei and growatt
infra/cdk/src/stacks/services/metrics/integrations/huawei/auth-cache.stack.ts
Both stacks are identical except vendor name string. A generic IntegrationAuthCacheStack({vendorName, ssmPathSuffix}) would eliminate the 4+ incoming copies for solis, fronius, solark, apsystems.
resolveChannels() duplicated between huawei and growatt metrics handlers
services/metrics/integrations/huawei/src/handlers/metrics.lambda.ts:70
resolveChannels (sources-array vs flat-params fallback for BAT-189 compat) is byte-for-byte identical in both handlers. With 12 vendor metrics workers coming, this function belongs in @batu/metrics-engine or a shared lib.
localDateOf() copied verbatim from victron (acknowledged in comment)
services/metrics/integrations/huawei/src/translation/points-to-batu.ts:192
The comment says '(Victron helper, verbatim)'. With solis/fronius/sma likely needing the same timezone utility, it should live in @batu/metrics-engine/src/lib/timezone.ts alongside the existing timezone.ts helper.
seed-metrics-catalog.ts credentialFields mirrored by hand — drift risk with 12 vendors
packages/database/src/seed-metrics-catalog.ts:155
Comment calls out that credentialFields must be kept in lockstep with manifests manually (no dep on @batu/integration-manifests to avoid cycles). A validation script or test that imports manifests and compares against seed data would catch drift before staging.
FusionSolar client mixes auth/explore/metrics/connection calls — reasonable now, watch at scale
services/metrics/integrations/huawei/src/engine/fusionsolar-client.ts
Section headers are clear; 285 lines is appropriate. No split needed now. Note for future: probeToken's frequency budget constraint (~1/5-min per user) should be documented in a comment near the function to prevent a future optimization that naïvely parallelizes auth refreshes.