allow to use json file as is without replacing/parsing anything by talgendler · Pull Request #299 · slackapi/slack-github-action

talgendler · 2024-03-31T13:32:34Z

Summary

Allows to treat payload within a file unchanged, no replacing {{ similar to how one would specify payload with all the slack blocks etc but using template strings to be parsed by github itself.

Requirements (place an `x` in each `[ ]`)

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

zimeg · 2024-04-02T17:48:24Z

Hey @talgendler 👋 Could you share a bit more about your use case? It's my understanding that payload-file-path is an alternative to payload and that these should behave similarly in regards to variable replacements.

talgendler · 2024-04-03T06:17:51Z

Hi @zimeg, you right, it works when you replace template variables with the values from github context. It doesn't replace values for functions or any boolean logic one might have like ${{ github.action == 'success' && 'Write this' || 'Write that' }}. Some will do all the calculation outside and only pass the calculated values to the file payload instead. For us it makes it much easier to share payloads between multiple repositories.

zimeg · 2024-04-04T20:31:16Z

It doesn't replace values for functions or any boolean logic

This is a great callout and a clever workaround is mentioned in #203 (comment) where more creative expressions are provided as environment variables. I think you're hinting at this already though!

I'm not against the change you're suggesting but I'm still trying to understand how you're planning to use an unparsed payload in your workflow and sharing these between repositories? I was imagining this was an alternative to file uploads (perhaps this is a good future enhancement) but it seems like you'd receive just the JSON payload file in your webhook URL and have to process it within Workflow Builder?

Apologies if I'm misunderstanding something and for the back-and-forth, I just want to make sure I'm grasping the gap this is addressing!

talgendler · 2024-04-05T16:09:18Z

@zimeg you got everything right, our solution is similar to what was suggested here in order to support multiple repositories we ended up creating a composite action which holds files and other shared payloads. This composite action is then used in multiple repositories.

zimeg · 2024-04-09T23:25:50Z

...we ended up creating a composite action which holds files and other shared payloads. This composite action is then used in multiple repositories.

@talgendler Does the composite action output the payload file path name for use in a repository workflow? Or are you calling this action from the composite one? I'm still not certain how you're trying to use the unparsed payload file 🤔

In any case I can see how this could be used from a workflow in Slack so will start reviewing now!

Edit: Also please excuse those "Error" workflows - I canceled the waiting job with hopes it would disappear.

zimeg

📝 Left some feedback on the change and would love your thoughts around the naming of this attribute. Things are looking good already but still left a few nits for tidiness that I think should be addressed. Also, thank you for including a test!

📚 We'll want to document this field and defaults in the README too. Let me know if you want suggestions for this since I can imagine a few different ways to make a note of this, the existing payload-file-path, and other values for Technique 1.

zimeg · 2024-04-09T23:29:01Z

.gitignore

 node_modules
 .DS_Store
 .nyc_output
-dist


This can be kept since a build step is handled by the publish.yaml workflow

zimeg · 2024-04-09T23:30:18Z

action.yml

  payload-file-path: # path to JSON payload to send to Slack via webhook
    description: 'path to JSON payload to send to Slack if webhook route. If not supplied, json from GitHub event will be sent instead. If payload is provided, it will take preference over this field'
    required: false
+  use-file-as-valid-json:


I'm thinking it might be better to use payload-file-path-parsed with a default true value here instead to match the payload-file-path parameter. Open to your thoughts on this though!

zimeg · 2024-04-09T23:30:57Z

src/slack-send.js

 const markup = require('markup-js');
 const { HttpsProxyAgent } = require('https-proxy-agent');
 const { parseURL } = require('whatwg-url');
-


Nit: Let's keep the space between external and internal dependencies.

zimeg · 2024-04-09T23:39:59Z

src/slack-send.js


      try {
        await axios.post(webhookUrl, payload, axiosOpts);
+        console.log(JSON.stringify(payload));


This might not be desired behavior for a successful POST since it could expose values in the action logs. Not so against it, but I'm wondering if there's a strong reason for including it?

zimeg · 2024-04-09T23:41:48Z

src/slack-send.js

        console.log('axios post failed, double check the payload being sent includes the keys Slack expects');
-        console.log(payload);
+        // console.log(payload);
+        console.log(JSON.stringify(payload));


It might be a good time to remove the commented logs and perhaps enhance the outputs in this section with console.error?

Suggested change

console.log(JSON.stringify(payload));

console.error(`Attempted to POST payload: ${JSON.stringify(payload)}`);

test/slack-send-test.js

zimeg · 2024-04-09T23:50:08Z

action.yml

    description: 'path to JSON payload to send to Slack if webhook route. If not supplied, json from GitHub event will be sent instead. If payload is provided, it will take preference over this field'
    required: false
+  use-file-as-valid-json:
+    description: 'Do not try to speically parse github context, markup or other parts. Assume file content is legit'


Depending on the final naming of this attribute we might want to update this to something like "Replace templated variables with the GitHub context and environment variables"

zimeg

Thanks for making these changes! I opened up finmenow#1 to swap the default value of payload-file-path-parsed to be true since we're replacing templated variables by default with this action. To me this matches the meaning of parsed a bit better.

Once those changes are included I'm feeling like this will be in a pretty good place. Of course, please let me know if you think the meaning of the false value shouldn't change!

fix: default to true when parsing and replacing values in payload json files

zimeg

@talgendler thanks a ton for suggesting and implementing this feature! Going to tag a release after merging. 🚀

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | OpenSSF | |---|---|---|---|---| | [slackapi/slack-github-action](https://togithub.com/slackapi/slack-github-action) | action | minor | `v1.25.0` -> `v1.26.0` | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/slackapi/slack-github-action/badge)](https://securityscorecards.dev/viewer/?uri=github.com/slackapi/slack-github-action) | --- ### Release Notes <details> <summary>slackapi/slack-github-action (slackapi/slack-github-action)</summary> ### [`v1.26.0`](https://togithub.com/slackapi/slack-github-action/releases/tag/v1.26.0): Slack Send V1.26.0 [Compare Source](https://togithub.com/slackapi/slack-github-action/compare/v1.25.0...v1.26.0) #### What's Changed This release provides an escape hatch for sending the JSON content of a payload file exactly as is, without replacing any templated variables! Previously a payload file was parsed and templated variables were replaced with values from `github.context` and `github.env`. Any undefined variables were replaced with `???` in this process, which might have caused questions. That remains the default behavior, but now the JSON contents of a payload file can be sent exactly as written by setting the `payload-file-path-parsed` input to `false`: ```yaml - name: Send custom JSON data to Slack workflow id: slack uses: slackapi/slack-github-action@v1.26.0 with: payload-file-path: "./payload-slack-content.json" payload-file-path-parsed: false env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} ``` With this change, the contents of the example `payload-slack-content.json` will be sent to a webhook URL exactly as is! #### Recent commits ##### Enhancements - allow to use json file as is without replacing/parsing anything by [@talgendler](https://togithub.com/talgendler) in [https://github.com/slackapi/slack-github-action/pull/299](https://togithub.com/slackapi/slack-github-action/pull/299) ##### Documentation - docs(readme): adjust whitespace in env assignment by [@paulo9mv](https://togithub.com/paulo9mv) in [https://github.com/slackapi/slack-github-action/pull/296](https://togithub.com/slackapi/slack-github-action/pull/296) ##### Maintenance - ci(test): collect environment secrets from a prepared staging environment by [@zimeg](https://togithub.com/zimeg) in [https://github.com/slackapi/slack-github-action/pull/294](https://togithub.com/slackapi/slack-github-action/pull/294) - ci(test): share environment secrets with pull requests from forked prs by [@zimeg](https://togithub.com/zimeg) in [https://github.com/slackapi/slack-github-action/pull/297](https://togithub.com/slackapi/slack-github-action/pull/297) ##### Dependencies - Bump eslint-plugin-jsdoc from 46.10.1 to 48.2.1 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/295](https://togithub.com/slackapi/slack-github-action/pull/295) - Bump eslint from 8.56.0 to 8.57.0 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/289](https://togithub.com/slackapi/slack-github-action/pull/289) - Bump mocha from 10.2.0 to 10.3.0 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/288](https://togithub.com/slackapi/slack-github-action/pull/288) - Bump https-proxy-agent from 7.0.2 to 7.0.4 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/290](https://togithub.com/slackapi/slack-github-action/pull/290) - Bump [@slack/web-api](https://togithub.com/slack/web-api) from 6.12.0 to 7.0.2 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/287](https://togithub.com/slackapi/slack-github-action/pull/287) - Bump mocha from 10.3.0 to 10.4.0 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/300](https://togithub.com/slackapi/slack-github-action/pull/300) - Bump axios from 1.6.7 to 1.6.8 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/301](https://togithub.com/slackapi/slack-github-action/pull/301) - Bump eslint-plugin-jsdoc from 48.2.1 to 48.2.2 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/slackapi/slack-github-action/pull/302](https://togithub.com/slackapi/slack-github-action/pull/302) #### New Contributors - [@paulo9mv](https://togithub.com/paulo9mv) made their first contribution in [https://github.com/slackapi/slack-github-action/pull/296](https://togithub.com/slackapi/slack-github-action/pull/296) - [@talgendler](https://togithub.com/talgendler) made their first contribution in [https://github.com/slackapi/slack-github-action/pull/299](https://togithub.com/slackapi/slack-github-action/pull/299) **Full Changelog**: slackapi/slack-github-action@v1.25.0...v1.26.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 10pm every weekday,before 5am every weekday,every weekend" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/settlemint/btp-docs).  Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

allow to use json file as is without replacing/parsing anything

e2528c8

salesforce-cla bot added the cla:signed label Mar 31, 2024

talgendler had a problem deploying to staging March 31, 2024 13:32 — with GitHub Actions Failure

expose dist folder for internal use

c11b8cd

talgendler had a problem deploying to staging March 31, 2024 13:34 — with GitHub Actions Failure

debug errors

f0479f5

talgendler had a problem deploying to staging March 31, 2024 13:44 — with GitHub Actions Error

debug errors v2

9abceb0

talgendler temporarily deployed to staging March 31, 2024 14:00 — with GitHub Actions Inactive

zimeg added enhancement New feature or request semver:minor labels Apr 2, 2024

zimeg reviewed Apr 9, 2024

View reviewed changes

Merge branch 'slackapi:main' into main

1374271

talgendler had a problem deploying to staging April 10, 2024 09:04 — with GitHub Actions Failure

talgendler had a problem deploying to staging April 10, 2024 09:05 — with GitHub Actions Failure

requested fixes in slackapi#299 PR

d6f3bad

talgendler had a problem deploying to staging April 10, 2024 09:49 — with GitHub Actions Failure

talgendler requested a review from zimeg April 11, 2024 14:48

fix: default to parsing and replacing payload json files

7fa92a6

zimeg added this to the 1.26 milestone Apr 12, 2024

zimeg reviewed Apr 12, 2024

View reviewed changes

Merge pull request #1 from zimeg/fix-payload-file-path-parsed-default

84038e9

fix: default to true when parsing and replacing values in payload json files

talgendler temporarily deployed to staging April 17, 2024 08:59 — with GitHub Actions Inactive

zimeg approved these changes Apr 19, 2024

View reviewed changes

zimeg merged commit 47d8e42 into slackapi:main Apr 19, 2024

zimeg mentioned this pull request Sep 1, 2024

Support parsing and replacement of templated variables in input payloads #330

Closed

13 tasks

	console.log(JSON.stringify(payload));
	console.error(`Attempted to POST payload: ${JSON.stringify(payload)}`);

Conversation

talgendler commented Mar 31, 2024

Summary

Requirements (place an x in each [ ])

Uh oh!

zimeg commented Apr 2, 2024

Uh oh!

talgendler commented Apr 3, 2024

Uh oh!

zimeg commented Apr 4, 2024

Uh oh!

talgendler commented Apr 5, 2024

Uh oh!

zimeg commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

zimeg Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

zimeg Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

zimeg Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

zimeg Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zimeg Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Requirements (place an `x` in each `[ ]`)

zimeg commented Apr 9, 2024 •

edited

Loading