cloudfoundry-community / slack-notification-resource

Concourse CI resource for sending notifications to Slack.
MIT License
75 stars 82 forks source link

Interpolation no longer supported in v1.4.0 #49

Closed iswix-llc closed 3 years ago

iswix-llc commented 6 years ago

In my pipeline I reference v$(cat version/number) and it's worked until today. I changed my resource tag to v1.3.1 and it started working.

Expected Result: v1.0.0 (some SEMVER)

Broken Result: v$(cat version/number)

drnic commented 6 years ago

Investigating.

drnic commented 6 years ago

It looks like https://github.com/cloudfoundry-community/slack-notification-resource/commit/7999ada4d62a1c665a076722691cf662ad897f25 removed support for this.

/cc @tprobinson @jhunt

@iswix-llc I've retitled this issue

iswix-llc commented 6 years ago

Does that imply intentionally or accidentally?

drnic commented 6 years ago

@iswix-llc I intentionally retitled the issue to reflect what I understand is the nature of the problem for @tprobinson @jhunt when they get notified and come see the issue. If I've got the title wrong, then perhaps I've misunderstood your bug.

Perhaps it will also help if you paste in a sample of your pipeline.yml job that used to work but now does.

iswix-llc commented 6 years ago

I'm sorry. I meant is it intentional for interpolation to no longer be supported or is it accidental and the support will be returning?

My most trimmed down / redacted sample would be:

resource_types:
  - name: slack-notification
    type: docker-image
    source:
      repository: cfcommunity/slack-notification-resource
      tag: v1.3.1
resources:
jobs:       
  - name: build
    on_success:
      do:    
        - put: slack
          params:
            channel: "((slackchannel))"
            username: {{cicd-github-username}}
            icon_url: "http://icons.iconarchive.com/icons/zyotism/2001-space-odyssey/128/HAL-9000-icon.png"
            attachments: [{"color": "good", "text": "$BUILD_PIPELINE_NAME v$(cat version/number).0 succeeded! (<$ATC_EXTERNAL_URL/teams/main/pipelines/$BUILD_PIPELINE_NAME/jobs/build/builds/$BUILD_NAME|Concourse #$BUILD_NAME>"} ]
drnic commented 6 years ago

@iswix-llc I personally don't know. I was just trying to help by cutting v1.4.0. I assume for the immediate future it won't come back? Or there is some other way to interpolate?

The commit title was "Remove double attachments interpolation" - does that mean there is some other path to interpolation for attachments? /cc @tprobinson @jhunt

tprobinson commented 6 years ago

Sorry for my late reply.

@drnic In reference to 7999ada, it appeared interpolation was already performed on the attachments, and the block that I removed (from my perspective) appeared to do the same thing twice and break escape sequences while it did so.

Is "interpolation" in this case supposed to be full Bash eval? I can't find any reference to that in this resource's documents, only that it accepts environment variables and Concourse variables. If so, I can take a closer look at this and try to re-create the original intention while preserving escapes.

drnic commented 6 years ago

@tprobinson I'm only the person who cut v1.4.0 assuming all the commits on HEAD were ok to go out. @iswix-llc had a use case shown above that used to work in v1.3.1 but is now broken. Suggestions on path forward?

tprobinson commented 6 years ago

In a case like this I would be inclined to question whether the use case is supposed to be valid-- as in, whether the ability to eval commands is intended, considering that it's not listed in the documentation and I can't figure out what part of the code is supposed to be doing that. I would probably suggest a middle ground feature that regulates this ability by providing a new parameter like vars_from_commands that populates variables for this. However since I'm not an actual project maintainer and am the cause of this breakage, I'll see if I can figure out how to restore this ability without breaking escape sequences.

drnic commented 6 years ago

I'd argue this project doesn't have "maintainer" per se. Your offer to help would be awesome.

iswix-llc commented 6 years ago

I tracked down the commit and related issue 27. It didn't specifically say bash interpolation but it supported it. It's been in there since Feb 2017 so removing it would break backwards compatibility.

During my process of learning Concourse I'm not sure how it is that I came to use this. Mabe it was a tutorial or perhaps it just kinda came to me and worked so I kept on going.

tprobinson commented 6 years ago

The commit in question looks to be 1064cb0.

iswix-llc commented 6 years ago

It looked like https://github.com/cloudfoundry-community/slack-notification-resource/commit/b05bea91e5f03573eeef50db42093ad9df64f0ce#diff-c68271a63ddbc431c307beb7d2918275 to me.