Skip to content

[snapshot] Add --publish so download works from outside the cluster network - #463

Open
kkozoriz wants to merge 7 commits into
mainfrom
feature/snapshot-download-publish
Open

[snapshot] Add --publish so download works from outside the cluster network#463
kkozoriz wants to merge 7 commits into
mainfrom
feature/snapshot-download-publish

Conversation

@kkozoriz

@kkozoriz kkozoriz commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Problem

d8 snapshot download could only pull volume bytes from the exporter pod's in-cluster service, so it needed direct network access to the cluster's internal service network. Pulling a snapshot out from outside the cluster was not possible.

Stacked on #462 (feature/snapshot-upload-publish) - review that one first.

Fix

  • --publish on d8 snapshot download: pull volume bytes through the published Ingress endpoint (status.publicURL) instead of the exporter's in-cluster service.
  • Unset means autodetect; explicit --publish=false keeps the old path.
  • Publish is threaded through the whole download pipeline and the DataExport export request, not just parsed at the CLI.
  • Same bearer-token-only constraint as upload: Ingress does not forward a client certificate to the exporter pod.

Before / After

Before After
Download from a machine with no route to the cluster service network not possible d8 snapshot download ... --publish=true
Endpoint exporter pod service, internal CA (status.ca) Ingress status.publicURL, public certificate
image

Validation

Real run against a live test cluster: a 4-volume snapshot (2 block + 2 filesystem) downloaded with --publish=true from an external machine with no route into the cluster network.

The resulting archive is byte-for-byte identical (sha256, all 4 volumes) to both an internal-network download of the same snapshot and the live volume data on the cluster.

That run also hit 8 real transient network interruptions; every one was recovered by the retry fix stacked on this branch (#465), with no checksum mismatch.

Tests

  • internal/snapshot/cmd/download/download_test.go - flag default/visibility, explicit set, example
  • internal/snapshot/exporter/{export,dataexport,http}_test.go - publish endpoint selection and 401 hinting on the export path
  • internal/snapshot/pipeline/config_test.go - publish threaded through pipeline config

… client

Add ValidateHTTPSURL (transport) and NewSafeClientForConfig (safe client)
as the foundational pieces needed to build an HTTPS client against a
published (ingress) DataImport endpoint.

Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
Add spec.publish to the DataImport built by d8 snapshot upload, align
it alongside spec.ttl on reuse, wait on status.publicURL when publish
is enabled, and switch to a merged TLS trust pool with 401/403
diagnostics on that path.

Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
Wire the flag through to the DataImport importer, auto-detecting the
upload mode when unset, and document the bearer-token requirement for
the published path.

Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
SetTLSCAData merged a trust pool but never reset the caller's inherited
insecure-skip-tls-verify/tls-server-name, so Go skipped certificate
verification entirely on that path — any endpoint could receive the
real Kubernetes bearer token. Force verification on unconditionally,
in both the rest.Config and the cloned transport, since client-go may
already have baked the insecure flag into a base transport before this
wrapper runs.

Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
Add spec.publish to the DataExport built by d8 snapshot download,
upgrade it (one-way, optimistic-locked) on an adopted CR, wait on
status.publicURL when publish is enabled, and switch to a merged TLS
trust pool with 401/403 diagnostics on that path.

Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
@kkozoriz kkozoriz changed the title [snapshot] d8 snapshot download publish [snapshot] Add --publish so download works from outside the cluster network Aug 30, 2026
@kkozoriz
kkozoriz marked this pull request as ready for review August 31, 2026 18:10
@kkozoriz
kkozoriz requested a review from ldmonster as a code owner August 31, 2026 18:10
@kkozoriz kkozoriz self-assigned this Aug 31, 2026
kkozoriz added a commit that referenced this pull request Aug 31, 2026
…463

The team merges with "Squash and merge", which drops the #462 -> #463 -> #464
ancestry. Git then falls back to origin/main as the merge base and sees #462's
changes -- already present here -- as competing additions, producing two
conflicts that a plain merge chain never hits:

  CONFLICT (content): internal/snapshot/transport/http.go
  CONFLICT (add/add): pkg/libsaferequest/client/http_test.go

Both were purely structural, so reshape them instead of changing behaviour:

  - http_test.go is created by #462 too, so an add/add conflicts unless both
    sides match byte for byte. Restore it to #462's exact content and move the
    7 SetTLSCAData tests and their 3 helpers to a new http_tls_test.go, which
    exists on neither side of main and merges as a clean single-side add.

  - The "keep in sync" note sat at the end of a doc comment block #462 had just
    added, so the two additions were adjacent and conflicted. Move it just
    inside SetTLSCAData, a region #462 does not touch.

No production code changes: pkg/libsaferequest/client/http.go is untouched and
internal/snapshot/transport/http.go now matches #463 apart from the relocated
comment. All 8 tests are preserved and still pass under -race.

Verified from origin/main: squash #462, squash #463, merge #464, merge #465 --
all four clean, then build, vet and go test -race clean over
./internal/snapshot/... and ./pkg/libsaferequest/...

Signed-off-by: Konstantin Kozoriz <konstantin.kozoriz@flant.com>
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.

1 participant