diff --git a/controllers/mover/rclone/mover.go b/controllers/mover/rclone/mover.go index 9cbe697b7..b24cc2a00 100644 --- a/controllers/mover/rclone/mover.go +++ b/controllers/mover/rclone/mover.go @@ -246,7 +246,11 @@ func (m *Mover) ensureJob(ctx context.Context, dataPVC *corev1.PersistentVolumeC } job.Spec.Parallelism = ¶llelism - envVars := []corev1.EnvVar{ + envVars := []corev1.EnvVar{} + // Rclone env vars if they are in the secret + envVars = utils.AppendRCloneEnvVars(rcloneConfigSecret, envVars) + + defaultEnvVars := []corev1.EnvVar{ {Name: "RCLONE_CONFIG", Value: "/rclone-config/rclone.conf"}, {Name: "RCLONE_DEST_PATH", Value: *m.rcloneDestPath}, {Name: "DIRECTION", Value: direction}, @@ -254,6 +258,10 @@ func (m *Mover) ensureJob(ctx context.Context, dataPVC *corev1.PersistentVolumeC {Name: "RCLONE_CONFIG_SECTION", Value: *m.rcloneConfigSection}, } + // Add our defaults after RCLONE_ env vars so any duplicates will be + // overridden by the defaults + envVars = append(envVars, defaultEnvVars...) + // Cluster-wide proxy settings envVars = utils.AppendEnvVarsForClusterWideProxy(envVars) diff --git a/controllers/mover/rclone/rclone_test.go b/controllers/mover/rclone/rclone_test.go index bb99ff035..d2e375ba4 100644 --- a/controllers/mover/rclone/rclone_test.go +++ b/controllers/mover/rclone/rclone_test.go @@ -1323,6 +1323,126 @@ var _ = Describe("Rclone as a source", func() { }) }) + When("RCLONE_ env vars are in the rclone secret", func() { + BeforeEach(func() { + rcloneConfigSecret.StringData = map[string]string{ + "rclone.config": "datahere", + "RCLONE_CONFIG_MYS3_TYPE": "mys3", + "RCLONE_CONFIG_MYS3_ACCESS_KEY_ID": "9999", + "RCLONE_CONFIG_MYS3_SECRET_ACCESS_KEY": "111222", + "RCLONE_BWLIMIT": "5M", + "othervar": "abc123", + "RCLONE_CONFIG": "bad", // Should not override what we set + } + }) + + It("Should set the env vars in the mover job pod", func() { + j, e := mover.ensureJob(ctx, sPVC, sa, rcloneConfigSecret, nil) // Using sPVC as dataPVC (i.e. direct) + Expect(e).NotTo(HaveOccurred()) + Expect(j).To(BeNil()) // hasn't completed + nsn := types.NamespacedName{Name: jobName, Namespace: ns.Name} + job = &batchv1.Job{} + Expect(k8sClient.Get(ctx, nsn, job)).To(Succeed()) + + env := job.Spec.Template.Spec.Containers[0].Env + + // First validate all our normal env vars should be set + validateJobEnvVars(env, true) + + t := true + + // Validate our RCLONE_ env vars got set + Expect(env).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_CONFIG_MYS3_TYPE", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rcloneConfigSecret.GetName(), + }, + Key: "RCLONE_CONFIG_MYS3_TYPE", + Optional: &t, + }, + }, + })) + Expect(env).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_CONFIG_MYS3_ACCESS_KEY_ID", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rcloneConfigSecret.GetName(), + }, + Key: "RCLONE_CONFIG_MYS3_ACCESS_KEY_ID", + Optional: &t, + }, + }, + })) + Expect(env).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_CONFIG_MYS3_SECRET_ACCESS_KEY", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rcloneConfigSecret.GetName(), + }, + Key: "RCLONE_CONFIG_MYS3_SECRET_ACCESS_KEY", + Optional: &t, + }, + }, + })) + Expect(env).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_BWLIMIT", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rcloneConfigSecret.GetName(), + }, + Key: "RCLONE_BWLIMIT", + Optional: &t, + }, + }, + })) + // This one should still get set + Expect(env).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_CONFIG", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rcloneConfigSecret.GetName(), + }, + Key: "RCLONE_CONFIG", + Optional: &t, + }, + }, + })) + + // Confirm the RCLONE_CONFIG env var we set is after + // the one the user tried to override in the rclone secret + // to avoid users accidentally overriding something we need in + // the mover job + rcloneConfigEnvVars := []corev1.EnvVar{} + for _, envVar := range env { + if envVar.Name == "RCLONE_CONFIG" { + rcloneConfigEnvVars = append(rcloneConfigEnvVars, envVar) + } + } + Expect(len(rcloneConfigEnvVars)).To(Equal(2)) + first := rcloneConfigEnvVars[0] + second := rcloneConfigEnvVars[1] + // First one in the env array should be the one from the secret + Expect(first.ValueFrom).To(Equal(&corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rcloneConfigSecret.GetName(), + }, + Key: "RCLONE_CONFIG", + Optional: &t, + }, + })) + // Second one should be the one we set for the rclone mover + // (this one will take precedence in the mover pod) + Expect(second.Value).To(Equal("/rclone-config/rclone.conf")) + }) + }) + It("Should have correct volume mounts", func() { j, e := mover.ensureJob(ctx, sPVC, sa, rcloneConfigSecret, nil) // Using sPVC as dataPVC (i.e. direct) Expect(e).NotTo(HaveOccurred()) @@ -1963,10 +2083,13 @@ func validateJobEnvVars(env []corev1.EnvVar, isSource bool) { func validateEnvVar(env []corev1.EnvVar, envVarName, envVarExpectedValue string) { found := false - for _, envVar := range env { + // Reverse over range, look for env vars from the end first + for i := len(env) - 1; i >= 0; i-- { + envVar := env[i] if envVar.Name == envVarName { found = true Expect(envVar.Value).To(Equal(envVarExpectedValue)) + break } } Expect(found).To(BeTrue()) diff --git a/controllers/mover/restic/mover.go b/controllers/mover/restic/mover.go index a0491da3b..8a8591e32 100644 --- a/controllers/mover/restic/mover.go +++ b/controllers/mover/restic/mover.go @@ -22,9 +22,7 @@ import ( "errors" "fmt" "path" - "sort" "strconv" - "strings" "time" "github.com/go-logr/logr" @@ -409,7 +407,7 @@ func (m *Mover) ensureJob(ctx context.Context, cachePVC *corev1.PersistentVolume } // Rclone env vars for restic if they are in the secret - envVars = appendRCloneEnvVars(repo, envVars) + envVars = utils.AppendRCloneEnvVars(repo, envVars) // Cluster-wide proxy settings envVars = utils.AppendEnvVarsForClusterWideProxy(envVars) @@ -636,24 +634,3 @@ func generateForgetOptions(policy *volsyncv1alpha1.ResticRetainPolicy) string { } return forget } - -// Append k/v from the restic secret if they start with RCLONE_ -func appendRCloneEnvVars(resticSecret *corev1.Secret, envVars []corev1.EnvVar) []corev1.EnvVar { - rcloneKeys := []string{} - for key := range resticSecret.Data { - if strings.HasPrefix(key, "RCLONE_") { - rcloneKeys = append(rcloneKeys, key) - } - } - - if len(rcloneKeys) > 0 { - // Sort so we do not keep re-ordering env vars (range over map keys will give us inconsistent sorting) - sort.Strings(rcloneKeys) - - for _, sortedKey := range rcloneKeys { - envVars = append(envVars, utils.EnvFromSecret(resticSecret.Name, sortedKey, true)) - } - } - - return envVars -} diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index 38c01dbc9..07be9f57c 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -21,6 +21,7 @@ import ( "context" "fmt" "os" + "sort" "strings" "github.com/go-logr/logr" @@ -187,6 +188,27 @@ func AppendEnvVarsForClusterWideProxy(envVars []corev1.EnvVar) []corev1.EnvVar { return envVars } +// Append k/v from the secret if they start with RCLONE_ +func AppendRCloneEnvVars(secret *corev1.Secret, envVars []corev1.EnvVar) []corev1.EnvVar { + rcloneKeys := []string{} + for key := range secret.Data { + if strings.HasPrefix(key, "RCLONE_") { + rcloneKeys = append(rcloneKeys, key) + } + } + + if len(rcloneKeys) > 0 { + // Sort so we do not keep re-ordering env vars (range over map keys will give us inconsistent sorting) + sort.Strings(rcloneKeys) + + for _, sortedKey := range rcloneKeys { + envVars = append(envVars, EnvFromSecret(secret.Name, sortedKey, true)) + } + } + + return envVars +} + // Updates to set the securityContext, podLabels on mover pod in the spec and resourceRequirements on the mover // containers based on what is set in the MoverConfig func UpdatePodTemplateSpecFromMoverConfig(podTemplateSpec *corev1.PodTemplateSpec, diff --git a/controllers/utils/utils_test.go b/controllers/utils/utils_test.go index 257fe836d..0b9a5a6bd 100644 --- a/controllers/utils/utils_test.go +++ b/controllers/utils/utils_test.go @@ -225,6 +225,117 @@ var _ = Describe("utils tests", func() { }) }) + Describe("AppendRCloneEnvVars", func() { + envVarsOrig := []corev1.EnvVar{ + { + Name: "existingvar1", + Value: "value1", + }, + { + Name: "EXISTINGVAR2", + Value: "VALUE2", + }, + } + + var envVars []corev1.EnvVar + var secret *corev1.Secret + + BeforeEach(func() { + // Reset envVars back to initial starting value for test + envVars = make([]corev1.EnvVar, len(envVarsOrig)) + copy(envVars, envVarsOrig) + + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret-withrclonevars", + Namespace: "test-secret-namespace", + }, + Data: map[string][]byte{}, + } + }) + + When("no secret env vars are set", func() { + It("Should not modify the existing env vars", func() { + envVars = utils.AppendRCloneEnvVars(secret, envVars) + Expect(envVars).To(Equal(envVarsOrig)) + }) + }) + + When("no env vars containing RCLONE_ are set in the secret", func() { + BeforeEach(func() { + secret.Data = map[string][]byte{ + "testkey": []byte("pineapples"), + "NOT_RCLONE_VAR": []byte("kiwis"), + "OTHER_VAR": []byte("oranges"), + } + }) + It("Should not modify the existing env vars", func() { + envVars = utils.AppendRCloneEnvVars(secret, envVars) + Expect(envVars).To(Equal(envVarsOrig)) + }) + }) + + When("RCLONE_ env vars are set", func() { + BeforeEach(func() { + secret.Data = map[string][]byte{ + "RCLONE_TESTVAR1": []byte("veryimportant"), + "RCLONE_TESTVAR2": []byte("evenmoreimportant"), + "NOT_RCLONE_VAR": []byte("shouldntbeset"), + "RCLONE_BWLIMIT": []byte("5M:10M"), + } + }) + + It("Should set the appropriate env vars", func() { + envVars = utils.AppendRCloneEnvVars(secret, envVars) + for i := range envVarsOrig { + // Original env vars should still be set + origVar := envVarsOrig[i] + Expect(envVars).To(ContainElements(origVar)) + } + + t := true + + // RCLONE_ env vars from secret should be set + Expect(envVars).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_TESTVAR1", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret.GetName(), + }, + Key: "RCLONE_TESTVAR1", + Optional: &t, + }, + }, + })) + Expect(envVars).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_TESTVAR2", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret.GetName(), + }, + Key: "RCLONE_TESTVAR2", + Optional: &t, + }, + }, + })) + Expect(envVars).To(ContainElement(corev1.EnvVar{ + Name: "RCLONE_BWLIMIT", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret.GetName(), + }, + Key: "RCLONE_BWLIMIT", + Optional: &t, + }, + }, + })) + }) + }) + }) + Describe("UpdatePodTemplateSpecFromMoverConfig", func() { When("no pod template spec", func() { It("should not fail", func() { diff --git a/docs/usage/rclone/rclone-secret.rst b/docs/usage/rclone/rclone-secret.rst index e1daee129..a998c538f 100644 --- a/docs/usage/rclone/rclone-secret.rst +++ b/docs/usage/rclone/rclone-secret.rst @@ -5,6 +5,8 @@ Understanding ``rclone-secret`` The Rclone Secret provides the configuration details to locate and access the intermediary storage system. It is mounted as a secret on the Rclone data mover pod and provided to the Rclone executable. +The secret should contain the key ``rclone.conf`` that contains the contents of your rclone.conf file. Here is an +example rclone.conf: .. code:: none @@ -48,3 +50,36 @@ Secret can be created via: rclone-secret Opaque 1 17s This Secret should be created on both the source and the destination locations. + +Using ``RCLONE_`` environment variables in ``rclone-secret`` +============================================================ + +Rclone has the ability to set environment variables for configuration. Environment variables that +start with ``RCLONE_`` can be set as key/value pairs in the ``rclone-secret`` and they will be passed +to the rclone mover job. + +Here is an example ``rclone-secret`` that sets ``RCLONE_BWLIMIT`` to 5M: + +.. code-block:: yaml + + apiVersion: v1 + kind: Secret + metadata: + name: rclone-secret + type: Opaque + stringData: + # equivalent to the --bwlimit command line flag + RCLONE_BWLIMIT: 5M + # rclone.conf + rclone.conf: | + [s3-bucket] + type = s3 + provider = Minio + env_auth = false + access_key_id = user1 + secret_access_key = abc123 + region = us-east-1 + endpoint = http://minio.minio.svc.cluster.local:9000 + +For detailed information on Rclone environment variables see the +`Rclone environment variable documentation `_.