865799 - switch to setting show_diff => false only on "sensitive" files after 3.2.0 upgrade

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: Puppet, task)

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug865799.patch

I'm not sure how best to review this, since the risk is that something with a password was omitted.

Comment 2

[Justin Wood (:Callek)]

dustin: I'm considering tackling this over the weekend or early next week, is that timescale ok?

My reason for the defer is I think this warrants a direct "every file resource" peek to make sure we set every secret one we care about to false. I suspect doing a thorough review here is worth the time.

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

That's fine - there's no great urgency here.  Rather, I'm trying to clean out the backlog of low-priority PuppetAgain bugs.  But let's not let it bitrot.

Comment 4

[Justin Wood (:Callek)]

Comment on attachment 801746 [details] [diff] [review]
bug865799.patch

Review of attachment 801746 [details] [diff] [review]:
-----------------------------------------------------------------

All things currently in this patch are good... however,...

*** Things I see that I think need show_diff=false, ***

D:\Sources\puppet\modules\bmm\manifests\httpd.pp
        "/opt/bmm/www/squashfs":
        "/opt/bmm/www/artifacts":
   - since they source from private dir's.

D:\Sources\puppet\modules\packages\manifests\aptrepo.pp
        "/etc/apt/${repo_name}-pubkey.txt":
   - despite being a text file, any changes here won't actually be useful for a human to read.

D:\Sources\puppet\modules\packages\manifests\yumrepo.pp
        "/etc/pki/${repo_name}-pubkey.txt":
   - same reason as aptrepo.pp

D:\Sources\puppet\modules\packages\manifests\mozilla\google_api.pp
        "/builds/gapi.data":
   - It's purged when we loan the machine, therefore it shouldn't be here.

***** Things Unsure on ****

D:\Sources\puppet\modules\bmm\manifests\tftpd.pp
        "/var/lib/tftpboot/panda-live/uImage":
        "/var/lib/tftpboot/panda-live/uInitrd":

**** While touching all these file resources ****

D:\Sources\puppet\modules\puppet\manifests\puppetize_sh.pp
        "${users::root::home}/puppetize.log":
  - Can be removed from managed, the referenced bug is resolved

------

For next patch either a part 2, or a new patch+interdiff is fine. (r- strictly for the google api, everything else you can try to convince me on if you disagree)

::: modules/httpd/manifests/config.pp
@@ +25,5 @@
>                          owner => "$httpd::settings::owner",
>                          group => "$httpd::settings::group",
> +                        content => $content,
> +                        # don't show the diff, since sometimes httpd configs contain passwords
> +                        show_diff => false;

we could make this a template var that we use instead ;-)  but good-for-now

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug865799-interdiff.patch

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug865799-r2.patch