search

NixOS / nixops

NixOps is a tool for deploying to NixOS machines in a network or cloud.
https://nixos.org/nixops
GNU Lesser General Public License v3.0
1.83k stars 363 forks source link

Broken API key management in DigitalOcean backend #628

Open joepie91 opened 7 years ago

joepie91 commented 7 years ago

While trying to get a DigitalOcean test deployment working, I've had a read through the backend implementation, and I suspect that there are some issues with it.

The first issue is that the digitalOcean.authToken property is never actually stored on the DigitalOceanState instance in any way, rather only on the DigitalOceanDefinition (as auth_token). At the same time, <DigitalOceanState>.get_auth_token() attempts to read it from self.auth_token, which because of the aforementioned issue doesn't exist.

This means that the authentication token - if not set through an environment variable, for which I created a separate PR (#626) - will always be None, resulting in the following error:

machine.> creating droplet ...error: No token provided. Please use a valid token

The immediate solution would be to do something like self.auth_token = defn.auth_token in the create() call, but this would result in another issue concerning API credential management; I've filed an issue about that in #627.

cc @teh

teh commented 7 years ago

I think my immediate preferred solution for this (not the more abstract #627) would be to only allow the env var. That way there at least is no ambiguity in the code. @joepie91 what do you think?

I don't have a strong opinion on a more general key management, it's not an area I'm good at.

joepie91 commented 7 years ago

Yeah, I think that would be a reasonable short-term solution. It'd require a change of the documentation as well, though - currently, the environment variable is only documented in the option (which would obviously disappear).

I'd definitely like to see a well-integrated solution for key management that doesn't rely on environment variables at some point, so that credentials can be easily managed and used alongside the rest of the NixOps configuration, especially when dealing with multiple sets of credentials for different networks... but that's probably going to have to involve a change to the core of NixOps itself.

EDIT: Ah, I see a fix bringing it in line with EC2 behaviour was already committed. Not sure how that would tie into your suggestion, or that in #627.