Chore/builtin virtual cloud

[robmorgan]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
infraspec	Ready	Preview, Comment	Dec 30, 2025 3:31am

[claude]

Code Review: PR #198 - Chore/builtin virtual cloud

Summary

This PR updates documentation to reflect that Virtual Cloud is now embedded within InfraSpec rather than requiring a separate infraspec-api service. The changes primarily update messaging in README.md and add a new live testing guide.

---

✅ Strengths

1. Clear messaging improvements: The updated README better emphasizes the embedded emulator approach and the ability to test against both virtual and real AWS.

2. Helpful new documentation: The live-testing.mdx guide provides practical guidance for users testing against real AWS, including important cleanup considerations.

3. Proper commit conventions: Both commits follow the conventional commits format correctly (chore(docs):).

4. Improved value proposition: Lines 23-26 in README.md better articulate InfraSpec's collaborative benefits for platform, security, and compliance stakeholders.

---

🐛 Issues Found

1. Repetitive/Unclear Wording (live-testing.mdx:3-5)

"This is useful for testing your infrastructure code in a live environment, 
or for testing your infrastructure code against a live environment."

This sentence is circular and redundant. Consider rewording to:

"This is useful for end-to-end validation in actual AWS environments or for 
testing production-like configurations."

2. Incomplete Documentation Sweep

The changes remove references to Virtual Cloud as an external service from README.md, but many other documentation files still reference the old architecture:

• website/src/content/virtual-cloud.mdx - Describes Virtual Cloud as a "managed service"
• website/src/content/pricing.mdx - References Virtual Cloud access as a paid feature
• website/src/content/vs-localstack.mdx - Compares Virtual Cloud as an external service
• CLAUDE.md:228 - Still references infraspec-api and checking infraspec-api/AGENTS.md
• CLAUDE.md:244-245 - Troubleshooting mentions "Virtual Cloud 403/404 errors" related to infraspec-api

Recommendation: Create follow-up issues/PRs to update these files consistently, or expand this PR to include a complete documentation sweep.

3. Missing Context in README.md:68

- 💰 **Cost effective** - Use Virtual Cloud to eliminate AWS testing costs

Now that Virtual Cloud is embedded, this bullet needs updating. Consider:

- 💰 **Cost effective** - Built-in emulator eliminates AWS testing costs

4. Broken Link Potential (README.md:147)

→ [**Learn more about live testing on AWS**](https://infraspec.sh/docs/guides/live-testing)

Ensure this URL path will be correct once the site is deployed. The new file is at website/src/content/docs/guides/live-testing.mdx which should map correctly, but verify the routing configuration.

5. Inconsistent Flag Documentation

README.md:141 shows --live flag for testing against real AWS, but:

• website/src/content/vs-localstack.mdx:299 references --virtual-cloud flag
• The messaging is now inverted (embedded is default, not opt-in)

Recommendation: Verify the CLI flag behavior and ensure all documentation uses the correct flag name.

---

💡 Suggestions

1. Enhance live-testing.mdx with examples

Add practical examples showing:

• How to configure AWS credentials for different scenarios (IAM roles, profiles, CI/CD)
• Common pitfalls when testing against live AWS
• Cost estimation tips
• Examples of using AWS Organizations/SCPs for additional safety

2. Add troubleshooting section to live-testing.mdx

Include:

• What to do if cleanup fails
• How to identify orphaned resources
• AWS cost tracking recommendations
• Rate limiting considerations

3. Consider migration guide

If any users were previously using the external Virtual Cloud service, they may need guidance on:

• How the embedded version differs
• Any breaking changes
• Performance characteristics

---

🔒 Security Considerations

✅ No security issues identified. The changes are documentation-only.

---

🧪 Testing

⚠️ Action Required: While this PR only modifies documentation, please verify:

1. The new live-testing.mdx file renders correctly on the website
2. All internal links resolve properly
3. The --live flag is correctly documented and matches actual CLI behavior

---

📊 Test Coverage

N/A - Documentation changes only.

---

✍️ Commit Message Quality

✅ Both commits follow conventional commits correctly:

• chore(docs): virtual cloud internal tweaks
• chore(docs): vc no longer needed for dev

---

🎯 Overall Assessment

Status: Approved with recommendations

This PR makes important documentation updates to reflect the embedded Virtual Cloud architecture. The changes are well-intentioned and improve clarity in the README. However, this appears to be part of a larger documentation migration that isn't complete.

Recommended Actions:

1. Fix the repetitive wording in live-testing.mdx (lines 3-5)
2. Create follow-up issues to update remaining Virtual Cloud references across the website
3. Verify the --live vs --virtual-cloud flag naming is consistent
4. Test that the new documentation renders correctly on infraspec.sh

The core changes are solid and can be merged once the wording issue in live-testing.mdx is addressed.

---

References:

• CLAUDE.md guidelines followed ✅
• Conventional commits format ✅
• Documentation standards maintained ✅

[claude]

Pull Request Review: Built-in Virtual Cloud

Summary

This PR successfully migrates from an external InfraSpec API dependency to a built-in AWS emulator, which is a significant architectural improvement. The changes are primarily documentation updates reflecting this new default behavior.

✅ Strengths

1. Clean Dependency Removal: Properly removes all references to external infraspec-api and infraspec-cloud services
2. Comprehensive Documentation: Updates span multiple documentation files (CLAUDE.md, README.md, test docs, website)
3. Consistent Messaging: The "emulator by default, --live flag for real AWS" pattern is clearly communicated throughout
4. Improved Developer Experience: Removing Docker Compose dependencies for local testing simplifies the setup

📝 Code Quality & Best Practices

Documentation Changes (CLAUDE.md)

• ✅ Formatting improvements with proper heading spacing
• ✅ Accurate reflection of new architecture (embedded emulator vs external API)
• ✅ Correct import path updates in CloudMirror templates (infraspec-api → infraspec)
• ✅ Removal of authentication section (no longer needed for embedded emulator)

User-Facing Documentation (README.md)

• ✅ Simplified value proposition focusing on built-in emulator benefits
• ✅ Removed confusing external service references
• ✅ Clear feature ordering (fast feedback moved up appropriately)

Test Infrastructure (test/README.md, docker-compose.yml)

• ✅ Removed unnecessary infraspec-api container and environment setup scripts
• ✅ Kept httpbin container for HTTP-specific tests (correct decision)
• ✅ Updated instructions are clear and actionable

🔍 Potential Issues & Suggestions

Minor Issues

1. CLAUDE.md:255 - Outdated troubleshooting text:

   • "InfraSpec API connectivity": Ensure InfraSpec API is running and accessible on port 3687
   • "Test failures": Verify InfraSpec API services are running

   These should be removed or updated to reflect the embedded emulator.

2. website/src/content/docs/guides/live-testing.mdx:11 - Broken link reference:

   • References /docs/providers/aws/configure page
   • Verify this documentation page exists at the referenced path

3. Missing Information - Consider adding:

   • How the embedded emulator is started (automatically? on-demand?)
   • Which port the embedded emulator uses (if any)
   • Troubleshooting steps for emulator-specific issues

Suggestions for Improvement

1. Getting Started Example (website/src/content/docs/getting-started.mdx)

   • The new "Hello World" Terraform example is simpler (good!)
   • Consider mentioning that this runs against the emulator by default

2. Commit Message
   Per your Conventional Commits standard in CLAUDE.md, the PR title should follow the format:

   • chore: migrate to built-in virtual cloud emulator
     or
   • docs: update documentation for built-in emulator

   Current title "Chore/builtin virtual cloud" doesn't follow the spec (should use lowercase, colon separator).

🔒 Security Considerations

• ✅ No security concerns
• ✅ Removal of external API dependencies actually improves security posture
• ✅ No credentials or sensitive data exposed

🧪 Test Coverage

• ✅ Documentation updates are consistent with test infrastructure changes
• ⚠️ No code changes to review, but ensure CI/CD is updated to reflect new workflow
• 💡 Consider adding a test case to verify emulator starts correctly by default

⚡ Performance Considerations

• ✅ Built-in emulator should be faster than external API calls
• ✅ No Docker overhead for basic testing workflows
• ✅ The "10-100x faster" claim in README is well-positioned

📊 Overall Assessment

Rating: ✅ Approve with minor suggestions

This is a solid documentation update that accurately reflects a significant architectural improvement. The changes are well-thought-out and consistent across all documentation surfaces.

Recommended Actions Before Merge:

1. Clean up outdated "InfraSpec API" references in CLAUDE.md troubleshooting section (lines 255, 258)
2. Verify the /docs/providers/aws/configure link exists in the website
3. Consider updating the PR title to follow Conventional Commits format
4. Optional: Add a brief explanation of how the emulator starts/runs in the getting started docs

Nice-to-Haves (Future Work):

• Add troubleshooting section specific to the embedded emulator
• Document emulator limitations vs real AWS (if any)
• Add performance comparison metrics

---

Great work on this migration! The shift to a built-in emulator significantly improves the developer experience. 🚀

[claude]

Pull Request Review: Chore/builtin virtual cloud

Summary

This PR transitions InfraSpec from using an external InfraSpec Cloud API to a builtin AWS emulator embedded directly in the repository. The changes are primarily documentation updates and cleanup of external dependencies. Overall, this is a solid refactoring that improves the project's self-containment and reduces external dependencies.

Strengths

1. Improved Self-Containment

• Embedding the AWS emulator eliminates dependency on external services
• Simplifies development setup and reduces onboarding friction
• Better alignment with open-source principles

2. Comprehensive Documentation Updates

• CLAUDE.md is thoroughly updated with emulator architecture details
• Clear migration from "InfraSpec API" to "builtin AWS emulator" terminology
• Added mandatory response building rules and code patterns to prevent technical debt

3. Cleanup of Obsolete Infrastructure

• Removed external API references from docker-compose.yml
• Deleted unused test scripts (run-tests.sh, set-env-vars.sh)
• Cleaned up CloudMirror template import paths

Issues & Recommendations

1. Documentation Inconsistencies

Issue: The README.md still mentions "InfraSpec API" in one place:

- InfraSpec API (for AWS emulation during testing)

Location: Line 35 in the updated README.md (Development Setup > Prerequisites section)

Recommendation: Update to "Builtin AWS emulator" for consistency.

---

2. Missing File Reference

Issue: CLAUDE.md references infrastructure that may not exist:

- InfraSpec API (for AWS emulation during testing)

Location: CLAUDE.md line 35

Recommendation: Update to match the new embedded emulator terminology.

---

3. Incomplete Test Documentation

Issue: test/README.md mentions "InfraSpec Cloud API" and token setup:

### Using InfraSpec Cloud API
Build the binary and run features with the `--virtual-cloud` flag:

Observation: The diff shows this section was updated to "Builtin InfraSpec AWS Emulator", so this appears to be handled correctly in the PR.

---

4. Potential Breaking Changes

Issue: The PR removes the --virtual-cloud flag references and the docker-compose service for infraspec-api.

Impact:

• Any existing CI/CD pipelines using --virtual-cloud flag will break
• Local development environments using docker-compose may need updates
• Documentation examples in blog posts or external tutorials will be outdated

Recommendation:

• Add a migration guide or changelog entry documenting this breaking change
• Consider deprecation warnings if the flag is still supported
• Update any example scripts in the repository

---

5. CloudMirror Template Import Path

Issue: CloudMirror templates updated import paths from:

"github.com/robmorgan/infraspec-api/internal/emulator"

to:

"github.com/robmorgan/infraspec/internal/emulator"

Observation: This is correct and necessary. However, verify that:

• All generated code from previous runs is regenerated
• No lingering references to the old path exist in generated files

Recommendation: Run a repository-wide search to ensure no old import paths remain:

grep -r "infraspec-api/internal/emulator" .

---

6. Website Documentation Gap

Issue: A new file website/src/content/docs/guides/live-testing.mdx is added but content is minimal (17 lines).

Observation: The content appears complete for a getting-started guide, but consider:

• Adding examples of different AWS credential methods
• Including troubleshooting section for common AWS auth errors
• Documenting IAM permission requirements

Recommendation: Expand this guide with practical examples and common pitfalls.

---

7. Docker Compose Service Removal

Issue: The infraspec-api service is completely removed from docker-compose.yml, leaving only httpbin.

Impact: This is intentional but worth noting that:

• Developers who had docker-compose running will need to restart it
• The compose file now has minimal value (single service)

Recommendation: Consider whether docker-compose.yml is still needed, or if it should be moved to a test-specific location.

---

Code Quality Assessment

Positive Patterns

• Consistent terminology updates across all documentation
• Proper cleanup of obsolete scripts and configuration
• Clear separation of virtual cloud (emulator) vs live testing

Potential Issues

• No automated tests verifying the emulator integration still works
• No validation that CloudMirror-generated code compiles with new import paths

Security Considerations

• Positive: Removing external API dependency reduces attack surface
• Positive: No credentials or secrets management changes
• Neutral: AWS credential handling remains unchanged

Performance Considerations

• Positive: Embedded emulator should be faster (no network overhead)
• Positive: Local testing becomes more deterministic
• Unknown: Memory/resource usage of embedded emulator vs external API

Test Coverage

Missing Coverage:

• No tests verify that features still work with the embedded emulator
• No integration tests confirm CloudMirror templates generate valid code
• No backward compatibility tests for the removed --virtual-cloud flag

Recommendation: Add integration tests that verify:

./infraspec features/aws/s3/s3_bucket.feature  # Should use embedded emulator
./infraspec --live features/aws/s3/s3_bucket.feature  # Should use real AWS

Action Items

Must Fix (Blocking)

1. ✅ Verify no lingering infraspec-api import paths exist
2. ⚠️ Add migration notes for breaking changes (--virtual-cloud flag removal)

Should Fix (Recommended)

3. 📝 Expand live-testing.mdx documentation with examples
4. 🧪 Add integration test to verify embedded emulator works end-to-end
5. 📋 Update CONTRIBUTING.md to reflect the embedded emulator architecture

Nice to Have

6. 📚 Add troubleshooting section for embedded emulator
7. 🔍 Document performance comparison: embedded vs external API
8. 🗑️ Consider removing docker-compose.yml if only httpbin remains

Conclusion

This is a well-executed refactoring that simplifies the project architecture. The documentation updates are comprehensive and the cleanup is thorough. The main concerns are:

1. Ensuring no breaking changes are undocumented
2. Verifying all generated code uses new import paths
3. Adding tests to prevent regression

Recommendation: Approve with minor revisions to address breaking change documentation and verification testing.

---

Review Checklist:

• ✅ Code quality and best practices: Good
• ✅ Potential bugs: None identified, but needs verification testing
• ✅ Performance: Expected improvement (embedded vs network)
• ✅ Security: Improved (reduced external dependencies)
• ⚠️ Test coverage: Needs integration tests for emulator

Overall Assessment: Approve pending minor documentation updates and verification testing.