slackapi / deno-slack-sdk

SDK for building Run on Slack apps using Deno
https://api.slack.com/automation
MIT License
162 stars 27 forks source link

[BUG] Adding a Outgoing domain requires reinstall #214

Open tomas-zijdemans-vipps opened 1 year ago

tomas-zijdemans-vipps commented 1 year ago

The deno-slack versions "deno-slack-sdk/": "https://deno.land/x/deno_slack_sdk@2.2.0/", "deno-slack-api/": "https://deno.land/x/deno_slack_api@2.1.1/",

Deno runtime version deno 1.36.4 (release, aarch64-apple-darwin)

OS info ProductName: macOS ProductVersion: 13.5.2 BuildVersion: 22G91 Darwin Kernel Version 22.6.0

Describe the bug My app has been live for some time with an integration to Salesforce (using outgoing domain). I now wrote a new integration for Marketing Cloud, adding the outgoing domain as needed. That worked fine using 'Slack Run', but when using 'Slack deploy' I would get an error error: 'Requires net access to "mcm....." After scratching my head a lot and testing a wide array of tests - the simple solution was to do a full uninstall 'Slack add delete" and then "Slack deploy" 🀯 Until this bug can be fixed (maybe in the CLI?), I suggest the documentation is updated so others save time πŸ˜„ (it's also quite inconvenient to have to delete and then reinstall, since all triggers are lost and needs to be re-added. Additionally I need to add the App to all relevant channels +++)

filmaj commented 1 year ago

Hey thanks for filing! We will take a look at this.. that definitely seems like odd behaviour and should not happen so my gut tells me this is a bug (possibly with one of the server-side APIs the CLI interacts with during slack deploy)

WilliamBergamin commented 1 year ago

@filmaj I was able to partially reproduce this behavior, I was eventually able to get the app to work after running slack deploy a few more times

Steps:

  1. Create a new app with slack create (I used the hello world sample)
  2. Deploy the app using slack deploy
  3. Test out that the app works
  4. Add the following in the SlackFunction
    const resp = await fetch("https://www.google.com/");
    console.log(resp.status);
  5. Run slack deploy again
  6. Test the workflow -- the function should fail with the following activity (slack activity -t)
    Caught error from user supplied module: PermissionDenied: Detected missing network permissions; add the domain to your manifest's `outgoingDomains`. Original message: Requires net access to "www.google.com", run again with the --allow-net flag
  7. Add outgoingDomains: ["www.google.com"], in the manifest
  8. Run slack deploy again
  9. Test the workflow -- the function seemed to fail with no error (slack activity -t)
    
    2023-09-11 11:37:06 [info] [Wf] (Trace=Tr) Workflow step 2 of 3 started
    2023-09-11 11:37:06 [info] [Fn] (Trace=Tr) Function 'Generate a greeting' (app function) started
    2023-09-11 11:37:08 [info] [Fn] (Trace=Tr) Function output:

2023-09-11 11:37:08 [error] [Fn] (Trace=Tr) Function 'Generate a greeting' (app function) failed event_dispatch_failed



NOTE:
This seems to be inconsistent behavior, I was able to get the app to work properly with the `fetch` call by executing `slack deploy` a few times more after following these steps, will ask others to reproduce this

@tomas-zijdemans-vipps could you share what version of the cli was used to deploy the app? (output of `slack version`)
WilliamBergamin commented 1 year ago

This was tested by others, adding the outgoing_domains in the manifest.ts seemed to resolve their issue

tomas-zijdemans-vipps commented 1 year ago

slack version gives "Using slack v2.9.0"

brizandrew commented 1 year ago

We're also running into this problem. Our app works when we run slack run without issue, but once we try to deploy we get errors saying: Requires net access to "www.xx-example-xx.com. I tried deploying a few more times to no avail. Unfortunately, uninstalling the app is not a potential solution for us as we depend on the existing deployment for critical tasks.

Is there any update on confirming this bug? Is there a way to check the deployed manifest file to validate it? I've tried slack manifest info, but that looks to be pulling from my local file.

We're using slack version v2.9.0.

filmaj commented 1 year ago

@brizandrew slack manifest validate will validate your manifest against the latest live production schema, but that gets implicitly done when running many app-related commands such as run and deploy among others.

Going to look into this to see if I can reproduce tomorrow.

filmaj commented 1 year ago

Oh, do either of your workspaces have admin controls enabled? That is, do your applications require admin approval upon updating/deploying? Wondering if that has something to do with this.

Also, if you are OK sharing your app IDs with me (I am @filmaj on our community.slack.com instance), and you can give me a rough timeframe for when you saw such issues happen, I can work with the deploy infra team to see if we can find any hints as to what is going on that way.

tomas-zijdemans-vipps commented 1 year ago

We do require admin approval upon deploying new apps, but since I'm an Admin - I do not need that.

Will try to get into community.slack.com :)

brizandrew commented 1 year ago

I'm also an admin so I don't approve my own deployments, but others in my workspace do. Just sent you my app id on slack community.

Thanks for looking into it!

filmaj commented 1 year ago

One more question @brizandrew and @tomas-zijdemans-vipps : are your workspaces on an Enterprise or Grid plan? That is, is it an 'org' with multiple sub-workspaces?

tomas-zijdemans-vipps commented 1 year ago

We are on Enterprise Select. No grid or multiple workspaces

filmaj commented 1 year ago

Hmm, I tested on a business workspace (not enterprise), with a separate account that is part of the Workspace Admin group, and this admin group has app-management permissions. App approval is set up to be required. My steps were:

  1. as the Admin, deploy a new app. This app has no outgoingDomains but its custom function was set up to fetch from google.com. slack deploy worked fine for this app with this user as the Admin app management permissions allowed for deploying without approval. The function fails as expected since the domain is missing from outgoingDomains.
  2. added the relevant domain to the manifest, re-deployed the app.
  3. custom function runs as expected.

Going to try a few other combinations but I'm finding this one hard to reproduce :/

filmaj commented 1 year ago

Update here: we are able to reproduce the core of this bug, I think:

We have a bug somewhere on our backend - we are investigating / addressing. Thanks for the reports and patience as we work through this!

brizandrew commented 1 year ago

Wanted to add I tried this workaround and am still getting the same error. Here's what we tried:

Thanks for checking this out. Happy to help debug as we have new theories.

filmaj commented 1 year ago

OK I am here to report another data point that may be a cause for confusion here: after someone slack deploys their app, and asks for admin approval, and the admin approves the app, the dev must redeploy their app. That is, any code or app manifest changes (including outgoingDomains) that were part of a deploy that hit the 'admin approval required' wall did not make it to Slack's backend. Meaning, any such changes post-approval need to be redeployed.

That doesn't address the case where the person running slack deploy is also the admin (and thus should have implicit approval to deploy at will). I will keep digging on this.

filmaj commented 1 year ago

Another update: we have a fix for the situation where the user running slack deploy is also the workspace admin ready and it is being rolled out to production as I type this. It is my end-of-work-day, though, but first thing tomorrow in my morning I will double-check that everything is working and update this issue.

We are also working on some instruction updates to both the CLI as well as Slackbot messaging in the case where the user deploying the app is not an admin. We are tweaking the messaging/instructions to make clear that non-admins need to re-deploy the app after an admin approval.

filmaj commented 1 year ago

In my testing, as an admin in an enterprise workspace with app admin approval enabled, this now works.

Does it work for you as well now?

sethetter commented 11 months ago

πŸ‘‹ I'm running into this issue! I am the workspace admin. I installed and deployed the app. I deployed the function and workflow first without adding the domain to outgoingDomains, then made the change and redeployed. Can't seem to get past the error though.

Any workarounds I can do here would be appreciated!

filmaj commented 11 months ago

@sethetter what kind of Slack plan are you on? Is it a standalone workspace or part of an org?

sethetter commented 11 months ago

@filmaj standalone I believe. We’re a small nonprofit with a single slack space, if that helps.

sethetter commented 11 months ago

Deleting and redeploying the app got things working again!

lukassr commented 3 months ago

I'm running into this issue. I am a workspace owner. My app started with one domain in the array of outgoingDomains with a fetch and it was deployed with this only domain.

Now, I added a second fetch with another domain and added it in the outgoingDomains array. In local, both fetch works seamlessly, but when I deployed it, the second fetch didn't work. Things I've tried:

I can see that the manifest updates correctly, since when I deleted the first domain the fetch stopped working. But for some reason I cannot make a second domain to work, or use a domain different from the first one when it was deployed.

shakepay-tl commented 3 months ago

I had the exact same bug, updated an existing app with brand new independent triggers/workflows/functions that was communicating with a new external domain, added this domain to outgoingDomains in the manifest and the app worked fine in local but not in remote (deployed). Switching around the 2 values in the outgoingDomains didn't work.

Un-installing, deleting, and re-installing the app worked.

slack app uninstall
slack delete
slack deploy
tzuo9 commented 4 days ago

Facing the same issue. Local is working but remote is not after slack deploy. Also it passed the validation check.