Add plan results to PR as a comment

xt0rted commented 3 years ago

Is your enhancement request related to a problem? Please describe.

Right now you have to look through the log output to see what the proposed changes are, it'd be a lot nicer if this could be added to the PR as a comment for easier review.

Describe the solution you'd like

I'd like to see a plan logger that comments on the PR, if being run from one, with a table of the proposed changes. There's a version of this in the octoDNS repo which can be found in https://github.com/octodns/octodns/pull/156#issuecomment-374038862 along with examples of what the comment looks like.

Describe alternatives you've considered

Creating my own action/scripts which I'd rather not do since I've never worked with python.

Additional context

solvaholic commented 3 years ago

I like this idea, thank you @xt0rted !

To be sure I'm thinking clearly about what you're after.. Would it meet your needs to add an octodns-sync input like add-pr-comment:

        uses: solvaholic/octodns-sync@v2
        with:
          config_path: public.yaml
          add-pr-comment: Yes

And, when that's set, octodns-sync adds the HTML-formatted plan comment to the PR that triggered the action, like in the example you provided?

Adding a comment will be straightforward, with pull_request.issue_url in the triggering payload. I'll figure out how to get the planhtml output and put together something we can test.

xt0rted commented 3 years ago

Yea, I figured making this opt-in would be the best way to do it. It's been awhile since I played with octodns but from what I recall the code in that issue worked fine for me back when it was first posted. At the time I was running it locally, I never got far enough to deploy it so hopefully this is even easier now with actions.

The other settings that might be useful are one for the api token and the name & email of the account leaving the comment.

solvaholic commented 3 years ago

Yea, I figured making this opt-in would be the best way to do it.

10-4, thank you for confirming.

The other settings that might be useful are one for the api token and the name & email of the account leaving the comment.

👍, adding the PR comment may need a token. Would you use the name and email for attributing the comment? or something else?

I'll figure out how to get the planhtml output...

After adding this to /config/public.yaml:

manager:
  plan_outputs:
    html:
      class: octodns.provider.plan.PlanHtml

This command:

octodns-sync --config-file="/config/public.yaml" 2>/dev/null

Produced this output:

example.com.

route53

Operation	Name	Type	TTL	Value	Source
Create	llama	CNAME	3600	nnn01.example.com.	config
Summary: Creates=1, Updates=0, Deletes=0, Existing Records=17

solvaholic commented 3 years ago

I'm thinking:

The plan is what @thattommyhall wanted, in #11, not necessarily the rest of the octodns-sync output
Write the log to one file and the plan to another
If add-pr-comment is enabled, post the contents of the output file as a comment on the triggering pull request
This'll fit naturally into entrypoint.sh

This shell script:

#!/bin/bash
_needle='<!--- octodns/plan --->'
_header="## octoDNS Plan for {sha}"
_footer="Automatically generated by [solvaholic/octodns-sync](https://github.com/solvaholic/octodns-sync)"

_config_path="/config/public.yaml"
_planfile="/octodns-sync.plan"
_logfile="/octodns-sync.log"

echo "${_needle}" > "${_planfile}"
echo " " >> "${_planfile}"
echo "${_header}" >> "${_planfile}"
echo " " >> "${_planfile}"

script "${_logfile}" -e -c "octodns-sync --config-file=\"${_config_path}\" >>\"${_planfile}\""

echo " " >> "${_planfile}"
echo "${_footer}" >> "${_planfile}"

Produces the output similar to the example:

octoDNS Plan for {sha}

winans.net.

route53

Operation	Name	Type	TTL	Value	Source
Create	llama	CNAME	3600	ams01.winans.net.	config
Summary: Creates=1, Updates=0, Deletes=0, Existing Records=17

Automatically generated by solvaholic/octodns-sync

xt0rted commented 3 years ago

Would you use the name and email for attributing the comment? or something else?

After looking this over again the name & email options aren't needed since there's no commit activity. The reason I mentioned it is because I try to keep automated activity associated with the github-actions[bot]/github-actions[bot]@users.noreply.github.com user so it doesn't get confused with explicit user activity. Since this is an issue comment the user it's associated with is based on the token used which is the bot account for GITHUB_TOKEN.

I like the idea of saving the plan output. Can this be saved as json or yaml? That could make it even more useful if you wanted to work with the data in a later step.

For now I just want a comment on the PR, but down the road I might want to post to slack the changes that were just applied. I don't want to ask for features I may never use though.

solvaholic commented 3 years ago

After looking this over again the name & email options aren't needed since there's no commit activity.

Cool, thank you for explaining @xt0rted

I like the idea of saving the plan output. Can this be saved as json or yaml? That could make it even more useful if you wanted to work with the data in a later step.

I think that functionality could definitely be added. The plan output is produced by octodns's plan.py, which currently has Plans for Logger, Markdown, and Html. As long as the sync command puts the output you're looking for on stdout, it'll be saved as the plan output.

For now I just want a comment on the PR, but down the road I might want to post to slack the changes that were just applied. I don't want to ask for features I may never use though.

I appreciate your requests, and your consideration 🙇

As of d42b3f1e138d7b1a58eae65674352633b69f3ed3 the issue35 branch is functional enough to test. Most of the details to test it are already in the readme. Here's the gist:

Set plan_outputs in your octodns config, for example public.yaml:

manager:
plan_outputs:
  html:
    class: octodns.provider.plan.PlanHtml

Configure your workflow like:

on:
pull_request:
jobs:
test:
  runs-on: ubuntu-latest
  steps:
    - uses: actions/checkout@v2
    - uses: solvaholic/octodns-sync@issue35
      with:
        config_path: public.yaml
        add_pr_comment: 'Yes'
        pr_comment_token: '${{ github.token }}'

If you have a chance to try it out, please let me know what you think.

solvaholic commented 3 years ago

@xt0rted this feature is available and documented in main now, so uses: solvaholic/octodns-sync@main should get it.

Please let me know what questions or feedback you have. 🙇

xt0rted commented 3 years ago

I'm just now testing this update and have a few issues I'm running into.

The first thing I noticed was if there are errors running OctoDNS a comment is still left on the pr. I don't think this is really an issue, it's just not what I expected since OctoDNS didn't successfully run. I figured this step would throw an error and the workflow would end here. My initial run had the wrong api credentials which resulted in the following log output:

Traceback (most recent call last):
  File "/usr/local/bin/octodns-sync", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.7/site-packages/octodns/cmds/sync.py", line 37, in main
    manager = Manager(args.config_file)
  File "/usr/local/lib/python3.7/site-packages/octodns/manager.py", line 118, in __init__
    self.providers[provider_name] = _class(provider_name, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/octodns/provider/rackspace.py", line 64, in __init__
    auth_token, dns_endpoint = self._get_auth_token(username, api_key)
  File "/usr/local/lib/python3.7/site-packages/octodns/provider/rackspace.py", line 85, in _get_auth_token
    x['name'] == 'cloudDNS'][0]['endpoints'][0]['publicURL']
IndexError: list index out of range

And this is the comment that was left:

github com_paramax_dns_pull_2

The second thing is each time the action runs it adds a new comment to the PR which can get very noisy. The logger in https://github.com/octodns/octodns/pull/156#issuecomment-374038862 checks the PR comments for one with  and updates it if found or creates a new comment if it wasn't.

~~The third is I keep getting an error saying my zone file isn't found.~~ This is resolved, my config paths were wrong as I expected.

solvaholic commented 3 years ago

I'm just now testing this update and have a few issues I'm running into.

Fantastic, thank you for letting me know @xt0rted :bow:

I'll set up new issues for these now.

solvaholic / octodns-sync