added Arch Intent docs#1483
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds comprehensive architectural intent documentation for the Accelerated Publishing feature by introducing two new checklist documents (security-checklist.md and engineering-checklist.md) and removing an older, less detailed checklist (engineering-security-checklist.md). The new documentation provides thorough coverage of the Next.js server integration for accelerating content publishing from Drupal CMS to VA.gov, targeting a ~5-minute publishing time.
Key changes:
- Added detailed security checklist covering monitoring plans, process privileges, endpoint security, incident response, and architecture diagrams
- Added comprehensive engineering checklist documenting backend changes, API interactions, infrastructure, rollout strategy, and testing approach
- Removed redundant engineering-security-checklist.md file now superseded by the new documentation
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
| docs/arch-intent/security-checklist.md | New comprehensive security checklist covering architecture, monitoring, API endpoints, data handling, and incident response plans for the Next.js/Drupal integration |
| docs/arch-intent/engineering-security-checklist.md | Removed older, less detailed checklist that has been superseded by the two new documentation files |
| docs/arch-intent/engineering-checklist.md | New detailed engineering checklist documenting product description, backend changes, API patterns, infrastructure, testing strategy, and rollout plans |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Nextjs Env - ~10 mins [Tim and Edmund to comment] | ||
| - Drupal Env - ~15 mins [Tim and Edmund to comment] |
There was a problem hiding this comment.
Placeholder comments should be removed or addressed. This line contains incomplete information with placeholders for "Tim and Edmund to comment".
| - Nextjs Env - ~10 mins [Tim and Edmund to comment] | |
| - Drupal Env - ~15 mins [Tim and Edmund to comment] | |
| - Nextjs Env - ~10 mins | |
| - Drupal Env - ~15 mins |
spelling/grammar Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
cvalarida
left a comment
There was a problem hiding this comment.
The only meaningful comments I had were:
- Adding a README for a little more context to these docs
- Adding an option to redirect traffic to the S3 bucket in our vulnerability remediation strategy
The rest were mostly just nit-picky edits because I'm kind of annoying about spelling, and consistency. 😅 🙈
There was a problem hiding this comment.
Looks like this one isn't used, yeah?
There was a problem hiding this comment.
Can you add a brief explanation of what this file is? Preferably everything in this directory with a README.md. It serves a very specific purpose, and without the context of the collaboration cycle, I can see engineers in a couple years reading this and wondering why it's here.
Another thing I'd recommend is adding a little changelog at the bottom of this and the security checklist because they're both very much snapshot-in-time type documents. It's possible to leverage git history for that, but that's assuming the PRs that modify the files will have good descriptions. 🤷
|
|
||
| **Incident Response Plan, including Points of Contact for your system and dependent VA back-ends.** | ||
|
|
||
| - **If a security vulnerability is discovered or reported in this code base, what is the plan and timeline for rolling out the fix?** |
There was a problem hiding this comment.
It's worth calling out that this question about the "timeline for rolling out the fix" assumes that a rollout with a fix is needed to mitigate the vulnerability.
Because we'll have automated backups to S3 and the RevProxy will be setup to direct traffic to that already, the shorter path to mitigate exposure to a newly-discovered vulnerability in next-build may involve a deploying a change to the RevProxy to direct all traffic to the Next Build S3 bucket instead of the Next Build server. We'll have no downtime, and the side effect would be a rollback in some content to the last backup, which isn't the worst.
This would be useful if the fix takes longer than the estimated 5 minutes.
There was a problem hiding this comment.
Revproxy changes take approximately 20 minutes to deploy once approved and merged. If we can move faster than that here, we should do what the doc suggests.
| - Deployment executes and updates the production environment | ||
| - Nextjs Env - ~10 mins | ||
| - Drupal Env - ~90 mins | ||
| - No VA.gov downtime due to the deployment process. The CMS has some downtime as part of its deployment process, but this is not public facing. |
There was a problem hiding this comment.
The CMS has some downtime as part of its deployment process
Maybe? Depends on what the vulnerability is, but I'd expect we wouldn't need to make any changes to Drupal in most cases. If that's true, that also cuts out a solid 90-ish minutes off of the estimated time to roll out a fix.
Co-authored-by: Chris Valarida <cvalarida@gmail.com>
Removing unused file.
Description
This pull request updates the engineering documentation for the Accelerated Publishing project by adding a comprehensive and detailed checklist in
engineering-checklist.mdand removing a less detailed, somewhat redundant checklist fromengineering-security-checklist.md. The new documentation provides a thorough overview of the product, infrastructure, backend changes, API interactions, monitoring, rollout, and maintenance plans for the introduction of a Next.js server to accelerate content publishing from Drupal CMS to VA.gov.Key documentation updates:
Addition of detailed engineering checklist:
engineering-checklist.mdthat describes the architecture, backend changes (including the introduction of a Next.js server for Incremental Static Regeneration), failover logic in the RevProxy, internal API connections, monitoring/observability, infrastructure, rollout plan, and test strategy.Removal of outdated/redundant checklist:
engineering-security-checklist.md, which is now superseded by the new documentation.Infrastructure and backend enhancements:
Rollout and maintenance planning:
Testing and observability:
Ticket
#22652