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

Support for Organizational-Level Action Variables in Payload Parsing #330

Open
4 of 13 tasks
hemupadhyay26 opened this issue Aug 16, 2024 · 6 comments
Open
4 of 13 tasks
Labels
needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info question Further information is requested

Comments

@hemupadhyay26
Copy link

hemupadhyay26 commented Aug 16, 2024

Description

We are currently managing our GitHub Action payloads at the organizational level. However, we've encountered an issue where the action variables are not being correctly parsed or passed when generating the payload. This results in the payload not being fully supported, leading to errors when actions run.

To address this, we need to ensure that any payload created using organizational-level action variables is fail-safe and parsed correctly when received.


Problem Statement

  1. Organizational Action Variables: We aim to define and use action variables at the organizational level, but the variables are not being correctly processed when generating payloads.

  2. Payload Parsing: The action payload does not fully parse when passed, leading to incomplete or incorrect data being sent.


Proposed Solution

  • Implement logic to re-parse the payload upon receiving it, ensuring it fully supports organizational-level variables.
  • Add fail-safe checks to verify that the payload structure is correct and contains all necessary data before processing.

Acceptance Criteria

  • Organizational-level action variables are supported and correctly parsed into the payload.
  • The action can successfully process the payload without errors, ensuring data integrity.
  • Implement validation checks to ensure payload structure is complete and correct before processing.

Additional Context

This enhancement will improve the robustness of our CI/CD pipeline by ensuring that organizational-level configurations work seamlessly with GitHub Actions.

This format should make it clear and actionable for the development team!

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • example code related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • 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.
  • I've searched for any related issues and avoided creating a duplicate issue.

Screenshot 2024-08-16 204623

@zimeg zimeg added question Further information is requested needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info labels Aug 16, 2024
@zimeg
Copy link
Member

zimeg commented Aug 16, 2024

Hey @hemupadhyay26 👋 At the moment it's unclear why this might be happening, but I do agree that variables stored at an organization level should be used in payloads!

Can you share the .github/workflows implementation you're using for this? I'm most curious why just the 3rd message in the above image has data but the others remain templated... 🤔

Marking this as a question for now, but if we're finding that this should be working or needs additional implementation we can change it to a bug or enhancement!

@hemupadhyay26
Copy link
Author

Screenshot 2024-08-16 204623
Hey @zimeg it is the actual message previously I have attached is screenshot of multiple case that I was trying

name: deploy

on:
  workflow_dispatch:
  push:
    branches:
      - 'dev'

concurrency:
  group: ${{ github.workflow }}
  cancel-in-progress: true


jobs:
  deploy:
    runs-on: ubuntu-latest
    timeout-minutes: 5
  
    steps:
    - name: Checkout repository.
      uses: actions/checkout@v4      
    - name: Post to a Slack channel
      if: always() && github.event_name == 'push'
      id: slack
      uses: slackapi/slack-github-action@v1.26.0
      env:
        SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
        SLACK_COLOR: ${{ job.status == 'success' && '#36a64f' || '#FF0000' }}  # Ternary operator to set color based on job status
        JOB_STATUS: ${{ job.status }}
      with:    
        channel-id: '...'    
        payload: ${{vars.DETAIL_SLACK_NOTIFICATION}}

This is the scenario I was using to run my workflow

@zimeg
Copy link
Member

zimeg commented Aug 26, 2024

@hemupadhyay26 Thanks for sharing this one! Something in the workflow is standing out to me that might be causing this, along with how templated variables are replaced in workflows 👀

When using the payload parameter no additional replacements happen for templated variables. The one replacement that is happening happens when GitHub starts the step. AFAIK, all templated string values are replaced with matching variables inline and before calling slackapi/slack-github-action.

Using a similar example, that means the following workflow file input:

    - name: Post to a Slack channel
      uses: slackapi/slack-github-action
      env:
        SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
      with:
        payload: ${{ vars.DETAIL_SLACK_NOTIFICATION }}

Would expand to something like:

    - name: Post to a Slack channel
      uses: slackapi/slack-github-action
      env:
        SLACK_BOT_TOKEN: "xoxb-example"
      with:
        payload: "GitHub Action build result: ${{ job.status }}"

Which would then send that payload text as is!

📣 To instead wait for this replacement to happen from within the action, you'll have to use payload-file-path.

This might not be an ideal workaround, but I believe you can write a temporary file using the same vars.DETAIL_SLACK_NOTIFICATION value, then use that as input for this action? I'm hoping organization environment variables work just the same in this case, but please let me know if this is causing similar issues!

@hemupadhyay26
Copy link
Author

@zimeg I have tried to make the file in the workflow using the ${{ vars.DETAIL_SLACK_NOTIFICATION }} content and pass it to the payload-file-path it works fine but it makes my repository to be edited in runtime so if we could have no interaction with repository data so it will be more reliable

@hemupadhyay26
Copy link
Author

hemupadhyay26 commented Sep 1, 2024

I think we could achieve by adding the parsing when we get data through the payload as well so we could resolve that issue

@zimeg
Copy link
Member

zimeg commented Sep 1, 2024

@hemupadhyay26 Great to know this is working as expected with payload-file-path parsed - thanks for confirming!

I agree that the runtime edits to the repository are not be ideal, but I'm wondering if another way exists to parse these variables in a payload 🤔

📚 Some background context

We've found strangeness in the parsing logic from within the action code where "...only values of the github.context and the step env can be retrieved..." (#203) that can sometimes make attempting these replacements less than ideal.

An option to skip these replacements for files was added with the payload-file-path-parsed parameter in #299. This defaults to true to remain compatible with existing behaviors, but is changing to false with current @v2 plans.

⚙️ Some thoughts

At the moment I'm hesitant to expand what we're parsing since this has caused confusion before, but now could be a good time to revisit the scope of what payload-file-path-parsed is! Perhaps it could work for a more general payload as payload-templated or something similar?

One quick concern for this would be a meaningless false value when defining templated YAML inline... Those values will be replaced by the workflow, so a payload-templated: false value might cause even more confusion 😬

Edit: This input could be string though! We could do additional parsing if it's true and nothing when it's empty? 👀 I'm thinking this could work if we error if false is provided also:

- uses: slackapi/slack-github-action
  with:
    payload: ${{ vars.DETAIL_SLACK_NOTIFICATION }}
    payload-templated: true

I'm open to continuing this discussion here (though my responses will lag these next few weeks), but believe this could be related to #312 🚀

Before considering those wider changes, I'm also wondering if you can share more about DETAIL_SLACK_NOTIFICATION? Is this a value that is changing between runs? I haven't yet seen the dynamic payload which is making this interesting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants