1231493 - Adds Puppet Support for secrets and keys for Win 7 and Win 10 testers

Status: RESOLVED FIXED

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

Comment 1

[Mark Cornmesser [:markco] OOO 2024/04/15]

Attachment: BUG1231493-Win7Slave.patch

Comment 2

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

Comment on attachment 8702940 [details] [diff] [review]
BUG1231493-Win7Slave.patch

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

::: modules/disableservices/manifests/winreport.pp
@@ -6,4 @@
>  
>  class disableservices::winreport {
>      if ($env_os_version != 2008) {
> -        service { "Windows Error Reporting Service":

Does this change break something else where it is called "Windows Error Reporting Service"?

Comment 3

[Mark Cornmesser [:markco] OOO 2024/04/15]

Comment on attachment 8702940 [details] [diff] [review]
BUG1231493-Win7Slave.patch

https://hg.mozilla.org/build/puppet/rev/b4d42b0986ca

Comment 4

[Mark Cornmesser [:markco] OOO 2024/04/15]

It doesn't break anything. It was not actually being applied for 2008, and I was leaving the manifest as a place holder for when it was needed on the testers.

Comment 5

[Mark Cornmesser [:markco] OOO 2024/04/15]

Attachment: BUG1231493-Win7Token.patch

Comment 6

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

Comment on attachment 8704220 [details] [diff] [review]
BUG1231493-Win7Token.patch

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

::: modules/toplevel/manifests/slave.pp
@@ +18,4 @@
>      if ($::operatingsystem == "Darwin") or ($::operatingsystem == "Windows") {
>          include users::builder::autologin
>      }
> +    # The initial pass for support for Win 7 is meant to only support secrets 

Trailing space (travis will complain)

Comment 7

[Mark Cornmesser [:markco] OOO 2024/04/15]

Comment on attachment 8704220 [details] [diff] [review]
BUG1231493-Win7Token.patch

https://hg.mozilla.org/build/puppet/rev/3d913988a833

Comment 8

[Mark Cornmesser [:markco] OOO 2024/04/15]

Attachment: BUG1231493-Win10Slave.patch

Comment 9

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

Comment on attachment 8708533 [details] [diff] [review]
BUG1231493-Win10Slave.patch

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

::: modules/packages/manifests/pkgzip.pp
@@ +8,5 @@
>      $installersource_posix = 'c:/InstallerSource/puppetagain.pub.build.mozilla.org'
>      $installersource_win = 'c:\InstallerSource\puppetagain.pub.build.mozilla.org'
> +    $quoted_7zip =  $hardwaremodel ? {
> +        x64 => '"C:\Program Files (x86)\7-Zip\7z.exe"',
> +        default => '"C:\Program Files\7-Zip\7z.exe"',

What happened here?  x64 -> (x86)??

Comment 10

[Justin Wood (:Callek)]

(In reply to Dustin J. Mitchell [:dustin] from comment #9)
> Comment on attachment 8708533 [details] [diff] [review]
> BUG1231493-Win10Slave.patch
> 
> Review of attachment 8708533 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: modules/packages/manifests/pkgzip.pp
> @@ +8,5 @@
> >      $installersource_posix = 'c:/InstallerSource/puppetagain.pub.build.mozilla.org'
> >      $installersource_win = 'c:\InstallerSource\puppetagain.pub.build.mozilla.org'
> > +    $quoted_7zip =  $hardwaremodel ? {
> > +        x64 => '"C:\Program Files (x86)\7-Zip\7z.exe"',
> > +        default => '"C:\Program Files\7-Zip\7z.exe"',
> 
> What happened here?  x64 -> (x86)??

Without speaking to complete patch validity, that is a *common* idiom on windows, "Program Files (x86)" tends to house x86 binaries on a 64 bit windows, rather than "Program Files\" and sometimes its done transparently to the programs installer depending on what API's they use. It's odd to see but usually valid.

Comment 11

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

Comment on attachment 8708533 [details] [diff] [review]
BUG1231493-Win10Slave.patch

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

::: modules/packages/manifests/pkgzip.pp
@@ +8,5 @@
>      $installersource_posix = 'c:/InstallerSource/puppetagain.pub.build.mozilla.org'
>      $installersource_win = 'c:\InstallerSource\puppetagain.pub.build.mozilla.org'
> +    $quoted_7zip =  $hardwaremodel ? {
> +        x64 => '"C:\Program Files (x86)\7-Zip\7z.exe"',
> +        default => '"C:\Program Files\7-Zip\7z.exe"',

Ah, I see, so it used to treat "x64" as the default, and now treats "x86" as the default, but also switched the values.  So I don't think this has any effect, but is correct..

Comment 12

[Mark Cornmesser [:markco] OOO 2024/04/15]

Attachment: BUG1231493-Win10Slave.patch

A small change form the last patch. For the 7zip directory. I made the directory for the x64 OSes the default in the conditionals and the Win 7, which comes up as i686 for the new variable as the exception. The variable that was being use had to change because it was not available within the Win 10 environment.

Comment 13

[Mark Cornmesser [:markco] OOO 2024/04/15]

Comment on attachment 8708562 [details] [diff] [review]
BUG1231493-Win10Slave.patch

https://hg.mozilla.org/build/puppet/rev/249ea52f8252