sous-chefs / openvpn

Development repository for the openvpn cookbook
https://supermarket.chef.io/cookbooks/openvpn
Apache License 2.0
98 stars 160 forks source link

Dynamic and custom configuration of client configs #87

Closed phutchins closed 6 years ago

phutchins commented 8 years ago

Allows users to more granularly and dynamically control the client config created.

flaccid commented 8 years ago

Can we have the tests pass first please?

flaccid commented 8 years ago

Any reason this wouldn't simply use the existing LWRP and config attribute https://github.com/xhost-cookbooks/openvpn/blob/master/providers/conf.rb ?

flaccid commented 8 years ago

@phutchins just circling back on this one - keen to hear your thoughts, perhaps I am misunderstanding.. let me know please.

phutchins commented 8 years ago

Doing it this way allows us to set different parameters on the client and server side. The changes also allow for flags vs key/value config options.

flaccid commented 8 years ago

I'd like to avoid going back to separate keys for client and server configuration. I think the requirement for the client template to be not hard coded is what we should be looking at specifically. I think there must be a more elegant way to do this. I will have a play soon.

phutchins commented 8 years ago

What is the reason for not having separate keys for client and server? That seems like an arbitrary restriction.

I definitely considered ways to merge flags and config values but the options seemed more messy than this way...

devsibwarra commented 8 years ago

It would be nice to see a generic template file, without any static lines, that takes the config hash and converts that into a file that OpenVPN can consume. It looks like your clients.conf template change has part of this already, but I'd like seeing the other hard-coded lines moved out

Instead of adding a client_flags setting, you can just add the desired options to your config hash like so

  'config' => {
    'comp-lzo' => '',
    'persist-key' => '',
    'persist-tun' => '',
    'nobind' => '',
    ...
  }

I do this with our server config to add the opt-verify flag, but this should work just fine with your tweaks to the client template.

phutchins commented 8 years ago

Definitely. Thats pretty much what I was going for. When all of the values are in the same config object however it will drop them into the template out of order and I desired a config with the single word entries (flags) together, and the config params together. That is more of a organization and readability thing over functionality so I could see it being useful both ways...

The functionality in the client.conf template does already exist to parse an empty value as a single word entry so this wouldn't be an issue.

We could merge client_flags with client_config, however, I want the ability to have different config values, naming, etc... between my server config and client config so I want those separate. They could default to the same but I dynamically name these certs for the client and the client config needs to be able to reflect that.

Some of the values, proto, for instance, will always be shared between server and client so I can individually add those to the client_config object setting them to the servers value. This way it can be changed by changing only the value in 'config'.

Thoughts?

phutchins commented 8 years ago

@devsibwarra I just pushed some changes in line with our discussion. Take a look and let me know your thoughts... Tests aren't passing on my machine so we'll figure that out once we land on a direction.

devsibwarra commented 8 years ago

Since there are numerous defaults being set for the client config, there should be a way for a user to bypass/ignore some of the unwanted default settings, rather than have a wrapper script do node.rm_default(...).

I've done this in the past with other cookbooks by setting the value to nil in my environment/role configs and skipping that value in the template loop.

damacus commented 6 years ago

I'm going to close this one down now. If you want to re-open this please feel free!

Thanks! Dan

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.