Repository navigation
fix: DNS resolution flakiness in prod, and let backfill scripts run there - #2693
Merged
Merged
Conversation
…here Two independent problems found while chasing prod DNS failures (getaddrinfo EAI_AGAIN on Strava/Coros/Suunto calls) and trying to actually run a backfill script in prod: 1. Docker's embedded DNS resolver (127.0.0.11) can drop one of glibc's parallel A/AAAA queries. Confirmed empirically in prod: the custom upstream DNS servers configured in the infra repo's compose file were correctly applied to the container (verified via docker inspect) and directly reachable/functional (verified with a direct query bypassing 127.0.0.11), yet the app's normal resolution path still failed - isolating the fault to Docker's embedded resolver itself, not the network or the upstream servers. This is the exact same issue already fixed for v6_api's containers (docker/scripts/entrypoint.sh there); c2c_tracking never had the equivalent fix, since it has no custom entrypoint at all. docker-entrypoint.sh appends `options single-request-reopen` to /etc/resolv.conf at container start, forcing sequential A/AAAA queries instead. Since this needs root (resolv.conf isn't writable by the app's unprivileged user) and the image now boots as root to apply it, the entrypoint drops to the node user via `su` before exec'ing the actual app. Verified locally: DNS fix applied, app process runs as uid 1000 not root, and SIGTERM still reaches it correctly for graceful shutdown (exit code 143, confirmed via a trap handler, not just timing). 2. The production image couldn't run scripts/backfill-*.ts at all: `scripts/` and `tsconfig.json` were never copied into the image, and `ts-node`/`typescript` were pruned as devDependencies. Now copied through both build stages, and ts-node/typescript moved to `dependencies` so `npm prune --omit=dev` keeps them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
🟢 Coverage ∅ diff coverage · +0.00% coverage variation
Metric Results Coverage variation ✅ +0.00% coverage variation (-3.00%) Diff coverage ✅ ∅ diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (f312b61) 2214 2037 92.01% Head commit (a05a29e) 2215 (+1) 2038 (+1) 92.01% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#2693) 0 0 ∅ (not applicable) Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
ts-node pulls in diff@4.0.2, vulnerable to a ReDoS in parsePatch/ applyPatch (GHSA-73rr-hh4g-fpgx). Pre-existing, but only surfaced as a production-scope Snyk failure once ts-node moved from devDependencies to dependencies in the previous commit. Force the patched version via npm overrides rather than waiting on ts-node to bump its own dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contexte
Deux problèmes trouvés en creusant les échecs DNS en prod (
getaddrinfo EAI_AGAINsur Strava/Coros/Suunto) et en essayant de faire tourner un script de backfill en prod.1. DNS flaky - résolveur embarqué Docker
Confirmé empiriquement en prod : les serveurs DNS custom configurés dans le compose de l'infra sont bien appliqués au container (
docker inspectconfirmeHostConfig.Dns) et directement joignables/fonctionnels (requête directe en contournant 127.0.0.11 → succès), mais le chemin de résolution normal de l'appli échoue quand même. Ça isole le problème au résolveur DNS embarqué de Docker (127.0.0.11) lui-même : glibc envoie les requêtes A et AAAA en parallèle, et certains environnements de résolveur embarqué perdent une des deux réponses.C'est exactement le même bug déjà corrigé pour les containers
v6_api(docker/scripts/entrypoint.shlà-bas) -c2c_trackingn'avait jamais eu l'équivalent, faute d'entrypoint custom du tout.docker-entrypoint.shajouteoptions single-request-reopenà/etc/resolv.confau démarrage (force des requêtes séquentielles). Ça nécessite root (resolv.conf n'est pas modifiable par l'utilisateur applicatif), donc l'image démarre maintenant en root et bascule vers l'utilisateurnodeviasujuste avant de lancer l'appli.Vérifié en local (l'image complète était trop lourde à builder sur la machine du moment, donc testé par couches sur une image déjà construite) :
options single-request-reopenprésent dans/etc/resolv.conf)node), pas roottrap, pas juste un chronométrage)2. Les scripts de backfill ne pouvaient pas tourner en prod
scripts/ettsconfig.jsonn'étaient jamais copiés dans l'image, etts-node/typescriptétaient supprimés (devDependencies, prunées parnpm prune --omit=dev). Maintenant copiés dans les deux étapes du build, etts-node/typescriptdéplacés versdependenciespour survivre au prune.Test plan