Files
velero/pkg/cmd/util/downloadrequest/downloadrequest.go
T
3cd6c2e533
Run the E2E test on kind / setup-test-matrix (push) Successful in 4s
e2e-test-kind.yaml / extract (push) Failing after 9s
Run the E2E test on kind / get-go-version (push) Failing after 11s
push.yml / extract (push) Failing after 6s
Run the E2E test on kind / build (push) Skipped
Run the E2E test on kind / run-e2e-test (push) Skipped
Main CI / get-go-version (push) Failing after 7s
Main CI / Build (push) Skipped
Skip signing a download URL when no artifacts can exist yet (#10252)
* Skip signing a download URL when no artifacts can exist yet

Reported in #10232: a DownloadRequest for a backup that never ran still
reaches Processed with a signed URL, and fetching it returns 404.

The controller already has the backup, and the restore for restore
targets, in hand before it signs, so checking the phase costs no extra
call to the object store.

The check is deliberately narrow. It refuses only the pre-execution
phases, where nothing has been written for any target kind: New, Queued,
ReadyToStart and FailedValidation for backups, New and FailedValidation
for restores. InProgress onwards may hold a partial log or other
artifacts, and Deleting may still hold all of them, so those keep the
behaviour callers have today.

That matters because velero backup download has no client side phase
check of its own, unlike backup logs and restore logs. Reusing the
allowlist from pkg/cmd/cli/backup/logs.go would have changed what
backup download can fetch; this does not.

A backup with an empty phase is left alone as well, since that state is
transient and the caller can retry.

Refs #10232

Signed-off-by: saral <ilovegojo2580@gmail.com>

* Derive the phase coverage test from the generated CRDs

The previous test built a slice of phases by hand and asserted its own
length, so it passed no matter what the API did. Adding a fourteenth
backup phase would not have failed it.

This reads the status.phase enum out of the generated CRDs, via the
exported v1crds.CRDs that pkg/install already uses. The enum comes from
the same kubebuilder markers as the Go constants, so a phase added to
the API fails here until it is classified.

Verified by removing Deleting from the expectations, which now fails with
'BackupPhase "Deleting" is served by the CRD but not classified'.

Signed-off-by: saral <ilovegojo2580@gmail.com>

* Use US spelling in comments to satisfy the misspell linter

golangci-lint runs misspell, which flags behaviour as a misspelling of
behavior. Comments only, no functional change.

Signed-off-by: saral <ilovegojo2580@gmail.com>

* Set a Failed phase with a reason when the guard refuses to sign

The guard added in the previous commit left the request at New with no URL, so
the CLI polled until its own timeout and then reported that the backup storage
location may be unavailable. The BSL is fine; the backup never ran.

DownloadRequestPhase gains Failed and DownloadRequestStatus gains Message. The
controller sets both where it refuses, and the CLI stops as soon as it sees the
phase and surfaces the message instead of its generic timeout error.

Adding an enum value is additive, per the direction on the PR discussion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: saral <ilovegojo2580@gmail.com>

---------

Signed-off-by: saral <ilovegojo2580@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-21 17:26:21 +08:00

253 lines
7.2 KiB
Go

/*
Copyright the Velero contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package downloadrequest
import (
"compress/gzip"
"context"
"crypto/tls"
"crypto/x509"
"fmt"
"io"
"net/http"
"net/url"
"os"
"time"
"github.com/cockroachdb/errors"
"github.com/google/uuid"
kbclient "sigs.k8s.io/controller-runtime/pkg/client"
veleroV1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
)
// ErrNotFound is exported for external packages to check for when a file is
// not found
var ErrNotFound = errors.New("file not found")
var ErrDownloadRequestDownloadURLTimeout = errors.New("download request download url timeout, check velero server logs for errors. backup storage location may not be available")
var unzipLimit int64 = 1024 * 1024 * 1024 // 1GB limit
// ErrDownloadRequestFailed is returned when the server refused the request and gave no
// reason. The controller sets a message in every path that fails today, so this is a
// fallback rather than the usual case.
var ErrDownloadRequestFailed = errors.New("download request failed, check velero server logs for errors")
func Stream(
ctx context.Context,
kbClient kbclient.Client,
namespace, name string,
kind veleroV1api.DownloadTargetKind,
w io.Writer,
timeout time.Duration,
insecureSkipTLSVerify bool,
caCertFile string,
) error {
return StreamWithBSLCACert(ctx, kbClient, namespace, name, kind, w, timeout, insecureSkipTLSVerify, caCertFile, "")
}
// StreamWithBSLCACert is like Stream but accepts an additional bslCACert parameter
// that contains the cacert from the BackupStorageLocation config
func StreamWithBSLCACert(
ctx context.Context,
kbClient kbclient.Client,
namespace, name string,
kind veleroV1api.DownloadTargetKind,
w io.Writer,
timeout time.Duration,
insecureSkipTLSVerify bool,
caCertFile string,
bslCACert string,
) error {
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
downloadURL, err := getDownloadURL(ctx, kbClient, namespace, name, kind)
if err != nil {
return err
}
if err := download(ctx, downloadURL, kind, w, insecureSkipTLSVerify, caCertFile, bslCACert); err != nil {
return err
}
return nil
}
func getDownloadURL(
ctx context.Context,
kbClient kbclient.Client,
namespace, name string,
kind veleroV1api.DownloadTargetKind,
) (string, error) {
uuid, err := uuid.NewRandom()
if err != nil {
return "", err
}
reqName := fmt.Sprintf("%s-%s", name, uuid.String())
created := builder.ForDownloadRequest(namespace, reqName).Target(kind, name).Result()
if err := kbClient.Create(ctx, created, &kbclient.CreateOptions{}); err != nil {
return "", errors.WithStack(err)
}
for {
select {
case <-ctx.Done():
return "", ErrDownloadRequestDownloadURLTimeout
case <-time.After(25 * time.Millisecond):
updated := &veleroV1api.DownloadRequest{}
if err := kbClient.Get(ctx, kbclient.ObjectKey{Name: created.Name, Namespace: namespace}, updated); err != nil {
return "", errors.WithStack(err)
}
if updated.Status.DownloadURL != "" {
return updated.Status.DownloadURL, nil
}
// Failed is terminal. Waiting for a URL that will never be signed would end in
// ErrDownloadRequestDownloadURLTimeout, which blames the storage location for
// something the status already explains.
if updated.Status.Phase == veleroV1api.DownloadRequestPhaseFailed {
if updated.Status.Message != "" {
return "", errors.New(updated.Status.Message)
}
return "", ErrDownloadRequestFailed
}
}
}
}
func download(
ctx context.Context,
downloadURL string,
kind veleroV1api.DownloadTargetKind,
w io.Writer,
insecureSkipTLSVerify bool,
caCertFile string,
caCertByteString string,
) error {
var caPool *x509.CertPool
var err error
// Initialize caPool once
caPool, err = x509.SystemCertPool()
if err != nil {
caPool = x509.NewCertPool()
}
// Try to load CA cert from file first
if len(caCertFile) > 0 {
caCert, err := os.ReadFile(caCertFile)
if err != nil {
// If caCertFile fails and BSL cert is available, fall back to it
if len(caCertByteString) > 0 {
fmt.Fprintf(os.Stderr, "Warning: Failed to open CA certificate file %s: %v. Using CA certificate from backup storage location instead.\n", caCertFile, err)
caPool.AppendCertsFromPEM([]byte(caCertByteString))
} else {
// If no BSL cert available, return the original error
return errors.Wrapf(err, "couldn't open cacert")
}
} else {
caPool.AppendCertsFromPEM(caCert)
}
} else if len(caCertByteString) > 0 {
// If no caCertFile specified, use BSL cert if available
caPool.AppendCertsFromPEM([]byte(caCertByteString))
}
defaultTransport := http.DefaultTransport.(*http.Transport)
// same settings as the default transport
// aside from TLSClientConfig
httpClient := new(http.Client)
httpClient.Transport = &http.Transport{
TLSClientConfig: &tls.Config{
InsecureSkipVerify: insecureSkipTLSVerify, //nolint:gosec // This parameter is useful for some scenarios.
RootCAs: caPool,
},
DialContext: defaultTransport.DialContext,
ForceAttemptHTTP2: defaultTransport.ForceAttemptHTTP2,
MaxIdleConns: defaultTransport.MaxIdleConns,
Proxy: defaultTransport.Proxy,
TLSHandshakeTimeout: defaultTransport.TLSHandshakeTimeout,
ExpectContinueTimeout: defaultTransport.ExpectContinueTimeout,
}
httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil)
if err != nil {
return err
}
resp, err := httpClient.Do(httpReq)
if err != nil {
if urlErr, ok := err.(*url.Error); ok {
if _, ok := urlErr.Err.(x509.UnknownAuthorityError); ok {
return fmt.Errorf("%s\n\nThe --insecure-skip-tls-verify flag can also be used to accept any TLS certificate for the download, but it is susceptible to man-in-the-middle attacks", err.Error())
}
}
return err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
body, err := io.ReadAll(resp.Body)
if err != nil {
return errors.Wrapf(err, "request failed: unable to decode response body")
}
if resp.StatusCode == http.StatusNotFound {
return ErrNotFound
}
return errors.Errorf("request failed: %v", string(body))
}
var r io.Reader = resp.Body
var gzipReader *gzip.Reader
if kind != veleroV1api.DownloadTargetKindBackupContents {
// need to decompress logs
var err error
gzipReader, err = gzip.NewReader(resp.Body)
if err != nil {
return err
}
defer gzipReader.Close()
r = io.LimitReader(gzipReader, unzipLimit)
}
_, err = io.Copy(w, r)
if err != nil {
return err
}
if gzipReader != nil {
var buf [1]byte
n, err := gzipReader.Read(buf[:])
if n > 0 || err == nil {
return errors.Errorf("decompressed data exceeds the limit")
}
if err != io.EOF {
return err
}
}
return nil
}