Implement master side token generation for signing on demand

RESOLVED FIXED

Status

Release Engineering
General
P2
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: rail, Assigned: rail)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 578257 [details] [diff] [review]
token generation code

Requirements:

1) master talks to the signing server over https
2) master validates https cert
3) master uses user/password http authentication over https
4) custom step should support multiple signing servers and should retry n times before switching to another server from the list
5) token should be downloaded from master to slave

Please, see attached patch.
Attachment #578257 - Flags: review?(catlee)
Attachment #578257 - Flags: review?(bhearsum)
Comment on attachment 578257 [details] [diff] [review]
token generation code

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

r+ with these fixed

::: process/factory.py
@@ +1130,5 @@
> +        server_cert = os.path.join(
> +            os.path.dirname(build.paths.__file__),
> +            '../../../release/signing/host.cert')
> +        token = WithProperties('%(basedir)s/build/token')
> +        nonce = WithProperties('%(basedir)s/build/nonce')

I think we can use 'build/token', 'build/nonce' here without properties

@@ +1143,5 @@
> +        ))
> +        self.addStep(SigningServerAuthenication(
> +            servers=self.signingServers,
> +            server_cert=server_cert,
> +            s="",

this is a bit weird. can we make SigningServerAuthentication use a default empty string?

::: steps/signing.py
@@ +114,5 @@
> +    def downloadSignature(self, res):
> +        self.s = res
> +        StringDownload.start(self)
> +
> +    def interrupt(self, reason='Interrupted'):

Should you be calling the base class's interrupt here as well?
Attachment #578257 - Flags: review?(catlee) → review+
Attachment #578257 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 2

6 years ago
Created attachment 578556 [details] [diff] [review]
token generation code

* Address comments
* l10n repacks work now
Attachment #578257 - Attachment is obsolete: true
Attachment #578556 - Flags: review?(catlee)

Updated

6 years ago
Attachment #578556 - Flags: review?(catlee) → review+

Updated

6 years ago
Blocks: 509158
(Assignee)

Comment 3

6 years ago
Created attachment 578686 [details] [diff] [review]
token generation code

* moved token generation after the compile step
* tested in staging
* keeping r+
Attachment #578556 - Attachment is obsolete: true
Attachment #578686 - Flags: review+
(Assignee)

Comment 4

6 years ago
Comment on attachment 578686 [details] [diff] [review]
token generation code

http://hg.mozilla.org/build/buildbotcustom/rev/202dfd424ea1
Attachment #578686 - Flags: checked-in+
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.