oliviabarrick / fluxcloud

Slack notifications for Weave Flux without Weave Cloud
Apache License 2.0
181 stars 73 forks source link

COMMIT_TEMPLATE env var not used #23

Closed proskehy closed 4 years ago

proskehy commented 5 years ago

When trying to use fluxcloud notifications with a private bitbucket-based repository, the COMMIT_TEMPLATE does not seem to do anything.

From the kubectl describe pod command:

Environment:
  ...
  COMMIT_TEMPLATE:   {{ .VCSLink }}/commits/{{ .Commit }}

Resulting link in the notification (with placeholder words for the domain and project/repo): http://git.private.com/projects/myproject/repos/my-flux-repo/commit/sha

Edit: not sure if I should create a separate issue for that, but when I change the chart in my flux repo, no notification is sent, though the event can be seen in the fluxcloud logs, is that an intentional behaviour?

jbarrick-mesosphere commented 5 years ago

Which version of fluxcloud are you running?

proskehy commented 5 years ago

I took the yaml file from the examples folder, so that should be fluxcloud:v0.3.0.

proskehy commented 5 years ago

@jbarrick-mesosphere Update: the template IS actually used. I was just looking for it in the wrong place.

I took a proper look at the JSON structure representing the notification and the template is applied in the title_link field, but not in the text field.

Looking at the code, I see that the issue lies in the hardcoded link format in the body_template. I see that it is customizable, so I guess I could just replace the whole body_template with the correct link format, but I was thinking it might be more intuitive that the commit_template change somehow applies into the default body_template too.

However, I wasn't able to figure out a smart way of incorporating this into the code (I've never used Go before, so I'm probably not the best person to be looking into that, but I wanted to try). Do you have any ideas on how this can be achieved?

Edit: After thinking about it, I guess that just mentioning the need to edit the body_template as well in the readme would be enough :)

ylohnitram commented 5 years ago

fluxcloud v0.3.6

$ grep -HRi '/commit' * | grep -v '\.md:' chart/templates/deployment.yaml: value: "{{ .VCSLink }}/commits/{{ .Commit }}" chart/values.yaml: commitTemplate: "{{ .VCSLink }}/commits/{{ .Commit }}"

I would expect then see in the logs commits instead of commit, however this is the result: $ klf -c fluxcloud flux-5c697f999f-t9s2z | grep -oE 'https://.*/commit.*/.*' | awk -F '/' '{ print $8 }' | sort -u commit

fluxcloud logs output example: ` Request for:/v6/events {"id":0,"serviceIDs":["default:helmrelease/deployment-test-container-crash"],"type":"sync","startedAt":"2019-05-23T14:20:14.40360327Z","endedAt":"2019-05-23T14:20:14.40360327Z","logLevel":"info","metadata":{"commits":[{"revision":"eb1184c765f590ae3a7ac24b273f42e20821e19f","message":"test fluxcloud commit template"}],"includes":{"other":true}}} {#weave-flux-notif :ghost: Flux Operator [{#4286f4 Applied flux changes to cluster ###HIDDEN_URL###/commits/eb1184c765f590ae3a7ac24b273f42e20821e19f Event: Sync: eb1184c, default:helmrelease/deployment-test-container-crash Commits:

Resources updated:

jseiser commented 4 years ago

Same issue, here. The Link to the commit in the title is correct, but not in the body.

`Event: {{ .EventString }} {{ if and (ne .EventType "commit") (gt (len .Commits) 0) }}Commits: {{ range .Commits }}

Is it possible to overwrite this from the helm chart?

proskehy commented 4 years ago

@jseiser how are you deploying with Helm?

I'm using the deployment from examples/fluxcloud.yaml and I just modified it by adding the env var:

        - name: BODY_TEMPLATE
          value: |
            Event: {{ .EventString }}
            {{ if and (ne .EventType "commit") (gt (len .Commits) 0) }}Commits:
            {{ range .Commits }}
            * {{ call $.FormatLink (print $.VCSLink "/commits/" .Revision) (truncate .Revision 7) }}: {{ .Message }}
            {{end}}{{end}}
            {{ if (gt (len .EventServiceIDs) 0) }}Resources updated:
            {{ range .EventServiceIDs }}
            * {{ . }}
            {{ end }}{{ end }}
            {{ if gt (len .Errors) 0 }}Errors:
            {{ range .Errors }}
            Resource {{ .ID }}, file: {{ .Path }}:

            > {{ .Error }}
            {{ end }}{{ end }}
jseiser commented 4 years ago

@proskehy

This fixed it, I wasnt doing the value: | correctly.

oliviabarrick commented 4 years ago

Are there any remaining issues here or should we close this?

proskehy commented 4 years ago

@jseiser glad to hear that! :) @justinbarrick I think we can close this (mentioning this in the readme would probably be a good idea)

oliviabarrick commented 4 years ago

Agreed. Always happy to approve pull requests.