Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix checking Reaper and the SSHd containers when _HUB_IMAGE_NAME_PREFIX provided #751

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

silh
Copy link
Contributor

@silh silh commented Apr 9, 2024

Additional fix for the issue #747.

Thanks to @joyrex2001 for pointing to ruyk image name comparison problems.

The changes were tested with specifying TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX=docker.io for the test run.

Copy link

netlify bot commented Apr 9, 2024

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 7d206c9
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/6616ca96cbde710007fe4521
😎 Deploy Preview https://deploy-preview-751--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@silh
Copy link
Contributor Author

silh commented Apr 9, 2024

As we set the reaper's image name it's not possible to add an autotest for that, unless this value is calculated every time.

@cristianrgreco cristianrgreco added bug Something isn't working patch Backward compatible bug fix labels Apr 9, 2024
@cristianrgreco
Copy link
Collaborator

@silh is this related? #752. Could you please have a look at the codeql warning to see if it's relevant?

@silh
Copy link
Contributor Author

silh commented Apr 10, 2024

@silh is this related? #752. Could you please have a look at the codeql warning to see if it's relevant?

Hey @cristianrgreco, codeql was right. I fixed that regex.

@silh
Copy link
Contributor Author

silh commented Apr 10, 2024

Regarding #752 - it was fixed in #748 partially (at least the 401 problem). This PR also should fix it working with TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX having a value such as "custom.domain.com/". Previously this would create an incorrect image name of "/real_image_name"

… consisting of a registry with a slash but without path
@cristianrgreco
Copy link
Collaborator

Thanks for pushing these fixes @silh. I'll merge and release once the build is passing

@cristianrgreco cristianrgreco changed the title Fixed checking the reaper and the sshd containers when _HUB_IMAGE_NAME_PREFIX is specified Fix checking Reaper and the SSHd containers when _HUB_IMAGE_NAME_PREFIX provided Apr 10, 2024
@silh
Copy link
Contributor Author

silh commented Apr 10, 2024

Hey @cristianrgreco, now reading it #752 more thoroughly I am not entirely sure if these fixes would resolve the problem.

@AlessioFranceschi are you able to use your ECR images without TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX but specigying their name directly when creating a container? If so - then the recent changes should help solving the issue.

@AlessioFranceschi
Copy link

Hey @cristianrgreco, now reading it #752 more thoroughly I am not entirely sure if these fixes would resolve the problem.

@AlessioFranceschi are you able to use your ECR images without TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX but specigying their name directly when creating a container? If so - then the recent changes should help solving the issue.

Yes by not using the env but putting the registry url together with the image name it works. Thanks

@cristianrgreco cristianrgreco merged commit 58e57df into testcontainers:main Apr 11, 2024
118 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Backward compatible bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants