Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cloud-init: SSH key injection improvements #15015

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+446 −341
Open

cloud-init: SSH key injection improvements #15015

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
de50d37
lxd/device: Outer variable for correct logging
hamistao Feb 17, 2025
07b9f92
lxd: Move `MergeSSHKeyCloudConfig` to new `cloudinit` package
hamistao Feb 18, 2025
7004837
lxd: Move tests to `cloudinit` package
hamistao Feb 18, 2025
f4fc8a1
lxd/cloudinit: Refactor SSH key injection logic
hamistao Feb 22, 2025
06974a6
lxd/devlxd: Refactor `devlxdConfigKeyGetHandler`
hamistao Feb 22, 2025
c523ead
lxd/device: Use refactored `cloudinit` package
hamistao Feb 23, 2025
3797d73
lxd/cloudinit: Logic to prevent overwriting `vendor-data` config
hamistao Feb 23, 2025
f2dd3a5
lxd/cloudinit: Adapt unit tests to work after refactoring
hamistao Feb 22, 2025
22d8c5e
lxd/util: Check for "#cloud-init"
hamistao Feb 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 12 additions & 35 deletions lxd/device/disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"golang.org/x/sys/unix"

"github.com/canonical/lxd/lxd/cgroup"
"github.com/canonical/lxd/lxd/cloudinit"
"github.com/canonical/lxd/lxd/db"
"github.com/canonical/lxd/lxd/db/cluster"
"github.com/canonical/lxd/lxd/db/warningtype"
Expand All @@ -27,7 +28,6 @@ import (
storagePools "github.com/canonical/lxd/lxd/storage"
storageDrivers "github.com/canonical/lxd/lxd/storage/drivers"
"github.com/canonical/lxd/lxd/storage/filesystem"
"github.com/canonical/lxd/lxd/util"
"github.com/canonical/lxd/lxd/warnings"
"github.com/canonical/lxd/shared"
"github.com/canonical/lxd/shared/api"
Expand Down Expand Up @@ -2540,55 +2540,32 @@ func (d *disk) generateVMConfigDrive() (string, error) {

instanceConfig := d.inst.ExpandedConfig()

// Use an empty vendor-data file if no custom vendor-data supplied.
vendorDataKey := "cloud-init.vendor-data"
vendorData, ok := instanceConfig[vendorDataKey]
if !ok {
vendorDataKey = "user.vendor-data"
vendorData = instanceConfig[vendorDataKey]
if vendorData == "" {
vendorData = "#cloud-config\n{}"
}
// Get raw data from instance config.
vendorDataKey := cloudinit.GetEffectiveConfigKey(instanceConfig, "vendor-data")
userDataKey := cloudinit.GetEffectiveConfigKey(instanceConfig, "user-data")
vendorData, userData := cloudinit.GetResultingCloudConfig(instanceConfig, vendorDataKey, userDataKey, d.inst.Name(), d.inst.Project().Name)
Copy link
Member

Choose a reason for hiding this comment

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

In my view it would be clearer to return a cloudinit.Config (or similarly named) struct that contained fields for userdata, vendordata etc rather than multiple return values as it allows you to pass around the overall cloud config.

Copy link
Member

Choose a reason for hiding this comment

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

Could we have the logic for GetEffectiveConfigKey moved inside GetResultingCloudConfig and then avoid the caller needing to figure this stuff out by just having a requestedKey argument that the the cloudinit package figures out what needs to be returned?

Copy link
Member

Choose a reason for hiding this comment

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

Rather than "Resulting" lets use "Effective" here as that aligns more with other concepts in LXD (effective project for example).


// Use an empty cloud-config file if no custom *-data is supplied.
if vendorData == "" {
vendorData = "#cloud-config\n{}"
}

// Merge additional SSH keys present on the instance config into vendorData.
vendorData, err = util.MergeSSHKeyCloudConfig(instanceConfig, vendorData)
if err != nil {
logger.Warn("Failed merging SSH keys into cloud-init seed data, abstain from injecting additional keys", logger.Ctx{"err": err, "project": d.inst.Project().Name, "instance": d.inst.Name(), "dataConfigKey": vendorDataKey})
if userData == "" {
userData = "#cloud-config\n{}"
}

err = os.WriteFile(filepath.Join(scratchDir, "vendor-data"), []byte(vendorData), 0400)
if err != nil {
return "", err
}

// Use an empty user-data file if no custom user-data supplied.
userDataKey := "cloud-init.user-data"
userData, ok := instanceConfig[userDataKey]
if !ok {
userDataKey = "user.user-data"
userData = instanceConfig[userDataKey]
if userData == "" {
userData = "#cloud-config\n{}"
}
}

// Merge additional SSH keys present on the instance config into userData.
userData, err = util.MergeSSHKeyCloudConfig(instanceConfig, userData)
if err != nil {
logger.Warn("Failed merging SSH keys into cloud-init seed data, abstain from injecting additional keys", logger.Ctx{"err": err, "project": d.inst.Project().Name, "instance": d.inst.Name(), "dataConfigKey": userDataKey})
}

err = os.WriteFile(filepath.Join(scratchDir, "user-data"), []byte(userData), 0400)
if err != nil {
return "", err
}

// Include a network-config file if the user configured it.
networkConfig, ok := instanceConfig["cloud-init.network-config"]
if !ok {
networkConfig = instanceConfig["user.network-config"]
}
networkConfig := instanceConfig[cloudinit.GetEffectiveConfigKey(instanceConfig, "network-config")]

if networkConfig != "" {
err = os.WriteFile(filepath.Join(scratchDir, "network-config"), []byte(networkConfig), 0400)
Expand Down