Repository navigation
fix(environment): compose project scoping, the http host port, and the openapi lint - #42
Merged
Merged
Conversation
…cation ignis and buem-gateway both defaulted HOST_PORT to 8080, and both declare the compose project name building-simulation because they are expected to run side by side. Starting the second failed on the bind with an error naming neither service. 8080 was not ignis's to keep either way. The EnerPlanET platform's Keycloak binds 0.0.0.0:8080 and is part of the platform stack rather than an optional component, so anyone bringing up environment/http alongside the platform hit the same failure. Binding to loopback did not avoid it: a wildcard bind holds the port on every interface, so a later bind to 127.0.0.1:8080 fails with address already in use, and the reverse order fails too. Verified against a host with Keycloak running. 8088 rather than a nearer port: 8081 is buem-gateway's, and 8083 to 8087 and 8089 are each declared as a host port somewhere in the workspace, 8087 being the geothermal service's. 8088 is declared nowhere and bound by nothing. Both .env.example files now name the whole allocation rather than warning in general terms that a host port can collide. A general warning is what failed to stop two repos settling on the same value. The https pairing of 443 and 8443 is unchanged and is recorded the same way. Nothing about the container side moves. APP_PORT stays 8080, so only the left half of the published mapping changes and the image, the health check and the proxy upstream are untouched.
Building Configurator's dev server falls back to 5174 when 5173 is taken. The CORS middleware compares Origin by exact string, so localhost and 127.0.0.1 are different origins and a page served from either spelling needs its own entry, as http://localhost:8000 and http://127.0.0.1:8000 already have. Local development origins only; a deployment sets ALLOWED_ORIGINS to the real browser origins, or leaves it unset for server-to-server callers.
Removing the API key took the top-level `security` and `securitySchemes` blocks with it. Redocly's recommended ruleset requires every operation to have security defined on it or at the root, so the docs workflow's lint step failed with one error per operation and the MkDocs deploy stopped running. An empty list is the OpenAPI way to say that no scheme is required, which is what is true here, so the spec now states it rather than leaving it unstated. An ignore file would have suppressed the report without recording the fact. Two `operation-4xx-response` warnings remain, on GET /ignis/health and GET /api/v1/fields. Both had 403 as their only 4XX and neither can return a 4XX now: health takes no input and fields is static metadata with no parameters. Documenting a status code the server never sends would be worse than the warning. Warnings do not fail the step.
Every compose file declared `building-simulation`, and so did
buem-gateway's, on the reasoning that a project name groups the
containers of one concern. A Compose project is not a label. It is the
unit Compose takes destructive action against, and sharing one across
repositories put a cross-repository delete behind a flag people use
casually.
Measured with two throwaway compose files sharing a project name:
up in each no orphan warning at all
ps from one lists the other file's container
down from one removes only its own, then fails to
remove the shared network, still in use
down --remove-orphans from one removes the other file's container,
silently, nothing naming what it took
So `docker compose down --remove-orphans` in buem-gateway destroyed the
running ignis stack, with no message identifying it. The projects are now
ignis-http and ignis-https, and buem-gateway declares its own. Neither
repository resolves the other by container name, so the shared network
carried nothing and separate projects cost nothing.
Splitting by transport alone was rejected: ignis-http and buem-gateway's
http stack would have stayed in one project, and that is the pair meant
to run side by side.
ignis-db-data is pinned to its existing name. Compose prefixes a volume
with the project name unless pinned, so without this the two environments
would mount separate empty databases and switching transport would lose
the data every time, not once at upgrade. The pin also means an existing
checkout needs no reseed. caddy-data is left unpinned: it exists only in
the quickstart file, where the CA is deliberately never added to a trust
store, so regenerating it costs one click-through.
Container names are unchanged and remain unique across the host, which is
what still keeps the two environments from running at once. An old stack
must come down before the renamed one starts; `up` fails on the container
name rather than replacing it.
The note gave `docker compose -p building-simulation down` for coming down from a pre-rename checkout. That targets the project, not this repository, so on a machine where another service still declares the old project name it removes that service's containers too, naming neither. It is the exact hazard this rename exists to close, published as the upgrade step. Removing the three containers by name reaches nothing else, since container names are unique across the host, and `docker stop` before `docker rm` gives the database a clean shutdown rather than a SIGKILL. The project form is now called out as one not to use, with the reason, because it is the form that looks more correct.
`up` reports that building-simulation_ignis-db-data was created for a different project and suggests `external: true`. The warning is inherent to the pin: one volume is deliberately shared by two Compose projects, so whichever project did not create it is always the one Compose complains about. Taking the suggestion would break a first run. An external volume must exist before `up`, so a clean machine would fail rather than create it, and `cd environment/http && docker compose up` is the thing these directories exist to make work. Recorded in the compose files and the getting-started guide so the suggestion is not followed later.
fa48ddd's message and ADR-005 both stated that no other repository resolved these containers by name, so separate networks cost nothing. That was wrong. tentacron had joined building-simulation_default and resolved ignis-app by container name; the project rename broke it with "no such host" until it joined the new network. The commit message cannot be corrected without rewriting a pushed branch, so the correction is recorded here and in the ADR, which is what a later reader consults. The general problem is now the open question on ADR-005. A Compose project's default network takes its name from the project, which makes it an implementation detail rather than an interface, and a caller that attaches to one gets no signal when it changes: the service starts normally and fails at its first outbound call. That is how this was found, downstream, by someone chasing timeouts rather than reading a start-up error. A purpose-named external network is the likely answer and is not applied here. Declaring an external network that does not exist makes a Compose file fail to start, verified the same way as the equivalent volume case, so it belongs in an opt-in overlay rather than in a base file that has to work on a clean machine with no prior setup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mainis currently red and the docs site has not deployed since #41 merged. The first commit here fixes that. The rest closes two ways the Compose setup could bite: a default host port that could never bind, and a project name shared across repositories.1. The OpenAPI spec no longer lints
This is the part that needs merging soon. Removing the API key in #41 took the top-level
securityandsecuritySchemesblocks with it. Redocly's recommended ruleset requires security to be defined per-operation or at the root, soLint OpenAPI specificationin the MkDocs workflow now fails with one error per operation, seven in total. Themkdocs buildand deploy steps run after it, so the published site has been frozen since the merge.The fix is one line:
That is the OpenAPI way to say no scheme is required, which is what is true here.
redocly lint --generate-ignore-filewould also have silenced it, but an ignore file suppresses the report without recording the fact, and the fact is the point.Two
operation-4xx-responsewarnings remain, onGET /ignis/healthandGET /api/v1/fields. Both had403as their only 4XX and neither can return one now: health takes no input, fields is static metadata with no parameters. Documenting a status code the server never sends would be worse than the warning, and warnings do not fail the step.2. The HTTP host port moves from 8080 to 8088
8080 collides with the platform. Keycloak binds
0.0.0.0:8080and is part of the EnerPlanET platform stack rather than an optional component. Anyone runningenvironment/httpalongside the platform gotaddress already in use, with an error naming neither service.HOST_BIND=127.0.0.1does not help: a wildcard bind holds the port on every interface, so a later loopback bind fails, and the reverse order fails too. Measured, not reasoned about:This had not bitten anyone only because
ignis-apppublishes no port in the HTTPS environment. It appears the first time someone follows the HTTP quickstart, which is the path the getting-started guide now leads with.8080 also collided with buem-gateway, which published the same default. Why 8088: 8081 is buem-gateway's, and 8083 to 8087 and 8089 are each declared as a host port somewhere in the workspace, 8087 being the geothermal service's. 8088 is declared nowhere and bound by nothing.
APP_PORTstays 8080. Only the left half of the published mapping moves, so the image, the health check and the proxy upstream are untouched.3. The Compose project is scoped per repository and transport
Every compose file here declared
name: building-simulation, and so did buem-gateway's, on the reasoning that a project name groups the containers of one concern. That reasoning was mine and it was wrong. A Compose project is not a label; it is the unit Compose takes destructive action against. Two throwaway compose files sharing a project name:upin eachpsfrom onedownfrom onedown --remove-orphansfrom oneSo
docker compose down --remove-orphansin buem-gateway destroyed the running ignis stack with no message identifying it. Projects are nowignis-httpandignis-https. Neither repository resolves the other by container name, so the shared network carried nothing and separate projects cost nothing.Splitting by transport alone (
building-simulation-http) was considered and rejected:ignis-httpand buem-gateway's http stack would have stayed in one project, and that is exactly the pair meant to run side by side.ignis-db-datais pinned to its existing volume name. Compose prefixes a volume with the project name unless pinned, so without this the two environments would mount separate empty databases and switching transport would lose the data every time, not once at upgrade. The pin also means an existing checkout needs no reseed.caddy-datais left unpinned: it exists only in the quickstart file, where the CA is deliberately never added to a trust store, so regenerating it costs one click-through.Container names are unchanged and remain unique across the host, which is what still keeps the two environments from running at once.
!!! warning
An existing stack must come down before the renamed one starts.
upfails on the container name rather than replacing it. The getting-started guide carries this note.Please check my judgement here
Both
.env.examplefiles name the full port allocation, including buem-gateway's number, rather than warning in general terms that a host port can collide. The general warning is exactly what failed to stop two repos settling on the same value. The cost is a second place that goes stale if either service moves again. That trade looks right at two repos and would stop being right at four, at which point the allocation wants one shared place both repos point at.4. Two more browser origins for local development
environment/{http,https}/env/common.envgainhttp://localhost:5174and the127.0.0.1spelling of both Vite ports. The CORS middleware comparesOriginby exact string, solocalhostand127.0.0.1are different origins and a page served from either spelling needs its own entry. The file already paired both spellings for port 8000. Development origins only.Base URL change
Anything pointing at
http://localhost:8080for the HTTP environment becomeshttp://localhost:8088. Updated here:docs/api.md,docs/getting-started.md, the OpenAPIserverslist, anddocumentation/content/07-deployment-view.md.Verification
npx @redocly/cli lint --format=github-actionsmkdocs build --strictmake -C documentation(arc42 PDF)docker compose config, all five filesignis-http/ignis-https,ignis-db-datapinned, published 8088localhost:8080,HOST_PORT:-8080, straybuilding-simulationThe stacks were not booted from this branch: both environments share container names, so bringing one up would have replaced the HTTPS stack running on the host.
ADR-005 in
documentation/content/09-architectural-decisions.mdis rewritten to record the project-naming decision and close the open question it carried, which had anticipated exactly this ("two repos each declaring the same namespace will coexist but Compose may warn about orphan containers").Still outstanding from #41
documentation/figures/06-runtime-view-sequence.pdfand07-deployment-view.drawio.pdfstill draw the API-key sequence and the 403. Re-exporting them fromdocumentation/figures/diagrams.drawioneeds drawio, so the prose and captions were corrected and the figures deliberately left alone rather than edited into an inconsistent state.