Skip to content

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

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

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

Conversation

@LombardiDaniel
Copy link
Copy Markdown
Contributor

@LombardiDaniel LombardiDaniel commented Dec 1, 2023

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
Copy link
Copy Markdown
Contributor

@milas milas left a comment

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!

Copy link
Copy Markdown
Contributor

@milas milas left a comment

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