Skip to content

e2e: add private registry pull/push regression test#7007

Open
lohitkolluri wants to merge 1 commit into
docker:masterfrom
lohitkolluri:e2e/private-registry-pull-push-5965
Open

e2e: add private registry pull/push regression test#7007
lohitkolluri wants to merge 1 commit into
docker:masterfrom
lohitkolluri:e2e/private-registry-pull-push-5965

Conversation

@lohitkolluri

@lohitkolluri lohitkolluri commented May 26, 2026

Copy link
Copy Markdown

This adds an e2e regression test for authenticated pull/push against a private registry, covering the auth regression reported in #5963.


What's included

  • New privateregistry service in the e2e Compose stack that runs a Docker registry with htpasswd authentication on port 5001, and a --insecure-registry flag for it in the engine container.

  • TestPullPushPrivateRepository test that:

    • Tags the test alpine image for the private registry
    • Verifies that an unauthenticated push is rejected (no auth credentials configured)
    • Verifies that an authenticated push succeeds
    • Removes the local image, then verifies that an unauthenticated pull is rejected
    • Verifies that an authenticated pull succeeds
  • Auth config entries for privateregistry:5001 in the e2e fixture config, with test credentials stored in a new e2e/testdata/registry/auth/htpasswd file.

  • 90-second retry loop in the test that re-runs docker commands when the registry or DNS is not yet ready (covers the container startup race in CI). Transient errors are detected by matching known DNS/network failure strings; all other failures are returned immediately.

  • Service health wait loop in scripts/test/e2e/run that polls docker compose ps for all three services (registry, privateregistry, engine) before connecting the test runner to the Compose network. If any service fails to start within 120 seconds, it prints service logs and exits with a clear error instead of letting tests hang on DNS timeouts.


About the earlier attempt (#6940)

The earlier version of this test failed in CI because the htpasswd volume mount in compose-env.yaml used a path resolved relative to the project root (./e2e/testdata/registry/auth), but Compose resolves volume paths relative to the compose file's directory (e2e/). The auth file never mounted, the registry started without authentication, and every registry operation either succeeded unauthenticated (giving false negatives) or timed out with a DNS error when the registry failed to start entirely. This version uses ./testdata/registry/auth so the mount resolves correctly.


Closes #5965.

@lohitkolluri lohitkolluri reopened this May 26, 2026
@lohitkolluri lohitkolluri force-pushed the e2e/private-registry-pull-push-5965 branch 2 times, most recently from aa3a10d to b2e3d39 Compare May 26, 2026 13:46
@lohitkolluri lohitkolluri changed the title e2e/image: add private registry pull/push regression test e2e: add private registry pull/push regression test May 26, 2026
@codecov-commenter

codecov-commenter commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
e2e/internal/fixtures/fixtures.go 0.00% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@lohitkolluri lohitkolluri force-pushed the e2e/private-registry-pull-push-5965 branch 7 times, most recently from febb222 to 236b3d4 Compare May 27, 2026 15:57
@lohitkolluri

Copy link
Copy Markdown
Author

@vvoland @thaJeztah Codecov is reporting 0% patch coverage for e2e/internal/fixtures because the unit-test coverage job does not include the e2e/ tree. The added lines are exercised by TestPullPushPrivateRepository in the e2e suite, and the related e2e jobs are passing.

Is this acceptable as-is, or would you prefer additional coverage changes here?

@vvoland vvoland requested a review from docker-agent June 10, 2026 10:30
@lohitkolluri lohitkolluri force-pushed the e2e/private-registry-pull-push-5965 branch 2 times, most recently from b5f226f to 2fbbd74 Compare June 10, 2026 10:56
@vvoland vvoland requested review from docker-agent and removed request for docker-agent June 10, 2026 12:02
Comment thread e2e/compose-env.yaml
@lohitkolluri lohitkolluri requested a review from vvoland June 11, 2026 14:48
Comment thread e2e/image/private_test.go Outdated
Comment thread e2e/testdata/registry/certs/ca.crt Outdated
lohitkolluri added a commit to lohitkolluri/cli that referenced this pull request Jun 11, 2026
Address reviewer feedback on docker/cli PR docker#7007:
- Merge TestPullPushPrivateRepository and
  TestPullPushTlsRepository into one test with
  "insecure" and "tls" subtests
- Generate TLS certs at setup time instead of
  committing them
- Remove committed cert files from git

Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
@lohitkolluri lohitkolluri requested a review from vvoland June 11, 2026 17:22
@lohitkolluri

Copy link
Copy Markdown
Author

I pushed two more commits to address the CI failures:

  • Connhelper-ssh variant: The connhelper-ssh Dockerfile was missing the CA certificate trust setup that the main Dockerfile already had — all 8 tests (connhelper-ssh) jobs failed with x509: certificate signed by unknown authority on the TLS registry test. Fixed by adding COPY registry/certs/ca.crt and RUN update-ca-certificates to Dockerfile.connhelper-ssh.

  • Cert generation guard: The cert existence check in scripts/test/e2e/run only verified ca.crt, so if any of the other three generated files was missing, gen-certs.sh wouldn't re-run. Changed it to loop through all four files before deciding to generate.

@thaJeztah thaJeztah added this to the 29.6.1 milestone Jun 12, 2026
@lohitkolluri

Copy link
Copy Markdown
Author

Should i squash all commits into one?

@vvoland

vvoland commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Yes

@vvoland

vvoland commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

/review

@vvoland vvoland requested review from docker-agent and removed request for docker-agent June 12, 2026 17:19
@lohitkolluri

Copy link
Copy Markdown
Author

Once the CI run finishes, I'll squash the commits and push the changes.

@docker-agent

Copy link
Copy Markdown

⏱️ PR Review Timed Out — The review agent hit the 1800 s time limit before completing. This usually happens on large or complex diffs. You can re-trigger with /review — if it times out again, consider splitting the PR into smaller pieces.

@lohitkolluri

Copy link
Copy Markdown
Author

I did a quick RCA on both failures.

For [e2e / tests (connhelper-ssh, debian, 25)], I believe the failure is related to TestProcessTermination using a 10s timeout. The test sends a TERM signal and expects the container to exit within that window, but on the Debian + Engine 25 combination the attach stream appears to close more slowly than on Alpine/Engine 27. Increasing the WaitOnCmd timeout in e2e/container/run_test.go from 10s to 20s consistently addresses the failure in my testing.

For [test / ctn (test-coverage)], my understanding is that this is unrelated to the changes in this PR. The coverage job excludes the e2e package (grep -vE '/e2e/'), so an e2e-only change should not affect it. It looks like a pre-existing unit test issue that would need to be fixed independently.

Could you take a quick look and let me know if my assessment seems reasonable? If you think the coverage failure is worth investigating, I'm happy to dig into it and open a separate PR if needed.

@lohitkolluri lohitkolluri force-pushed the e2e/private-registry-pull-push-5965 branch from 16df6a4 to 9706732 Compare June 12, 2026 18:24
lohitkolluri added a commit to lohitkolluri/cli that referenced this pull request Jun 12, 2026
Address reviewer feedback on docker/cli PR docker#7007:
- Merge TestPullPushPrivateRepository and
  TestPullPushTlsRepository into one test with
  "insecure" and "tls" subtests
- Generate TLS certs at setup time instead of
  committing them
- Remove committed cert files from git

Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
@lohitkolluri lohitkolluri force-pushed the e2e/private-registry-pull-push-5965 branch from 9706732 to fd735c2 Compare June 12, 2026 18:39
This adds an e2e regression test for authenticated pull/push against a private registry, covering the auth regression reported in docker#5963.

Includes:
- New privateregistry service in the e2e Compose stack with htpasswd auth on port 5001, and --insecure-registry for the engine container.
- TestPullPushPrivateRepository test that verifies authenticated push/pull and rejects unauthenticated operations.
- Auth config and test credentials in e2e/testdata/registry/.
- 90-second retry loop for transient DNS/container startup races.
- Service health wait loop in scripts/test/e2e/run.
- Increase TestProcessTermination timeout from 10s to 20s for connhelper-ssh + engine 25 combination.
- Connhelper-ssh engine Dockerfile for private registry integration.

Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
@lohitkolluri lohitkolluri force-pushed the e2e/private-registry-pull-push-5965 branch from fd735c2 to d9698ed Compare June 12, 2026 18:52
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.

Add e2e test for authorized pull/push

6 participants