Skip to content

Require normalized project names#314

Merged
glours merged 1 commit intocompose-spec:masterfrom
mbland:update-project-name-spec
Mar 3, 2023
Merged

Require normalized project names#314
glours merged 1 commit intocompose-spec:masterfrom
mbland:update-project-name-spec

Conversation

@mbland
Copy link
Contributor

@mbland mbland commented Mar 1, 2023

What this PR does / why we need it:

This brings the compose-spec in line with current docker compose behavior regarding acceptable project names.

compose-spec/compose-go#261 changed docker compose behavior to require normalized project names as input, instead of normalizing project names automatically. This landed in compose-spec/compose-go v1.2.5 and docker/compose v2.5.1.

However, the current error message "... is not a valid project name" gives no indication why a name isn't valid. This is surprising for users whose previously working project names no longer work with newer versions of docker compose. The valid format also isn't yet documented anywhere.

This is the first step towards documenting the valid format. If this change lands, I can follow up with corresponding changes to:

I also plan to try updating WithName from cli/options.go in the compose-spec/compose-go repo to expand upon the error message.

Which issue(s) this PR fixes:

Fixes #311.

Fixes docker/compose#9741.

Fixes docker/compose#9741.

compose-spec/compose-go#261 changed `docker compose` behavior to require
normalized project names as input, instead of normalizing project names
automatically. This landed in compose-spec/compose-go v1.2.5 and
docker/compose v2.5.1.

However, the current error message "... is not a valid project name"
gives no indication why a name isn't valid. This is surprising for users
whose previously working project names no longer work with newer
versions of `docker compose`. The valid format also isn't yet documented
anywhere.

This is the first step towards documenting the valid format. If this
change lands, I can follow up with corresponding changes to:

- https://docs.docker.com/compose/reference/#use--p-to-specify-a-project-name
- https://docs.docker.com/compose/environment-variables/envvars/#compose_project_name
- https://docs.docker.com/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

I also plan to try updating `WithName` from cli/options.go in the
compose-spec/compose-go repo to expand upon the error message.

Signed-off-by: Mike Bland <mbland@acm.org>
Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks :)

@mbland
Copy link
Contributor Author

mbland commented Mar 3, 2023

@laurazard @ndeloof Thanks for the approvals. When might someone merge it?

Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@glours glours merged commit 58de9f4 into compose-spec:master Mar 3, 2023
@glours
Copy link
Contributor

glours commented Mar 3, 2023

@mbland do you have PR ready for compose-go?

@mbland mbland deleted the update-project-name-spec branch March 3, 2023 14:43
@mbland
Copy link
Contributor Author

mbland commented Mar 3, 2023

@glours Thanks for the approval and merge. I've just cooked up a working change for compose-go and will have a PR out in about an hour.

mbland added a commit to mbland/compose-go that referenced this pull request Mar 3, 2023
PR compose-spec#261 changed `docker compose` behavior to require normalized project
names as input, instead of normalizing project names automatically. This
landed in compose-spec/compose-go v1.2.5 and docker/compose v2.5.1.

However, the previous error message gave no indication why a name isn't
valid.  This is surprising for users whose previously working project
names no longer work with newer versions of `docker compose`.

As of compose-spec/compose-spec#314, the spec now contains the text:

> Project name MUST contain only lowercase letters, decimal digits,
> dashes, and underscores, and MUST begin with a lowercase letter or
> decimal digit.

This updated error message provides a concise version of this
requirement based on regular expression character range notation.

See also:

- compose-spec/compose-spec#311
- docker/compose#9741

Signed-off-by: Mike Bland <mbland@acm.org>
@mbland
Copy link
Contributor Author

mbland commented Mar 3, 2023

@glours Took me a little over an hour to get around to it, but it's up: compose-spec/compose-go#362.

mbland added a commit to mbland/compose-go that referenced this pull request Mar 3, 2023
PR compose-spec#261 changed `docker compose` behavior to require normalized project
names as input, instead of normalizing project names automatically. This
landed in compose-spec/compose-go v1.2.5 and docker/compose v2.5.1.

However, the previous error message gave no indication why a name isn't
valid.  This is surprising for users whose previously working project
names no longer work with newer versions of `docker compose`.

As of compose-spec/compose-spec#314, the spec now contains the text:

> Project name MUST contain only lowercase letters, decimal digits,
> dashes, and underscores, and MUST begin with a lowercase letter or
> decimal digit.

This updated error message provides a concise version of this
requirement based on regular expression character range notation.

See also:

- compose-spec/compose-spec#311
- docker/compose#9741

Signed-off-by: Mike Bland <mbland@acm.org>
mbland added a commit to mbland/compose-go that referenced this pull request Mar 3, 2023
This update landed in compose-spec/compose-spec#314, reflecting the new
project name spec:

> Project name MUST contain only lowercase letters, decimal digits,
> dashes, and underscores, and MUST begin with a lowercase letter or
> decimal digit.

Signed-off-by: Mike Bland <mbland@acm.org>
mbland added a commit to mbland/docs that referenced this pull request Mar 3, 2023
These changes reflect current `docker compose` behavior regarding
acceptable project names.

compose-spec/compose-go#261 changed `docker compose` behavior to require
normalized project names as input, instead of normalizing project names
automatically. This landed in compose-spec/compose-go v1.2.5 and
docker/compose v2.5.1.

compose-spec/compose-spec#314 updated the compose spec, and
compose-spec/compose-go#362 added information to the error message
specifying the correct project name format.

Local development URLs:
- http://localhost:4000/compose/reference/#use--p-to-specify-a-project-name
- http://localhost:4000/compose/environment-variables/envvars/#compose_project_name
- http://localhost:4000/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Production URLs:
- https://docs.docker.com/compose/reference/#use--p-to-specify-a-project-name
- https://docs.docker.com/compose/environment-variables/envvars/#compose_project_name
- https://docs.docker.com/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Signed-off-by: Mike Bland <mbland@acm.org>
mbland added a commit to mbland/compose-go that referenced this pull request 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 <mbland@acm.org>
mbland added a commit to mbland/docs that referenced this pull request Mar 19, 2023
These changes reflect current `docker compose` behavior regarding
acceptable project names.

compose-spec/compose-go#261 changed `docker compose` behavior to require
normalized project names as input, instead of normalizing project names
automatically. This landed in compose-spec/compose-go v1.2.5 and
docker/compose v2.5.1.

compose-spec/compose-spec#314 updated the compose spec, and
compose-spec/compose-go#362 added information to the error message
specifying the correct project name format.

Local development URLs:
- http://localhost:4000/compose/reference/#use--p-to-specify-a-project-name
- http://localhost:4000/compose/environment-variables/envvars/#compose_project_name
- http://localhost:4000/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Production URLs:
- https://docs.docker.com/compose/reference/#use--p-to-specify-a-project-name
- https://docs.docker.com/compose/environment-variables/envvars/#compose_project_name
- https://docs.docker.com/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Signed-off-by: Mike Bland <mbland@acm.org>
mbland added a commit to mbland/docs that referenced this pull request Mar 19, 2023
These changes reflect updated `docker compose` behavior regarding
acceptable project names.

compose-spec/compose-go#261 changed `docker compose` behavior to require
normalized project names as input when using `-p`. Previously `docker
compose` normalized project names automatically, leading to errors for
some users after the change landed in compose-spec/compose-go v1.2.5 and
docker/compose v2.5.1.

compose-spec/compose-spec#314 updated the compose spec, effectively
enforcing the same constraint for the `name:` config file property.
compose-spec/compose-go#362 added information to the error message
specifying the correct project name format. compose-spec/compose-go#364
added enforcement of the new project name requirements for all project
name mechanisms (`-p`, `name:`, `COMPOSE_PROJECT_NAME`, project dir,
working dir).

Local development URLs:
- http://localhost:4000/compose/reference/#use--p-to-specify-a-project-name
- http://localhost:4000/compose/environment-variables/envvars/#compose_project_name
- http://localhost:4000/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Production URLs:
- https://docs.docker.com/compose/reference/#use--p-to-specify-a-project-name
- https://docs.docker.com/compose/environment-variables/envvars/#compose_project_name
- https://docs.docker.com/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Signed-off-by: Mike Bland <mbland@acm.org>
aevesdocker pushed a commit to docker/docs that referenced this pull request Mar 22, 2023
* Add detailed project name requirements, mechanisms

These changes reflect updated `docker compose` behavior regarding
acceptable project names.

compose-spec/compose-go#261 changed `docker compose` behavior to require
normalized project names as input when using `-p`. Previously `docker
compose` normalized project names automatically, leading to errors for
some users after the change landed in compose-spec/compose-go v1.2.5 and
docker/compose v2.5.1.

compose-spec/compose-spec#314 updated the compose spec, effectively
enforcing the same constraint for the `name:` config file property.
compose-spec/compose-go#362 added information to the error message
specifying the correct project name format. compose-spec/compose-go#364
added enforcement of the new project name requirements for all project
name mechanisms (`-p`, `name:`, `COMPOSE_PROJECT_NAME`, project dir,
working dir).

Local development URLs:
- http://localhost:4000/compose/reference/#use--p-to-specify-a-project-name
- http://localhost:4000/compose/environment-variables/envvars/#compose_project_name
- http://localhost:4000/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Production URLs:
- https://docs.docker.com/compose/reference/#use--p-to-specify-a-project-name
- https://docs.docker.com/compose/environment-variables/envvars/#compose_project_name
- https://docs.docker.com/engine/reference/commandline/compose/#use--p-to-specify-a-project-name

Signed-off-by: Mike Bland <mbland@acm.org>

* Revert _data/compose-cli/docker_compose.yaml

This reverts part of commit 9ce29a8.

As @glours noted in the review of #16915, this content comes from the
upstream docker/compose repo. He's opened docker/compose#10390 to apply
the update there instead.

Signed-off-by: Mike Bland <mbland@acm.org>

---------

Signed-off-by: Mike Bland <mbland@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Require normalized project names docker compose restricts project name, where the spec does not

4 participants