Add a job that runs diagnostics

cisagov / skeleton-generic

A generic skeleton project for quickly getting a new cisagov project started.

Creative Commons Zero v1.0 Universal

14 stars 11 forks source link

Add a job that runs diagnostics #144

Closed jsf9k closed 1 year ago

jsf9k commented 1 year ago

🗣 Description

This pull request adds a diagnostics job that runs these GitHub Actions:

💭 Motivation and context

The GitHub Actions listed above are added in a separate diagnostics job. As configured the actions should never fail, but they will print out information that may be useful in diagnosing workflow failures.

🧪 Testing

All automated tests pass. I verified that the expected outputs appear in the GitHub Actions log (also here).

✅ Pre-approval checklist

[x] This PR has an informative and human-readable title.
[x] Changes are limited to a single goal - eschew scope creep!
[x] All relevant type-of-change labels have been added.
[x] I have read the CONTRIBUTING document.
[x] These code changes follow cisagov code standards.
[x] All new and existing tests pass.

jsf9k commented 1 year ago

Thanks for setting up this PR! I'm on board but I have two thoughts for consideration. First, would you please add this to the list of commented out dependencies in the Dependabot configuration:

https://github.com/cisagov/skeleton-generic/blob/bd762fe4c4b41eca45fe8f0e3f3fa26bbfa0345c/.github/dependabot.yml#L15-L21

Second, and this one is more a thought, what do you think about adding a needs block to the lint job and requiring the diagnostics job? Even if the diagnostics job does not currently leverage the ability for the Action to fail depending on the GitHub status, if the job has a problem while running that will generally not be a good sign for the lint job to run.

@mcdonnnj - Please see commits 3e51119 and f88b9d2.

mcdonnnj commented 1 year ago

@jsf9k Would you please update the PR description to include the addition of the step-security/harden-runner Action?

jsf9k commented 1 year ago

@jsf9k Would you please update the PR description to include the addition of the step-security/harden-runner Action?

I just did it while you were writing your comment. Synchronicity!

jsf9k commented 1 year ago

I'm don't think that the step-security/harden-runner Action is covering the lint job when it is inserted only in the diagnostics job. I think that Action may need to be duplicated in each workflow job to actually provide the desired coverage. Am I right, @cisagov/team-ois?

I created commit 2bddec4 to test this hypothesis.

mcdonnnj commented 1 year ago

I'm don't think that the step-security/harden-runner Action is covering the lint job when it is inserted only in the diagnostics job. I think that Action may need to be duplicated in each workflow job to actually provide the desired coverage. Am I right, @cisagov/team-ois?

I created commit 2bddec4 to test this hypothesis.

Oh, right. Each job is its own runner so it would need to be used in every job you want hardened.

jsf9k commented 1 year ago

I created commit 2bddec4 to test this hypothesis.

Looks like I was correct, so I'll leave the commit in place.

mcdonnnj commented 1 year ago

I created commit 2bddec4 to test this hypothesis.

Looks like I was correct, so I'll leave the commit in place.

@jsf9k Yes, please see my comment in https://github.com/cisagov/skeleton-generic/pull/144#issuecomment-1699551781. Also you may want to mention that the step-security/harden-runner Action must be added in every job (and that you added it to the lint job) in the description to better track when the kraken this change is included in is being resolved.

jsf9k commented 1 year ago

Also you may want to mention that the step-security/harden-runner Action must be added in every job (and that you added it to the lint job) in the description to better track when the kraken this change is included in is being resolved.

Please see commit 69dd196.