Skip to content

Request review: exact Redpanda registry egress host - #307

Merged
v-robaiken merged 1 commit into
dependabot:mainfrom
eitanziv:eitanziv/allowlist-redpanda-registry
Oct 12, 2026
Merged

v-robaiken merged 1 commit into
dependabot:mainfrom
eitanziv:eitanziv/allowlist-redpanda-registry

Conversation

@eitanziv

@eitanziv eitanziv commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What are you trying to accomplish?

Request maintainer security review of an exact docker.redpanda.com default allowlist entry for Docker/Compose update discovery. A real Compose update job fails with * egress not allowlisted docker.redpanda.com when fetching /v2/redpandadata/redpanda/tags/list: https://github.com/eitanziv/ops/actions/runs/37855156075 .

Redpanda's official quickstart uses this registry: https://docs.redpanda.com/streaming/current/get-started/quick-start/ . The patch adds one exact host to the Docker anchor (shared by Compose/devcontainers), a realistic positive request, and a blocked-child regression probe. No wildcard, credentials, redirect storage hosts, or handler changes.

Anything you want to highlight for special attention from reviewers?

Ready for maintainer review. Explicit maintainer sign-off on the host-wide exposure is required before merge. The allowlist matches hostname rather than path/method, and defaults apply to every job. Redpanda describes this endpoint as a Docker Hub mirror/reflector: https://docs.redpanda.com/streaming/current/upgrade/k-rolling-upgrade/ . Provider ownership alone does not prove every path serves provider-only content. Please confirm whether this mirror belongs in defaults under the repository's triage policy, or advise a per-job alternative.

Safe direct HTTPS probes (no redirects followed, public DNS addresses checked, no personal credentials): the registry advertises an auth.docker.io anonymous pull token; the existing allowlist already includes that token host. An anonymous scoped token allows the Redpanda tag listing (200,817 tags). A manifest request returned429, so complete image/blob download routing is NOT verified. A non-Redpanda library/alpine tag listing returned404 both unauthenticated and with a valid anonymous token; this is one negative probe, not proof of all path restrictions. No token values are included.

A separate Forgejo host report is deliberately excluded because external-user registration/package namespaces introduce an unresolved user-upload boundary. This PR should not be taken as approval to add that forge globally.

How will you know you've accomplished your goal?

  • Maintainers explicitly accept this host's security classification.
  • Build, vet, all TestEgress tests, and native full race/shuffle/count2 suite passed using official checksum-verified Go1.27.2.
  • Mutating the entry to .docker.redpanda.com makes the child-host test fail; the exact entry was restored and TestEgress passed again.
  • After upstream deployment, real updater discovery reaches this endpoint without the proxy403. That end-to-end result remains pending; auth/rate-limit/download failures are separate evidence.

Docker's script/test could not run because this environment has no Docker daemon. The equivalent native go test -race -shuffle=on -count=2 ./... passed, but this does not certify the prescribed container or all linters. The changed Go file is gofmt-formatted; unrelated existing checkout files were not reformatted.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

Upstream CI readback: container tests and lint passed;107 of108 checks passed. The python-pip-compile smoke fixture expects pycparser3.0 but generated output contains3.1. This failure remains unresolved. Maintainer rerun was requested because this contributor cannot rerun upstream Actions or formally request a team review. Implementation review approved exactheadc85d518; maintainer security acceptance remains required.

Signed-off-by: Codex <codex@openai.com>
@eitanziv

eitanziv commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Independent GPT-6.1-sol/high review of exact head c85d518 approved the narrow implementation with no actionable code defects: exact-host entry, Docker alias placement, positive endpoint test and blocked-child test are consistent with the handler's matching semantics. This is code review approval only, not a maintainer security exception.

Merge/readiness remains held for explicit maintainer acceptance of the Docker Hub reflector's host-wide exposure. The account submitting this contribution has no upstream merge permission; no bypass is requested.

Live upstream validation readback:107 of108 checks passed. CI Build ran the prescribed container test step successfully; Lint passed yamllint, mod tidy, gofmt, build and vet. e2e(python-pip-compile) failed and remains under diagnosis. Local native full race/shuffle/count2 and exact-host mutation checks passed. Reviewer inspected source/mutation logic but could not independently rerun WSL tests; execution evidence is from the contributing lead and upstream CI.

@eitanziv

eitanziv commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Smoke failure diagnosis: e2e(python-pip-compile) completes registry requests and update computation but fails expected-output comparison. The retained log shows generated requirements changing pycparser3.0 to3.1 where the smoke fixture expects3.0, followed by update failed expectations. No docker.redpanda.com request or egress403 appears in this failed job. This supports dependency/fixture drift rather than an observed failure of the new host rule; it does not convert the failing check into a pass. No unrelated smoke fixtures have been changed. Maintainer diagnosis/rerun remains appropriate.

@eitanziv
eitanziv marked this pull request as ready for review October 9, 2026 06:39
@eitanziv
eitanziv requested a review from a team as a code owner October 9, 2026 06:39
@eitanziv

eitanziv commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Ready for maintainer review; not requesting a bypass of the security classification gate.

Please decide whether exact docker.redpanda.com is acceptable in the global defaults under the add-egress-allowlist-domain policy. Redpanda documents this as a Docker Hub reflector. Anonymous Redpanda tags succeed; an unrelated library/alpine path returns404, but that single negative probe does not establish comprehensive path restrictions. The proposal allows hostname-wide access for all jobs and therefore needs your explicit acceptance of that exposure. If unsuitable, please advise the supported per-job anonymous-registry configuration; type/url-only docker-registry declarations were rejected by the hosted config validator in the affected repository.

Please also rerun or diagnose e2e(python-pip-compile), run37863744710, job113605452677. Its actual output contains pycparser3.1 where the expected fixture contains3.0; requests complete and no Redpanda-host block appears. I attempted a failed-jobs-only rerun, but GitHub requires upstream admin rights. I have not changed unrelated fixtures or treated the failed check as passing.

Exact head remains c85d518. Independent implementation review approved; upstream container tests/lint and107/108checks passed. Forgejo remains excluded. End-to-end registry remediation remains pending upstream acceptance/deployment.

@v-robaiken v-robaiken added the allowlist Changes the default egress allowlist label Oct 12, 2026
@v-robaiken
v-robaiken merged commit cc77706 into dependabot:main Oct 12, 2026
107 of 108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allowlist Changes the default egress allowlist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants