Ensure we dont mess with windows permissions with Unix ones

[lynxman]

We found issues with unix file definitions on our Windows servers in which the file would be unreadable by Windows due to permissions disappearing, this fixes it.

Once this is merged I'll start working on getting this branch up to speed with master

[solarkennedy]

I don't like the case statements.

Instead, how about setting the defaults for the file resource in the class?

case windows
File {
  owner => undef,
  group => undef,
  mode => ?
}
case linux {
File {
  owner => $user,
  group => $group,
  mode => 0644,
}
}

https://docs.puppet.com/puppet/4.9/lang_defaults.html

[lynxman]

Cool! Will do that way :)

[lynxman]

I've done something slightly different, instead of redefining the whole module File references I've moved the references to params.pp as they should and set the right ones over there, let me know if that's okay!

[solarkennedy]

for style, can you compact all the vertical whitespace in the "windows" block to make it more obvious to the eye that these are all for windows, and then align arrows?

[solarkennedy]

just do ',' and not ';'

[lynxman]

Oh crap, I missed that one, sorry!

[solarkennedy]

Just style stuff (https://travis-ci.org/solarkennedy/puppet-consul/jobs/205823470) and I'll click the button!

[lynxman]

Done! Once this is merged I'll start working on integrating all this to the latest master so Windows works out of the box

[solarkennedy]

I'm not sure why the tests are failing... I looks like puppet-archive, but we pin in fixtures.yml, I'm not sure how a breaking change would get through...

[solarkennedy]

Can you try rebasing on master to see if the tests pass now?

[lynxman]

Hi,
Sorry for the late reply, I was out!
This windows branch is so far behind master that the changes break there, I was thinking about doing a separate PR to integrate windows changes in master so this branch can be closed, which way do you prefer to go?

[solarkennedy]

@lynxman it's all good. Do whatever you are git-comfortable with.

[lynxman]

Cool, Let me sort it out and will come back with passing tests :)

[solarkennedy]

This may be obsoleted by #352

[bastelfreak]

Hi @lynxman, is this still needed after #403 got merged?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.