Azure / kubernetes-keyvault-flexvol

Azure keyvault integration with Kubernetes via a Flex Volume
MIT License
253 stars 84 forks source link

Support .NET configuration conventions for structured data #66

Closed thomaseyde closed 5 years ago

thomaseyde commented 5 years ago

The .NET convention for structured keys is using underscores where json or other structured format is not available, like environment variable names and, in this case, file names.

For some reason, Azure Key Vault doesn't allow underscores in key names, so dashes are used instead. And the logical thing is to use the KeyPerFileConfigurationProvider and call config.AddKeyPerFile("/kv") to load all values. But this provides doesn't handle double dashes.

It would be stupid to write yet another configuration provider to handle this, and when the convention is established, the FlexVolume provider should replace double dashes with double underscores. Or provide a variable to specify the delimiter.

ritazh commented 5 years ago

@thomaseyde can you please provide an example of the name of the object in Azure Key Vault, what you expect to see in the container and what this solution has produced?

If I understand correctly, it sounds like we are creating the file with the name of the key vault object, which is not what your application expects. Is that correct?

thomaseyde commented 5 years ago

Given two secrets in Azure Key Vault, named Key and Structured--Key, and the volume mounted to /kv, I expect two files in /kv, named Key and Structured__Key.

That is, all double-dashes (--)in Key Vault names are replaced with double underscores (__).

This solution produces two files with names unchanged, Key and Structured--Key. And that last name is not recognized by configuration providers in .NET to be structured, and will not be mapped into an object like this:

class Structured
{
  public string Key { get; set; }
}
ritazh commented 5 years ago

@thomaseyde Thanks for the details. For this scenario, the recommendation is to make a copy of the file from inside the container to the desired filename. For example cp /kvmnt/Structured--Key Structured__Key. This solution is agnostic to platforms as it mounts the content exactly as it's stored in key vault. Once the file is mount to your container, your application can process it as needed.

thomaseyde commented 5 years ago

I can appreciate the desire to be agnostic, still I hope you will reconsider and find a solution to the problem.

Given that Azure Key Vault is not meant only for .NET developers, you have no choice but to be platform agnostic. This means the default behavior should be as it is.

But, from a .NET developer's perspective, it's expected that the mounted volume contains usable filenames so we can load the configuration using the typical tools provided in .NET.

It would be awkward to have to rename all files in the mounted volume before loading them. That would require inconvenient boilerplate for all configurations where none were required. It's also not the idiomatic way to load configuration values.

The best thing, of course, would be to have usable names from the start. But when that's not possible, having the provider fix this would be next best.

I can't believe it would be that hard to add an additional parameter to replace -- with __. Even better, let us specify what to replace and what with.

khenidak commented 5 years ago

It is a valid request. Specially valid for applications that are migrating to Kubernetes. One way of doing this, is to maintain the current behavior and provide a parameter to do the replacement needed (maybe a reg exp?)

ritazh commented 5 years ago

That's right. The alias feature should satisfy this request, where the values inkeyvaultobjectalias will be the mounted file names.

For example:

flexVolume:
      driver: "azure/kv"
      secretRef:
        name: kvcreds # mounting point to the pod
      options:
        usepodidentity: "false"
        keyvaultname: "testkeyvault"
        keyvaultobjectnames: "test--secret1;test--secret2"
        keyvaultobjecttypes: secret;secret # OPTIONS: secret, key, cert
        keyvaultobjectversions: "testversion1;testversion2"
        keyvaultobjectalias: "test__secret1;test__secret2"
        resourcegroup: "testresourcegroup"
        subscriptionid: "testsub"
        tenantid: "testtenant"
thomaseyde commented 5 years ago

That would work. But the syntax will be clumsy for many values. Also, if a value doesn't have an alias, what do one write? And when something goes wrong, it's hard to match alias and type to the object name.

Can't the above be written something like:

flexVolume:
      driver: "azure/kv"
      secretRef:
        name: kvcreds # mounting point to the pod
      options:
        usepodidentity: "false"
        keyvaultname: "testkeyvault"
        secrets:
        - test--secret1;test__secret1 # with alias
        - test--secret2;test__secret2 # with alias
        - secret3 # no alias
        # keys: # same syntax as secrets
        # certs: # same syntax as secrets
        resourcegroup: "testresourcegroup"
        subscriptionid: "testsub"
        tenantid: "testtenant"