[WIP] Switch to Puppet data types #244

wyardley · 2017-10-09T20:27:15Z

This adds Puppet 4 data types.

bastelfreak · 2017-10-09T20:46:17Z

.fixtures.yml

@@ -2,10 +2,10 @@ fixtures:
  repositories:
    stdlib:
      repo: git://github.com/puppetlabs/puppetlabs-stdlib
-      ref: '4.6.0'
+      ref: '4.20.0'


IMO we should unpin the modules here. In general we test against master to find breaking changes before they are released.

@bastelfreak works for me, I'll adjust in #242

wyardley · 2017-10-09T22:52:52Z

spec/defines/openvpn_server_spec.rb

@@ -166,7 +166,7 @@
        'key_cn'          => 'yolo',
        'key_name'        => 'burp',
        'key_ou'          => 'NSA',
-        'verb'            => 'mute',


@luxflux do you know why verb is 'mute' here? It seems like mute is a separate param, and the docs suggest that verb should be an int as in the client?

Yeah, this seems to be wrong. Let's use an integer...
From https://openvpn.net/index.php/open-source/documentation/manuals/65-openvpn-20x-manpage.html:

--verb n
Set output verbosity to n (default=1). Each level shows all info from the previous levels. Level 3 is recommended if you want a good summary of what's happening without being swamped by output.
0 -- No output except fatal errors.
1 to 4 -- Normal usage range.
5 -- Output R and W characters to the console for each packet read and write, uppercase is used for TCP/UDP packets and lowercase is used for TUN/TAP packets.
6 to 11 -- Debug info range (see errlevel.h for additional information on debug levels).

I think we need we need to update this line as well.

luxflux · 2017-10-11T07:16:09Z

spec/defines/openvpn_server_spec.rb

@@ -166,7 +166,7 @@
        'key_cn'          => 'yolo',
        'key_name'        => 'burp',
        'key_ou'          => 'NSA',
-        'verb'            => 'mute',


Yeah, this seems to be wrong. Let's use an integer...
From https://openvpn.net/index.php/open-source/documentation/manuals/65-openvpn-20x-manpage.html:

--verb n
Set output verbosity to n (default=1). Each level shows all info from the previous levels. Level 3 is recommended if you want a good summary of what's happening without being swamped by output.
0 -- No output except fatal errors.
1 to 4 -- Normal usage range.
5 -- Output R and W characters to the console for each packet read and write, uppercase is used for TCP/UDP packets and lowercase is used for TUN/TAP packets.
6 to 11 -- Debug info range (see errlevel.h for additional information on debug levels).

I think we need we need to update this line as well.

wyardley · 2017-10-11T17:39:21Z

@luxflux edited that other line as well. The bulk of the tests currently fail with the same error, if you get a sec, could you see what I need to adjust?

Puppet::Error:
        Could not find resource 'Openvpn::Ca[winterthur]' for relationship on 'Openvpn::Client[winti-client1]'

I think the ordering on / from the base class is not needed, though maybe that's part of the problem?

luxflux · 2017-10-12T08:03:42Z

@wyardley I'm not used to the Puppet data types. But what I can say is that a openvpn::client needs a openvpn::ca to work (directories, signing CA, ...) and that there is quite some magic involved here because a openvpn::ca may be used by multiple servers. To cover this, there is this pick statement in client.pp.

Does this help?

wyardley · 2017-10-19T22:22:13Z

@juniorsysadmin any thoughts on the test failures here?

juniorsysadmin · 2017-10-21T12:22:07Z

@wyardley Hmm, I was going to suggest re-arrangement of pre_condition to be first in the defined types but that shouldn't have an effect. I take it you have tried sprinkling

Puppet::Util::Log.level = :debug
Puppet::Util::Log.newdestination(:console)

in the rspec, and turning off parallel rspec to see if that is helpful in finding the problem?

bastelfreak · 2017-11-17T22:55:29Z

manifests/ca.pp

-  $key_name     = '',
-  $key_ou       = '',
-  $tls_auth     = false,
+  String $country,


datatypes \o/

bastelfreak · 2017-11-17T22:57:05Z

manifests/server.pp

-    if $extca_server_key_file == undef { fail('extca_server_key_file has to be specified in extca mode') }
-    if $extca_dh_file == undef and !$remote and $tls_server { fail('cant enable tls_server: missing extca_dh_file') }
-    if $extca_tls_auth_key_file == undef and !$remote and $tls_auth { fail('cant enable tls_auth: missing extca_tls_auth_key_file') }
+    if $extca_ca_cert_file == undef {


How do you like this?

unless $extca_ca_cert_file {}

This is maybe easier to read?

I prefer that syntax myself, though I know others don't. I'm wondering if those sort of changes are best made after we at least get the tests passing w/ the updated code, and maybe in a separate PR?

I was actually having the exact same thought, though, when I was rebasing it.

yeah, we should not clutter up this PR.

should be re-reviewed

wyardley · 2017-12-07T07:14:28Z

I've rebased again, however, not sure how much time I'll be able to devote to this (it's not a module I use), and the tests still have the same failures. Is anyone interested in taking a deeper look into the test failures, or should I close this?

wyardley · 2017-12-13T19:01:50Z

I'm going to close this; agree with some comments that this is too much all at once, and not that familiar with the module, so I'll let someone else pick up this torch if they're interested.

bastelfreak reviewed Oct 9, 2017

View reviewed changes

wyardley commented Oct 9, 2017

View reviewed changes

luxflux previously requested changes Oct 11, 2017

View reviewed changes

bastelfreak reviewed Nov 17, 2017

View reviewed changes

manifests/ca.pp

$key_name = '',

$key_ou = '',

$tls_auth = false,

String $country,

Copy link

Member

bastelfreak Nov 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datatypes \o/

bastelfreak reviewed Nov 17, 2017

View reviewed changes

wyardley added the tests-fail label Nov 17, 2017

Switch to Puppet data types

d41e6c0

wyardley added the backwards-incompatible label Dec 7, 2017

wyardley closed this Dec 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Switch to Puppet data types #244

[WIP] Switch to Puppet data types #244

wyardley commented Oct 9, 2017 •

edited

Loading

bastelfreak Oct 9, 2017

wyardley Oct 9, 2017

wyardley Oct 9, 2017

luxflux Oct 11, 2017 •

edited

Loading

luxflux Oct 11, 2017 •

edited

Loading

wyardley commented Oct 11, 2017

luxflux commented Oct 12, 2017

wyardley commented Oct 19, 2017

juniorsysadmin commented Oct 21, 2017

bastelfreak Nov 17, 2017

bastelfreak Nov 17, 2017

wyardley Nov 17, 2017

wyardley Nov 17, 2017

bastelfreak Nov 17, 2017

wyardley commented Dec 7, 2017

wyardley commented Dec 13, 2017

[WIP] Switch to Puppet data types #244

[WIP] Switch to Puppet data types #244

Conversation

wyardley commented Oct 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luxflux Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

luxflux Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

wyardley commented Oct 11, 2017

luxflux commented Oct 12, 2017

wyardley commented Oct 19, 2017

juniorsysadmin commented Oct 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wyardley commented Dec 7, 2017

wyardley commented Dec 13, 2017

wyardley commented Oct 9, 2017 •

edited

Loading

luxflux Oct 11, 2017 •

edited

Loading

luxflux Oct 11, 2017 •

edited

Loading