Skip to content

Fix #3153 - Validate tag before build using OCI regex - #3191

Merged
milas merged 4 commits into
docker:mainfrom
LombardiDaniel:main
Dec 5, 2023
Merged

Fix #3153 - Validate tag before build using OCI regex#3191
milas merged 4 commits into
docker:mainfrom
LombardiDaniel:main

Conversation

@LombardiDaniel

@LombardiDaniel LombardiDaniel commented Dec 1, 2023

Copy link
Copy Markdown
Contributor

Fix #3153, but guarantees that it woud raise the appropriate error.

The regex expression is:
^[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(\/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*(:[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127})?$

I made it by combining ^[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(\/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)* and the (:[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127})? group. Both expressions are from the opencontainers/distribution-spec repo, as cited in the docker docs.

I also wrote a test for testing a build function call with erroneous tag value.

@LombardiDaniel LombardiDaniel changed the title check for tag match before build using OCI regex Validate tag before build using OCI regex Dec 1, 2023
@LombardiDaniel LombardiDaniel changed the title Validate tag before build using OCI regex Validate tag before build using OCI regex - Fix #3153 Dec 1, 2023
@LombardiDaniel LombardiDaniel changed the title Validate tag before build using OCI regex - Fix #3153 Fix #3153 - Validate tag before build using OCI regex Dec 1, 2023

@milas milas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR! I just kicked off CI and there's some failures - once those are taken care of, I'll get this merged:

  • Lint errors - make ruff (will run inside Docker container)
  • Unit tests (need to update example tag) - make unit-test-py3 (will run inside Docker container)

Let me know if you have any trouble

Signed-off-by: Daniel Lombardi <lombardi.daniel.o@gmail.com>
Signed-off-by: Daniel Lombardi <lombardi.daniel.o@gmail.com>
…l.com>

Signed-off-by: Daniel Lombardi <lombardi.daniel.o@gmail.com>
…di.daniel.o@gmail.com>

Signed-off-by: Daniel Lombardi <lombardi.daniel.o@gmail.com>
@LombardiDaniel

Copy link
Copy Markdown
Contributor Author

@milas Done! the error was due to me changing the exception type and not updating the test!

@milas milas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍🏻

Out of curiosity, was it you or the linter that reformatted the test file? (Not a big deal, just surprised)

@milas
milas merged commit a9b5494 into docker:main Dec 5, 2023
@LombardiDaniel

Copy link
Copy Markdown
Contributor Author

No no, i had made a change to better represent the error, before it was TypeError but changed to errors.DockerException to better represent the scenario. But had forgotten to change the test accordingly

@consideRatio

Copy link
Copy Markdown

I opened #3240 related to this PR

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.

build images with tag with https prefix never ends

3 participants