Project name constraints only applied to -p, name: config property
#363
Comments
mbland
added a commit
to mbland/compose-go
that referenced
this issue
Mar 4, 2023
Applies the constraints articulated in compose-spec/compose-spec#314 for project names from `COMPOSE_PROJECT_NAME`, the project dir, or the working dir. Before this change, the constraints applied only when using `docker compose -p` or the `name:` config file property. Non-normalized project names from `COMPOSE_PROJECT_NAME`, the project dir, or the working dir were still allowed, which `docker compose` would then normalize. The constraints are enforced by: - `cli.ProjectFromOptions()` setting `withNamePrecedenceLoad()` as an option to `loader.Load()`. - `loader.Load()` applies `withNamePrecedenceLoad()`, which tries to get the project name from the command line options, then `COMPOSE_PROJECT_NAME`, then the project or working dir. - `withNamePrecedenceLoad()` calls `loader.Options.SetProjectName()`, which now also saves the original name as well as the normalized name. - `loader.Load()` then calls `loader.projectName()`, which reads the project name from the config file(s) if it's not already set. - `loader.projectName()` then applies the new `CheckOriginalProjectNameIsNormalized()` function extracted from `cli.WithName()`. Only a handful of test cases required updating in `cli/options_test.go`. These test cases were checking that invalid project names were rejected by `cli.WithName()`, but that `cli.ProjectFromOptions()` would normalize them. Now `cli.ProjectFromOptions()` rejects them with the same error. There's no need for new test cases specifically for `COMPOSE_PROJECT_NAME` or directory-based project names, because the `CheckOriginalProjectNameIsNormalized()` function is applied regardless of the source. Other existing test cases are validating that `cli.ProjectFromOptions` does also use `COMPOSE_PROJECT_NAME` or directory names when necessary. Closes compose-spec#363. Signed-off-by: Mike Bland <[email protected]>
|
As you can see, I got the itch and filed #364. I'm in no rush, but wanted to get it drafted while it was still fresh in my mind. I understand if there's some caution regarding how this might affect existing configurations, if there's other changes required, or if you'd prefer to pass on this change. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The project name constraints articulated in compose-spec/compose-spec#314 are only enforced when using
docker compose -por thename:config file property (once #362 lands indocker compose). Non-normalized project names fromCOMPOSE_PROJECT_NAME, the project dir, or the working dir are still allowed, whichdocker composewill then normalize.I'm happy to implement the necessary changes to enforce the project name constraints across all mechanisms. I'm also happy to update the documentation PR to reflect the current behavior instead, if it's desirable to leave the current behavior as is for some reason.
Background
I noticed this while preparing the following documentation change after having compose-spec/compose-spec#314 and #362 merged today:
I experimented locally using all the mechanisms described in that change to confirm the current behavior.
It seems the JSON schema change from #362 will catch invalid
name:values in the config (per the loader/loader_test.go failure in that PR) once it lands indocker compose. However, theCOMPOSE_PROJECT_NAME, project dir, and current dir mechanisms would need an update.Maybe users are less likely to encounter issues when the project name is defined via the other mechanisms. Still, it would seem more correct and less surprising to enforce the project name constraints across all the available mechanisms.
Related issues
The text was updated successfully, but these errors were encountered: