Skip to content

feat: Upload DCs secrets - #1173

Open
sambuc wants to merge 3 commits into
sambuc/chore/rename-path-variables-for-clarityfrom
sambuc/feat/upload-dcs-secrets
Open

feat: Upload DCs secrets#1173
sambuc wants to merge 3 commits into
sambuc/chore/rename-path-variables-for-clarityfrom
sambuc/feat/upload-dcs-secrets

Conversation

@sambuc

@sambuc sambuc commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

@sambuc
sambuc marked this pull request as ready for review July 16, 2026 15:47
@sambuc
sambuc requested review from a team and olevski as code owners July 16, 2026 15:47
Comment on lines +648 to +655
switch as.Spec.SessionLocation {
case Remote:
return as.RemoteSessionDataSources()
case Local:
return as.LocalSessionDataSources()
default:
panic("invalid session location")
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do a little bit of defensive programming here: this method should work even if the CRD is outdated and doesn't have the location field.

See also: #1121, #1120

Suggested change
switch as.Spec.SessionLocation {
case Remote:
return as.RemoteSessionDataSources()
case Local:
return as.LocalSessionDataSources()
default:
panic("invalid session location")
}
if as.Spec.SessionLocation == Remote {
return as.RemoteSessionDataSources()
}
// as.Spec.SessionLocation == Local
return as.LocalSessionDataSources()

{
Name: fmt.Sprintf("%s-%s", prefix, as.Name),
VolumeSource: v1.VolumeSource{
Secret: &v1.SecretVolumeSource{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add Optional: ptr.To(false), no?

VolumeSource: v1.VolumeSource{
Secret: &v1.SecretVolumeSource{
SecretName: as.InternalSecretName(),
DefaultMode: ptr.To(int32(256)), // decimal value of 0400 for the access flags (chmod-like)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DefaultMode: ptr.To(int32(256)), // decimal value of 0400 for the access flags (chmod-like)
DefaultMode: ptr.To(int32(0400)), // chmod: r-- --- ---

}
volMounts := []v1.VolumeMount{
{
Name: fmt.Sprintf("%s-%s", prefix, as.Name),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will have two dashes

Suggested change
Name: fmt.Sprintf("%s-%s", prefix, as.Name),
Name: fmt.Sprintf("%s%s", prefix, as.Name),

Comment on lines +15 to +17
const UserSecretProxyFolder = "/secrets-user"
const DataConnectorProxyFolder = "/secrets-dcs"
const DataConnectorSecretProxyFolder = "/secrets-dcs-secrets"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proxy?

const DataConnectorSecretProxyFolder = "/secrets-dcs-secrets"

type DataConnector struct {
root string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is root a private field?

Comment on lines +59 to +61
if dir.IsDir() || strings.HasPrefix(dir.Name(), "..") {
continue
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.ReadDir() will not return . or ..

Suggested change
if dir.IsDir() || strings.HasPrefix(dir.Name(), "..") {
continue
}
if dir.IsDir() {
continue
}


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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a local file, you should use filepath.Join() and not path.Join()

Comment on lines +97 to +98
// Put it in a file, which will allow for passing to `rclone obscure -` before being placed in the config file.
configFiles[k] = v

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we do this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants