azure-init: Accept providing user groups via the CLI

[jeremycline]

Debian uses "sudo" as the group for having do-anything sudo permissions, where-as Fedora uses "wheel". Otherwise the same binary works fine for both. I don't see an advantage to baking the groups into the binary, so this is a take on runtime configuration.

Accept a list of supplementary groups to use when provisioning the user so the same binary can be used for both. Values can be provided using the "-g" or "--groups" argument, or by setting the "AZURE_INIT_USER_GROUPS" environment variable. If no groups are provided, the default remains "wheel".

I found this helpful when testing #105. We could expand this to allow more runtime tweaks to, for example, the backend in use if folks like this.

Help text for the CLI is:

$ azure-init --help
Minimal provisioning agent for Azure

Create a user, add SSH public keys, and set the hostname.

Usage: azure-init [OPTIONS]

Options:
  -g, --groups <GROUPS>
          List of supplementary groups of the provisioned user account.

          Values can be comma-separated and the argument can be provided multiple times.

          [env: AZURE_INIT_USER_GROUPS=]
          [default: wheel]

  -h, --help
          Print help (see a summary with '-h')

[anhvoms]

Can you add a unit test for this?

[jeremycline]

Can you add a unit test for this?

Sure thing. I take it you're okay with the general idea, then?

Also, I had to force clap <= 4.4 because our MSRV is 1.71 and clap bumped from 1.70 to 1.74 in 4.5. Sadly, I can't just specify "4" and have cargo find the latest one with a MSRV of the current toolchain... Maybe there's a way to make this work without having to artificially pin minor releases when we work with both.

[anhvoms]

Can you add a unit test for this?

Sure thing. I take it you're okay with the general idea, then?

Also, I had to force clap <= 4.4 because our MSRV is 1.71 and clap bumped from 1.70 to 1.74 in 4.5. Sadly, I can't just specify "4" and have cargo find the latest one with a MSRV of the current toolchain... Maybe there's a way to make this work without having to artificially pin minor releases when we work with both.

Yeah, I'm good with providing the configurable groups via CLI and env var. One concern is that as we start adding more features, the list of cli arguments will start growing longer and longer, which is probably not a big deal.
Long term speaking, I think we can have configuration file (e.g., /etc/azure-init/azure-init.conf) that can provide this kind of configuration (and many others, such as hostname backend, username backend, etc...)

[jeremycline]

Okay, I've dropped a few functional tests in that use the CLI a little bit. It's tricky to unit test this sort of thing without mocking out everything. I could add a --dry-run type thing to just show what it would do, I suppose, and use that when testing things.

For what it's worth, my plan with #105 is to have it run on PRs so it'll actually run on a handful of distributions and that'll definitely exercise this, and I'm happy to not have this merged until then.

If you'd prefer everything live in a configuration file, I can change this to do a config file, as well.

[anhvoms]

Okay, I've dropped a few functional tests in that use the CLI a little bit. It's tricky to unit test this sort of thing without mocking out everything. I could add a --dry-run type thing to just show what it would do, I suppose, and use that when testing things.

For what it's worth, my plan with #105 is to have it run on PRs so it'll actually run on a handful of distributions and that'll definitely exercise this, and I'm happy to not have this merged until then.

If you'd prefer everything live in a configuration file, I can change this to do a config file, as well.

This looks fine for me. The cli arguments and configuration files aren't mutually exclusive. We should have a configuration builder at some point (See #65). Once we do that, we just need to clarify/document the order of precedence when applying configs (cli vs env var vs config file). Btw, we need to document here what is the behavior of env var vs cli (I read clap documentation but it is not obvious to me what happens when the user specifies both, whether unintentionally or intentionally)

[jeremycline]

Okay, I've dropped a few functional tests in that use the CLI a little bit. It's tricky to unit test this sort of thing without mocking out everything. I could add a --dry-run type thing to just show what it would do, I suppose, and use that when testing things.
For what it's worth, my plan with #105 is to have it run on PRs so it'll actually run on a handful of distributions and that'll definitely exercise this, and I'm happy to not have this merged until then.
If you'd prefer everything live in a configuration file, I can change this to do a config file, as well.

This looks fine for me. The cli arguments and configuration files aren't mutually exclusive. We should have a configuration builder at some point (See #65). Once we do that, we just need to clarify/document the order of precedence when applying configs (cli vs env var vs config file). Btw, we need to document here what is the behavior of env var vs cli (I read clap documentation but it is not obvious to me what happens when the user specifies both, whether unintentionally or intentionally)

I'm pleased to report clap does what one would expect: arguments override environment variables and there is no merging. e.g. AZURE_INIT_USER_GROUPS=deal azure-init --groups=wheel results in a group list of ["wheel"], not ["wheel", "deal"]. Omitting --groups=wheel sets the groups to ["deal"].

I've added a note to the CLI help text spelling that out. It would probably be good to add a test to assert this behavior in case clap breaks it at some point. I'll need to think a bit about how best to test this.

[dongsupark]

Happy to see CLI parameters added again.