refactor(ci): use GitHub variables for non-sensitive info

[gustavovalverde]

Motivation

We've been using values that are variable across multiple workflows,
and those can only be changed if modifying the workflows, but we should
be able to change the values without committing new changes in the code
for this purpose we're now using GitHub Variables, and even moving
non-sensitive information into variables instead of secrets. Allowing
more flexibility and other scenarios that should be easier to manage,
like deploying to Mainnet or Testnet.

Closes: #6312

This will also make it easier to fix our actual main branch failures

Specifications

https://docs.github.com/en/actions/learn-github-actions/variables#defining-configuration-variables-for-multiple-workflows

Solution

Use GitHub variables for:

• Our GCP Project name
• Our registry URLs
• GCP machine types
• Other variables that have an impact on all or most workflows

Review

Anyone from the DevOps team, after test are passing (or before for desing feedback)

Check the actual values here: https://github.com/ZcashFoundation/zebra/settings/variables/actions

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
  • Does it change concurrent code, unsafe code, or consensus rules?
• How do you know it works? Does it have tests?

[codecov]

Codecov Report

Merging #6357 (5534c74) into main (403c074) will decrease coverage by 0.25%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6357      +/-   ##
==========================================
- Coverage   77.90%   77.66%   -0.25%     
==========================================
  Files         304      304              
  Lines       39665    39665              
==========================================
- Hits        30902    30806      -96     
- Misses       8763     8859      +96     

[gustavovalverde]

This should also make it easier to fix main branch CI failures, when related to CPUs not being available on a specific region.

[teor2345]

I like this change, I think it will make things a lot easier!

But I'm not sure how to review or test it, since we only run some workflows on main or on specific events (like releases).

[teor2345]

This is probably something for the next refactor, but can you change some of the secrets that don't need to be confidential into variables?

it's much easier to edit the value if we know what the previous value was.

[mpguerra]

This is probably something for the next refactor, but can you change some of the secrets that don't need to be confidential into variables?

it's much easier to edit the value if we know what the previous value was.

Can this be merged "as is" with follow ups in other PRs? Otherwise this will have to be parked until Sprint 8

[teor2345]

This is probably something for the next refactor, but can you change some of the secrets that don't need to be confidential into variables?
it's much easier to edit the value if we know what the previous value was.

Can this be merged "as is" with follow ups in other PRs? Otherwise this will have to be parked until Sprint 8

Unfortunately, this PR uses a single variable for the machine type, but our current CI uses two different machine types. Changing all workflows to a single machine type could break CI or be really expensive, so we need to fix that before we merge:
#6357 (comment)

[teor2345]

All the other suggestions could be done as follow-ups.

[gustavovalverde]

Aside from secrets.DOCKERHUB_USERNAME, all other non-sensitive information was replaced with a variable.

[teor2345]

Thanks for these fixes, it looks good!

[teor2345]

This PR failed in the merge queue, because it tried to create a disk using the incorrect network name:

ERROR: (gcloud.compute.images.create) argument --labels: Bad value [Mainnet]: Only hyphens (-), underscores (_), lowercase characters, and numbers are allowed. International characters are allowed.

https://github.com/ZcashFoundation/zebra/actions/runs/4683572250/jobs/8299559492?pr=6493#step:13:81

I have applied a quick fix to use the correct network variable, and a fix for another incorrect network default variable.

In general, it would help if we put more details in variable names, so that it's clearer what they should be used for:

• vars.ZCASH_NETWORK -> vars.DEFAULT_ZCASH_NETWORK
• ${NETWORK} -> ${NETWORK_LOWERCASE}

(I didn't make these changes in this PR, happy to leave them to a later refactor.)

[gustavovalverde]

I have applied a quick fix to use the correct network variable, and a fix for another incorrect network default variable.

In general, it would help if we put more details in variable names, so that it's clearer what they should be used for:

vars.ZCASH_NETWORK -> vars.DEFAULT_ZCASH_NETWORK
${NETWORK} -> ${NETWORK_LOWERCASE}

Thanks for this, I'll take this into account for further refactors I have pending.

[teor2345]

Failed in the merge queue due to #6498

[teor2345]

@Mergifyio refresh

[mergify]

refresh

✅ Pull request refreshed