ansible / ansible-documentation

Ansible community documentation
https://docs.ansible.com/
GNU General Public License v3.0
80 stars 467 forks source link

Output workflow parameters for approval details #1650

Closed samccann closed 3 months ago

samccann commented 3 months ago

Display the input parameters for the Ansible package docs build workflow so approvers can verify the correct settings before approving a deploy of the docs to prod or test.

samccann commented 3 months ago

@oraNod I think I fixed the extra whitespace problem. Can you take another look pls?

oraNod commented 3 months ago

@samccann I feel like this is good to merge but no harm giving it a couple more days to give others a chance to review and comment.

webknjaz commented 3 months ago

This info will definitely be useful when we need to review and approve deployments.

In that case, perhaps append it to ${{ GITHUB_JOB_SUMMARY }} too. Maybe, wrap as a Markdown code block, though.

oraNod commented 3 months ago

This info will definitely be useful when we need to review and approve deployments.

In that case, perhaps append it to ${{ GITHUB_JOB_SUMMARY }} too. Maybe, wrap as a Markdown code block, though.

This is a great suggestion and really makes an improvement. I found it was tricky to wrap the json in a code block without getting formatting issues. Since we can do markdown, I'd suggest adding a heading and pointing out only the details about the deployment that we really care about.

What about something like this?

- name: Log the workflow inputs
      run: |
        echo '## Deployment details :rocket:' >> "${GITHUB_STEP_SUMMARY}"
        echo 'Environment:' "${{ github.event.inputs.deployment-environment }}" >> "${GITHUB_STEP_SUMMARY}"
        echo 'Package version:' "${{ github.event.inputs.ansible-package-version }}" >> "${GITHUB_STEP_SUMMARY}"
        echo '```' >> "${GITHUB_STEP_SUMMARY}"

That could give us a summary page like this one: https://github.com/oraNod/ansible-documentation/actions/runs/9700881720/attempts/1#summary-26773300092

samccann commented 3 months ago

@oraNod @webknjaz thanks this helps a lot! I'm wondering if we can expand on don's example (which has each input appended to GITHUB_STEP_SUMMARY) to something that would take the current github.event.inputs in its entirety and output it in job summary?

It would likely not look as pretty as Don's example, but it would have the benefit of not having to remember to update that step of the workflow if we change input parameters in the future.

samccann commented 3 months ago

just to give a comparison - dumping the existing stuff to the job summary looks as follows and has the benefit of not having to ever update that part of the workflow: image

Using @oraNod's example of manually adding each input to the markdown job summary output looks (just a couple of inputs shown as an example): image

That one is definitely easier to read and we could probably get fancier with the markdown if we wanted to. So the question is do we go for:

We're about to drop the language input parameter, but historically, these inputs haven't changed much over time.

oraNod commented 3 months ago

@oraNod @webknjaz thanks this helps a lot! I'm wondering if we can expand on don's example (which has each input appended to GITHUB_STEP_SUMMARY) to something that would take the current github.event.inputs in its entirety and output it in job summary?

It would likely not look as pretty as Don's example, but it would have the benefit of not having to remember to update that step of the workflow if we change input parameters in the future.

You can include all the inputs in the summary, like I did here: https://github.com/oraNod/ansible-documentation/commit/c0ec56dd70bb6e967b18748b5110a276cae5231f

That results in a summary page such as: https://github.com/oraNod/ansible-documentation/actions/runs/9700669374#summary-26772605246

As I mentioned in my previous comment, I was wrangling with json formatting in that codeblock. I thought perhaps - for the purposes of the deployment approval - it might look nicer and more obvious if we highlighted only the inputs that were relevant, deployment environment and version. I suppose you could combine things.

IMO I don't think we're likely to change the inputs . That one about the language doesn't really pertain to the deployment section anyway. It's kind of a deprecated input.

There would be obvious errors if we, say, removed an input but didn't make the corresponding change in what we're logging to the summary. And the fix is easy. So I don't feel like there is a huge maintenance burden. You could add a comment to the input but that seems excessive.

oraNod commented 3 months ago

I guess my summary is that the intent of this PR is to make the details of the deployment destination readily apparent to approvers. It makes sense to have that scoped to only the relevant details for the person approving or rejecting the deployment job.

oraNod commented 3 months ago

Dumping json in a codeblock doesn't seem much better than just putting them in the log files. Adding more human readable plain text is a more elegant result.

samccann commented 3 months ago

Thanks. I'll review the inputs later and update this pr with the prettier option so we can take a look again at it.

samccann commented 3 months ago

Ok the output should look like this now:

image