docs: port ranges, OpenSearch default image, quick-start fixes, MWAA index row, parity TODOs - #2690
docs: port ranges, OpenSearch default image, quick-start fixes, MWAA index row, parity TODOs#2690sam-fakhreddine wants to merge 8 commits into
Conversation
…xy port ranges (cherry picked from commit 8424039361a5f280d4fa04aba527f22787411d17)
…DPOINT variable (cherry picked from commit efee53d0e044b8ea2200e3df6d4794a1bb89856f)
…ault (cherry picked from commit 8372880983fd7a4803d78f8e9d8389c0177d7f3e)
…iption The MWAA webserver proxy listed in the same section forwards HTTP, so describing every in-Floci proxy as a TCP proxy is inaccurate. (cherry picked from commit e362f4c5e928f1775f3530d4cd5f6c6afbe5395a)
…il nav entry MWAA has had a page and nav entry since it shipped but was never added to the services table. Separately, mkdocs.yml listed CloudTrail twice in the nav.
|
| Filename | Overview |
|---|---|
| docs/configuration/ports.md | Adds missing proxy ranges and now lists only the current Lambda Runtime API range. |
| docs/getting-started/quick-start.md | Corrects the endpoint variable, ECR registry port, and tightly scoped firewall ranges. |
| docs/service-api-parity-todos.md | Adds a consolidated, linked inventory of service compatibility work. |
| docs/services/index.md | The attempted MWAA addition was correctly deduplicated, leaving one consistent canonical row. |
| mkdocs.yml | Removes the duplicate CloudTrail navigation entry without disturbing service navigation. |
Reviews (4): Last reviewed commit: "docs(issues-0009): rewrite as project-fa..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR batches several documentation corrections across Floci’s user/docs site: port-range references, environment variable defaults, quick-start commands, MkDocs navigation, and a new parity TODO inventory.
Changes:
- Updates port-range documentation (adds MemoryDB/Neptune/MWAA proxies; adjusts proxy wording) and quick-start examples (ECR endpoint var + registry port).
- Corrects OpenSearch default-image documentation to reflect that
FLOCI_SERVICES_OPENSEARCH_DEFAULT_IMAGEis unset by default and resolved perEngineVersion. - Adds MWAA to the services index table, removes a duplicate CloudTrail nav entry, and introduces a consolidated parity TODO document.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates documented default for FLOCI_SERVICES_OPENSEARCH_DEFAULT_IMAGE to “unset” behavior. |
| mkdocs.yml | Removes a duplicate CloudTrail navigation entry. |
| docs/services/index.md | Adds MWAA to the services operation-count table. |
| docs/service-api-parity-todos.md | New consolidated parity/TODO inventory document. |
| docs/getting-started/quick-start.md | Fixes ECR quick-start commands and adds tighter UFW examples. |
| docs/configuration/ports.md | Expands the port-range reference to cover more proxy services and adjusts proxy wording. |
| docs/configuration/environment-variables.md | Corrects OpenSearch default-image documentation to match “unset” behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…docs Address PR floci-io#2690 review findings (Greptile/Copilot): - docs/configuration/ports.md and docs/getting-started/quick-start.md still listed the retired Lambda Runtime API range (9200-9299) and a never-valid ECR range (5000-5099) alongside the current 12000-12499 and 5100-5199 ranges. Drop the stale entries. - docs/services/index.md had a duplicate MWAA row (added by 9a8a6e5) with a conflicting endpoint description; the service matrix already carried MWAA. Remove the duplicate; verified via `make docs-sync` that no other tables are stale. - docs/service-api-parity-todos.md links to four investigation epic docs (issues/0009, codebuild-local-agent, codepipeline-v2, network-data-plane) that existed on throwaway/rebase-onto-upstream but were never merged into any docs branch. Restore them from that branch so the links resolve instead of stripping the references.
pgermosen
left a comment
There was a problem hiding this comment.
The six described fixes check out — I spot-checked the OpenSearch default-image claim directly against EmulatorConfig.java (the field really is an unannotated Optional, no fixed default, confirming the doc correction) and confirmed the port-range and duplicate-MWAA-row bot findings are resolved.
Before going further though, the PR summary describes six small fixes, but the diff also includes four larger files — three multi-hundred-line investigation/architecture documents and one issue-tracking doc — that aren't mentioned anywhere in the description. I see from the thread that these were restored from another branch specifically to fix broken links a new file introduced, so I understand how they got here. Three of the four (the CodeBuild local-agent, CodePipeline V2, and network-data-plane investigation docs) read as legitimate, general project documentation — decision frameworks written for the project rather than for one person — so I don't have a content objection to them, just a visibility one: a reviewer relying on the summary alone would have no idea they're part of this PR at all.
The fourth one, the account+region-scoping audit issue doc, is a different case worth a closer look. It references a workflow script, a specific automation tool, and a particular subagent/model orchestration setup, none of which exist anywhere in this repository, and its own decision log reads in the voice of one person's private working notes rather than something another contributor could pick up and act on. The underlying technical concern it describes, ambient account or region resolution being used in place of an already-known explicit value, sounds like a genuinely worthwhile thing to track for this project. But as written, nobody besides the author can act on the file, since the tooling it references doesn't exist here. Would you be open to either rewriting that one as a project-facing issue describing just the bug pattern and the affected files, or letting me know if there's a reason it should stay as-is?
…s tooling refs pgermosen (PR floci-io#2690 review) flagged that this file referenced a workflow script, a subagent/model orchestration setup, and a shell script under scripts/static-checks/ that don't exist anywhere in this repo, and read like one person's private working notes rather than something another contributor could act on. Rewrite it to describe the underlying bug pattern (ambient account/region resolution used in place of an already-resolved explicit value), cite the three confirmed fixes by commit SHA instead of nonexistent issue file paths, and replace the proprietary detection tooling with a plain shell one-liner any contributor can run. The candidate file list, service-domain scope, and the AWS global-services exclusion list are unchanged.
|
Thanks for the close look, and for spot-checking the OpenSearch claim against On visibility: you're right that the summary didn't mention the four restored files. Updated the PR description to call out that On the fourth file, Pushed as |
…s tooling refs pgermosen (PR floci-io#2690 review) flagged that this file referenced a workflow script, a subagent/model orchestration setup, and a shell script under scripts/static-checks/ that don't exist anywhere in this repo, and read like one person's private working notes rather than something another contributor could act on. Rewrite it to describe the underlying bug pattern (ambient account/region resolution used in place of an already-resolved explicit value), cite the three confirmed fixes by commit SHA instead of nonexistent issue file paths, and replace the proprietary detection tooling with a plain shell one-liner any contributor can run. The candidate file list, service-domain scope, and the AWS global-services exclusion list are unchanged.
483747e to
0784ef2
Compare
Summary
Six small, independent documentation fixes, batched into one PR to avoid opening six near-trivial PRs:
docs/configuration/ports.md, which previously only covered ElastiCache and RDS.FLOCI_SERVICES_OPENSEARCH_DEFAULT_IMAGEis unset by default (images resolve per requestedEngineVersion, seeOpenSearchDomainManager.java:151/EmulatorConfig.java:1045,Optional<String> defaultImage()), butREADME.mdanddocs/configuration/environment-variables.mdboth documented a fixed default image string. Corrected both.$AWS_ENDPOINTvariable (the guide defines$AWS_ENDPOINT_URLearlier but the ECR section used the wrong name).docs/services/index.md(MWAA has had a service page and nav entry since it shipped but was never added to this specific table), and dropped a duplicateCloudTrailnav entry frommkdocs.yml.docs/service-api-parity-todos.md, consolidating scattered service API parity TODOs into one tracked document. This doc links to four investigation/epic files that existed on an unmerged branch but not on any docs branch — restoring them was needed to avoid shipping broken links: three general project docs (docs/services/codebuild-local-agent-investigation-epic.md,docs/services/codepipeline-v2-epic.md,docs/services/network-data-plane-investigation-epic.md) and one issue-tracking doc (issues/0009-epic-account-region-scoping-audit.md, rewritten to drop references to tooling that doesn't exist in this repo and read as a project-facing bug-pattern writeup instead of private notes).All content was verified as still current against
upstream/mainbefore pushing — this is a docs-only branch that had drifted since being cut, so it was rebased onto currentupstream/mainfirst, and each fix was re-checked against the current code/docs state rather than assumed still applicable.Deliberate scope notes
feature/docs, previously at a base ~128 commits behind currentmain). That earlier push contained 4 of these 6 commits (same fixes, different SHAs after rebase); the 2 newest commits (MWAA operations-table row + parity-todos consolidation) were not yet present there. No open PR existed against that branch, so it was force-updated in place rather than opened as a second competing PR..javafiles touched, so the wire-fidelity extractor and local code-review steps in the PR-maintenance runbook do not apply.Type of change
AWS Compatibility
Checklist
upstream/mainrtk mvn --ultra-compact clean test-compile— BUILD SUCCESS (no non-doc files changed)make docs-check— passes clean