Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use concat for ovpn generation. #176

Merged
merged 3 commits into from
Oct 19, 2015

Conversation

jkroepke
Copy link
Contributor

Hi,

I refactor the generation of the ovpn file. Its support inline tls-auth now (see #175).

Concat Module is already required in metdata.json

Tested on Ubuntu 14.04 w/ puppet 3.8.2 and OpenVPN 2.3.7 for Windows (VPN client).

Also, the concat files are located in /var/lib/puppet/concat, which only read read by puppet group and root.

@luxflux
Copy link
Contributor

luxflux commented Oct 10, 2015

Thank you for your work! 🍻 Looks quite good! 🌟
What do you think, could you create a new definition called openvpn::ovpn and move all the ovpn stuff in there? Afterwards we can call openvpn::ovpn from openvpn::client.

@jkroepke
Copy link
Contributor Author

In my opinion, If you want to splitt openvpn::client and openvpn::ovpn, you should do it with tar, too. Maybe openvpn::client::ovpn for more a better style.

@luxflux
Copy link
Contributor

luxflux commented Oct 13, 2015

Yep, this sound right. Do you mind doing this? Or maybe just the ovpn part?

@jkroepke
Copy link
Contributor Author

While i'm trying this, I got some dependency cycles.

For example, the tblk file are require the ovpn file must be run before the tar command.

I think one complex class is good enough, Refactoring can be later, not?

@luxflux
Copy link
Contributor

luxflux commented Oct 19, 2015

Okay, I am fine with this.

@luxflux
Copy link
Contributor

luxflux commented Oct 19, 2015

Thanks a lot for your work!

luxflux added a commit that referenced this pull request Oct 19, 2015
@luxflux luxflux merged commit 35b69e4 into voxpupuli:master Oct 19, 2015
luxflux added a commit that referenced this pull request Oct 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants