fix(backups): probe the Redis snapshot with the trust its Job used

The default Redis strategy script verifies the S3 endpoint against
ca.crt, then tls.crt, from the Secret mounted as the s3-ca volume
(backupStorage.endpointCASecretName), and only without either honours
S3_INSECURE_SKIP_VERIFY=true. The probe always verified against the
system roots, so on a cluster with a private CA, such as a self-signed
SeaweedFS, every Redis and Valkey backup succeeded with no artifact.

Read the same settings from the pod spec of the completed backup Job, in
the order the script applies them, and pass them to the S3 connection.
The shared transport gains an InsecureSkipVerify switch that is set only
from that explicit opt-in.

The projector always stores the endpoint as a bare host, so the branch
that parsed a scheme out of it was unreachable; drop it and its test.
Cover the endpoint and access-key terms of the Secret completeness check.

Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
This commit is contained in:
Andrey Kolkov committed 2026-11-09 16:52:06 +07:00
1 parent 2b5fa1f0eb
commit 1d80a543dc
4 files changed
+263 -50

No files matched your search

+10 -3
View File
@@ -54,6 +54,9 @@ type s3Connection struct {
// what the writer of the backup trusted, no more.
CAPEM []byte
CAOnly bool
// InsecureSkipVerify mirrors an explicit admin opt-in a strategy
// already applies to its own uploads; it is never a fallback.
InsecureSkipVerify bool
}
// objectStore reads back what a backup wrote: the Secrets it was written
@@ -105,14 +108,18 @@ func newMinioObjectLister(conn s3Connection) (s3ObjectLister, error) {
Region: conn.Region,
Secure: conn.Secure,
}
if conn.Secure && len(conn.CAPEM) > 0 {
if conn.Secure && (len(conn.CAPEM) > 0 || conn.InsecureSkipVerify) {
// Start from minio's transport to keep its proxy and timeout settings.
tr, err := minio.DefaultTransport(true)
if err != nil {
return nil, err
}
if tr.TLSClientConfig.RootCAs, err = endpointTrustPool(conn, tr.TLSClientConfig.RootCAs); err != nil {
return nil, err
if len(conn.CAPEM) > 0 {
if tr.TLSClientConfig.RootCAs, err = endpointTrustPool(conn, tr.TLSClientConfig.RootCAs); err != nil {
return nil, err
}
} else {
tr.TLSClientConfig.InsecureSkipVerify = true
}
opts.Transport = tr
}
@@ -5,10 +5,9 @@ import (
"context"
"errors"
"fmt"
"net/url"
"strings"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
backupsv1alpha1 "github.com/cozystack/cozystack/api/backups/v1alpha1"
@@ -17,11 +16,12 @@ import (
// redisArtifact locates the RDB snapshot the backup Job uploaded and returns
// an Artifact carrying its s3:// URI and size. The default Redis strategy
// takes its S3 coordinates from the projected backup credentials Secret, so
// the object is looked up there. Returns nil - leaving Status.Artifact
// absent - when the Secret cannot be read or the object is not found: a
// custom strategy that writes elsewhere must not get a URI pointing at an
// object that does not exist.
func (r *BackupJobReconciler) redisArtifact(ctx context.Context, namespace, objectKey string) *backupsv1alpha1.BackupArtifact {
// the object is looked up there, with the TLS trust the Job's pod applied
// (see redisS3Trust). Returns nil - leaving Status.Artifact absent - when
// the Secret cannot be read or the object is not found: a custom strategy
// that writes elsewhere must not get a URI pointing at an object that does
// not exist.
func (r *BackupJobReconciler) redisArtifact(ctx context.Context, namespace, objectKey string, podSpec *corev1.PodSpec) *backupsv1alpha1.BackupArtifact {
logger := getLogger(ctx)
ctx, cancel := context.WithTimeout(ctx, artifactSizeProbeTimeout)
defer cancel()
@@ -31,6 +31,10 @@ func (r *BackupJobReconciler) redisArtifact(ctx context.Context, namespace, obje
logger.Info("cannot resolve backup credentials to locate the Redis snapshot", "objectKey", objectKey, "error", err)
return nil
}
if conn.CAPEM, conn.InsecureSkipVerify, err = r.redisS3Trust(ctx, namespace, podSpec); err != nil {
logger.Info("cannot read the S3 endpoint CA to locate the Redis snapshot", "objectKey", objectKey, "error", err)
return nil
}
lister, err := r.s3Lister(conn)
if err != nil {
logger.Info("cannot build S3 client to locate the Redis snapshot", "objectKey", objectKey, "error", err)
@@ -48,8 +52,8 @@ func (r *BackupJobReconciler) redisArtifact(ctx context.Context, namespace, obje
}
// projectedS3Connection reads the backup credentials Secret the controller
// projects into the application namespace. Its endpoint is a bare host,
// which the strategy scripts reach over https.
// projects into the application namespace. The projector always stores the
// endpoint as a bare host, which the strategy scripts reach over https.
func (r *BackupJobReconciler) projectedS3Connection(ctx context.Context, namespace string) (string, s3Connection, error) {
var conn s3Connection
name := r.CredentialsConfig.TargetSecretName
@@ -68,13 +72,63 @@ func (r *BackupJobReconciler) projectedS3Connection(ctx context.Context, namespa
return "", conn, fmt.Errorf("secret %s/%s lacks bucketName, endpoint or access keys", namespace, name)
}
conn.Endpoint, conn.Secure = value("endpoint"), true
if strings.Contains(conn.Endpoint, "://") {
u, err := url.Parse(conn.Endpoint)
if err != nil || u.Host == "" {
return "", conn, fmt.Errorf("invalid endpoint %q in secret %s/%s", conn.Endpoint, namespace, name)
}
conn.Endpoint, conn.Secure = u.Host, u.Scheme == "https"
}
conn.Region = value("region")
return bucket, conn, nil
}
const (
redisS3CAVolume = "s3-ca"
redisS3InsecureSkipEnv = "S3_INSECURE_SKIP_VERIFY"
)
// redisS3Trust mirrors how the default strategy script verifies the S3
// endpoint: it trusts ca.crt, then tls.crt, from the Secret mounted as the
// s3-ca volume, and only when neither exists does it honour
// S3_INSECURE_SKIP_VERIFY=true. Both are read from the pod spec the backup
// Job ran with. A missing CA Secret is not an error, because the script
// mounts it optionally.
func (r *BackupJobReconciler) redisS3Trust(ctx context.Context, namespace string, podSpec *corev1.PodSpec) ([]byte, bool, error) {
if podSpec == nil {
return nil, false, nil
}
for _, v := range podSpec.Volumes {
if v.Name != redisS3CAVolume || v.Secret == nil {
continue
}
secret := &corev1.Secret{}
err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: v.Secret.SecretName}, secret)
if apierrors.IsNotFound(err) {
break
}
if err != nil {
return nil, false, err
}
for _, file := range []string{"ca.crt", "tls.crt"} {
if pem := secret.Data[secretKeyForFile(v.Secret.Items, file)]; len(pem) > 0 {
return pem, false, nil
}
}
}
for _, c := range podSpec.Containers {
for _, e := range c.Env {
if e.Name == redisS3InsecureSkipEnv && e.Value == "true" {
return nil, true, nil
}
}
}
return nil, false, nil
}
// secretKeyForFile returns the Secret key a secret volume projects to file:
// the key itself when the volume lists no items.
func secretKeyForFile(items []corev1.KeyToPath, file string) string {
if len(items) == 0 {
return file
}
for _, it := range items {
if it.Path == file {
return it.Key
}
}
return ""
}
@@ -3,15 +3,18 @@ package backupcontroller
import (
"context"
"encoding/pem"
"errors"
"net/http"
"net/http/httptest"
"strings"
"testing"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
backupsv1alpha1 "github.com/cozystack/cozystack/api/backups/v1alpha1"
)
@@ -52,7 +55,7 @@ func TestRedisArtifact(t *testing.T) {
t.Run("stats the snapshot with the projected credentials", func(t *testing.T) {
l := &fakeS3Lister{statSize: 2048}
r := redisReconciler(t, l, projectedCredsSecret(nil))
got := r.redisArtifact(context.Background(), "tenant", redisTestKey)
got := r.redisArtifact(context.Background(), "tenant", redisTestKey, nil)
if got == nil || got.URI != "s3://cozy-backups/"+redisTestKey || got.SizeBytes != 2048 {
t.Fatalf("artifact = %#v", got)
}
@@ -64,28 +67,18 @@ func TestRedisArtifact(t *testing.T) {
}
})
t.Run("an endpoint with a scheme sets the transport", func(t *testing.T) {
l := &fakeS3Lister{statSize: 1}
r := redisReconciler(t, l, projectedCredsSecret(map[string]string{"endpoint": "http://seaweedfs:8333"}))
if got := r.redisArtifact(context.Background(), "tenant", redisTestKey); got == nil {
t.Fatal("artifact = nil")
}
if l.conn.Endpoint != "seaweedfs:8333" || l.conn.Secure {
t.Errorf("connection = %+v", l.conn)
}
})
unset := map[string]struct {
lister *fakeS3Lister
objs []client.Object
noTarget bool
}{
"missing object": {lister: &fakeS3Lister{statErr: errors.New("NoSuchKey")}, objs: []client.Object{projectedCredsSecret(nil)}},
"missing Secret": {lister: &fakeS3Lister{statSize: 1}},
"missing bucket": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(map[string]string{"bucketName": ""})}},
"missing keys": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(map[string]string{"AWS_SECRET_ACCESS_KEY": ""})}},
"no target name": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(nil)}, noTarget: true},
"broken endpoint": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(map[string]string{"endpoint": "https://"})}},
"missing object": {lister: &fakeS3Lister{statErr: errors.New("NoSuchKey")}, objs: []client.Object{projectedCredsSecret(nil)}},
"missing Secret": {lister: &fakeS3Lister{statSize: 1}},
"missing bucket": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(map[string]string{"bucketName": ""})}},
"missing keys": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(map[string]string{"AWS_SECRET_ACCESS_KEY": ""})}},
"no target name": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(nil)}, noTarget: true},
"missing endpoint": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(map[string]string{"endpoint": ""})}},
"missing access key": {lister: &fakeS3Lister{statSize: 1}, objs: []client.Object{projectedCredsSecret(map[string]string{"AWS_ACCESS_KEY_ID": ""})}},
}
for name, tc := range unset {
t.Run(name+" leaves the artifact unset", func(t *testing.T) {
@@ -93,19 +86,20 @@ func TestRedisArtifact(t *testing.T) {
if tc.noTarget {
r.CredentialsConfig.TargetSecretName = ""
}
if got := r.redisArtifact(context.Background(), "tenant", redisTestKey); got != nil {
if got := r.redisArtifact(context.Background(), "tenant", redisTestKey, nil); got != nil {
t.Errorf("artifact = %#v, want nil", got)
}
})
}
}
// TestRedisArtifact_MinioClient drives the real minio client against a stub
// that answers the HEAD request, so addressing, signing and the
// Content-Length parsing are exercised together.
// TestRedisArtifact_MinioClient runs the real minio client against a TLS
// stub with a self-signed certificate that answers the HEAD request. It
// covers, in one pass, the trust settings the backup Job used, request
// signing and Content-Length parsing.
func TestRedisArtifact_MinioClient(t *testing.T) {
var gotMethod, gotPath, gotAuth string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
gotMethod, gotPath, gotAuth = req.Method, req.URL.Path, req.Header.Get("Authorization")
w.Header().Set("Content-Length", "4096")
w.Header().Set("ETag", `"abc"`)
@@ -113,15 +107,24 @@ func TestRedisArtifact_MinioClient(t *testing.T) {
w.WriteHeader(http.StatusOK)
}))
defer srv.Close()
caPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: srv.Certificate().Raw})
creds := projectedCredsSecret(map[string]string{"endpoint": srv.URL})
c := newMariaDBStrategyTestClient(t, creds)
creds := projectedCredsSecret(map[string]string{"endpoint": strings.TrimPrefix(srv.URL, "https://")})
ca := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "s3-endpoint-ca"},
Data: map[string][]byte{"ca.crt": caPEM},
}
c := newMariaDBStrategyTestClient(t, creds, ca)
r := &BackupJobReconciler{Client: c, Scheme: c.Scheme()}
r.CredentialsConfig.TargetSecretName = "cozy-backups-creds"
got := r.redisArtifact(context.Background(), "tenant", redisTestKey)
if got := r.redisArtifact(context.Background(), "tenant", redisTestKey, &corev1.PodSpec{}); got != nil {
t.Errorf("without the Job's trust settings: artifact = %#v, want nil (untrusted certificate)", got)
}
got := r.redisArtifact(context.Background(), "tenant", redisTestKey, redisPodSpec("s3-endpoint-ca", ""))
if got == nil || got.SizeBytes != 4096 {
t.Fatalf("artifact = %#v, want SizeBytes 4096", got)
t.Fatalf("with the s3-ca volume: artifact = %#v, want SizeBytes 4096", got)
}
if gotMethod != http.MethodHead || gotPath != "/cozy-backups/"+redisTestKey {
t.Errorf("request %s %s, want HEAD /cozy-backups/%s", gotMethod, gotPath, redisTestKey)
@@ -129,6 +132,84 @@ func TestRedisArtifact_MinioClient(t *testing.T) {
if !strings.Contains(gotAuth, "Credential=AKID/") {
t.Errorf("request not signed with the Secret's access key: %q", gotAuth)
}
if got := r.redisArtifact(context.Background(), "tenant", redisTestKey, redisPodSpec("", "true")); got == nil || got.SizeBytes != 4096 {
t.Errorf("with S3_INSECURE_SKIP_VERIFY=true: artifact = %#v, want SizeBytes 4096", got)
}
}
// redisPodSpec builds the parts of the backup Job's pod spec that
// redisS3Trust reads: the s3-ca volume when caSecret is set, and the
// S3_INSECURE_SKIP_VERIFY variable.
func redisPodSpec(caSecret, skipVerify string) *corev1.PodSpec {
spec := &corev1.PodSpec{Containers: []corev1.Container{{
Name: "backup",
Env: []corev1.EnvVar{{Name: "S3_INSECURE_SKIP_VERIFY", Value: skipVerify}},
}}}
if caSecret != "" {
spec.Volumes = []corev1.Volume{{
Name: "s3-ca",
VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: caSecret}},
}}
}
return spec
}
func TestRedisS3Trust(t *testing.T) {
caSecret := func(data map[string]string) *corev1.Secret {
s := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "s3-endpoint-ca"}, Data: map[string][]byte{}}
for k, v := range data {
s.Data[k] = []byte(v)
}
return s
}
cases := map[string]struct {
spec *corev1.PodSpec
objs []client.Object
wantCA string
wantSkip bool
}{
"ca.crt wins over tls.crt and skip-verify": {
spec: redisPodSpec("s3-endpoint-ca", "true"),
objs: []client.Object{caSecret(map[string]string{"ca.crt": "CA", "tls.crt": "TLS"})},
wantCA: "CA",
},
"tls.crt when there is no ca.crt": {
spec: redisPodSpec("s3-endpoint-ca", ""),
objs: []client.Object{caSecret(map[string]string{"tls.crt": "TLS"})},
wantCA: "TLS",
},
"volume items map the file to another key": {
spec: func() *corev1.PodSpec {
s := redisPodSpec("s3-endpoint-ca", "")
s.Volumes[0].Secret.Items = []corev1.KeyToPath{{Key: "bundle.pem", Path: "ca.crt"}}
return s
}(),
objs: []client.Object{caSecret(map[string]string{"bundle.pem": "MAPPED", "ca.crt": "UNMAPPED"})},
wantCA: "MAPPED",
},
"missing CA Secret falls through to skip-verify": {
spec: redisPodSpec("s3-endpoint-ca", "true"),
wantSkip: true,
},
"skip-verify only when it is exactly true": {
spec: redisPodSpec("", "false"),
},
"no pod spec": {},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
c := newMariaDBStrategyTestClient(t, tc.objs...)
r := &BackupJobReconciler{Client: c}
ca, skip, err := r.redisS3Trust(context.Background(), "tenant", tc.spec)
if err != nil {
t.Fatalf("redisS3Trust: %v", err)
}
if string(ca) != tc.wantCA || skip != tc.wantSkip {
t.Errorf("trust = (%q, %v), want (%q, %v)", ca, skip, tc.wantCA, tc.wantSkip)
}
})
}
}
func TestCreateRedisBackupArtifact_Artifact(t *testing.T) {
@@ -143,7 +224,7 @@ func TestCreateRedisBackupArtifact_Artifact(t *testing.T) {
t.Run("records the snapshot location and size", func(t *testing.T) {
l := &fakeS3Lister{statSize: 777}
r := redisReconciler(t, l, j, projectedCredsSecret(nil))
got, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey)
got, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey, nil)
if err != nil {
t.Fatalf("createRedisBackupArtifact: %v", err)
}
@@ -156,7 +237,7 @@ func TestCreateRedisBackupArtifact_Artifact(t *testing.T) {
existing := &backupsv1alpha1.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "bj-1"}}
l := &fakeS3Lister{statSize: 777}
r := redisReconciler(t, l, j, existing, projectedCredsSecret(nil))
if _, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey); err != nil {
if _, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey, nil); err != nil {
t.Fatalf("createRedisBackupArtifact: %v", err)
}
if l.calls != 0 {
@@ -164,3 +245,73 @@ func TestCreateRedisBackupArtifact_Artifact(t *testing.T) {
}
})
}
// TestReconcileRedis_ArtifactUsesTheJobTrust pins the wiring from the
// completed backup Job to the probe: the s3-ca volume of the pod the Job
// ran must reach the S3 connection.
func TestReconcileRedis_ArtifactUsesTheJobTrust(t *testing.T) {
app := newRedisApp("cache", "tenant-test")
strategy := newRedisStrategy("redis-strategy")
now := metav1.Now()
backupJob := &backupsv1alpha1.BackupJob{
ObjectMeta: metav1.ObjectMeta{Name: "test-bj", Namespace: "tenant-test"},
Spec: backupsv1alpha1.BackupJobSpec{
ApplicationRef: newRedisAppRef("cache"),
BackupClassName: "cozy-default",
},
Status: backupsv1alpha1.BackupJobStatus{StartedAt: &now, Phase: backupsv1alpha1.BackupJobPhaseRunning},
}
completedK8sJob := &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{
Name: jobNameForBackupJob(backupJob),
Namespace: backupJob.Namespace,
Labels: map[string]string{
backupsv1alpha1.OwningJobNameLabel: backupJob.Name,
backupsv1alpha1.OwningJobNamespaceLabel: backupJob.Namespace,
},
},
Spec: batchv1.JobSpec{Template: corev1.PodTemplateSpec{Spec: *redisPodSpec("s3-endpoint-ca", "")}},
Status: batchv1.JobStatus{
CompletionTime: &now,
Conditions: []batchv1.JobCondition{{Type: batchv1.JobComplete, Status: corev1.ConditionTrue}},
},
}
creds := projectedCredsSecret(nil)
creds.Namespace = "tenant-test"
ca := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "tenant-test", Name: "s3-endpoint-ca"},
Data: map[string][]byte{"ca.crt": []byte("CA")},
}
resolved := newRedisResolved("redis-strategy", map[string]string{"bucketName": "redis-bucket"})
r, _ := newRedisTestEnv(t, app, clientfake.NewClientBuilder().WithObjects(backupJob, strategy, completedK8sJob, creds, ca))
l := &fakeS3Lister{statSize: 512}
r.CredentialsConfig.TargetSecretName = "cozy-backups-creds"
r.newS3ObjectLister = func(conn s3Connection) (s3ObjectLister, error) {
l.conn = conn
return l, nil
}
ctx := context.Background()
if _, err := r.reconcileRedis(ctx, backupJob, resolved); err != nil {
t.Fatalf("reconcileRedis() error = %v", err)
}
if string(l.conn.CAPEM) != "CA" {
t.Errorf("connection CA = %q, want the s3-ca volume's ca.crt", l.conn.CAPEM)
}
updated := &backupsv1alpha1.BackupJob{}
if err := r.Get(ctx, client.ObjectKeyFromObject(backupJob), updated); err != nil {
t.Fatalf("get backupjob: %v", err)
}
if updated.Status.BackupRef == nil {
t.Fatalf("expected BackupRef set")
}
created := &backupsv1alpha1.Backup{}
if err := r.Get(ctx, client.ObjectKey{Namespace: backupJob.Namespace, Name: updated.Status.BackupRef.Name}, created); err != nil {
t.Fatalf("get Backup: %v", err)
}
if created.Status.Artifact == nil || created.Status.Artifact.SizeBytes != 512 {
t.Errorf("Artifact = %#v, want SizeBytes 512", created.Status.Artifact)
}
}
@@ -366,7 +366,7 @@ func (r *BackupJobReconciler) reconcileRedis(ctx context.Context, j *backupsv1al
if j.Status.BackupRef != nil {
return ctrl.Result{}, nil
}
artifact, err := r.createRedisBackupArtifact(ctx, j, resolved, objectKey)
artifact, err := r.createRedisBackupArtifact(ctx, j, resolved, objectKey, &batchJob.Spec.Template.Spec)
if err != nil {
return r.failBackupArtifact(ctx, j, err)
}
@@ -471,6 +471,7 @@ func (r *BackupJobReconciler) createRedisBackupArtifact(
j *backupsv1alpha1.BackupJob,
resolved *ResolvedBackupConfig,
objectKey string,
podSpec *corev1.PodSpec,
) (*backupsv1alpha1.Backup, error) {
existing, probe := r.existingBackup(ctx, j.Namespace, j.Name)
if existing != nil {
@@ -497,7 +498,7 @@ func (r *BackupJobReconciler) createRedisBackupArtifact(
},
}
if probe {
backup.Status.Artifact = r.redisArtifact(ctx, j.Namespace, objectKey)
backup.Status.Artifact = r.redisArtifact(ctx, j.Namespace, objectKey, podSpec)
}
if j.Spec.PlanRef != nil {
backup.Spec.PlanRef = j.Spec.PlanRef