Closed baurmatt closed 1 year ago
Puppetfiles
.These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.
Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.
I think dropping Puppet 5 compatibility code deserves at least a mention in the changelog. I'd suggest to submit that as its own PR for visiblity and then rebase this once it's merged.
I don't think it's worth the trouble creating two separate MRs, rebased, ... Officially, Puppet 5 support has been drop with https://github.com/puppetlabs/puppetlabs-concat/pull/685. This just manifests it.
Hey @baurmatt, thanks for taking the time to contribute to our module. Regarding the Puppet 5 code removal topic, while it is true that it has been deprecated for a while, we still prefer (where possible) to have changes properly separated and documented (title and description) in different PRs. This helps us and the rest of the community keep better track of changes being made. Otherwise, pinpointing certain changes can often become a quite challenging.
We would appreciate if you could push the Puppet 5 code removal changes in a different PR. Sorry for the inconvenience.
i have created #761 for the puppet5 removal, once that's merged this should be able to be rebased
I'm still not convinced the r[:content].unwrap
line is reachable though. In other types, where I've had a property/parameter that was a hash and within that there's a sensitive, I'd have to unwrap it, but otherwise I think it's automagic???
Sorry, got sick :( Yeah, seems like this isn't needed. I've removed the unwrap
part.
Thanks for adding Sensitive support! But it appears that the entire content
has to be wrapped in Sensitive
. It's not possible to use epp()
to render the content where certain variables contain Sensitive
values.
Opened a bug here: https://tickets.puppetlabs.com/browse/MODULES-11429
For example, this doesn't work as expected. A warning is thrown and the sensitive content is not redacted:
concat::fragment { 'foo':
target => $foo,
order => '01',
content => epp('module/template.epp',
{
'password' => Sensitive($password),
'other'. => $other,
}
),
}
Warning: /Concat_fragment[foo]: Unable to mark 'content' as sensitive: content is a parameter and not a property, and cannot be automatically redacted.
@natemccurdy Thanks for bringing this up! I wasn't able to reproduce this with version 7.4.0, but with 8.0.0. So something must have been broken with the latest version.
(Talking about concat version, not Puppet)
This seems to be broken with https://github.com/puppetlabs/puppetlabs-concat/commit/bc6478ae57ac25be6a53c8c64eca179853c3a598#diff-176bed02b2636bba558658d6d610f63f1c894e45cfa6d7b54e8257cc57ba5d49L98
Ping @LukasAud :)
oh, you definitely can't just rename that function just because rubocop doesn't like it beginning with set_
...
I'll open a PR.
Fixed in https://github.com/puppetlabs/puppetlabs-concat/pull/777
@LukasAud Are there any other modules where a similar change might have slipped in?
Maybe remove this https://github.com/puppetlabs/puppetlabs-vcsrepo/blob/main/.rubocop_todo.yml#L45 and explicitly disable the cop on the offending line
@LukasAud Are there any other modules where a similar change might have slipped in?
https://github.com/puppetlabs/puppetlabs-postgresql/pull/1433 was exactly the same thing.
https://github.com/puppetlabs/puppetlabs-chocolatey/blob/bc40015f36960b241651953eaa9aa832a183cbcc/.rubocop_todo.yml#L59 also maybe an accident waiting to happen
Good news is I've not been able to find any other modules that have the breaking change in them.
Fixes #742.