Conversation
There was a problem hiding this comment.
Pull request overview
This PR reorganizes and expands documentation (new integration/deployment guides, cross-linking), and refactors backend code to remove barrel exports while consolidating shared validation and streaming callback patterns.
Changes:
- Added/updated multiple docs pages (new Kubernetes deployment guide, integration setup guides, repo structure guide, and troubleshooting references).
- Refactored backend imports by removing several
index.tsbarrel files and updating call sites accordingly. - Introduced shared Zod schemas (
commonSchemas.ts) and centralized streaming callback creation inStreamingExecutionManager.
Reviewed changes
Copilot reviewed 54 out of 68 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/user-guide.md | Adds troubleshooting reference and Bolt output interpretation section; trims streaming documentation. |
| docs/repo_structure_and_config.md | New documentation describing repo dotfiles/config directories. |
| docs/puppetdb-api.md | Updates PuppetDB setup guide link path. |
| docs/kubernetes-deployment.md | New Kubernetes deployment guide and example manifests. |
| docs/integrations/puppetserver.md | Adds manual API verification section for Puppetserver. |
| docs/integrations/puppetdb.md | Adds manual API verification section for PuppetDB. |
| docs/integrations/hiera.md | New Hiera integration setup guide. |
| docs/integrations/bolt.md | New Bolt integration setup guide (whitelist, package tasks, perf tuning). |
| docs/integrations-api.md | Updates PuppetDB setup guide link path in related docs. |
| docs/docker-deployment.md | Improves container path explanation; replaces env var reference with link to configuration guide; updates PuppetDB setup link path. |
| docs/development/puppetdb-puppetserver-api-endpoints.md | Adds “Generated” timestamp header. |
| docs/development/SECURITY_AUDIT.md | Adds a security audit report document. |
| docs/development/BACKEND_REFACTORING_OPPORTUNITIES.md | Adds backend refactoring opportunities report. |
| docs/configuration.md | Replaces large inline sections with links to dedicated docs (Bolt/Hiera/Docker/Kubernetes/Troubleshooting). |
| docs/architecture.md | Updates PuppetDB setup guide link path in related docs. |
| backend/test/integrations/PuppetDBService.test.ts | Updates imports after removal of PuppetDB barrel exports. |
| backend/test/integration/re-execution.test.ts | Updates middleware imports after middleware barrel removal. |
| backend/test/integration/puppetserver-nodes.test.ts | Updates middleware imports and adjusts mock method signature/behavior. |
| backend/test/integration/integration-status.test.ts | Updates middleware imports and enriches mock facts structure. |
| backend/test/integration/external-api-errors-expert-mode.test.ts | Updates PuppetDB error imports and fixes boolean assertion logic. |
| backend/test/integration/expert-mode-routes.test.ts | Updates middleware imports after middleware barrel removal. |
| backend/test/integration/api.test.ts | Updates route wiring to new router signatures and middleware imports. |
| backend/test/debug-inventory-route.test.ts | Updates middleware imports after middleware barrel removal. |
| backend/test/debug-expert-mode.test.ts | Updates middleware imports after middleware barrel removal. |
| backend/src/validation/index.ts | Removes validation barrel exports. |
| backend/src/validation/commonSchemas.ts | Adds shared Zod param schemas for id and nodeId. |
| backend/src/services/StreamingExecutionManager.ts | Adds createStreamingCallback() helper and exports StreamingCallback type. |
| backend/src/server.ts | Updates imports to non-barrel paths (middleware + BoltPlugin). |
| backend/src/routes/tasks.ts | Uses shared NodeIdParamSchema; uses createStreamingCallback(). |
| backend/src/routes/puppetHistory.ts | Uses shared NodeIdParamSchema. |
| backend/src/routes/puppet.ts | Uses shared NodeIdParamSchema; uses createStreamingCallback(). |
| backend/src/routes/packages.ts | Uses createStreamingCallback() for streaming hooks. |
| backend/src/routes/inventory.ts | Uses shared NodeIdParamSchema. |
| backend/src/routes/integrations/puppetdb.ts | Updates imports from PuppetDBClient; types catch blocks as unknown. |
| backend/src/routes/hiera.ts | Uses shared NodeParamSchema for nodeId params. |
| backend/src/routes/facts.ts | Uses shared NodeIdParamSchema. |
| backend/src/routes/commands.ts | Uses shared NodeIdParamSchema; uses createStreamingCallback() and IntegrationManager-based inventory check. |
| backend/src/middleware/index.ts | Removes middleware barrel exports. |
| backend/src/middleware/errorHandler.ts | Updates ErrorHandlingService import to non-barrel path. |
| backend/src/integrations/puppetserver/index.ts | Removes Puppetserver barrel exports. |
| backend/src/integrations/puppetserver/PuppetserverService.ts | Consolidates SimpleCache usage via utils/caching. |
| backend/src/integrations/puppetdb/index.ts | Removes PuppetDB barrel exports. |
| backend/src/integrations/puppetdb/PuppetDBService.ts | Consolidates SimpleCache usage via utils/caching. |
| backend/src/integrations/index.ts | Removes integrations barrel exports. |
| backend/src/integrations/hiera/index.ts | Removes Hiera barrel exports. |
| backend/src/integrations/bolt/index.ts | Removes Bolt barrel exports. |
| backend/src/errors/index.ts | Removes errors barrel exports. |
| backend/src/errors/ErrorHandlingService.ts | Improves sensitive-data obfuscation logic and safe-object recursion checks. |
| backend/src/database/index.ts | Removes database barrel exports. |
| backend/src/config/index.ts | Removes config barrel exports. |
| backend/src/bolt/index.ts | Removes bolt barrel exports. |
| README.md | Simplifies documentation sections and updates integration setup links. |
| .kiro/undo/flatten-node-detail-tabs.md | Adds/records implementation plan notes (Kiro). |
| .kiro/todo/docker-improvements.md | Adds Docker improvements TODO list (Kiro). |
| .kiro/summaries/certificate-removal-analysis.md | Adds certificate removal analysis (Kiro). |
| .kiro/summaries/bolt-error-output-fix.md | Adds summary of Bolt error output fix (Kiro). |
| .kiro/scripts/split-routes-analysis.cjs | Adds route-splitting analysis script (Kiro). |
| .kiro/scripts/debug-inventory-linking.js | Adds inventory linking debug script (Kiro). |
| .kiro/done/multi-debug-info-enhancement.md | Adds completion notes for multi-debug info enhancement (Kiro). |
| .kiro/done/lint-fixes.md | Adds lint-fix tracking notes (Kiro). |
| .kiro/done/hiera-class-detection-fix.md | Adds completion notes for hiera class detection fix (Kiro). |
| .kiro/done/expert-mode-test-issues.md | Adds expert-mode test issue notes (Kiro). |
| .kiro/done/expert-mode-stale-debug-info.md | Adds expert-mode stale debug-info fix notes (Kiro). |
| .kiro/done/configuration-analysis.md | Adds configuration investigation notes (Kiro). |
| .kiro/done/certificate-removal-tasks.md | Adds certificate-removal task checklist (Kiro). |
| - [Integration Color Coding](#integration-color-coding) | ||
| - [Expert Mode](#expert-mode) | ||
| - [Realtime Streaming Output](#realtime-streaming-output) | ||
| - [Interpreting Bolt Command Output](#interpreting-bolt-command-output) | ||
| - [Puppet Reports Filtering](#puppet-reports-filtering) |
There was a problem hiding this comment.
The Table of Contents doesn’t include the new "Troubleshooting" section (see header at line 1559), so navigation links are now incomplete. Add a TOC entry for Troubleshooting (or remove/relocate the section if it shouldn’t be in the user guide).
- Remove duplicate and non-existent TOC entries - Fix typos (Detaila→Details, Contrainer→Container, padawi→pabawi) - Standardize capitalization (Bolt, SSH, PuppetCA) - Fix duplicate 'the the' instances - Improve clarity: 'eventual' → 'any required/any external' - Align TOC with actual document structure
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 70 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
docs/integrations/puppetdb.md:362
- The PuppetDB reports query example uses
curl -X GET ... -d '{...}'. Many servers/proxies ignore request bodies on GET, and PuppetDB typically expects queries via POST body or via a?query=URL parameter. Consider switching this example to POST (or using a URL-encodedqueryparameter) so it works reliably.
| * **`specs/`**, the directory where Kiro stores its files for Spc driven development. Subdirs for each implementation plan | ||
| * **`hooks/`**, the directory which Kiro uses to trigger hooks under certain conditions |
There was a problem hiding this comment.
Typo: "Spc driven development" should be "spec-driven development" (or "spec-driven development").
| - name: pabawi | ||
| image: pabawi:0.1.0 | ||
| ports: |
There was a problem hiding this comment.
The Kubernetes manifest example uses image: pabawi:0.1.0, which appears out of date relative to the rest of the docs (v0.5.0). Consider using a placeholder (your-registry/pabawi:<version>) or aligning the tag with the current release to avoid copy/paste deploying an incorrect version.
| if (typeof data === "object" && data !== null) { | ||
| const sanitized: Record<string, unknown> = {}; | ||
| for (const [key, value] of Object.entries(data)) { | ||
| const lowerKey = key.toLowerCase(); | ||
| if ( | ||
| lowerKey.includes("password") || | ||
| lowerKey.includes("token") || | ||
| lowerKey.includes("secret") || | ||
| lowerKey.includes("key") | ||
| ) { | ||
| if (this.shouldObfuscateKey(key)) { | ||
| sanitized[key] = "***"; | ||
| } else { | ||
| } else if (this.isSafeObject(value)) { | ||
| sanitized[key] = this.sanitizeSensitiveData(value); | ||
| } else { | ||
| sanitized[key] = value; | ||
| } |
There was a problem hiding this comment.
sanitizeSensitiveData() no longer recursively sanitizes arrays (and other non-plain objects): Object.entries() will iterate array indices, but isSafeObject(value) returns false for arrays, so nested objects inside arrays (e.g., lists of headers, request objects, or raw API responses) will be returned unsanitized. This can leak tokens/secrets in expert-mode error responses. Consider explicitly handling arrays (map + recurse) and/or broadening the recursion condition to include arrays while still avoiding problematic prototypes.
| npm run test:e2e:headed | ||
| ``` | ||
|
|
||
| See [E2E Testing Guide](docs/e2e-testing.md) for detailed information about end-to-end testing. |
There was a problem hiding this comment.
This link is likely wrong relative to this file’s location: docs/development.md linking to docs/e2e-testing.md will resolve to docs/docs/e2e-testing.md. Use a relative link like ./e2e-testing.md.
| - [PuppetDB Integration Setup](./integrations/puppetdb.md) | ||
| - [Puppetserver Setup](./uppetserver-integration-setup.md) | ||
| - [Configuration Guide](./configuration.md) |
There was a problem hiding this comment.
The Puppetserver setup link points to ./uppetserver-integration-setup.md, which doesn’t exist (typo / outdated path). It should link to the current Puppetserver guide under ./integrations/puppetserver.md.
| - [PuppetDB Integration Setup](./integrations/puppetdb.md) | ||
| - [Puppetserver Setup](./uppetserver-integration-setup.md) | ||
| - [Troubleshooting Guide](./troubleshooting.md) |
There was a problem hiding this comment.
The Puppetserver setup link points to ./uppetserver-integration-setup.md, which doesn’t exist. It should link to the current guide at ./integrations/puppetserver.md.
| - [Configuration Guide](./configuration.md) | ||
| - [PuppetDB Integration Setup](./puppetdb-integration-setup.md) | ||
| - [PuppetDB Integration Setup](./integrations/puppetdb.md) | ||
| - [Puppetserver Setup](./uppetserver-integration-setup.md) | ||
| - [Troubleshooting Guide](./troubleshooting.md) |
There was a problem hiding this comment.
The Puppetserver setup link points to ./uppetserver-integration-setup.md, which doesn’t exist. It should link to the current guide at ./integrations/puppetserver.md.
| To have an idea of Pabawi awesomeness, here some random screenshots | ||
|
|
||
| *Puppet run reports with detailed metrics and Bolt Execution history with filtering* | ||
| <img src="docs/screenshots/pabawi-screenshots.png" alt="Pababi Screenshots" width="1024"> |
There was a problem hiding this comment.
The screenshot alt text has a typo: "Pababi" → "Pabawi".
| <img src="docs/screenshots/pabawi-screenshots.png" alt="Pababi Screenshots" width="1024"> | |
| <img src="docs/screenshots/pabawi-screenshots.png" alt="Pabawi Screenshots" width="1024"> |
| # Create your configuration file in your current directory (paths in .env are relative to the container) | ||
| .env | ||
|
|
||
| # Run the example42/pabawi image mounting your pabawi dir | ||
| docker run -d \ | ||
| --name pabawi \ | ||
| --user "$(id -u):1001" \ # Your user must be able to read all the mounted files | ||
| --user "$(id -u):1001" \ | ||
| -p 127.0.0.1:3000:3000 \ | ||
| --platform "amd64" \ # amd64 or arm64 | ||
| -v "$(pwd):/bolt-project:ro" \ | ||
| -v "$(pwd)/data:/data" \ | ||
| -v "$(pwd)/certs:/certs" \ | ||
| -v "$HOME/.ssh:/home/pabawi/.ssh" \ | ||
| -v "$(pwd):/data" \ | ||
| --env-file ./env \ | ||
| example42/padawi:latest | ||
| example42/pabawi:latest |
There was a problem hiding this comment.
The Docker run example is inconsistent/confusing: it tells users to create a .env file but then passes --env-file ./env, and there’s a standalone .env line that isn’t a command. This will be hard to follow as-is; consider showing an explicit cp ... .env/touch .env step and using --env-file ./.env consistently.
- Remove default type parameters in PuppetDBService and PuppetserverService - Fixes eslint @typescript-eslint/no-unnecessary-type-arguments violations
| # files are in your $(cwd)/pabawi dir which is mounted to /pabawi in the container | ||
|
|
||
| PORT=3000 | ||
| HOST=localhost |
There was a problem hiding this comment.
For Docker deployments, binding to HOST=localhost inside the container will typically make the service unreachable via published ports (the process listens only on the container loopback). Use HOST=0.0.0.0 (or omit HOST if the app defaults appropriately) so port mapping works.
| HOST=localhost | |
| HOST=0.0.0.0 |
| if [[ -f ".env" ]]; then | ||
| env_file=".env" | ||
| print_status "Loading environment from .env in current directory" | ||
| elif [[ -f "$PROJECT_ROOT/backend/.env" ]]; then | ||
| env_file="$PROJECT_ROOT/backend/.env" | ||
| print_status "Loading environment from $env_file" |
There was a problem hiding this comment.
The .env lookup is dependent on the caller’s current working directory (-f ".env"). This makes the script behave differently if invoked from elsewhere (e.g., cd scripts && ./generate-pabawi-cert.sh). Since the script already knows PROJECT_ROOT, prefer checking $PROJECT_ROOT/.env (and/or $PROJECT_ROOT/backend/.env) so the script is location-independent.
| - [Configuration Guide](./configuration.md) | ||
| - [PuppetDB Integration Setup](./puppetdb-integration-setup.md) | ||
| - [PuppetDB Integration Setup](./integrations/puppetdb.md) | ||
| - [Puppetserver Setup](./uppetserver-integration-setup.md) |
There was a problem hiding this comment.
This link target looks wrong (./uppetserver-integration-setup.md is missing the leading p and doesn’t match the new docs layout). Update it to the actual Puppetserver integration guide path (e.g., ./integrations/puppetserver.md).
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: pabawi | ||
| spec: | ||
| selector: | ||
| app: pabawi | ||
| ports: | ||
| - port: 80 | ||
| targetPort: 3000 | ||
| type: LoadBalancer | ||
| ``` |
There was a problem hiding this comment.
The Kubernetes Service example exposes Pabawi as a LoadBalancer on port 80, which will typically make this unauthenticated remote-operations UI reachable from outside the cluster. Because Pabawi has no built-in authentication and can execute arbitrary Bolt commands/tasks on your infrastructure, exposing it directly creates an easy path for an external attacker to run commands across all managed nodes. Consider documenting this risk explicitly and changing the example to use an internal-only Service (e.g., ClusterIP behind an authenticated ingress or VPN) rather than a public LoadBalancer by default.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Alessandro Franceschi <al@example42.com>
No description provided.