OpenVPN / puppet-openvpnas

Puppet module for managing the OpenVPN Access Server
BSD 2-Clause "Simplified" License
6 stars 5 forks source link

Read-only config properties #9

Open raybellis opened 5 months ago

raybellis commented 5 months ago

Is there a list anywhere of the properties that are read-only and/or otherwise not intended to be written to (c.f. the note about subscription.saved_state in the documentation) ?

I'd like to extend the openvpnas_config type so that those properties are not visible to Puppet as resources, and instead expose them as Puppet facts.

sahaqaa commented 5 months ago

Hello, i will ask developers and will reply back within approx 1-2 days

sahaqaa commented 5 months ago

@raybellis Reply from developers: only "subscription.saved_state" property has this behavior (i.e. even if you overwrite that value, AS will put a new value in by itself to 'fix it').

All other settings are regular i.e. "what you put in it - is what you get".

If you could make "subscription.saved_state" to be not visible so no one would try to configure it - it would be good.

But exposing "subscription.saved_state" as a Puppet fact, in my opinion, doesn't make sense. Because it is not used by anything for anything.

Facts are supposed to be used or referenced in some way or to help in node classification. In this case exposing "subscription.saved_state" as Puppet fact will not bring any advantages.

raybellis commented 1 month ago

I didn't get notified of the followup comments. I'll send a PR to exclude subscription.saved_state from the Puppet openvpnas_config types exposed.

Re: the latter point, though, Facts have a lot more use than that. Custom facts are often the best way to expose system state, especially if you need Puppet manifests to make conditional decisions based on that state, which can't actually be done with resources.

For example, upgrade.current_version exposed as a Fact could be used to check whether the running version differs from the installed version, to trigger a restart. I use similar Facts on our BIND and Apache servers for this purpose.

(I note that contrary to what the developers have said it would seem to make no sense to allow Puppet to overwrite that particular informational config property, ditto for upgrade.initial_version!)