style(ci): lint and standardize the actions structure

[gustavovalverde]

Motivation

When making changes that affected multiple actions, not having an standardized file made global replacements difficult, and sometimes it left unwanted code unchanged.

Designs

• Make all the actions standard in structure and indentation

Solution

• Refactor and lint the files to comply with https://json.schemastore.org/github-workflow.json

Review

@dconnolly can also review this

[teor2345]

I have some questions about this PR:

• How can we be sure we've made all these changes consistently? Did you use search and replace, or make each change manually?
• Is there an automated linter for the formatting rules you're using, or an automated formatter to apply the rules?

This PR changes job conditions and job names, which has previously caused CI to fail or skip tests. So I've marked it as "do not merge", until we decide how risky it is. (We're currently working on some urgent NU5 and lightwalletd changes.)

[gustavovalverde]

I used https://marketplace.visualstudio.com/items?itemName=cschleiden.vscode-github-actions to ensure the conditions were kept as we expected, and https://marketplace.visualstudio.com/items?itemName=redhat.vscode-yaml to lint based on GitHub Action's json schema https://json.schemastore.org/github-workflow.json (mostly automatic)

The manual changes requested to comply were:

• Using macos-latest as macOS-latest does not exists in the schema
• Remove if conditions that were not evaluating as there was no context to evaluate from
• Use required and missing keys from the inputs in workflow_dispatch

Changes after revision:

• Revert to single quotes (fewer diff)
• Lint Mergify and Dependabot conf

[teor2345]

This seems like a potentially risky fix, that's isn't needed right now.

So I'd like to wait until after the high-priority NU5 updates are merged, and the NU5 testnet reactivation release is tagged.

[gustavovalverde]

@teor2345 @dconnolly I'd like to prioritize this PR, as it will help me identify common errors in other PRs, including parsing errors that won't be correctly parsed by GH (I added an action to lint other actions and verify shell errors)

[teor2345]

@teor2345 @dconnolly I'd like to prioritize this PR, as it will help me identify common errors in other PRs, including parsing errors that won't be correctly parsed by GH (I added an action to lint other actions and verify shell errors)

That makes sense. Did you need it to go in before we tag the NU5 testnet rollback version?

(We're currently waiting for the zcashd team to release 4.7.0 and pick an activation height. Hopefully that will happen some time in the next few days.)

Here's what I think this PR does:

No behaviour change:

• use standard formatting

Behaviour change:

• change when some workflows run
• add a job that lints workflows

Unsure:

• remove some job conditions - only a behaviour change if the conditions were skipping the job

Do any of these changes need to merge sooner?

If we need some changes sooner, we could reduce the risk by splitting it into multiple PRs. Or by testing a Rust PR immediately after it merges, and reverting if it causes problems.

[dconnolly]

Can we make sure actionlint runs at least any time any .github/workflows/* file is changed? I'm so happy to have a GH workflow linter!

[gustavovalverde]

@teor2345 this is ready. The shellcheck can be validated later, those shell commands have been like that for a while, I don't think those recommendations are completely valid for the specific use case.

I fixed the checkpoint_sync issue as that one was needed.

[teor2345]

@teor2345 this is ready. The shellcheck can be validated later, those shell commands have been like that for a while, I don't think those recommendations are completely valid for the specific use case.

I checked each shellcheck warning.

Some could be fixed, but none of them seem to be causing problems right now. They will be ok as long as the names don't contain special characters. (And we control those names.)

Some of them might need to be ignored.

[teor2345]

Looks good, thanks for your patience with this!

[teor2345]

Merge queue: sync past mandatory checkpoint:

Error: No such container: klt-sync-checkpoint-4078-merge-97382fe-qxsz

https://github.com/ZcashFoundation/zebra/runs/5963991145?check_suite_focus=true#step:9:115

[teor2345]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[teor2345]

The "Test full validation sync from cached state" job logs this warning:

Warning: The "create_credentials_file" option is true, but the current GitHub workspace is empty. Did you forget to use "actions/checkout" before this step? If you do not intend to share authentication with future steps in this job, set "create_credentials_file" to false.

https://github.com/ZcashFoundation/zebra/runs/5964749623?check_suite_focus=true#step:4:71

[gustavovalverde]

Yeah, I have that warning pending. I don't want to do checkout if it's not needed, but this is working without using checkout, but I'm not sure it will still work if I remove the create_credentials_file. I'll test that in the documentation PR

[teor2345]

I'm still seeing these container name errors:

the full validation sync fails with the wrong image name

Merge queue: sync past mandatory checkpoint:

Error: No such container: klt-sync-checkpoint-4078-merge-97382fe-qxsz

https://github.com/ZcashFoundation/zebra/runs/5963991145?check_suite_focus=true#step:9:115

[gustavovalverde]

This is, once again, the error caused by partitions in GCP

This happened before in other PRs, I'll follow up with the ticket. But it's not related to the name of the container.

I'll also create a PR to show this kind of issues when the container does not start, so it's easier for the team identifying the root cause without having to get into GCP and search for the logs

[teor2345]

Thanks, looks good!