Closed Bug 1424651 Opened 6 years ago Closed 6 years ago

Re-allow specifying symbol upload symbol token via a file.

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: tomprince, Assigned: tomprince)

References

Details

Attachments

(5 files)

The upload symbol code switched to fetching secrets from taskcluster in Bug 1422740. Thunderbird is still (although hopefully not for much longer) using buildbot to build nightlies, so I would like to re-add support for getting the token from a local file as well.
Blocks: 1424334
Comment on attachment 8936206 [details]
Bug 1424651: Factor out getting taskcluster secret; .mielczarek

https://reviewboard.mozilla.org/r/206972/#review212874
Attachment #8936206 - Flags: review?(ted) → review+
Comment on attachment 8936207 [details]
Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek

https://reviewboard.mozilla.org/r/206974/#review212876

::: toolkit/crashreporter/tools/upload_symbols.py:87
(Diff revision 1)
> +    else:
> +        log.error('You must set the SYMBOL_SECRET or SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE environment variables!')
> +        return 1
>  
> -    if os.environ.get('MOZ_SCM_LEVEL', '1') == '1':
> +    # Allow overwriting of the upload url with an environmental variable
> +    if 'SOCORRO_SYMBOL_UPLOAD_URL' in os.environ:

Do you actually need to override the symbol upload URL? I removed this because I didn't think it was actually useful.
Attachment #8936207 - Flags: review?(ted) → review+
Comment on attachment 8936208 [details]
Bug 1424651: Fix upload_symbols.py documentation; .mielczarek

https://reviewboard.mozilla.org/r/206976/#review212878

::: toolkit/crashreporter/tools/upload_symbols.py:11
(Diff revision 1)
>  #
>  # This script uploads a symbol zip file passed on the commandline
>  # to the Tecken symbol upload API at https://symbols.mozilla.org/ .
>  #
> +# Using this script requires you to have generated an authentication
>  # token in the Tecken web interface. You must store the token in a Taskcluster

Might be good to rephrase this a little, like "You can either ..."
Attachment #8936208 - Flags: review?(ted) → review+
Comment on attachment 8936209 [details]
Bug 1424651: Remove unused SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE mozconfig variable; .mielczarek

https://reviewboard.mozilla.org/r/206978/#review212892

Hah, I didn't even notice those variables were still there!

::: Makefile.in:306
(Diff revision 1)
>  buildsymbols:
>  endif
>  
>  uploadsymbols:
>  ifdef MOZ_CRASHREPORTER
> -ifdef SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE
> +ifneq ($(strip $(SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE)$(SYMBOL_SECRET),)

I'd just remove this conditional entirely. The script should produce a useful error message if someone screws it up.
Attachment #8936209 - Flags: review?(ted) → review+
Comment on attachment 8936207 [details]
Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek

https://reviewboard.mozilla.org/r/206974/#review212876

> Do you actually need to override the symbol upload URL? I removed this because I didn't think it was actually useful.

Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1424236#c5 the token available on buildbot won't work with the new URL. I'd like to avoid needing to change that token, particularly as I'd need to figure out if that will affect esr52 builds.
Comment on attachment 8936208 [details]
Bug 1424651: Fix upload_symbols.py documentation; .mielczarek

https://reviewboard.mozilla.org/r/206976/#review212878

> Might be good to rephrase this a little, like "You can either ..."

I'm happy to change it. My original thought was to put the prefered and then mention the deprecated one after.
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/integration/autoland/rev/189fd4f1df41
Factor out getting taskcluster secret; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/6038fb7b458c
Re-allow specifying symbol upload symbol token via a file; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/746d96792d18
Fix upload_symbols.py documentation; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/10ebf78f32bb
Remove unused SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE mozconfig variable; r=ted.mielczarek
Comment on attachment 8936212 [details]
Bug 1424651: Move SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE to the Thunderbird specific mozconfig files;

Can you land this after the m-c parts merge from autoland? Thanks.
Flags: needinfo?(jorgk)
Comment on attachment 8936207 [details]
Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek

https://reviewboard.mozilla.org/r/206974/#review212918

::: toolkit/crashreporter/tools/upload_symbols.py:77
(Diff revision 1)
> -        log.error('You must set the SYMBOL_SECRET environment variable!')
> -        return 1
> -    auth_token = get_taskcluster_secret(secret_name)
> +        auth_token = get_taskcluster_secret(secret_name)
> +    elif 'SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE' not in os.environ:
> +        token_file = os.environ['SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE']
> +

just a quick request for clarification.
should this part not say 
 elif 'SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE' in os.environ:

if it isn't in the os.environ, I don't see how
you can get that value from the os.environ?

furthermore, by moving to symbols... you just
need to log on to the Tecken ui and get a new
symbol done.

Then afaik, since |make upload-symbols| goes to symbols.mozilla.org, there really isn't anything
to do with buildbot.  Just need to ensure that
the new symbols file are on the workers.
Comment on attachment 8936207 [details]
Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek

https://reviewboard.mozilla.org/r/206974/#review212918

> just a quick request for clarification.
> should this part not say 
>  elif 'SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE' in os.environ:
> 
> if it isn't in the os.environ, I don't see how
> you can get that value from the os.environ?
> 
> furthermore, by moving to symbols... you just
> need to log on to the Tecken ui and get a new
> symbol done.
> 
> Then afaik, since |make upload-symbols| goes to symbols.mozilla.org, there really isn't anything
> to do with buildbot.  Just need to ensure that
> the new symbols file are on the workers.

Yep. Thanks for catching this before it hit m-c.

It requires some work to get a new token onto the buildbot workers, and verify that doing that doesn't impact old branches that haven't switch to symbols.mozilla.org. Since TB will only be on buildbot through 59 (hopefully), it is easier to add this option in temporarily than deal with adding a new puppet secret.
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/integration/autoland/rev/0413e7241a3d
Factor out getting taskcluster secret; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/5eaf58103579
Re-allow specifying symbol upload symbol token via a file; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/f64f4a99ffca
Fix upload_symbols.py documentation; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/8d5b3c6a6387
Remove unused SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE mozconfig variable; r=ted.mielczarek
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/comm-central/rev/02fd46566f88
Move SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE to the Thunderbird specific mozconfig files; r=me
Flags: needinfo?(jorgk)
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/comm-central/rev/f14a2331480c
Specify SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE in linux32 mozconfigs too; r=me
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.