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

Passing custom template as env var or parameter causes JSON error #219

Closed
chrisbruford opened this issue Dec 17, 2020 · 9 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@chrisbruford
Copy link

chrisbruford commented Dec 17, 2020

Orb version: 4.1.3

What happened:

Passing valid JSON directly into the custom parameter works as expected; however when putting said JSON into an environment variable or parameter it causes a parse error.

I've tried:

Passing the JSON directly ✅
Passing the JSON as an env var ❌
Passing the JSON as a string parameter ❌
Passing the JSON as an env var parameter ❌

Here's how I'm setting and calling (the node call returns the JSON in question):

- run:
    name: Generate slack failure message
    when: always
    command: |
      echo "export SLACK_FAILURE_MSG='`node repo/slack-on-fail $CIRCLE_BUILD_URL`'" >> $BASH_ENV
- slack-custom:
    event: always
    template: SLACK_FAILURE_MSG

Here's the command I've setup:

slack-custom:
    parameters:
      template:
        type: env_var_name
        default: SLACK_FAILURE_MSG
      event:
        type: string
        default: always
    steps:
      - run:
          name: Show template
          command: |
            echo ${<<parameters.template>>}
      - slack/notify:
          custom: ${<<parameters.template>>}
          event: "<<parameters.event>>"

The "Show template" step runs as expected and outputs the valid JSON

here's the output from the slack/notify step:

/bin/bash: warning: setlocale: LC_ALL: cannot change locale (***********)
It appears you have a Slack Webhook token present in this job.
Please note, Webhooks are no longer used for the Slack Orb (v4 +).
Follow the setup guide available in the wiki: https://github.com/CircleCI-Public/slack-orb/wiki/Setup
Posting Status
Checking For JQ + CURL: Debian
34 packages can be upgraded. Run 'apt list --upgradable' to see them.
The following additional packages will be installed:
  libjq1 libonig5
The following NEW packages will be installed:
  jq libjq1 libonig5
0 upgraded, 3 newly installed, 0 to remove and 34 not upgraded.
Need to get 313 kB of archives.
After this operation, 1062 kB of additional disk space will be used.
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = "***********",
	LANG = "***********"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
debconf: delaying package configuration, since apt-utils is not installed
Selecting previously unselected package libonig5:amd64.
(Reading database ... 
(Reading database ... 5%
(Reading database ... 10%
(Reading database ... 15%
(Reading database ... 20%
(Reading database ... 25%
(Reading database ... 30%
(Reading database ... 35%
(Reading database ... 40%
(Reading database ... 45%
(Reading database ... 50%
(Reading database ... 55%
(Reading database ... 60%
(Reading database ... 65%
(Reading database ... 70%
(Reading database ... 75%
(Reading database ... 80%
(Reading database ... 85%
(Reading database ... 90%
(Reading database ... 95%
(Reading database ... 100%
(Reading database ... 58731 files and directories currently installed.)
Preparing to unpack .../libonig5_6.9.4-1_amd64.deb ...
Unpacking libonig5:amd64 (6.9.4-1) ...
Selecting previously unselected package libjq1:amd64.
Preparing to unpack .../libjq1_1.6-1_amd64.deb ...
Unpacking libjq1:amd64 (1.6-1) ...
Selecting previously unselected package jq.
Preparing to unpack .../archives/jq_1.6-1_amd64.deb ...
Unpacking jq (1.6-1) ...
Setting up libonig5:amd64 (6.9.4-1) ...
Setting up libjq1:amd64 (1.6-1) ...
Setting up jq (1.6-1) ...
Processing triggers for libc-bin (2.31-0ubuntu9.1) ...
parse error: Invalid numeric literal at line 1, column 2
parse error: Invalid numeric literal at line 1, column 2

Exited with code exit status 4

Expected behavior:

The slack/notify command should be able to receive env vars and params in the custom parameter

@chrisbruford chrisbruford added the bug Something isn't working label Dec 17, 2020
@gmemstr
Copy link

gmemstr commented Dec 18, 2020

Are you able to show a small snippet from the

command: |
  echo ${<<parameters.template>>}

step? I'm curious what the JSON being echo'd to the environment variable looks like.

@chrisbruford
Copy link
Author

Sure, this is what I was using for the POC:

{“blocks”:[{“type”:“section”,“text”:{“type”:“mrkdwn”,“text”:“:warning: A job has failed”}},{“type”:“divider”},{“type”:“divider”},{“type”:“actions”,“elements”:[{“type”:“button”,“text”:{“type”:“plain_text”,“text”:“Visit job”},“value”:“visit_job”,“url”:“https://circleci.com/foobar“}]}]}

@KyleTryon
Copy link
Contributor

@chrisbruford chrisbruford

The reason passing the text in directly would work but an env var would not, is because of how the parameter is utilized as an env var. It is expecting a string and not evaluating the environment variable.

We inject the parameter as an environment variable:
https://github.com/CircleCI-Public/slack-orb/blob/master/src/commands/notify.yml#L63

SLACK_PARAM_CUSTOM: "<<parameters.custom>>"

This parameter is expecting a string. In order to accept an environment variable, we would need to first evaluate the value in $SLACK_PARAM_CUSTOM, this could potentially have unintended consequences for the string variant that it is currently designed for.

I believe however the use-case you are attempting to solve for can be achieved with what we are calling "dynamic templates".
https://github.com/CircleCI-Public/slack-orb/wiki/Dynamic-Templates

Could you tell me if this would work?

@chrisbruford
Copy link
Author

Thanks @KyleTryon Dynamic Templates worked perfectly - barely any change required at all; just use the env var param directly in template instead of custom

I wonder if the naming or documentation could be tidied up a bit to better signpost the differences between the two? to me "custom" is really a string and "template" is really an env var name - it wasn't clear until I started digging around in your code that I couldn't do what I wanted to do with custom - and then it felt like a bug rather than a feature/limitation of the implementation?

Just my 2 cents - great orb, and thanks for pointing me in the right direction!

@xavidop
Copy link

xavidop commented Jan 28, 2021

In my case doesn't work, I had to download the notify script, export all required variables and call that script in the same step.

@xavidop
Copy link

xavidop commented Jan 29, 2021

Also if you have a json template that contains a " which is a double quote scaped in a json file. The slack files becase there is one step that replaces all the " to " so it my double quotes scpaed turns into \". The you replace all " to " again, so the final output of my double quote scaped truns into just a \

@schellj
Copy link

schellj commented Jun 11, 2021

Also if you have a json template that contains a " which is a double quote scaped in a json file. The slack files becase there is one step that replaces all the " to " so it my double quotes scpaed turns into ". The you replace all " to " again, so the final output of my double quote scaped truns into just a \

I also ran into this issue (i.g., I'm including the git commit message in the Slack template and messages with double quotes would cause the Slack notification to fail to send) and ended up having to just strip out double quotes from the content.

@EricRibeiro
Copy link
Contributor

Hi @chrisbruford, I'm glad that the dynamic templates worked for you! I agree that the documentation could be more explicit about the difference between the template and custom parameters. I'm including this change in #319, along with a couple of helpful links.

@xavidop Are you still having this issue? If so, could you please post a snippet of your template? It sounds like @schellj's solution might work for you too.

@EricRibeiro
Copy link
Contributor

I'll close this issue due to staleness.

@xavidop, please open another issue if you are still having trouble with your template.

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

No branches or pull requests

6 participants