Merge pull request #21125 from jakecorrenti/single-ssh-key

machine: Use a single ssh key for all machines
This commit is contained in:
openshift-merge-bot[bot]
2024-01-05 12:55:41 +00:00
committed by GitHub
15 changed files with 120 additions and 103 deletions

View File

@ -41,9 +41,6 @@ func init() {
formatFlagName := "force"
flags.BoolVarP(&destroyOptions.Force, formatFlagName, "f", false, "Stop and do not prompt before rming")
keysFlagName := "save-keys"
flags.BoolVar(&destroyOptions.SaveKeys, keysFlagName, false, "Do not delete SSH keys")
ignitionFlagName := "save-ignition"
flags.BoolVar(&destroyOptions.SaveIgnition, ignitionFlagName, false, "Do not delete ignition file")

View File

@ -18478,10 +18478,6 @@ msgstr ""
msgid "Do not delete the VM image."
msgstr ""
#: ../../source/markdown/podman-machine-rm.1.md:43
msgid "**--save-keys**"
msgstr ""
#: ../../source/markdown/podman-machine-rm.1.md:45
msgid ""
"Do not delete the SSH keys for the VM. The system connection is always "

View File

@ -10,7 +10,7 @@ podman\-machine\-rm - Remove a virtual machine
Remove a virtual machine and its related files. What is actually deleted
depends on the virtual machine type. For all virtual machines, the generated
SSH keys and the podman system connection are deleted. The ignition files
podman system connections are deleted. The ignition files
generated for that VM are also removed as is its image file on the filesystem.
Users get a display of what is deleted and are required to confirm unless the option `--force`
@ -39,11 +39,6 @@ Do not delete the generated ignition file.
Do not delete the VM image.
#### **--save-keys**
Do not delete the SSH keys for the VM. The system connection is always
deleted.
## EXAMPLES
Remove a VM named "test1":
@ -53,8 +48,6 @@ $ podman machine rm test1
The following files will be deleted:
/home/user/.ssh/test1
/home/user/.ssh/test1.pub
/home/user/.config/containers/podman/machine/qemu/test1.ign
/home/user/.local/share/containers/podman/machine/qemu/test1_fedora-coreos-33.20210315.1.0-qemu.x86_64.qcow2
/home/user/.config/containers/podman/machine/qemu/test1.json

View File

@ -22,9 +22,9 @@ import (
"github.com/containers/podman/v4/pkg/machine"
"github.com/containers/podman/v4/pkg/machine/applehv/vfkit"
"github.com/containers/podman/v4/pkg/machine/define"
"github.com/containers/podman/v4/pkg/machine/ignition"
"github.com/containers/podman/v4/pkg/machine/sockets"
"github.com/containers/podman/v4/pkg/machine/vmconfigs"
"github.com/containers/podman/v4/pkg/machine/ignition"
"github.com/containers/podman/v4/pkg/strongunits"
"github.com/containers/podman/v4/pkg/systemd/parser"
"github.com/containers/podman/v4/pkg/util"
@ -202,7 +202,7 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
return false, err
}
m.IdentityPath = util.GetIdentityPath(m.Name)
m.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
m.Rootful = opts.Rootful
m.RemoteUsername = opts.Username
@ -241,11 +241,10 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
}
if len(opts.IgnitionPath) < 1 {
key, err = machine.CreateSSHKeys(m.IdentityPath)
key, err = machine.GetSSHKeys(m.IdentityPath)
if err != nil {
return false, err
}
callbackFuncs.Add(m.removeSSHKeys)
}
builder := ignition.NewIgnitionBuilder(ignition.DynamicIgnition{
@ -260,7 +259,8 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
})
if len(opts.IgnitionPath) > 0 {
return false, builder.BuildWithIgnitionFile(opts.IgnitionPath)
err = builder.BuildWithIgnitionFile(opts.IgnitionPath)
return false, err
}
if err := builder.GenerateIgnitionConfig(); err != nil {
@ -293,13 +293,6 @@ func createReadyUnitFile() (string, error) {
return readyUnit.ToString()
}
func (m *MacMachine) removeSSHKeys() error {
if err := os.Remove(fmt.Sprintf("%s.pub", m.IdentityPath)); err != nil {
logrus.Error(err)
}
return os.Remove(m.IdentityPath)
}
func (m *MacMachine) removeSystemConnections() error {
return machine.RemoveConnections(m.Name, fmt.Sprintf("%s-root", m.Name))
}
@ -344,9 +337,6 @@ func (m *MacMachine) Inspect() (*machine.InspectInfo, error) {
// collectFilesToDestroy retrieves the files that will be destroyed by `Remove`
func (m *MacMachine) collectFilesToDestroy(opts machine.RemoveOptions) []string {
files := []string{}
if !opts.SaveKeys {
files = append(files, m.IdentityPath, m.IdentityPath+".pub")
}
if !opts.SaveIgnition {
files = append(files, m.IgnitionFile.GetPath())
}

View File

@ -113,7 +113,6 @@ type StopOptions struct{}
type RemoveOptions struct {
Force bool
SaveKeys bool
SaveImage bool
SaveIgnition bool
}

View File

@ -1,3 +1,4 @@
package define
const UserCertsTargetPath = "/etc/containers/certs.d"
const DefaultIdentityName = "machine"

View File

@ -5,13 +5,11 @@ type rmMachine struct {
-f, --force Stop and do not prompt before rming
--save-ignition Do not delete ignition file
--save-image Do not delete the image file
--save-keys Do not delete SSH keys
*/
force bool
saveIgnition bool
saveImage bool
saveKeys bool
cmd []string
}
@ -27,9 +25,6 @@ func (i *rmMachine) buildCmd(m *machineTestBuilder) []string {
if i.saveImage {
cmd = append(cmd, "--save-image")
}
if i.saveKeys {
cmd = append(cmd, "--save-keys")
}
cmd = append(cmd, m.name)
i.cmd = cmd
return cmd
@ -49,8 +44,3 @@ func (i *rmMachine) withSaveImage() *rmMachine {
i.saveImage = true
return i
}
func (i *rmMachine) withSaveKeys() *rmMachine {
i.saveKeys = true
return i
}

View File

@ -303,7 +303,7 @@ var _ = Describe("podman machine init", func() {
img := inspectSession.outputToString()
rm := rmMachine{}
removeSession, err := mb.setCmd(rm.withForce().withSaveKeys()).run()
removeSession, err := mb.setCmd(rm.withForce()).run()
Expect(err).ToNot(HaveOccurred())
Expect(removeSession).To(Exit(0))
@ -313,20 +313,21 @@ var _ = Describe("podman machine init", func() {
Expect(err).ToNot(HaveOccurred())
Expect(ec).To(Equal(125))
// Clashing keys - init fails
i = new(initMachine)
session, err = mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(125))
// ensure files created by init are cleaned up on init failure
_, err = os.Stat(img)
Expect(err).To(HaveOccurred())
_, err = os.Stat(cfgpth)
Expect(err).To(HaveOccurred())
// WSL does not use ignition
if testProvider.VMType() != define.WSLVirt {
// Bad ignition path - init fails
i = new(initMachine)
i.ignitionPath = "/bad/path"
session, err = mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(125))
// ensure files created by init are cleaned up on init failure
_, err = os.Stat(img)
Expect(err).To(HaveOccurred())
_, err = os.Stat(cfgpth)
Expect(err).To(HaveOccurred())
_, err = os.Stat(ign)
Expect(err).To(HaveOccurred())
}

View File

@ -3,6 +3,7 @@ package e2e_test
import (
"fmt"
"os"
"path/filepath"
"github.com/containers/podman/v4/pkg/machine/define"
. "github.com/onsi/ginkgo/v2"
@ -80,7 +81,7 @@ var _ = Describe("podman machine rm", func() {
Expect(ec).To(Equal(125))
})
It("machine rm --save-keys, --save-ignition, --save-image", func() {
It("machine rm --save-ignition --save-image", func() {
i := new(initMachine)
session, err := mb.setCmd(i.withImagePath(mb.imagePath)).run()
Expect(err).ToNot(HaveOccurred())
@ -104,7 +105,7 @@ var _ = Describe("podman machine rm", func() {
img := inspectSession.outputToString()
rm := rmMachine{}
removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage().withSaveKeys()).run()
removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage()).run()
Expect(err).ToNot(HaveOccurred())
Expect(removeSession).To(Exit(0))
@ -127,4 +128,71 @@ var _ = Describe("podman machine rm", func() {
_, err = os.Stat(img)
Expect(err).ToNot(HaveOccurred())
})
It("Remove machine sharing ssh key with another machine", func() {
expectedIdentityPathSuffix := filepath.Join(".local", "share", "containers", "podman", "machine", define.DefaultIdentityName)
fooName := "foo"
foo := new(initMachine)
session, err := mb.setName(fooName).setCmd(foo.withImagePath(mb.imagePath)).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(0))
barName := "bar"
bar := new(initMachine)
session, err = mb.setName(barName).setCmd(bar.withImagePath(mb.imagePath).withNow()).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(0))
inspectFoo := new(inspectMachine)
inspectFoo = inspectFoo.withFormat("{{.SSHConfig.IdentityPath}}")
inspectSession, err := mb.setName(fooName).setCmd(inspectFoo).run()
Expect(err).ToNot(HaveOccurred())
Expect(inspectSession).To(Exit(0))
Expect(inspectSession.outputToString()).To(ContainSubstring(expectedIdentityPathSuffix))
fooIdentityPath := inspectSession.outputToString()
inspectBar := new(inspectMachine)
inspectBar = inspectBar.withFormat("{{.SSHConfig.IdentityPath}}")
inspectSession, err = mb.setName(barName).setCmd(inspectBar).run()
Expect(err).ToNot(HaveOccurred())
Expect(inspectSession).To(Exit(0))
Expect(inspectSession.outputToString()).To(Equal(fooIdentityPath))
rmFoo := new(rmMachine)
stop, err := mb.setName(fooName).setCmd(rmFoo.withForce()).run()
Expect(err).ToNot(HaveOccurred())
Expect(stop).To(Exit(0))
// removal of foo should not affect the ability to ssh into the bar machine
sshBar := new(sshMachine)
sshSession, err := mb.setName(barName).setCmd(sshBar.withSSHCommand([]string{"echo", "foo"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(sshSession).To(Exit(0))
})
It("Removing all machines doesn't delete ssh keys", func() {
fooName := "foo"
foo := new(initMachine)
session, err := mb.setName(fooName).setCmd(foo.withImagePath(mb.imagePath)).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(0))
inspectFoo := new(inspectMachine)
inspectFoo = inspectFoo.withFormat("{{.SSHConfig.IdentityPath}}")
inspectSession, err := mb.setName(fooName).setCmd(inspectFoo).run()
Expect(err).ToNot(HaveOccurred())
Expect(inspectSession).To(Exit(0))
fooIdentityPath := inspectSession.outputToString()
rmFoo := new(rmMachine)
stop, err := mb.setName(fooName).setCmd(rmFoo.withForce()).run()
Expect(err).ToNot(HaveOccurred())
Expect(stop).To(Exit(0))
_, err = os.Stat(fooIdentityPath)
Expect(err).ToNot(HaveOccurred())
_, err = os.Stat(fooIdentityPath + ".pub")
Expect(err).ToNot(HaveOccurred())
})
})

View File

@ -21,8 +21,8 @@ import (
"github.com/containers/podman/v4/pkg/machine"
"github.com/containers/podman/v4/pkg/machine/define"
"github.com/containers/podman/v4/pkg/machine/hyperv/vsock"
"github.com/containers/podman/v4/pkg/machine/vmconfigs"
"github.com/containers/podman/v4/pkg/machine/ignition"
"github.com/containers/podman/v4/pkg/machine/vmconfigs"
"github.com/containers/podman/v4/pkg/strongunits"
"github.com/containers/podman/v4/pkg/systemd/parser"
"github.com/containers/podman/v4/pkg/util"
@ -178,7 +178,7 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
return nil
})
m.IdentityPath = util.GetIdentityPath(m.Name)
m.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
if m.UID == 0 {
m.UID = 1000
@ -205,11 +205,10 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
callbackFuncs.Add(m.removeSystemConnections)
if len(opts.IgnitionPath) < 1 {
key, err = machine.CreateSSHKeys(m.IdentityPath)
key, err = machine.GetSSHKeys(m.IdentityPath)
if err != nil {
return false, err
}
callbackFuncs.Add(m.removeSSHKeys)
}
m.ResourceConfig = vmconfigs.ResourceConfig{
@ -233,7 +232,8 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
// If the user provides an ignition file, we need to
// copy it into the conf dir
if len(opts.IgnitionPath) > 0 {
return false, builder.BuildWithIgnitionFile(opts.IgnitionPath)
err = builder.BuildWithIgnitionFile(opts.IgnitionPath)
return false, err
}
callbackFuncs.Add(m.IgnitionFile.Delete)
@ -319,13 +319,6 @@ func (m *HyperVMachine) unregisterMachine() error {
return vm.Remove("")
}
func (m *HyperVMachine) removeSSHKeys() error {
if err := os.Remove(fmt.Sprintf("%s.pub", m.IdentityPath)); err != nil {
logrus.Error(err)
}
return os.Remove(m.IdentityPath)
}
func (m *HyperVMachine) removeSystemConnections() error {
return machine.RemoveConnections(m.Name, fmt.Sprintf("%s-root", m.Name))
}
@ -378,10 +371,6 @@ func (m *HyperVMachine) Inspect() (*machine.InspectInfo, error) {
// collectFilesToDestroy retrieves the files that will be destroyed by `Remove`
func (m *HyperVMachine) collectFilesToDestroy(opts machine.RemoveOptions, diskPath *string) []string {
files := []string{}
if !opts.SaveKeys {
files = append(files, m.IdentityPath, m.IdentityPath+".pub")
}
if !opts.SaveIgnition {
files = append(files, m.IgnitionFile.GetPath())
}

View File

@ -36,6 +36,20 @@ func CreateSSHKeys(writeLocation string) (string, error) {
return strings.TrimSuffix(string(b), "\n"), nil
}
// GetSSHKeys checks to see if there is a ssh key at the provided location.
// If not, we create the priv and pub keys. The ssh key is then returned.
func GetSSHKeys(identityPath string) (string, error) {
if _, err := os.Stat(identityPath); err == nil {
b, err := os.ReadFile(identityPath + ".pub")
if err != nil {
return "", err
}
return strings.TrimSuffix(string(b), "\n"), nil
}
return CreateSSHKeys(identityPath)
}
func CreateSSHKeysPrefix(identityPath string, passThru bool, skipExisting bool, prefix ...string) (string, error) {
_, e := os.Stat(identityPath)
if !skipExisting || errors.Is(e, os.ErrNotExist) {

View File

@ -124,7 +124,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
defer callbackFuncs.CleanIfErr(&err)
go callbackFuncs.CleanOnSignal()
v.IdentityPath = util.GetIdentityPath(v.Name)
v.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
v.Rootful = opts.Rootful
imagePath, strm, err := machine.Pull(opts.ImagePath, opts.Name, VirtualizationProvider())
@ -169,11 +169,10 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
// User has provided ignition file so keygen
// will be skipped.
if len(opts.IgnitionPath) < 1 {
key, err = machine.CreateSSHKeys(v.IdentityPath)
key, err = machine.GetSSHKeys(v.IdentityPath)
if err != nil {
return false, err
}
callbackFuncs.Add(v.removeSSHKeys)
}
// Run arch specific things that need to be done
if err = v.prepare(); err != nil {
@ -206,7 +205,8 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
// If the user provides an ignition file, we need to
// copy it into the conf dir
if len(opts.IgnitionPath) > 0 {
return false, builder.BuildWithIgnitionFile(opts.IgnitionPath)
err = builder.BuildWithIgnitionFile(opts.IgnitionPath)
return false, err
}
if err := builder.GenerateIgnitionConfig(); err != nil {
@ -238,13 +238,6 @@ func createReadyUnitFile() (string, error) {
return readyUnit.ToString()
}
func (v *MachineVM) removeSSHKeys() error {
if err := os.Remove(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil {
logrus.Error(err)
}
return os.Remove(v.IdentityPath)
}
func (v *MachineVM) removeSystemConnections() error {
return machine.RemoveConnections(v.Name, fmt.Sprintf("%s-root", v.Name))
}
@ -923,9 +916,6 @@ func NewQMPMonitor(network, name string, timeout time.Duration) (command.Monitor
func (v *MachineVM) collectFilesToDestroy(opts machine.RemoveOptions) ([]string, error) {
files := []string{}
// Collect all the files that need to be destroyed
if !opts.SaveKeys {
files = append(files, v.IdentityPath, v.IdentityPath+".pub")
}
if !opts.SaveIgnition {
files = append(files, v.getIgnitionFile())
}
@ -968,7 +958,7 @@ func (v *MachineVM) removeQMPMonitorSocketAndVMPidFile() {
}
}
// Remove deletes all the files associated with a machine including ssh keys, the image itself
// Remove deletes all the files associated with a machine including the image itself
func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() error, error) {
var (
files []string

View File

@ -19,8 +19,8 @@ import (
"github.com/containers/common/pkg/config"
"github.com/containers/podman/v4/pkg/machine"
"github.com/containers/podman/v4/pkg/machine/define"
"github.com/containers/podman/v4/pkg/machine/vmconfigs"
"github.com/containers/podman/v4/pkg/machine/ignition"
"github.com/containers/podman/v4/pkg/machine/vmconfigs"
"github.com/containers/podman/v4/pkg/machine/wsl/wutil"
"github.com/containers/podman/v4/pkg/util"
"github.com/containers/podman/v4/utils"
@ -411,7 +411,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
}
_ = setupWslProxyEnv()
v.IdentityPath = util.GetIdentityPath(v.Name)
v.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
v.Rootful = opts.Rootful
v.Version = currentMachineVersion
@ -452,7 +452,6 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
if err = createKeys(v, dist); err != nil {
return false, err
}
callbackFuncs.Add(v.removeSSHKeys)
// Cycle so that user change goes into effect
_ = terminateDist(dist)
@ -494,13 +493,6 @@ func (v *MachineVM) removeMachineImage() error {
return utils.GuardedRemoveAll(v.ImagePath)
}
func (v *MachineVM) removeSSHKeys() error {
if err := utils.GuardedRemoveAll(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil {
logrus.Error(err)
}
return utils.GuardedRemoveAll(v.IdentityPath)
}
func (v *MachineVM) removeSystemConnections() error {
return machine.RemoveConnections(v.Name, fmt.Sprintf("%s-root", v.Name))
}
@ -1057,7 +1049,7 @@ func wslPipe(input string, dist string, arg ...string) error {
}
func wslCreateKeys(identityPath string, dist string) (string, error) {
return machine.CreateSSHKeysPrefix(identityPath, true, false, "wsl", "-u", "root", "-d", dist)
return machine.CreateSSHKeysPrefix(identityPath, true, true, "wsl", "-u", "root", "-d", dist)
}
func runCmdPassThrough(name string, arg ...string) error {
@ -1605,9 +1597,6 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun
defer v.lock.Unlock()
// Collect all the files that need to be destroyed
if !opts.SaveKeys {
files = append(files, v.IdentityPath, v.IdentityPath+".pub")
}
if !opts.SaveImage {
files = append(files, v.ImagePath)
}

View File

@ -1184,7 +1184,7 @@ func ExitCode(err error) int {
}
func GetIdentityPath(name string) string {
return filepath.Join(homedir.Get(), ".ssh", name)
return filepath.Join(homedir.Get(), ".local", "share", "containers", "podman", "machine", name)
}
func Tmpdir() string {

View File

@ -531,7 +531,7 @@ func TestValidateSysctlBadSysctlWithExtraSpaces(t *testing.T) {
func TestGetIdentityPath(t *testing.T) {
name := "p-test"
identityPath := GetIdentityPath(name)
assert.Equal(t, identityPath, filepath.Join(homedir.Get(), ".ssh", name))
assert.Equal(t, identityPath, filepath.Join(homedir.Get(), ".local", "share", "containers", "podman", "machine", name))
}
func TestCoresToPeriodAndQuota(t *testing.T) {