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.

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

feat: Add microvm.credentialFiles for passing credentials to guests #337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Ramblurr
Copy link

@Ramblurr Ramblurr commented Feb 18, 2025

This commit implements microvm.credentialFiles a mechanism for passing
credentials into guest vms from the host.

Currently only support for qemu is implemented as I want to test the waters to
see if you're interested in this feature, Astro.

In addition to qmeu, cloud-hypervisor can be supported via smbios. But it depends
on this feature being added, and also #336 being merged to microvm.nix

cloud-hypervisor could be supported immediately, but then the secrets would be
visible in the ps output.

A cursory code search shows that the following additional hypervisors could be
supported:

  • crosvm: via fw_cfg, or smbios
  • alioth: via fw_cfg
  • stratovirt: via fw_cfg (maybe smbios)

kvmtool and firecracker both seem like they cannot be supported.

Related:

This commit implements `microvm.credentialFiles` a mechanism for passing
credentials into guest vms from the host.

Currently only support for qemu is implemented as I want to test the waters to
see if you're interested in this feature, Astro.

In addition to qmeu cloud-hypervisor can be supported via smbios. But it depends
on [this feature being added](cloud-hypervisor/cloud-hypervisor#6951 (comment)), and also astro#336 being merged to microvm.nix

cloud-hypervisor could be supported immediately, but then the secrets would be
visible in the ps output.

A cursory code search shows that the following additional hypervisors could be
supported:

- crosvm: via fw_cfg, or smbios
- alioth: via fw_cfg
- stratovirt: via fw_cfg (maybe smbios)

kvmtool and firecracker both seem like they cannot be supported.

Related:
- astro#259
- astro#52
Copy link

nix-ci-app bot commented Feb 18, 2025

NixCI is ready to run on this PR.
Maintainer: Comment nix-ci run to run now.

@astro
Copy link
Owner

astro commented Feb 18, 2025

That looks well to me.

Are you going to try adding the other hypervisors?

Where not supported, please add lib.throwIf or assert config.microvm.credentialFiles == [] so that users do notice whenever their desired config cannot be fulfilled.

@Ramblurr
Copy link
Author

Are you going to try adding the other hypervisors?

Yes I will attempt to implement it in the runners for the other hypervisors, and for those that I can't I'll add an assert. I'll also update the docs.

I'd like to implement a test for this too. Could you point me to where in checks/ I should add it? I thought somewhere in default.nix but there's a lot going on in there.

FYI this is how I am able to take advantage of the credential, by setting the SSH host key (which also lets me bootstrap my sops-nix in the guest).

  # this is part of the guest vms nixos config
  microvm.credentialFiles = {
    "SSH_HOST_KEY" = "/run/secrets/mymicrovm_sops_ssh_key"; # This file must exist on the host, since I use imperative vms, I have to make it manually.
  };
  systemd.services.sshd = {
    serviceConfig = {
      ImportCredential = "SSH_HOST_KEY";
    };
    preStart = ''
      mkdir -p /etc/ssh
      cat $CREDENTIALS_DIRECTORY/SSH_HOST_KEY > /etc/ssh/ssh_host_ed25519_key
      chmod 0600 /etc/ssh/ssh_host_ed25519_key
    '';
  };

I got ill, so it might be until next week before I'm able to return to these PRs.

@astro
Copy link
Owner

astro commented Feb 22, 2025

The top of checks/default.nix contains all the combinations to permute for a "test matrix". You can add add a list containing configurations with and without the feature. The idea is to be able to test for any feature combination but it is not yet CI as of today. Related: nix-community/infra#1598

Get well soon, and don't feel stressed from open tickets!

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