diff --git a/internal/backupcontroller/artifact_size.go b/internal/backupcontroller/artifact_size.go index dc23542b9..230ca3ce8 100644 --- a/internal/backupcontroller/artifact_size.go +++ b/internal/backupcontroller/artifact_size.go @@ -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 } diff --git a/internal/backupcontroller/redis_artifact_size.go b/internal/backupcontroller/redis_artifact_size.go index 0e76d0b9c..1c66f49af 100644 --- a/internal/backupcontroller/redis_artifact_size.go +++ b/internal/backupcontroller/redis_artifact_size.go @@ -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 "" +} diff --git a/internal/backupcontroller/redis_artifact_size_test.go b/internal/backupcontroller/redis_artifact_size_test.go index 458726b67..a167d8f4a 100644 --- a/internal/backupcontroller/redis_artifact_size_test.go +++ b/internal/backupcontroller/redis_artifact_size_test.go @@ -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) + } +} diff --git a/internal/backupcontroller/redisstrategy_controller.go b/internal/backupcontroller/redisstrategy_controller.go index 40f3a99aa..76030cb10 100644 --- a/internal/backupcontroller/redisstrategy_controller.go +++ b/internal/backupcontroller/redisstrategy_controller.go @@ -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