Issue 29517747: #1645 - Introduce mumble module

Issue 29517747: #1645 - Introduce mumble module (Closed)

Created:
Aug. 16, 2017, 7:01 p.m. by f.lopez

Modified:
Aug. 31, 2017, 9:02 p.m.

Reviewers:
mathias

CC:
Fred

Visibility:
Public.

Patch Set 2 : For comment 2 #

Created: Aug. 31, 2017, 7:34 p.m.

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+260 lines, -0 lines)			Patch
A	hiera/roles/voip.yaml	View	1	1 chunk	+9 lines, -0 lines	0 comments	Download
A	modules/adblockplus/manifests/mumble.pp	View	1	1 chunk	+114 lines, -0 lines	0 comments	Download
A	modules/adblockplus/templates/mumble-server.ini.erb	View	1	1 chunk	+83 lines, -0 lines	0 comments	Download
A	modules/private-stub/files/mumble.crt	View		1 chunk	+21 lines, -0 lines	0 comments	Download
A	modules/private-stub/files/mumble.key	View		1 chunk	+28 lines, -0 lines	0 comments	Download
M	modules/private-stub/hiera/hosts.yaml	View		1 chunk	+5 lines, -0 lines	0 comments	Download

Messages

Total messages: 4

Expand All Messages | Collapse All Messages

mathias

https://codereview.adblockplus.org/29517747/diff/29517748/hiera/roles/voip.yaml File hiera/roles/voip.yaml (right): https://codereview.adblockplus.org/29517747/diff/29517748/hiera/roles/voip.yaml#newcode7 hiera/roles/voip.yaml:7: adblockplus::mumble::server_password: securepassword Always use "changeme" for password defaults and ...

Aug. 24, 2017, 6:23 a.m. (2017-08-24 06:23:07 UTC) #2

f.lopez

https://codereview.adblockplus.org/29517747/diff/29517748/hiera/roles/voip.yaml File hiera/roles/voip.yaml (right): https://codereview.adblockplus.org/29517747/diff/29517748/hiera/roles/voip.yaml#newcode7 hiera/roles/voip.yaml:7: adblockplus::mumble::server_password: securepassword On 2017/08/24 06:23:07, mathias wrote: > Always ...

Aug. 25, 2017, 7:04 p.m. (2017-08-25 19:04:41 UTC) #3

https://codereview.adblockplus.org/29517747/diff/29517748/hiera/roles/voip.yaml
File hiera/roles/voip.yaml (right):

https://codereview.adblockplus.org/29517747/diff/29517748/hiera/roles/voip.ya...
hiera/roles/voip.yaml:7: adblockplus::mumble::server_password: securepassword
On 2017/08/24 06:23:07, mathias wrote:
> Always use "changeme" for password defaults and avoid explicitly configuring
> them in development (read: apply the default in Puppet code or similar, not
> Hiera).

Acknowledged.

https://codereview.adblockplus.org/29517747/diff/29517748/modules/adblockplus...
File modules/adblockplus/manifests/mumble.pp (right):

https://codereview.adblockplus.org/29517747/diff/29517748/modules/adblockplus...
modules/adblockplus/manifests/mumble.pp:11: # === Hiera (only) Parameters:
On 2017/08/24 06:23:07, mathias wrote:
> Why Hiera only?

Well, I wanted to use it with hiera only, but yeah it might not make sense to do
that.

https://codereview.adblockplus.org/29517747/diff/29517748/modules/adblockplus...
modules/adblockplus/manifests/mumble.pp:36: if ensure_state($ensure) {
On 2017/08/24 06:23:07, mathias wrote:
> The $ensure parameter is always undefined at this point - it is not declared
> anywhere. If it where, however, you couldn't overwrite it anyway, variables in
> Puppet are read-only after definition. You could use some $ensure_file or
> $ensure_foobar to derive from it - and pass the original (!) to the package
> resource (important! otherwise you corrupt requests for "latest" packages or a
> particular version). But deriving $ensure parameters should be done based on
> getparam(Package[...], 'ensure') anyway (to not miss some default defined
> globally) and, in this case, you do not even use the $ensure stuff anywhere
> except for the wrong use with the package resource below.

Acknowledged.

https://codereview.adblockplus.org/29517747/diff/29517748/modules/adblockplus...
modules/adblockplus/manifests/mumble.pp:67: if $mumble::certificate and
$mumble::private_key {
On 2017/08/24 06:23:07, mathias wrote:
> Please do not replicate the legacy "give me the name of a file in the private
> module" nonsense. Use a hash that may contain $source or $content (among other
> stuff) in fully qualified fashion, and merge it with your own parameters
before
> passing it to ensure_resource() or similar.

Ok fair enough, working on this. Gonna send a new patch soon.

https://codereview.adblockplus.org/29517747/diff/29517748/modules/private-stu...
File modules/private-stub/hiera/hosts.yaml (right):

https://codereview.adblockplus.org/29517747/diff/29517748/modules/private-stu...
modules/private-stub/hiera/hosts.yaml:173: role: voip
On 2017/08/24 06:23:07, mathias wrote:
> I'd call it `mumble-server` or something, `voip` is a bit too generic and not
a
> role (-word). How about `voip-server`? 

Acknowledged.

I like talk-1, it is more agnostic and work with our standard.

Expand All Messages | Collapse All Messages