msp: add support for secret volume mounts (#61185)

Closes https://github.com/sourcegraph/managed-services/issues/589, which blocks sourcegraph/managed-services#655

This allows operators to specify volumes to mount from secrets. The configuration is intentionally limited for now, for example:

```yaml
secretVolumes:
  myVolumeName:
    mountPath: "foobar.pem"
    secret: "foobar"
```

It has similar capabilities as `secretEnv`, e.g. being able to source from external secrets and automatically have the appropriate access provisioned.

## Test plan

n/a - we use this already internally with Redis certs. This just exposes the same functionality to operators

Author: bobheadxi

dev/managedservicesplatform/managedservicesplatform.go

@@ -108,6 +108,7 @@ func (r *Renderer) RenderEnvironment(
 		Image:           svc.Build.Image,
 		Service:         svc.Service,
 		SecretEnv:       env.SecretEnv,
+		SecretVolumes:   env.SecretVolumes,
 		PreventDestroys: preventDestroys,
 
 		IsFinalStageOfRollout: rolloutPipeline != nil,

dev/managedservicesplatform/spec/environment.go

@@ -78,6 +78,10 @@ type EnvironmentSpec struct {
 	// target project will be automatically granted.
 	SecretEnv map[string]string `yaml:"secretEnv,omitempty"`
 
+	// SecretVolumes configures volumes to mount from secrets. Keys are used
+	// as volume names.
+	SecretVolumes map[string]EnvironmentSecretVolume `yaml:"secretVolumes,omitempty"`
+
 	// Resources configures additional resources that a service may depend on.
 	Resources *EnvironmentResourcesSpec `yaml:"resources,omitempty"`
 
@@ -121,6 +125,12 @@ func (s EnvironmentSpec) Validate() []error {
 	errs = append(errs, s.Deploy.Validate()...)
 	errs = append(errs, s.Resources.Validate()...)
 	errs = append(errs, s.Instances.Validate()...)
+	for k, v := range s.SecretVolumes {
+		if k == "" {
+			errs = append(errs, errors.New("secretVolumes key cannot be empty"))
+		}
+		errs = append(errs, v.Validate()...)
+	}
 
 	// Validate service-specific specs
 	errs = append(errs, s.EnvironmentServiceSpec.Validate()...)
@@ -639,6 +649,30 @@ func (s *EnvironmentJobScheduleSpec) FindMaxCronInterval() (*time.Duration, erro
 	return &maxGap, nil
 }
 
+type EnvironmentSecretVolume struct {
+	// MountPath is the path within the container where the secret will be
+	// mounted. The mounted file is read-only.
+	MountPath string `yaml:"mountPath"`
+	// Secret is name of the secret in the service's project to populate in the
+	// environment.
+	//
+	// To point to a secret in another project, use the format
+	// 'projects/{project}/secrets/{secretName}' in the value. Access to the
+	// target project will be automatically granted.
+	Secret string `yaml:"secret"`
+}
+
+func (v EnvironmentSecretVolume) Validate() []error {
+	var errs []error
+	if v.MountPath == "" {
+		errs = append(errs, errors.New("mountPath is required"))
+	}
+	if v.Secret == "" {
+		errs = append(errs, errors.New("secret is required"))
+	}
+	return errs
+}
+
 type EnvironmentResourcesSpec struct {
 	// Redis, if provided, provisions a Redis instance backed by Cloud Memorystore.
 	// Details for using this Redis instance is automatically provided in

dev/managedservicesplatform/stacks/cloudrun/cloudrun.go

@@ -157,6 +157,9 @@ func NewStack(stacks *stack.Set, vars Variables) (crossStackOutput *CrossStackOu
 		return nil, errors.Wrap(err, "add user env vars")
 	}
 
+	// Add user-configured secret volumes
+	addContainerSecretVolumes(cloudRunBuilder, vars.Environment.SecretVolumes)
+
 	// Load image tag from tfvars.
 	imageTag := tfvar.New(stack, id, tfvar.Config{
 		VariableKey: tfVarKeyResolvedImageTag,
@@ -491,6 +494,24 @@ func addContainerEnvVars(
 	return nil
 }
 
+func addContainerSecretVolumes(
+	b builder.Builder,
+	volumes map[string]spec.EnvironmentSecretVolume,
+) {
+	keys := maps.Keys(volumes)
+	slices.Sort(keys)
+	for _, k := range keys {
+		v := volumes[k]
+		b.AddSecretVolume(k, v.MountPath,
+			builder.SecretRef{
+				Name:    v.Secret,
+				Version: "latest",
+			},
+			292, // 0444 read-only
+		)
+	}
+}
+
 func makeContainerResourceLimits(r spec.EnvironmentInstancesResourcesSpec) map[string]*string {
 	return map[string]*string{
 		"cpu":    pointers.Ptr(strconv.Itoa(r.CPU)),

dev/managedservicesplatform/stacks/iam/BUILD.bazel

@@ -35,6 +35,7 @@ go_test(
     srcs = ["iam_test.go"],
     embed = [":iam"],
     deps = [
+        "//dev/managedservicesplatform/spec",
         "//lib/pointers",
         "@com_github_hexops_autogold_v2//:autogold",
         "@com_github_stretchr_testify//assert",

dev/managedservicesplatform/stacks/iam/iam.go

@@ -41,6 +41,8 @@ type Variables struct {
 
 	// SecretEnv should be the environment config that sources from secrets.
 	SecretEnv map[string]string
+	// SecretVolumes should be the environment config that mounts volumes from secrets.
+	SecretVolumes map[string]spec.EnvironmentSecretVolume
 
 	// IsFinalStageOfRollout should be true if BuildRolloutPipelineConfiguration
 	// provides a non-nil configuration for an environment.
@@ -202,7 +204,7 @@ func NewStack(stacks *stack.Set, vars Variables) (*CrossStackOutput, error) {
 
 	// If any secret env secrets are external to this project, grant access to
 	// the referenced secrets.
-	if externalSecrets, err := extractExternalSecrets(vars.SecretEnv); err != nil {
+	if externalSecrets, err := extractExternalSecrets(vars.SecretEnv, vars.SecretVolumes); err != nil {
 		return nil, errors.Wrap(err, "extracting secret projects")
 	} else {
 		secretAccessID := id.Group("external_secret_access")
@@ -300,44 +302,70 @@ type externalSecret struct {
 	secretID  string
 }
 
-func extractExternalSecrets(secrets map[string]string) ([]externalSecret, error) {
+func extractExternalSecrets(secretEnv map[string]string, secretVolumes map[string]spec.EnvironmentSecretVolume) ([]externalSecret, error) {
 	var externalSecrets []externalSecret
 
-	// Sort for stability
-	secretKeys := maps.Keys(secrets)
+	// Add secretEnv
+	secretKeys := maps.Keys(secretEnv)
 	sort.Strings(secretKeys)
 	for _, k := range secretKeys {
-		secretName := secrets[k]
-		// Error on easy-to-make oopsies
-		if strings.HasPrefix(secretName, "project/") {
-			return nil, errors.Newf("invalid secret name %q: 'project/'-prefixed name provided, did you mean 'projects/'?",
-				secretName)
+		secretName := secretEnv[k]
+		es, err := getExternalSecretFromSecretName(k, secretName)
+		if err != nil {
+			return nil, err
 		}
-		// Crude check to tell users that they shouldn't include versions in their secrets
-		if strings.Contains(secretName, "/versions/") {
-			return nil, errors.Newf("invalid secret name %q: secrets should not be versioned with '/version/'",
-				secretName)
+		if es != nil {
+			externalSecrets = append(externalSecrets, *es)
 		}
-		// Check for 'projects/{project}/secrets/{secretName}'
-		if strings.HasPrefix(secretName, "projects/") {
-			secretNameParts := strings.SplitN(secretName, "/", 4)
-			if len(secretNameParts) != 4 {
-				return nil, errors.Newf("invalid secret name %q: expected 'projects/'-prefixed name to have 4 '/'-delimited parts",
-					secretName)
-			}
-			// Error on easy-to-make oopsies
-			if secretNameParts[2] != "secrets" {
-				return nil, errors.Newf("invalid secret name %q: found '/secret/' segment, did you mean '/secrets/'?",
-					secretName)
-			}
-
-			externalSecrets = append(externalSecrets, externalSecret{
-				key:       strings.ToLower(k),
-				projectID: secretNameParts[1],
-				secretID:  secretNameParts[3],
-			})
+	}
+
+	// Add secretVolumes
+	secretKeys = maps.Keys(secretVolumes)
+	sort.Strings(secretKeys)
+	for _, k := range secretKeys {
+		secretName := secretVolumes[k].Secret
+		es, err := getExternalSecretFromSecretName(fmt.Sprintf("volume_%s", k), secretName)
+		if err != nil {
+			return nil, err
+		}
+		if es != nil {
+			externalSecrets = append(externalSecrets, *es)
 		}
 	}
 
 	return externalSecrets, nil
 }
+
+func getExternalSecretFromSecretName(key, secretName string) (*externalSecret, error) {
+	// Error on easy-to-make oopsies
+	if strings.HasPrefix(secretName, "project/") {
+		return nil, errors.Newf("invalid secret name %q: 'project/'-prefixed name provided, did you mean 'projects/'?",
+			secretName)
+	}
+	// Crude check to tell users that they shouldn't include versions in their secrets
+	if strings.Contains(secretName, "/versions/") {
+		return nil, errors.Newf("invalid secret name %q: secrets should not be versioned with '/version/'",
+			secretName)
+	}
+	// Check for 'projects/{project}/secrets/{secretName}'
+	if strings.HasPrefix(secretName, "projects/") {
+		secretNameParts := strings.SplitN(secretName, "/", 4)
+		if len(secretNameParts) != 4 {
+			return nil, errors.Newf("invalid secret name %q: expected 'projects/'-prefixed name to have 4 '/'-delimited parts",
+				secretName)
+		}
+		// Error on easy-to-make oopsies
+		if secretNameParts[2] != "secrets" {
+			return nil, errors.Newf("invalid secret name %q: found '/secret/' segment, did you mean '/secrets/'?",
+				secretName)
+		}
+
+		return &externalSecret{
+			key:       strings.ToLower(key),
+			projectID: secretNameParts[1],
+			secretID:  secretNameParts[3],
+		}, nil
+	}
+
+	return nil, nil
+}

dev/managedservicesplatform/stacks/iam/iam_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/sourcegraph/sourcegraph/dev/managedservicesplatform/spec"
 	"github.com/sourcegraph/sourcegraph/lib/pointers"
 )
 
@@ -48,6 +49,7 @@ func TestExtractExternalSecrets(t *testing.T) {
 	for _, tc := range []struct {
 		name                string
 		secretEnv           map[string]string
+		secretVolumes       map[string]spec.EnvironmentSecretVolume
 		wantExternalSecrets []externalSecret
 		wantError           autogold.Value
 	}{
@@ -84,9 +86,38 @@ func TestExtractExternalSecrets(t *testing.T) {
 				secretID:  "BAR",
 			}},
 		},
+		{
+			name: "volumes has external secret",
+			secretVolumes: map[string]spec.EnvironmentSecretVolume{
+				"secret":  {Secret: "projects/foo/secrets/VOLUME"},
+				"secret2": {Secret: "SEKRET_VOLUME"},
+			},
+			wantExternalSecrets: []externalSecret{{
+				key:       "volume_secret",
+				projectID: "foo",
+				secretID:  "VOLUME",
+			}},
+		},
+		{
+			name:      "external secrets from volumes and env",
+			secretEnv: map[string]string{"SEKRET": "projects/foo/secrets/BAR", "NOT_EXTERNAL": "SEKRET"},
+			secretVolumes: map[string]spec.EnvironmentSecretVolume{
+				"secret":  {Secret: "projects/foo/secrets/VOLUME"},
+				"secret2": {Secret: "SEKRET_VOLUME"},
+			},
+			wantExternalSecrets: []externalSecret{{
+				key:       "sekret",
+				projectID: "foo",
+				secretID:  "BAR",
+			}, {
+				key:       "volume_secret",
+				projectID: "foo",
+				secretID:  "VOLUME",
+			}},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			got, err := extractExternalSecrets(tc.secretEnv)
+			got, err := extractExternalSecrets(tc.secretEnv, tc.secretVolumes)
 			if tc.wantError != nil {
 				require.Error(t, err)
 				tc.wantError.Equal(t, err.Error())