Fix certificate rotation policy correlation - #365
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91089f3979
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return cls.rotation_policy.has( | ||
| extract("day", cls.not_after - func.now()) <= RotationPolicy.days | ||
| ) |
There was a problem hiding this comment.
When a rotation-enabled certificate predates 484764d or was imported without a policy, its rotation_policy_id may still be NULL: the column remains nullable, and the automatically_enable_autorotate_* commands in lemur/certificates/cli.py only set rotation=True. This has() predicate is always false for those rows, whereas the previous expression selected them using an available policy, so get_all_pending_reissue() silently stops renewing these certificates and they can expire. Add a data backfill or an explicit default-policy fallback before requiring the relationship.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
the backfill has already run and commercial currently has zero active or rotation-enabled certs with a NULL policy. New certs also receive the default policy.
2 participants
Summary
in_rotation_windowSQL expression with each certificate's assigned rotation policyrotation_policiesRoot cause and impact
In #362 the hybrid SQL expression referenced
RotationPolicy.dayswithout joining it throughCertificate.rotation_policy_id. A certificate therefore qualified when its expiry matched any rotation policy, effectively applying the largest configured window to the entire rotation-enabled fleet.The expression now uses the
rotation_policyrelationship, producing a correlatedEXISTScondition. Certificates with no assigned policy remain ineligible, and unrelated policies can no longer affect them.Validation
flake8 lemur/certificates/models.py lemur/tests/test_certificates.pygit diff --checkrotation_policies.id = certificates.rotation_policy_idwith no top-level cross joinThe focused test is collected successfully, but could not execute locally because PostgreSQL is not running on
localhost:5432; CI will execute it against the project test database.