library: allow DNS provider configuration to be supplied without using environment variables

[vancluever]

Welcome

• Yes, I've searched similar issues on GitHub and didn't find any.

How do you use lego?

Library

Detailed Description

vancluever/terraform-provider-acme#235 describes an issue we're seeing in the Terraform ACME provider when multiple certificates that may share different configurations for the same DNS provider are created or renewed in parallel. The main issue lies in the fact that Terraform providers do not launch multiple processes for each resource being executed in a provider (if it did, this could scale to hundreds or even thousands of processes depending on the provider being utilized, which obviously would be bad). This means that multiple resources are going to clobber each other's DNS configurations via resetting of environment variables.

The current pattern for loading providers is using NewDNSProvider in each provider package (see the factory in dns_providers.go). The proposal is to provide another pattern side-by-side that will allow library consumers to supply a configuration to the constructor.

Proposal

The proposal is to provide a new function that would perform similar to NewDNSProvider, but take a configuration:

func NewDNSProviderWithConfig(config map[string]string)

This would process the configuration and hand-off initialization as appropriate in each provider (example: via the NewDNSProviderConfig function available to some providers).

Helpers

Helpers would be used to ensure that provider authors and maintainers do not need to maintain configuration reading. These would go in the platform/config package tree.

Configuration reader

A configuration reader would take a struct, reflect on its annotated values, and either set them from the passed in map[string]string, or read them from the environment.

An example configuration that the reader can take is below (an annotated version of the route53 provider):

type Route53Config struct {
    AccessKeyId        string        `lego:"AWS_ACCESS_KEY_ID"`
    SecretAccessKey    string        `lego:"AWS_SECRET_ACCESS_KEY"`
    Region             string        `lego:"AWS_REGION"`
    HostedZoneId       string        `lego:"AWS_HOSTED_ZONE_ID"`
    MaxRetries         int           `lego:"AWS_MAX_RETRIES"`
    TTL                int           `lego:"AWS_TTL"`
    PropagationTimeout time.Duration `lego:"AWS_PROPAGATION_TIMEOUT"`
    PollingInterval    time.Duration `lego:"AWS_POLLING_INTERVAL"`
}

The reader would fall back to the existing EnvOrDefault and _FILE functionality to ensure backwards compatibility.

Capability identification

To identify that a provider package is capable of the new configuration structure, an assignable value will be placed in the same package as the rest of the helpers. The provider can then alias to this value:

const HasWithConfig = config.HasWithConfig

Provider factory generators can then use this value to generate factories with the new provider constructor:

// Original
case "route53":
    return route53.NewDNSProvider()

// New (after checking for presence of route53.HasWithConfig)
​​case "route53":
    return route53.NewDNSProviderWithConfig(configMap)

Abandoned ideas

I was originally thinking that we'd need another interface for this, but given that the existing API seems to be pattern-based, I think we can just get away with simply extending the pattern as outlined above and continuing to rely on code generation and factories to handle any differences between provider capabilities (if there ends up being any after implementation).

[ldez]

Hello,

I'm not sure about "a new runtime configuration injection".

Can you be a bit more concrete about our solution/suggestion?

[ldez]

@vancluever I'm interested to know more about your proposal.

[vancluever]

Hey @ldez, been tied up the last few days. I'll respond with more detail in the next couple of days (may just be on the weekend). Sorry for the delay!

[vancluever]

Hey @ldez, updated the proposal. Hopefully this is much more helpful in understanding the issue and what I'm proposing! Let me know.

[ldez]

I'm not sure to understand your proposal, can you give a concrete example of the usage?

Currently, the env vars are one of the ways to provide configuration, the other way is to provide a structure.

Also, some providers cannot be used like that because the API clients (dependencies of lego) handle things internally with env vars or a file, so I think your current proposal will not work in those cases.

I don't think that using a map is the right way to handle configuration inside lego.

I need to understand the usage to be able to think another way.
For now, a concrete usage example is more important for me than the API or the lego internals.

I hope you are not trying to achieve something related to #1342 or #605

[vancluever]

@ldez I'm not too sure if I can give much more of an example of this usage outside of what's given without going much further into detail that I think would be a duplication of work that I can't really commit to at this time. I can always put in a pull request as an example of a rewritten provider as to what I mean.

Currently, the env vars are one of the ways to provide configuration, the other way is to provide a structure.

Correct, but the struct method is not consistent with the documented provider configurations. Just like the lego CLI, they are either set in the environment when Terraform is invoked, or passed in through a map. See our documentation in the acme_certificate resource and or in one of the DNS providers like the route53 provider.

Further to that not every provider has a NewDNSProviderConfig method, so it can't be relied on. Example: acmedns provider.

Also, some providers cannot be used like that because the API clients (dependencies of lego) handle things internally with env vars or a file, so I think your current proposal will not work in those cases.

I can't speak for every provider so this may very well be true. However some providers, ie: the route53 provider, should be able to be re-written to ensure that they accept configuration strictly from API and not from the environment.

I don't think that using a map is the right way to handle configuration inside lego.

It is the only real backwards-compatible method I can think of. My target is minimal to zero impact to UX, so a string-based key-value configuration method would need to exist. Using a map[string]string means that the method can still fall back to the environment if need be.

For now, a concrete usage example is more important for me than the API or the lego internals.

Consider that we load providers in exactly the same way that lego provides with the factory in dns_providers.go. Our auto-generated factory is here. I need a reliable constructor (read: present in every provider) that takes the exact same configuration as what would be expected in the environment and sets the provider configuration appropriately.

I hope you are not trying to achieve something related to #1342 or #605

I'm not suggesting adding a new provider to support multiple DNS providers. vancluever/terraform-provider-acme#235 is happening when attempting to renew multiple certificates that require different configurations for the same DNS provider (ie: different domains with different zone configurations).

(Incidentally, the TF provider already supports using multiple DNS providers on a single certificate and it did not require any extensions to lego. In fact, our approach sounds similar to #605, but we just did it locally. Again, this is not happening due to this.)

[ldez]

Correct, but the struct method is not consistent with the documented provider configurations

I don't really agree with that.
Only a few providers are not "consistent" but the majority are.
The "non-consistent" providers are like that mainly because of internal constraints inside their clients.

Further to that not every provider has a NewDNSProviderConfig method, so it can't be relied on.

Only 2 providers on 110: acme-dns and the manual provider.
I can add a NewDNSProviderConfig for acme-dns.
The manual provider is the manual provider, so there is no configuration and you don't use it.

It is the only real backward-compatible method I can think of.

I can see other ways, I will dig into your use case to try to find another approach.

I think we will have the same kind of discussion as in #1054, heterogeneous ways of configuring providers is a deeper issue than you might think.

I already spend time on that and a simple solution doesn't really exist.

I was hoping that you suggest a new way but for now, there is no new approach to the problem.

[vancluever]

@ldez when I say "consistent" I mean can I, as a consumer of the library, know exactly where the setting for a specific configuration value goes in the specific provider's configuration? As of right now, the answer to this seems to be no.

Consider the case of the route53 provider: how can I know that AWS_HOSTED_ZONE_ID translates to route53.Config.HostedZoneID? That shouldn't necessarily be something that the downstream consumer needs to worry about; ideally, there'd either be some way of determining the configuration schema so we could automate that translation.

That's pretty much the main issue for NewDNSProviderConfig right now I think.

Sorry I didn't look at the other providers. I must admit that I the other aforementioned issues with using a struct-based configuration (and the fact that it wasn't available across every provider) were the main reasons I didn't pursue that approach further.

I'll leave this issue open for now and if you can think of anything else, let me know!

[ldez]

Maybe we can move step by step: for example be able to not rely on the client's internal env vars parsing for route53 and lightsail (without dropping the current AWS client).
@vancluever if you can handle that it can be useful.

I will think about the "consistency" problem.

[ldez]

Consider the case of the route53 provider: how can I know that AWS_HOSTED_ZONE_ID translates to route53.Config.HostedZoneID?

I think route53 is a bad example but just for the technical challenge (I don't think I recommend that but yeah it's fun): https://go.dev/play/p/VrISa1N1rzN

[vancluever]

@ldez sounds good, I can help with that for sure. I'd suggest we still leave an option to use the default credential chain though as there could be some people relying on that functionality and it supports a large amount of things other than just environment variables (like instance profiles, and $HOME/.aws configuration file chains).

With regards to the posted example, I was actually thinking of using mapstructure. The main issue I can see with that approach is that it does not allow for going to the environment for values if they are not provided in the map. I'd suggest just unfurling that process and just reflecting over the config like so (with or without the annotations):

1. For each field:
   a. Check field for lego: struct tag for explicit key value.
   b. If annotation does not exist, create one by converting field name to all-caps snake case and combining with envNamespace as a prefix.
   c. Check for value in supplied map. If available, use that value.
   d. If not available in the map, look to environment for the value instead.
   e. If not found in environment, use a default value or error out.
   f. Do appropriate conversion for the value.
   g. Set the field.
2. Initialize the provider with the set values.

[ldez]

sounds good, I can help with that for sure. I'd suggest we still leave an option to use the default credential chain though as there could be some people relying on that functionality and it supports a large amount of things other than just environment variables (like instance profiles, and $HOME/.aws configuration file chains).

So let's go for a PR!

With regards to the posted example, I was actually thinking of using mapstructure.

My example was just a funny thing, I didn't want to suggest something, it was just fun.

I already created some complex parsing systems (ex: https://github.com/traefik/paerser/) so creating a basic parsing system is not a problem.
And as I said I will think about that, I think I can create something interesting, I will share it with you when my ideas will be more concrete.

[vancluever]

Hey @ldez I think I'm actually nearing a self-contained fix that I started over the holidays, and it seems stable; I've tested it in multiple environments, including TFC, and no issues, and we have one report of it correcting the initially reported issue. You can see vancluever/terraform-provider-acme#276 if you're curious. The fix involves re-running the plugin for every DNS challenge provider detected, each with its own environment set, communicating over a dedicated internal go-plugin transport that is independent of the one that communicates with Terraform itself.

This at least defers the need for non-environment DNS configuration in lego. As this is an unorthodox fix, I can't guarantee that it will last forever and I'd still love to see some progress towards this, but it definitely buys us a lot more time.

Once this is done and I have some cycles I'll do the lightsail provider too as previously discussed, but I thought I'd keep you in the loop!

[vancluever]

I'm closing this as I don't think it's worth it anymore compared to the lift required, given the plugin-based solution has been stable for pretty much the whole year now. If I need to, I'll revisit it at a later time. 🙂