Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Greptile SummaryThis PR adds a PowerDNS integration as a new App Connection and DNS provider for ACME certificate issuance. The implementation follows the established patterns for other DNS providers (Cloudflare, DNS Made Easy, Azure DNS) and correctly addresses the SSRF and DNS propagation retry issues raised in previous review threads. Key changes:
Confidence Score: 4/5
Important Files Changed
Reviews (3): Last reviewed commit: "Remove duplicate delay call in waitForDn..." | Re-trigger Greptile |
backend/src/services/certificate-authority/acme/dns-providers/powerdns.ts
Outdated
Show resolved
Hide resolved
|
Greptile Mitigations and Reviews have been applied :-) |
|
It seems Infisical is checking too fast if DNS Records are propagated, I have increased the attempts to 30. With 2000ms delay we have a maximum of 120 Seconds, which is enough for most providers to propagate. - const DNS_PROPAGATION_MAX_RETRIES = 5;
+ const DNS_PROPAGATION_MAX_RETRIES = 30;I also added an Exception on DNS Check Fail as previously, even though the dns check exceeded the maximum attempts, did not stop the request from going to the ACME Server. throw new Error(`DNS record "${lookupName}" with value "${expectedValue}" not found after ${DNS_PROPAGATION_MAX_RETRIES} attempts`); |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
Secret Detection was a false positive due to an accidental commit of the docker-compose.dev.yml, erased from the git history so the commit tree is clean. |
|
@greptileai Rescan PR |
backend/src/services/certificate-authority/acme/acme-certificate-authority-fns.ts
Outdated
Show resolved
Hide resolved
|
The following tests have been done:
DNS Propagation works perfectly fine now without Server Errors on ACME Side. |
…te-authority-fns.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Context
This PR adds PowerDNS Integration. I am not familiar with the code base so please bear with me :-)
Screenshots
Tested against Let's Encrypt Staging and PowerDNS Admin
Steps to verify the change
Type
Checklist
type(scope): short description(scope is optional, e.g.,fix: prevent crash on syncorfix(api): handle null response).