Skip to content

Fix trailing slash on Image Prefix#2747

Merged
mdelapenya merged 2 commits intotestcontainers:mainfrom
driverpt:fix-trailing-slash-on-image-prefix
Aug 27, 2024
Merged

Fix trailing slash on Image Prefix#2747
mdelapenya merged 2 commits intotestcontainers:mainfrom
driverpt:fix-trailing-slash-on-image-prefix

Conversation

@driverpt
Copy link
Contributor

@driverpt driverpt commented Aug 26, 2024

What does this PR do?

Fixes Trailing Slash on Image Prefix.

Why is it important?

Because we need to manually remove the Trailing Slash specifically for Go Projects. Where we want consistency between Testcontainers Java and Go

Related issues

How to test this PR

Unit Test Created

@driverpt driverpt requested a review from a team as a code owner August 26, 2024 08:51
@netlify
Copy link

netlify bot commented Aug 26, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 5be5853
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/66cc6edc0c41b90008a256c0
😎 Deploy Preview https://deploy-preview-2747--testcontainers-go.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.

@mdelapenya mdelapenya self-assigned this Aug 27, 2024
@mdelapenya mdelapenya added the bug An issue with the library label Aug 27, 2024
@mdelapenya
Copy link
Member

Hi @driverpt thanks for contributing the fix! I'm curious about the use case: from what I read from your message, you have projects in Java and in Go, using testcontainers, right? Can you share more about the configuration used in both? I guess you're setting the hub prefix in the properties file, and because it's shared across languages, it fails for Go. Is this assumption correct?

@driverpt
Copy link
Contributor Author

driverpt commented Aug 27, 2024

Ok, so what happens is that we are using a CDK Template for CodeBuild PR Builders (Java, Go and Typescript), all of them have the same TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX but only Go fails because instead of handling the Trailing slash, it just blindly adds // if you have a Trailing Slash on that Variable.

This PR properly handles this via url.JoinPath() instead of blindly Concatenating 2 strings

@mdelapenya
Copy link
Member

Yeah, sure, I'm not questioning the fix: I added the test myself with the current head and I saw it failing, so thanks for contributing the test too!

I'm just interested in how testcontainers is used across a set multi-language projects.

Copy link
Member

@mdelapenya mdelapenya 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!

@mdelapenya mdelapenya merged commit 0d9e934 into testcontainers:main Aug 27, 2024
@driverpt driverpt deleted the fix-trailing-slash-on-image-prefix branch August 27, 2024 14:05
mdelapenya added a commit that referenced this pull request Sep 3, 2024
* main:
  feat(wait): for file (#2731)
  feat(compose): select services via profiles (#2758)
  chore(deps): bump mkdocs-markdownextradata-plugin from 0.2.5 to 0.2.6 (#2761)
  fix: update template too (#2763)
  chore(deps): bump actions/checkout from 4.1.1 to 4.1.7 (#2762)
  chore(deps): bump mkdocs-include-markdown-plugin from 6.0.4 to 6.2.2 (#2760)
  fix: check if the discovered docker socket responds (#2741)
  Upgrade milvus-io/milvus-sdk-go to avoid checksum mismatch. (#2753)
  Fix trailing slash on Image Prefix (#2747)
  chore: use new testcontainers/ryuk:0.9.0 image (#2750)
  chore(deps): bump minimal Go version from 1.21 to 1.22 (#2743)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Sep 9, 2024
* main:
  docs: refine heading badges in README (testcontainers#2770)
  feat(wait): for file (testcontainers#2731)
  feat(compose): select services via profiles (testcontainers#2758)
  chore(deps): bump mkdocs-markdownextradata-plugin from 0.2.5 to 0.2.6 (testcontainers#2761)
  fix: update template too (testcontainers#2763)
  chore(deps): bump actions/checkout from 4.1.1 to 4.1.7 (testcontainers#2762)
  chore(deps): bump mkdocs-include-markdown-plugin from 6.0.4 to 6.2.2 (testcontainers#2760)
  fix: check if the discovered docker socket responds (testcontainers#2741)
  Upgrade milvus-io/milvus-sdk-go to avoid checksum mismatch. (testcontainers#2753)
  Fix trailing slash on Image Prefix (testcontainers#2747)
  chore: use new testcontainers/ryuk:0.9.0 image (testcontainers#2750)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Sep 11, 2024
* main:
  ci: add generate for mocks (testcontainers#2774)
  fix: docker config error handling when config file does not exist (testcontainers#2772)
  docs: refine heading badges in README (testcontainers#2770)
  feat(wait): for file (testcontainers#2731)
  feat(compose): select services via profiles (testcontainers#2758)
  chore(deps): bump mkdocs-markdownextradata-plugin from 0.2.5 to 0.2.6 (testcontainers#2761)
  fix: update template too (testcontainers#2763)
  chore(deps): bump actions/checkout from 4.1.1 to 4.1.7 (testcontainers#2762)
  chore(deps): bump mkdocs-include-markdown-plugin from 6.0.4 to 6.2.2 (testcontainers#2760)
  fix: check if the discovered docker socket responds (testcontainers#2741)
  Upgrade milvus-io/milvus-sdk-go to avoid checksum mismatch. (testcontainers#2753)
  Fix trailing slash on Image Prefix (testcontainers#2747)
  chore: use new testcontainers/ryuk:0.9.0 image (testcontainers#2750)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue with the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX trailing slash not handled properly

2 participants