From 7834e93307ff398647a1a282582f2c4687f31caf Mon Sep 17 00:00:00 2001 From: Lionel Sambuc Date: Thu, 4 Jun 2026 09:00:18 +0200 Subject: [PATCH 1/3] feat: Upload DCs secrets --- api/v1alpha1/amaltheasession_children.go | 123 +++++++++-- go.mod | 2 + go.sum | 4 + internal/common/data_connector.go | 198 ++++++++++++++++++ internal/remote/firecrest/controller.go | 205 ++++++++++++++----- internal/remote/firecrest/controller_test.go | 5 +- 6 files changed, 466 insertions(+), 71 deletions(-) create mode 100644 internal/common/data_connector.go diff --git a/api/v1alpha1/amaltheasession_children.go b/api/v1alpha1/amaltheasession_children.go index 552e31ef..12171664 100644 --- a/api/v1alpha1/amaltheasession_children.go +++ b/api/v1alpha1/amaltheasession_children.go @@ -4,16 +4,19 @@ import ( "context" "crypto/rand" "encoding/base64" + "encoding/json" "errors" "fmt" "io" "maps" "net/url" "os" + "path" "sort" "strings" "time" + "github.com/SwissDataScienceCenter/amalthea/internal/common" "github.com/SwissDataScienceCenter/amalthea/internal/controller/config" "gopkg.in/yaml.v3" appsv1 "k8s.io/api/apps/v1" @@ -642,11 +645,92 @@ func (cr *AmaltheaSession) AdoptedSecrets() v1.SecretList { // Assuming that the csi-rclone driver from https://github.com/SwissDataScienceCenter/csi-rclone // is installed, this will generate PVCs for the data sources that have the rclone type. func (as *AmaltheaSession) DataSources() ([]v1.PersistentVolumeClaim, []v1.Volume, []v1.VolumeMount) { - // TODO: Configure this for remote sessions - if as.Spec.SessionLocation == Remote { - return []v1.PersistentVolumeClaim{}, []v1.Volume{}, []v1.VolumeMount{} + switch as.Spec.SessionLocation { + case Remote: + return as.RemoteSessionDataSources() + case Local: + return as.LocalSessionDataSources() + default: + panic("invalid session location") + } +} + +func (as *AmaltheaSession) RemoteSessionDataSources() ([]v1.PersistentVolumeClaim, []v1.Volume, []v1.VolumeMount) { + pvcs := []v1.PersistentVolumeClaim{} + vols := []v1.Volume{ + { + Name: fmt.Sprintf("%s-%s", prefix, as.Name), + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: as.InternalSecretName(), + DefaultMode: ptr.To(int32(256)), // decimal value of 0400 for the access flags (chmod-like) + }, + }, + }, + } + volMounts := []v1.VolumeMount{ + { + Name: fmt.Sprintf("%s-%s", prefix, as.Name), + ReadOnly: true, + MountPath: common.UserSecretProxyFolder, + }, + } + + ids := 0 + for _, pv := range as.Spec.DataSources { + if pv.SecretRef.isAdopted() { + volName := fmt.Sprintf("%s%s-ds-%d", prefix, as.Name, ids) + vols = append( + vols, + v1.Volume{ + Name: volName, + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: pv.SecretRef.Name, + DefaultMode: ptr.To(int32(256)), // decimal value of 0400 for the access flags (chmod-like) + }, + }, + }, + ) + volMounts = append( + volMounts, + v1.VolumeMount{ + Name: volName, + ReadOnly: true, + MountPath: path.Join(common.DataConnectorProxyFolder, volName), + }, + ) + // If there is a user secret linked to the data connector, mount it as it contains required credentials + userSecretName := fmt.Sprintf("%s-secrets", pv.SecretRef.Name) + volNameSecret := fmt.Sprintf("%s-secrets", volName) + vols = append( + vols, + v1.Volume{ + Name: volNameSecret, + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: userSecretName, + Optional: ptr.To(true), + DefaultMode: ptr.To(int32(256)), // decimal value of 0400 for the access flags (chmod-like) + }, + }, + }, + ) + volMounts = append( + volMounts, + v1.VolumeMount{ + Name: volNameSecret, + ReadOnly: true, + MountPath: path.Join(common.DataConnectorSecretProxyFolder, volName), + }, + ) + ids += 1 + } } + return pvcs, vols, volMounts +} +func (as *AmaltheaSession) LocalSessionDataSources() ([]v1.PersistentVolumeClaim, []v1.Volume, []v1.VolumeMount) { pvcs := []v1.PersistentVolumeClaim{} vols := []v1.Volume{} volMounts := []v1.VolumeMount{} @@ -759,14 +843,30 @@ func (as *AmaltheaSession) Secret() v1.Secret { }, StringData: map[string]string{}, } - // Secret used to secure the tunnel for remote sessions + if as.Spec.SessionLocation == Remote { + // Secret used to secure the tunnel for remote sessions tunnelSecret, err := makeTunnelSecret(16) if err != nil { panic(err) } - - secret.StringData["WSTUNNEL_SECRET"] = tunnelSecret + secret.StringData["wstunnel_secret"] = tunnelSecret + + // Add the Datasources Specifications so that the proxy container can write them out to the HPC cluster + ids := 0 + for _, pv := range as.Spec.DataSources { + if pv.SecretRef.isAdopted() { + + var content []byte + content, err = json.Marshal(pv) + if err != nil { + panic(err) + } + + secret.StringData[fmt.Sprintf("%s%s-ds-%d", prefix, as.Name, ids)] = string(content) + ids += 1 + } + } } // Add the 'oidc' configuration if requested @@ -988,15 +1088,6 @@ func (cr *AmaltheaSession) sessionContainerRemote(volumeMounts []v1.VolumeMount) Name: "RSC_SERVER_PORT", Value: fmt.Sprintf("%d", RemoteSessionControllerPort), }, - v1.EnvVar{ - Name: "RSC_WSTUNNEL_SECRET", - ValueFrom: ptr.To(v1.EnvVarSource{ - SecretKeyRef: ptr.To(v1.SecretKeySelector{ - LocalObjectReference: v1.LocalObjectReference{Name: cr.InternalSecretName()}, - Key: "WSTUNNEL_SECRET", - }), - }), - }, ) if session.RemoteSecretRef != nil { @@ -1056,7 +1147,7 @@ func (cr *AmaltheaSession) tunnelContainer() v1.Container { ValueFrom: ptr.To(v1.EnvVarSource{ SecretKeyRef: ptr.To(v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{Name: cr.InternalSecretName()}, - Key: "WSTUNNEL_SECRET", + Key: "wstunnel_secret", }), }), }, diff --git a/go.mod b/go.mod index f95389bb..bafad934 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ tool fybrik.io/crdoc require ( github.com/distribution/reference v0.6.0 github.com/elazarl/goproxy v1.7.2 + github.com/fernet/fernet-go v0.0.0-20240119011108-303da6aec611 github.com/getkin/kin-openapi v0.132.0 github.com/getsentry/sentry-go v0.45.1 github.com/go-git/go-git/v5 v5.16.0 @@ -27,6 +28,7 @@ require ( github.com/spf13/viper v1.20.1 github.com/stretchr/testify v1.10.0 golang.org/x/sys v0.32.0 + gopkg.in/ini.v1 v1.67.0 gopkg.in/yaml.v3 v3.0.1 k8s.io/api v0.33.0 k8s.io/apimachinery v0.33.0 diff --git a/go.sum b/go.sum index 9722a7f2..8621dbbf 100644 --- a/go.sum +++ b/go.sum @@ -64,6 +64,8 @@ github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjT github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/fernet/fernet-go v0.0.0-20240119011108-303da6aec611 h1:JwYtKJ/DVEoIA5dH45OEU7uoryZY/gjd/BQiwwAOImM= +github.com/fernet/fernet-go v0.0.0-20240119011108-303da6aec611/go.mod h1:zHMNeYgqrTpKyjawjitDg0Osd1P/FmeA0SZLYK3RfLQ= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= @@ -440,6 +442,8 @@ gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWM gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= +gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= +gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME= diff --git a/internal/common/data_connector.go b/internal/common/data_connector.go new file mode 100644 index 00000000..c2efc6b5 --- /dev/null +++ b/internal/common/data_connector.go @@ -0,0 +1,198 @@ +package common + +import ( + "bytes" + "encoding/json" + "errors" + "os" + "path" + "strings" + + "github.com/fernet/fernet-go" + "gopkg.in/ini.v1" +) + +const UserSecretProxyFolder = "/secrets-user" +const DataConnectorProxyFolder = "/secrets-dcs" +const DataConnectorSecretProxyFolder = "/secrets-dcs-secrets" + +type DataConnector struct { + root string + + Name string + Remote string + + RemotePath string + MountOpt string + VfsOpt string + + ExtraArgs []string +} + +func (dc *DataConnector) fernetKey() (*fernet.Key, error) { + // the fernet key is mounted as part of the data source secret + if encodedKey, err := os.ReadFile(path.Join(dc.root, dc.Name, "secretKey")); err == nil { + return fernet.DecodeKey(string(encodedKey)) + } else { + return nil, err + } +} + +func (dc *DataConnector) dataConnectorSecrets() (map[string][]byte, error) { + var err error + + var fernetKey *fernet.Key + fernetKey, err = dc.fernetKey() + if err != nil { + return nil, err + } + + var dirEntries []os.DirEntry + dataConnectorSecretMountPoint := path.Join(DataConnectorSecretProxyFolder, dc.Name) + if dirEntries, err = os.ReadDir(dataConnectorSecretMountPoint); err != nil && !os.IsNotExist(err) { + return nil, err + } + + decodedSecrets := make(map[string][]byte) + for _, dir := range dirEntries { + // TODO: Add support for hierarchies ? + if dir.IsDir() || strings.HasPrefix(dir.Name(), "..") { + continue + } + + var content []byte + if content, err = os.ReadFile(path.Join(dataConnectorSecretMountPoint, dir.Name())); err != nil { + return nil, err + } + decodedSecrets[dir.Name()] = fernet.VerifyAndDecrypt(content, 0, []*fernet.Key{fernetKey}) + } + + return decodedSecrets, nil +} + +func (dc *DataConnector) ConfigFiles() (map[string][]byte, error) { + configFiles := map[string][]byte{} + + content, err := os.ReadFile(path.Join(dc.root, dc.Name, "configData")) + if err != nil { + return nil, err + } + + iniData, err := ini.Load(content) + if err != nil { + return nil, err + } + + section := iniData.Section(dc.Remote) + + dataConnectorSecrets, err := dc.dataConnectorSecrets() + if err != nil && !os.IsNotExist(err) { + return nil, err + } + + // Override values with secrets + for k, v := range dataConnectorSecrets { + switch k { + case "pass": + // Put it in a file, which will allow for passing to `rclone obscure -` before being placed in the config file. + configFiles[k] = v + default: + section.Key(k).SetValue(string(v)) + } + } + + buffer := new(bytes.Buffer) + if _, err = iniData.WriteTo(buffer); err != nil { + return nil, err + } + configFiles["configData"] = buffer.Bytes() + configFiles["remote"] = []byte(dc.Remote) + configFiles["remotePath"] = []byte(dc.RemotePath) + if len(dc.MountOpt) > 0 { + configFiles["mountOpt"] = []byte(dc.MountOpt) + } + if len(dc.VfsOpt) > 0 { + configFiles["vfsOpt"] = []byte(dc.VfsOpt) + } + if len(dc.ExtraArgs) > 0 { + configFiles["extraArgs"] = []byte(strings.Join(dc.ExtraArgs, " ")) + } + + return configFiles, err +} + +// Local definition of the types so that we may unmarshall into typed values +type cSessionSecretRef struct { + Name string `json:"name"` + Key string `json:"key,omitempty"` + Adopt bool `json:"adopt"` +} + +type cDataSource struct { + Type string `json:"type,omitempty"` + MountPath string `json:"mountPath,omitempty"` + AccessMode string `json:"accessMode,omitempty"` + SecretRef *cSessionSecretRef `json:"secretRef,omitempty"` +} + +func parsePV(name string) ([]string, error) { + extraArgs := []string{} + var content []byte + var err error + + if content, err = os.ReadFile(path.Join(UserSecretProxyFolder, name)); err != nil { + return nil, err + } + + ds := &cDataSource{} + if err = json.Unmarshal(content, ds); err != nil { + return nil, err + } + + if !strings.Contains(ds.AccessMode, "Write") { + extraArgs = append(extraArgs, "--read-only") + } + + return extraArgs, nil +} + +func LoadDataConnector(root, name string) (*DataConnector, error) { + var content []byte + var err error + + if content, err = os.ReadFile(path.Join(root, name, "remote")); err != nil { + return nil, err + } + remote := string(content) + + if content, err = os.ReadFile(path.Join(root, name, "remotePath")); err != nil { + return nil, err + } + remotePath := string(content) + + if content, err = os.ReadFile(path.Join(root, name, "mountOpt")); err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, err + } + mountOpt := string(content) + + if content, err = os.ReadFile(path.Join(root, name, "vfsOpt")); err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, err + } + vfsOpt := string(content) + + var extraArgs []string + if extraArgs, err = parsePV(name); err != nil { + return nil, err + } + + dc := &DataConnector{ + root, + name, + remote, + remotePath, + mountOpt, + vfsOpt, + extraArgs, + } + return dc, nil +} diff --git a/internal/remote/firecrest/controller.go b/internal/remote/firecrest/controller.go index 546934f2..c772e69e 100644 --- a/internal/remote/firecrest/controller.go +++ b/internal/remote/firecrest/controller.go @@ -35,6 +35,7 @@ import ( "time" "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1" + "github.com/SwissDataScienceCenter/amalthea/internal/common" "github.com/SwissDataScienceCenter/amalthea/internal/remote/config" "github.com/SwissDataScienceCenter/amalthea/internal/remote/firecrest/auth" "github.com/SwissDataScienceCenter/amalthea/internal/remote/models" @@ -126,51 +127,90 @@ func (c *FirecrestRemoteSessionController) Status(ctx context.Context) (state mo return c.currentStatus, c.currentStatusError } -func (c *FirecrestRemoteSessionController) uploadSecrets(ctx context.Context, remoteSecretsPath string) error { +func walkIfMatch(root string, filter func(dir os.DirEntry) bool, process func(dir os.DirEntry) error, onceBefore ...func() error) error { var err error - localSessionSecretsFolder, exists := os.LookupEnv("RENKU_SECRETS_PATH") - if !exists { - localSessionSecretsFolder = "/secrets" - } - - files, err := os.ReadDir(localSessionSecretsFolder) + dirEntries, err := os.ReadDir(root) if err != nil { if os.IsNotExist(err) { - // The folder does not exist if there are no user secrets defined. return nil } return err } + if onceBefore != nil && onceBefore[0] != nil { + if err = onceBefore[0](); err != nil { + return err + } + } + + for _, dirEntry := range dirEntries { + if filter(dirEntry) { + if err = process(dirEntry); err != nil { + return err + } + } + } + + return nil +} + +func ensurePrivateFolder(c *FirecrestRemoteSessionController, ctx context.Context, remotePath string) error { // Ensure the remote folder exists - err = c.mkdir(ctx, remoteSecretsPath, true) + err := c.mkdir(ctx, remotePath, true) if err != nil { return err } - err = c.chmod(ctx, remoteSecretsPath, "700") + err = c.chmod(ctx, remotePath, "700") if err != nil { return err } + return err +} - for _, file := range files { - if file.IsDir() { - // Ignore hierarchies for now, this will also skip "." and ".." as both are folders - continue - } - var content []byte - name := file.Name() - content, err = os.ReadFile(path.Join(localSessionSecretsFolder, name)) - if err != nil { - return err - } - err = c.uploadFile(ctx, remoteSecretsPath, name, content) - if err != nil { - return err - } - err = c.chmod(ctx, path.Join(remoteSecretsPath, name), "400") - if err != nil { +func (c *FirecrestRemoteSessionController) uploadSecretFromBuffer(ctx context.Context, remotePath, filename string, content []byte) error { + var err error + // ignore errors, we want this just to make sure we can write to it if the files exists + _ = c.chmod(ctx, path.Join(remotePath, filename), "700") + + if err = c.uploadFile(ctx, remotePath, filename, content); err != nil { + return err + } + + if err = c.chmod(ctx, path.Join(remotePath, filename), "400"); err != nil { + return err + } + + return err +} + +func (c *FirecrestRemoteSessionController) uploadSecret(ctx context.Context, localPath, remotePath, filename string) error { + var err error + var content []byte + + if content, err = os.ReadFile(path.Join(localPath, filename)); err != nil { + return err + } + + return c.uploadSecretFromBuffer(ctx, remotePath, filename, content) +} + +func (c *FirecrestRemoteSessionController) uploadDataConnector(ctx context.Context, remotePath string, dataConnector *common.DataConnector) error { + var err error + + remoteDataConnectorPath := path.Join(remotePath, dataConnector.Name) + if err = ensurePrivateFolder(c, ctx, remoteDataConnectorPath); err != nil { + return err + } + + var configFiles map[string][]byte + if configFiles, err = dataConnector.ConfigFiles(); err != nil { + return err + } + + for filename, content := range configFiles { + if err = c.uploadSecretFromBuffer(ctx, remoteDataConnectorPath, filename, content); err != nil { return err } } @@ -178,6 +218,48 @@ func (c *FirecrestRemoteSessionController) uploadSecrets(ctx context.Context, re return nil } +func isFile(dir os.DirEntry) bool { + return !dir.IsDir() +} + +func isDir(dir os.DirEntry) bool { + return dir.IsDir() +} + +func (c *FirecrestRemoteSessionController) uploadSecrets(ctx context.Context, localPath, remotePath string) error { + return walkIfMatch( + localPath, + func(dir os.DirEntry) bool { + return isFile(dir) && !strings.HasPrefix(dir.Name(), "..") + }, + func(a os.DirEntry) error { + filename := a.Name() + return c.uploadSecret(ctx, localPath, remotePath, filename) + }, + func() error { + return ensurePrivateFolder(c, ctx, remotePath) + }, + ) +} + +func (c *FirecrestRemoteSessionController) uploadDataConnectors(ctx context.Context, localPath, remotePath string) error { + return walkIfMatch( + localPath, + isDir, + func(dir os.DirEntry) error { + dc, err := common.LoadDataConnector(localPath, dir.Name()) + if err != nil { + return err + } + + return c.uploadDataConnector(ctx, remotePath, dc) + }, + func() error { + return ensurePrivateFolder(c, ctx, remotePath) + }, + ) +} + // Start sets up and starts the remote session using the FirecREST API // //nolint:gocyclo // TODO: can we break down session start? @@ -185,6 +267,7 @@ func (c *FirecrestRemoteSessionController) Start(ctx context.Context) error { // Please note: // * local*Path: A path on the current system // * remote*Path: A path on the remote firecrest system + // * container*Path: A path in the user container on the firecrest host. // Start a go routine to update the session status go c.periodicSessionStatus(ctx) @@ -250,32 +333,46 @@ func (c *FirecrestRemoteSessionController) Start(ctx context.Context) error { slog.Info("determined session path", "sessionPath", remoteSessionPath) // Setup secrets + localSecretsPath, exists := os.LookupEnv("RENKU_SECRETS_PATH") + if !exists { + localSecretsPath = "/secrets" + } + + // Makes sure that only the session owner can read session files + if err = ensurePrivateFolder(c, startCtx, remoteSessionPath); err != nil { + return err + } + remoteSecretsPath := path.Join(remoteSessionPath, "secrets") - err = c.mkdir(startCtx, remoteSecretsPath, true /* createParents */) - if err != nil { + if err = ensurePrivateFolder(c, startCtx, remoteSecretsPath); err != nil { return err } - // Makes sure that only the session owner can read session files - err = c.chmod(startCtx, remoteSessionPath, "700") + + err = c.uploadSecret(startCtx, common.UserSecretProxyFolder, remoteSecretsPath, "wstunnel_secret") if err != nil { return err } - // TODO: get wstunnel_secret as a config value - wstunnel_secret := os.Getenv("RSC_WSTUNNEL_SECRET") - if wstunnel_secret != "" { - err = c.uploadFile(startCtx, remoteSecretsPath, "wstunnel_secret", []byte(wstunnel_secret)) - if err != nil { - return err - } - err = c.chmod(startCtx, path.Join(remoteSecretsPath, "wstunnel_secret"), "400") - if err != nil { + + // Upload user secrets + remoteUserSecretsPath := path.Join(remoteSecretsPath, "user") + localUserSecretsPath := localSecretsPath // the secrets are stored directly, as is + + var dirEntries []os.DirEntry + if dirEntries, err = os.ReadDir(localUserSecretsPath); err != nil && !os.IsNotExist(err) { + return err + } + if len(dirEntries) == 0 { + // There are no secrets to mount + remoteUserSecretsPath = "" + } else { + if err = c.uploadSecrets(startCtx, localUserSecretsPath, remoteUserSecretsPath); err != nil { return err } } - // Upload user secrets - remoteUserSecretsPath := path.Join(remoteSecretsPath, "user") - err = c.uploadSecrets(startCtx, remoteUserSecretsPath) - if err != nil { + + remoteDataConnectorsPath := path.Join(remoteSecretsPath, "data_connectors") + localDataConnectorsPath := common.DataConnectorProxyFolder + if err = c.uploadDataConnectors(startCtx, localDataConnectorsPath, remoteDataConnectorsPath); err != nil { return err } @@ -356,8 +453,10 @@ func (c *FirecrestRemoteSessionController) Start(ctx context.Context) error { env["GIT_PROXY_HEALTH_PORT"] = fmt.Sprintf("%d", 65481) // git proxy port // Upload the session script - sessionScriptFinal := c.renderSessionScript(sessionScript, system.FileSystems, remoteUserSecretsPath) - err = c.uploadFile(ctx, remoteSessionPath, "session_script.sh", []byte(sessionScriptFinal)) + // We mirror the RENKU_SECRETS_PATH in the proxy and container final container, as it contains the user secrets, at + // the user secrets location (configurable by end-user) + sessionScriptFinal := c.renderSessionScript(sessionScript, system.FileSystems, remoteUserSecretsPath, localSecretsPath) + err = c.uploadFile(startCtx, remoteSessionPath, "session_script.sh", []byte(sessionScriptFinal)) if err != nil { return err } @@ -657,14 +756,14 @@ func (c *FirecrestRemoteSessionController) getCurrentStatus(ctx context.Context) return state, nil } -func (c *FirecrestRemoteSessionController) renderSessionScript(sessionScript string, fileSystems *[]FileSystem, secretsPath string) string { - return renderSessionScriptStatic(sessionScript, c.partition, fileSystems, secretsPath) +func (c *FirecrestRemoteSessionController) renderSessionScript(sessionScript string, fileSystems *[]FileSystem, nodeSecretsPath, containerSecretsPath string) string { + return renderSessionScriptStatic(sessionScript, c.partition, fileSystems, nodeSecretsPath, containerSecretsPath) } -func renderSessionScriptStatic(sessionScript, partition string, fileSystems *[]FileSystem, secretsPath string) string { +func renderSessionScriptStatic(sessionScript, partition string, fileSystems *[]FileSystem, nodeSecretsPath, containerSecretsPath string) string { sessionScriptFinal := removeMaintainersNotesFromScript(sessionScript) sessionScriptFinal = addSbatchDirectivesToScript(sessionScriptFinal, partition) - sessionScriptFinal = addSessionMountsToScript(sessionScriptFinal, fileSystems, secretsPath) + sessionScriptFinal = addSessionMountsToScript(sessionScriptFinal, fileSystems, nodeSecretsPath, containerSecretsPath) return sessionScriptFinal } @@ -689,7 +788,7 @@ func addSbatchDirectivesToScript(sessionScript, partition string) string { return strings.Replace(sessionScript, "#{{SBATCH_DIRECTIVES_PLACEHOLDER}}", directivesStr, 1) } -func addSessionMountsToScript(sessionScript string, fileSystems *[]FileSystem, secretsPath string) string { +func addSessionMountsToScript(sessionScript string, fileSystems *[]FileSystem, nodeSecretPath, containerSecretPath string) string { if fileSystems == nil { return strings.Replace(sessionScript, "#{{SESSION_MOUNTS_PLACEHOLDER}}", "", 1) } @@ -709,8 +808,8 @@ func addSessionMountsToScript(sessionScript string, fileSystems *[]FileSystem, s } // Add the secrets mount - if secretsPath != "" { - mounts = append(mounts, fmt.Sprintf("%s:/secrets:ro", secretsPath)) + if nodeSecretPath != "" && containerSecretPath != "" { + mounts = append(mounts, fmt.Sprintf("%s:%s:ro", nodeSecretPath, containerSecretPath)) } // Format mount list diff --git a/internal/remote/firecrest/controller_test.go b/internal/remote/firecrest/controller_test.go index 6b5e0106..3c7f129e 100644 --- a/internal/remote/firecrest/controller_test.go +++ b/internal/remote/firecrest/controller_test.go @@ -58,8 +58,9 @@ func TestRenderSessionScriptStatic(t *testing.T) { }, } secretsPath := "/secrets" + containerSecretsPath := "/container-secrets" - sessionScriptFinal := renderSessionScriptStatic(sessionScript, partition, &fileSystems, secretsPath) + sessionScriptFinal := renderSessionScriptStatic(sessionScript, partition, &fileSystems, secretsPath, containerSecretsPath) // Check that the rendered script starts with "#!/bin/bash" assert.Regexp(t, regexp.MustCompile("^#!/bin/bash"), sessionScriptFinal) @@ -86,6 +87,6 @@ func TestRenderSessionScriptStatic(t *testing.T) { assert.Contains(t, foundMounts, "\"/scratch:/scratch\"") assert.Contains(t, foundMounts, "\"/store:/store\"") assert.Contains(t, foundMounts, "\"/users:/home/users:ro\"") - assert.Contains(t, foundMounts, "\"/secrets:/secrets:ro\"") + assert.Contains(t, foundMounts, "\"/secrets:/container-secrets:ro\"") assert.Contains(t, foundMounts, "\"/cluster-specific:/cluster-specific\"") } From 66fb489714412a054035627ed6ab435c6dd3f429 Mon Sep 17 00:00:00 2001 From: Lionel Sambuc Date: Thu, 16 Jul 2026 16:54:18 +0200 Subject: [PATCH 2/3] fix: comment wording --- internal/remote/firecrest/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/remote/firecrest/controller.go b/internal/remote/firecrest/controller.go index c772e69e..9ae6068a 100644 --- a/internal/remote/firecrest/controller.go +++ b/internal/remote/firecrest/controller.go @@ -453,7 +453,7 @@ func (c *FirecrestRemoteSessionController) Start(ctx context.Context) error { env["GIT_PROXY_HEALTH_PORT"] = fmt.Sprintf("%d", 65481) // git proxy port // Upload the session script - // We mirror the RENKU_SECRETS_PATH in the proxy and container final container, as it contains the user secrets, at + // We mirror the RENKU_SECRETS_PATH in the proxy and final containers, as it contains the user secrets, at // the user secrets location (configurable by end-user) sessionScriptFinal := c.renderSessionScript(sessionScript, system.FileSystems, remoteUserSecretsPath, localSecretsPath) err = c.uploadFile(startCtx, remoteSessionPath, "session_script.sh", []byte(sessionScriptFinal)) From b192f91df54c8cd6880fc7c94a709dd467e12481 Mon Sep 17 00:00:00 2001 From: Lionel Sambuc Date: Thu, 16 Jul 2026 16:55:07 +0200 Subject: [PATCH 3/3] fix: replace node prefix with remote, as this is what is used already --- internal/remote/firecrest/controller.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/remote/firecrest/controller.go b/internal/remote/firecrest/controller.go index 9ae6068a..24e6adb0 100644 --- a/internal/remote/firecrest/controller.go +++ b/internal/remote/firecrest/controller.go @@ -756,14 +756,14 @@ func (c *FirecrestRemoteSessionController) getCurrentStatus(ctx context.Context) return state, nil } -func (c *FirecrestRemoteSessionController) renderSessionScript(sessionScript string, fileSystems *[]FileSystem, nodeSecretsPath, containerSecretsPath string) string { - return renderSessionScriptStatic(sessionScript, c.partition, fileSystems, nodeSecretsPath, containerSecretsPath) +func (c *FirecrestRemoteSessionController) renderSessionScript(sessionScript string, fileSystems *[]FileSystem, remoteSecretsPath, containerSecretsPath string) string { + return renderSessionScriptStatic(sessionScript, c.partition, fileSystems, remoteSecretsPath, containerSecretsPath) } -func renderSessionScriptStatic(sessionScript, partition string, fileSystems *[]FileSystem, nodeSecretsPath, containerSecretsPath string) string { +func renderSessionScriptStatic(sessionScript, partition string, fileSystems *[]FileSystem, remoteSecretsPath, containerSecretsPath string) string { sessionScriptFinal := removeMaintainersNotesFromScript(sessionScript) sessionScriptFinal = addSbatchDirectivesToScript(sessionScriptFinal, partition) - sessionScriptFinal = addSessionMountsToScript(sessionScriptFinal, fileSystems, nodeSecretsPath, containerSecretsPath) + sessionScriptFinal = addSessionMountsToScript(sessionScriptFinal, fileSystems, remoteSecretsPath, containerSecretsPath) return sessionScriptFinal } @@ -788,7 +788,7 @@ func addSbatchDirectivesToScript(sessionScript, partition string) string { return strings.Replace(sessionScript, "#{{SBATCH_DIRECTIVES_PLACEHOLDER}}", directivesStr, 1) } -func addSessionMountsToScript(sessionScript string, fileSystems *[]FileSystem, nodeSecretPath, containerSecretPath string) string { +func addSessionMountsToScript(sessionScript string, fileSystems *[]FileSystem, remoteSecretPath, containerSecretPath string) string { if fileSystems == nil { return strings.Replace(sessionScript, "#{{SESSION_MOUNTS_PLACEHOLDER}}", "", 1) } @@ -808,8 +808,8 @@ func addSessionMountsToScript(sessionScript string, fileSystems *[]FileSystem, n } // Add the secrets mount - if nodeSecretPath != "" && containerSecretPath != "" { - mounts = append(mounts, fmt.Sprintf("%s:%s:ro", nodeSecretPath, containerSecretPath)) + if remoteSecretPath != "" && containerSecretPath != "" { + mounts = append(mounts, fmt.Sprintf("%s:%s:ro", remoteSecretPath, containerSecretPath)) } // Format mount list