Skip to content

Conversation

@danbiwer
Copy link
Contributor

@danbiwer danbiwer commented Nov 5, 2025

Add tables describing all policies for each pre-built pack

@claude
Copy link
Contributor

claude bot commented Nov 5, 2025

Documentation Review

This PR adds detailed policy documentation for Pulumi's pre-built compliance packs. Overall, the content is well-structured and comprehensive. However, I've identified several issues that need to be addressed:

Critical Issues

1. Missing final newlines (AGENTS.md requirement)

All 11 new files are missing the required trailing newline. From line references in the files:

  • cis-aws.md:126 (ends after line 125)
  • cis-azure.md:115 (ends after line 114)
  • cis-gcp.md:113 (ends after line 112)
  • hitrust-aws.md:141 (ends after line 140)
  • hitrust-azure.md:98 (ends after line 97)
  • hitrust-gcp.md:83 (ends after line 82)
  • nist-aws.md:150 (ends after line 149)
  • pci-dss-aws.md:171 (ends after line 170)
  • pulumi-best-practices-aws.md:65 (ends after line 64)
  • pulumi-best-practices-azure.md:40 (ends after line 39)
  • pulumi-best-practices-gcp.md:46 (ends after line 45)

Fix: Add a newline at the end of each file:

sed -i '' 's/[[:space:]]*$//' content/docs/insights/policy/policy-packs/pre-built-packs/*.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/cis-aws.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/cis-azure.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/cis-gcp.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/hitrust-aws.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/hitrust-azure.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/hitrust-gcp.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/nist-aws.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/pci-dss-aws.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/pulumi-best-practices-aws.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/pulumi-best-practices-azure.md
echo >> content/docs/insights/policy/policy-packs/pre-built-packs/pulumi-best-practices-gcp.md

Minor Issues

2. Empty cells in policy tables

Some policies have empty "Framework Reference" and "Framework Specification" columns. For example, in pulumi-best-practices-aws.md:

  • Lines 35-38: Four policies have empty reference columns
  • This occurs in other files as well

Suggestion: Either fill in the missing framework references or add a note explaining why certain policies don't have framework mappings.

3. Grammar inconsistency in descriptions

Policy descriptions use mixed tenses and sentence structures:

  • Some start with "Ensure" (imperative): "Ensure RDS clusters have logging enabled"
  • Some start with "Ensures" (present tense): "Ensures CloudTrail trails have encryption enabled"
  • Some start with "Check" or "Checks": "Checks that RDS Clusters storage is encrypted"
  • Some are statements: "EC2 instances must have detailed monitoring enabled"

Recommendation: Standardize to one form, preferably starting with the imperative "Ensure" (without the 's') for consistency with technical documentation best practices.

Examples from cis-aws.md:

  • Line 11: "Checks that Neptune..." → "Ensure Neptune..."
  • Line 22: "Ensures AWS GuardDuty..." → "Ensure AWS GuardDuty..."
  • Line 55: "EC2 instances must have..." → "Ensure EC2 instances have..."

Positive Notes

  • ✓ Proper frontmatter with title, meta_desc, and h1
  • ✓ Clear table structure with appropriate headers
  • ✓ Links in the parent file (pre-built-packs.md) are correctly formatted
  • ✓ File organization follows the repository structure

Please address the critical issue (missing newlines) before merging, as this is a hard requirement per AGENTS.md. The grammar inconsistencies should also be fixed for better documentation quality.

Mention @claude if you'd like me to review the changes after updates or help with any fixes.

@pulumi-bot
Copy link
Collaborator

@pulumi-bot
Copy link
Collaborator

@pulumi-bot
Copy link
Collaborator

@pulumi-bot
Copy link
Collaborator

@danbiwer danbiwer requested a review from CamSoper November 6, 2025 14:35
@danbiwer danbiwer marked this pull request as ready for review November 6, 2025 14:35
@claude
Copy link
Contributor

claude bot commented Nov 6, 2025

Documentation Review

I've reviewed this PR that adds tables describing all policies for each pre-built pack. Overall, the structure and content organization look good. However, I found several grammar issues that need to be corrected.

Issues Found

Grammar: Subject-verb agreement errors

Multiple policy descriptions have subject-verb agreement errors. The pattern "Check/Checks that [plural subject] uses" is grammatically incorrect. It should be either:

  • "Check that [plural subject] use" (plural verb)
  • "Checks that [singular subject] uses" (when the subject is truly singular)

Examples from the files:

  1. CIS AWS (line 55 in content/docs/reference/pre-built-policy-packs/cis/aws.md):

    • Current: "Check that ELB Load Balancers uses access logging."
    • Should be: "Check that ELB Load Balancers use access logging."
  2. HITRUST AWS (multiple instances):

    • "Check that ELB Load Balancers uses access logging." → "use"
    • "Check that ELB Load Balancers uses more than one availability zone." → "use"
    • "Checks that CloudFront distributions uses secure/modern TLS encryption." → "use"
  3. NIST AWS (multiple instances):

    • Same patterns with "Load Balancers uses" → "use"
  4. PCI DSS AWS (multiple instances):

    • Same patterns throughout

All affected instances:
This error appears approximately 7 times with "ELB Load Balancers uses" and several more times with "CloudFront distributions uses" and similar plural subjects across multiple files.

Suggested fixes:

For descriptions starting with "Check that [plural noun] uses":

Check that ELB Load Balancers use access logging.

For descriptions starting with "Checks that [plural noun] uses":

Checks that CloudFront distributions use secure/modern TLS encryption.

Positive Observations

  • All files appear to have proper frontmatter structure
  • Table formatting is consistent across all files
  • Menu structure in config/_default/menus.yml is well-organized
  • Links in pre-built-packs.md correctly point to the new reference pages
  • No trailing whitespace detected
  • Content structure follows the repository conventions

Recommendation

Please correct all subject-verb agreement errors before merging. You can find these by searching for:

  • "uses" following plural subjects like "Load Balancers", "distributions", "Clusters", "Instances"

Mention @claude if you'd like me to help identify all specific instances or provide a complete list of lines to fix.

Copy link
Contributor

@CamSoper CamSoper left a comment

Choose a reason for hiding this comment

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

I love this, this is very thorough and helpful. No notes.

@danbiwer danbiwer merged commit b610307 into master Nov 6, 2025
16 checks passed
@danbiwer danbiwer deleted the danbiwer/add-policy-pack-info branch November 6, 2025 19:42
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