Skip to content

Conversation

@carlosmonastyrski
Copy link
Contributor

Description 📣

Revamps the PKI sync flow to work directly with certificates instead of the legacy subscribers entity, improves AWS syncs to allow reusing the same ARN during renewals, and includes several UI and UX enhancements across the certificate tables, tabs, and overall certificate management experience.

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

@maidul98
Copy link
Collaborator

maidul98 commented Oct 29, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR modernizes the PKI sync infrastructure by shifting from a legacy subscriber-based model to direct certificate management. The core enhancement enables AWS Certificate Manager syncs to preserve ARNs during certificate renewals by tracking alternative certificate names for identification.

Key Changes

  • Database schema: New CertificateSync join table tracks many-to-many relationships between PKI syncs and certificates with sync status tracking
  • Certificate-based syncing: Certificates can now be directly associated with PKI syncs instead of requiring subscriber entities
  • AWS ARN preservation: Added preserveArn option (default: true) to maintain consistent ARNs across certificate renewals by matching certificates via alternative names
  • Stable certificate naming: Certificate names now use profileId when available for consistent identification across renewals
  • Auto-sync integration: Certificate renewals automatically trigger PKI syncs with proper certificate replacement logic
  • Enhanced status tracking: Individual certificate sync statuses (pending/syncing/succeeded/failed) are tracked and updated throughout the sync lifecycle
  • REST endpoints: New API routes for managing certificate associations (POST/DELETE /pki-syncs/:id/certificates)
  • UI improvements: Replaced subscriber selection with direct certificate selection interface

Notable Implementation Details

  • Alternative names include legacy certificate IDs and original certificate IDs from renewals for backward compatibility
  • Validation errors are now separated from sync failures for better error granularity
  • SQL injection protection added to LIKE queries with RE2 regex-based sanitization
  • Certificate metadata map tracks ID-to-name relationships during sync operations
  • Tags are added separately when reusing ARNs due to AWS ACM API limitations

Confidence Score: 4/5

  • This PR is safe to merge with minor recommendations for consideration
  • The implementation is well-structured with proper database migrations, comprehensive error handling, and secure query patterns. The ARN preservation logic and certificate identification via alternative names is sound. Score is 4 rather than 5 due to the complexity of the AWS ACM tag timing logic (100ms delay) which may need monitoring in production, and the potential for edge cases in certificate name matching across renewals
  • backend/src/services/pki-sync/aws-certificate-manager/aws-certificate-manager-pki-sync-fns.ts - Contains complex ARN preservation and tagging logic that may require production monitoring

Important Files Changed

File Analysis

Filename Score Overview
backend/src/db/migrations/20251028120000_add-certificate-sync-table.ts 5/5 Creates new CertificateSync table to track certificate-to-PKI-sync relationships with proper foreign keys and indexes
backend/src/services/certificate-sync/certificate-sync-dal.ts 5/5 Implements comprehensive DAL for certificate sync operations including finding, adding, removing, and updating sync statuses
backend/src/services/pki-sync/aws-certificate-manager/aws-certificate-manager-pki-sync-fns.ts 4/5 Enhanced AWS ACM sync to support ARN preservation during renewals, alternative certificate names for identification, and improved error handling with validation errors tracked separately
backend/src/services/pki-sync/pki-sync-service.ts 5/5 Added certificate management endpoints for PKI syncs including adding/removing certificates and listing certificates with proper permission checks
backend/src/services/pki-sync/pki-sync-queue.ts 4/5 Updated queue processing to fetch certificates by IDs in addition to subscribers, generate stable certificate names using profileId, track alternative names for certificate identification, and update certificate sync statuses throughout the sync lifecycle

Sequence Diagram

sequenceDiagram
    participant User
    participant Frontend
    participant API
    participant PkiSyncService
    participant CertificateSyncDAL
    participant PkiSyncQueue
    participant AWS_ACM
    
    User->>Frontend: Create PKI Sync with certificates
    Frontend->>API: POST /pki-syncs
    API->>PkiSyncService: createPkiSync(certificateIds)
    PkiSyncService->>CertificateSyncDAL: addCertificates(pkiSyncId, certificateIds)
    CertificateSyncDAL-->>PkiSyncService: certificateSyncs created
    PkiSyncService->>PkiSyncQueue: queuePkiSyncSyncCertificatesById
    PkiSyncService-->>API: pkiSync created
    API-->>Frontend: Success
    
    Note over PkiSyncQueue: Background Queue Processing
    PkiSyncQueue->>CertificateSyncDAL: findCertificateIdsByPkiSyncId
    CertificateSyncDAL-->>PkiSyncQueue: certificateIds
    PkiSyncQueue->>CertificateSyncDAL: bulkUpdateSyncStatus(running)
    PkiSyncQueue->>AWS_ACM: Import certificates with ARN preservation
    
    alt Preserve ARN enabled
        AWS_ACM->>AWS_ACM: Reuse existing ARN for renewed certificates
        AWS_ACM->>AWS_ACM: Match by alternative names
    else New certificate
        AWS_ACM->>AWS_ACM: Create new certificate with tags
    end
    
    AWS_ACM-->>PkiSyncQueue: Sync results
    PkiSyncQueue->>CertificateSyncDAL: bulkUpdateSyncStatus(succeeded/failed)
    
    Note over User,AWS_ACM: Certificate Renewal Flow
    User->>API: Renew certificate
    API->>CertificateV3Service: renewCertificateFromProfile
    CertificateV3Service->>CertificateSyncDAL: replaceCertificateInSyncs(oldId, newId)
    CertificateV3Service->>PkiSyncQueue: triggerAutoSyncForCertificate
    PkiSyncQueue->>AWS_ACM: Sync renewed certificate to same ARN

Loading

63 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@sheensantoscapadngan
Copy link
Member

IGNORE PLACEMENT OF COMMENT

Minor but I think we should add a top margin for this one?
image

@carlosmonastyrski carlosmonastyrski merged commit 4818589 into main Nov 3, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants