Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed to be able to use env of Step #159

Closed
wants to merge 1 commit into from

Conversation

hnarimiya
Copy link
Contributor

Summary

Hello, thanks for this useful software !

I modified the env defined in step to be available in file.

Also, the process of replacing $ was strange, so I fixed it.

In addition, notes on using files were added to the README.

issues: #84

Requirements (place an x in each [ ])

@CLAassistant
Copy link

CLAassistant commented Dec 21, 2022

CLA assistant check
All committers have signed the CLA.

@WilliamBergamin WilliamBergamin added the bug Something isn't working label Dec 21, 2022
@WilliamBergamin WilliamBergamin added this to the 1.24 milestone Dec 21, 2022
@WilliamBergamin
Copy link
Contributor

Hi @hnarimiya thanks for the contribution 💯

I'm currently working on a work around for these integration test, will wait on others approval before merging this

@dale-satvu
Copy link

dale-satvu commented Feb 10, 2023

What's the status of this? Would be good to get this fix merged :)

Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like these changes render properly, at least based on the integration tests we ran with these changes. Any idea why?

@@ -1,13 +1,31 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the fields in this payload are not showing up correctly. Attached is a screenshot of the GitHub CI run from #163 (which is a copy of this PR), and how it looks like in our Slack workspace where the message gets posted:

Screenshot 2023-02-28 at 3 32 26 PM

In particular, it seems the following fields do not work as desired from the integration_test_file_payload test from main.yml: env.JOB_STATUS, github.payload.head_commit.url, github.payload.head_commit.author.name, github.payload.head_commit.message.

Any idea why this might be happening?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same seems to happen for us

@filmaj
Copy link
Contributor

filmaj commented May 17, 2023

We integrated this fix in #200, will release shortly.

@filmaj filmaj closed this May 17, 2023
@Darwiner
Copy link

Using v1.24.0, I just tried switching to using payload-file-path: and it seems to still not be working properly.

The rendering works fine using payload:.

Anyone else?

image

@filmaj
Copy link
Contributor

filmaj commented May 25, 2023

@Darwiner what event tripped the action and what does the file look like? We found in our testing that different events have different shapes and so those details matter.

@Darwiner
Copy link

@Darwiner what event tripped the action and what does the file look like? We found in our testing that different events have different shapes and so those details matter.

This was triggered via a PR open/synchronize, the top of the workflow file being:

on:
  workflow_dispatch:
  pull_request:
    types:
      - opened
      - synchronize
    branches:
      - main
      - master

As for the json file, it goes like this:

{
  "text": "GitHub Actions - Slack - Deploy Success",
  "blocks": [
    {
      "type": "section",
      "text": {
        "type": "mrkdwn",
        "text": ":rocket: *DEPLOY* - <${{ github.event.repository.html_url }}|${{ steps.vars.outputs.app }}>"
      }
    },
    {
      "type": "divider"
    },
    {
      "type": "section",
      "fields": [
        {
          "type": "mrkdwn",
          "text": "*Cluster* - ${{ steps.vars.outputs.gcp-project }} / ${{ inputs.cluster-location }} / ${{ inputs.cluster-name }}"
        },
        {
          "type": "mrkdwn",
          "text": "*Env* - ${{ steps.vars.outputs.env }}"
        },
        {
          "type": "mrkdwn",
          "text": "*Release* - ${{ steps.vars.outputs.release-version }}"
        },
        {
          "type": "mrkdwn",
          "text": "*PR* - <${{ github.event.pull_request.html_url || github.event.head_commit.url }}|${{ github.event.pull_request.title || github.event.head_commit.message }}>"
        },
        {
          "type": "mrkdwn",
          "text": "*Workflow* - <${{ github.event.repository.html_url }}/actions/runs/${{ github.run_id }}|Logs>"
        }
      ]
    },
    {
      "type": "divider"
    },
    {
      "type": "section",
      "text": {
        "type": "mrkdwn",
        "text": "*STATUS* - ${{ github.action_status }} :white_check_mark:"
      }
    }
  ]
}

Additionally, another piece of info that might or might not be relevant is that this is triggered via a composite action.

The action in the composite action being:

    - id: slack-prod-success
      uses: slackapi/[email protected]
      with:
        channel-id: ${{ inputs.slack-success-channel }}
        payload-file-path: ${{ github.action_path }}/payload-slack-success.json
      env:
        SLACK_BOT_TOKEN: ${{ inputs.slack-bot-token }}

@rd-florian-stagliano
Copy link

we have the same problem. the 'fix' didn't the rendering when you use a payload json file. We also see the github env variables rendered with '?'

@filmaj
Copy link
Contributor

filmaj commented May 27, 2023

I've filed #203 to follow-up more deeply.

We use this Action in this action's intergration tests, and they render correctly. So something weird is going on.

Will investigate more in #203.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants