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

feat: authenticate docker on PullImage #2446

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Mar 23, 2024

What does this PR do?

This authenticates on PullImage and improves the k3s LoadImages test so that a failure to start the pod will fail the test.

Why is it important?

I'm using k3s with private images, so I want to use the LoadImages feature. The instructions to do this included running PullImage first, which works on anonymous images, but not authenticated ones. By re-using the same auth code used to run a container, I can now use the LoadImages feature in k3s with private images.

While trying to figure out what was wrong I used Test_LoadImages which is quite nice, but accidentally had a problem where a container can fail to run, yet still pass the test. This lost me time, so I figured I would improve it for the next person.

Related issues

maybe #1685?

How to test this PR

Change the image in Test_LoadImages from "nginx" to a privately hosted image (like a private github repository). Before, it would fail to pull, and now it won't.

I'm using k3s and specifically its LoadImages function to avoid hassle
for images not published to a public repository. Through this, I noticed
we were accidentally only authenticating docker pulls when launching a
container.

This authenticates the same way, regardless of if you are launching a
container or pulling its image. This also improves the LoadImages test
as before, the pod could have not started due to an image problem, yet
still pass the test.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt codefromthecrypt requested a review from a team as a code owner March 23, 2024 09:22
Copy link

netlify bot commented Mar 23, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 836a8ec
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/65ffd7478235d300089fae84
😎 Deploy Preview https://deploy-preview-2446--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.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@mdelapenya mdelapenya self-assigned this Mar 25, 2024
@mdelapenya mdelapenya added the enhancement New feature or request label Mar 25, 2024
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.

This change makes total sense, thanks for adding it 🙇

LGTM!

@mdelapenya mdelapenya merged commit 3541728 into testcontainers:main Mar 25, 2024
100 checks passed
@codefromthecrypt codefromthecrypt deleted the fix-auth branch March 25, 2024 11:11
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Mar 26, 2024
* main:
  feat: add module to support InfluxDB v1.x (testcontainers#1703)
  feat: authenticate docker on PullImage (testcontainers#2446)
  feat: add distribution-registry module (testcontainers#2341)
  chore(deps): Bumping ChromaGo client version (testcontainers#2402)
  chore(deps): bump github.com/docker/docker from 25.0.3+incompatible to 25.0.5+incompatible (testcontainers#2444)
  feat: support passing io.Reader as ContainerFile (testcontainers#2401)
mdelapenya added a commit to stevenh/testcontainers-go that referenced this pull request Apr 3, 2024
* main: (21 commits)
  feat: optimizes file copies to and from containers (testcontainers#2450)
  fix(exec): updates the `Multiplexed` opt to combine stdout and stderr (testcontainers#2452)
  Upgrade neo4j module to use features from v0.29.1 of testcontainers-go (testcontainers#2463)
  bug:Fix AMQPS url (testcontainers#2462)
  chore: more compose updates in comments
  chore: use "docker compose" (v2) instead of "docker-compose" (v1) (testcontainers#2464)
  chore(deps): bump github/codeql-action from 2.22.12 to 3.24.9 (testcontainers#2459)
  refactor: Add Weaviate modules tests (testcontainers#2447)
  feat(exitcode): Add exit code sugar method (testcontainers#2342)
  feat: add module to support InfluxDB v1.x (testcontainers#1703)
  feat: authenticate docker on PullImage (testcontainers#2446)
  feat: add distribution-registry module (testcontainers#2341)
  chore(deps): Bumping ChromaGo client version (testcontainers#2402)
  chore(deps): bump github.com/docker/docker from 25.0.3+incompatible to 25.0.5+incompatible (testcontainers#2444)
  feat: support passing io.Reader as ContainerFile (testcontainers#2401)
  chore: bump ryuk to latest (testcontainers#2395)
  feat(MustConn): Add MustConnectionString on (some) dbs (testcontainers#2343)
  fix: typo in ci-test-go.yml (testcontainers#2394)
  feat: support for waiting for response headers (testcontainers#2349)
  chore(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0 (testcontainers#2392)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants