Add Helm rsync.fixPrivateKeyPerms option

[alex-vmw]

Add Helm rsync.fixPrivateKeyPerms option

[utkuozdemir]

The solution makes total sense to me. Please feel free to submit a PR with the fix you suggested.
(Do not forget to do task update-chart) after making changes in the chart.

[alex-vmw]

@utkuozdemir It appears that I underestimated this issue. SSH key is being mounted from a secret and apparently in Kubernetes secret mounts are always ReadOnly (kubernetes/kubernetes#62099). Since mounted SSH Key is always ReadOnly, trying to chmod it gives chmod: /root/.ssh/id_ed25519: Read-only file system error. That said, I think the only solution would be to copy the SSH key somewhere (/tmp maybe?) and somehow pass the -i /tmp/<PrivateKey> option to ssh command. That said, I need your guidance on what would be the best way to add that -i option to ssh? I see sshArgs defined in

pv-migrate/rsync/cmd.go

Line 34 in b563c1b

sshArgs := []string{

so do I need to create another command line option like --ssh-key-path and use the passed string to set the -i option to ssh and add it to sshArgsStr (https://github.com/utkuozdemir/pv-migrate/blob/master/rsync/cmd.go#L42)? In addition, the current rsync.fixPrivateKeyPerms option will probably need another option rsync.privateKeyCopyPath to be used together, so that we can fix permissions on the copy of the private key file.
P.S. We can also rename rsync.fixPrivateKeyPerms to rsync.copyPrivateKey to better match rsync.privateKeyCopyPath to be used together?

[utkuozdemir]

I see. No problem. What if we do something like:

• Always mount the private key to /tmp or /opt or something
• On rsync job, we
  • mkdir -p ~/.ssh
  • Copy it from /tmp into ~/.ssh
  • chmod if the flag is specified

Also, I'll ask you to please build and test manually & change/add integration tests for the further changes so we avoid further broken functionality.

[alex-vmw]

@utkuozdemir I like your idea, but I think it would be advantageous to not have a flag for chmod. I think chmod of the private key after copying should be done always by default. It wouldn't hurt anything and wouldn't require people to provide any extra flag for things to work correctly. WDYT?

[utkuozdemir]

Yep, actually that sounds reasonable - always mount it to a temp path, always copy and chmod 400. Should work just fine and less complexity. We can give it a shot.

[alex-vmw]

@utkuozdemir In my usecase, I don't need any changes for sshd deployment, but I see that sshd.privateKeyMountPath exists. I should skip modifying anything for sshd, right?

[utkuozdemir]

I had to look into the code to remember what it was used for, it seems I use it in the local strategy. Let's also change it to be the new way.

[utkuozdemir]

We'll probably have to pass a command to its container just like the rsync one.

[alex-vmw]

@utkuozdemir I also assume I would be removing the rsync.fixPrivateKeyPerms with this change, right?

[utkuozdemir]

Yep, we'll just make a minor release because of the breaking change and it should be fine.

[alex-vmw]

@utkuozdemir I was trying to verify that setting sshd.privateKeyMount=true is working, but even without any of my latest changes running pv-migrate with --helm-set sshd.privateKeyMount=true seems to fail (for any strategy).
P.S. Running --strategies "local" without also specifying --helm-set sshd.privateKeyMount=true seems to work fine.

[utkuozdemir]

Hm, interesting, we set it to true here: https://github.com/utkuozdemir/pv-migrate/blob/master/strategy/local.go#L197

And the integration test that tests local strategy passes without issues. Doing --helm-set sshd.privateKeyMount=true explicitly shouldn't change anything actually.