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

Add Slack v2 Connection Resource #381

Merged
merged 3 commits into from
Sep 9, 2021

Conversation

stmcallister
Copy link
Contributor

This adds a new resource for Slack v2 connections in PagerDuty. Fixes #73,#94,#280,#357,#365

Test results:

TF_ACC=1 go test -run "TestAccPagerDutySlackConnection" ./pagerduty -v -timeout 120m
=== RUN   TestAccPagerDutySlackConnection_import
--- PASS: TestAccPagerDutySlackConnection_import (13.32s)
=== RUN   TestAccPagerDutySlackConnectionTeam_import
--- PASS: TestAccPagerDutySlackConnectionTeam_import (6.10s)
=== RUN   TestAccPagerDutySlackConnection_Basic
--- PASS: TestAccPagerDutySlackConnection_Basic (14.02s)
=== RUN   TestAccPagerDutySlackConnection_Team
--- PASS: TestAccPagerDutySlackConnection_Team (6.11s)
=== RUN   TestAccPagerDutySlackConnection_Envar
--- PASS: TestAccPagerDutySlackConnection_Envar (4.48s)
PASS
ok  	github.com/terraform-providers/terraform-provider-pagerduty/pagerduty	44.947s

@stmcallister stmcallister requested a review from heimweh September 1, 2021 18:15
Copy link
Collaborator

@heimweh heimweh left a comment

Choose a reason for hiding this comment

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

Nice work! Can't wait to use this 🙂 🎉

pagerduty/resource_pagerduty_slack_connection.go Outdated Show resolved Hide resolved
pagerduty/resource_pagerduty_slack_connection.go Outdated Show resolved Hide resolved
@Glen-Moonpig
Copy link

I am using an API key with Full Access but getting a 401 for this resource only.

Error: POST API call to https://app.pagerduty.com/integration-slack/workspaces/SECRET/connections failed 401 Unauthorized. Code: 0, Errors: <nil>, Message: Unauthorized
  on .terraform/modules/pagerduty_service/main.tf line 82, in resource "pagerduty_slack_connection" "slack_connection":
  82: resource "pagerduty_slack_connection" "slack_connection" {

Any ideas why that would be? The Slack workspace has been added to PagerDuty and we have already set up some services using the web interface, now trying to add one using terraform.

@Glen-Moonpig
Copy link

I now see the documentation says
This resource requires a PagerDuty user-level API key set as the PAGERDUTY_USER_TOKEN environment variable.
That is painful 😓

@Glen-Moonpig
Copy link

It still doesn't work with a user-level token that has authorized PagerDuty access to slack 🤷

@m1n9o
Copy link

m1n9o commented Oct 21, 2021

@Glen-Moonpig I encountered the same problem. I was using PagerDuty Token with full access and was trying to create Slack V2 NG with terraform. It didn't work. Shouldn't we use Slack Token for it as well??? Any ideas??

cc: @stmcallister @jbfavre @heimweh

@Glen-Moonpig
Copy link

@Glen-Moonpig I encountered the same problem. I was using PagerDuty Token with full access and was trying to create Slack V2 NG with terraform. It didn't work. Shouldn't we use Slack Token for it as well??? Any ideas??

cc: @stmcallister @jbfavre @heimweh

I expect you need to use a user-level token because the user will need to have authorized OAuth access to their Slack account via the web. On the website UI for the Slack integration this is required to fetch a channel list.
I couldn't get it to work with any tokens though so gave up in the end..

@m1n9o
Copy link

m1n9o commented Oct 21, 2021

@Glen-Moonpig Hey mate, create a clean work directory and give it a try. I can manage resource smoothly now.

provider "pagerduty" {
  token = "whatever_token_here_api_access_token_or_user_level_token"
}

data "pagerduty_team" "foo" {
  name = "TEAMNAME"
}

data "pagerduty_priority" "p5" {
  name = "P5"
}

resource "pagerduty_slack_connection" "foo" {
  source_id = data.pagerduty_team.foo.id
  source_type = "team_reference"
  workspace_id = "..."
  channel_id = "..."
  notification_type = "responder"
  config {
    events = [
      "incident.triggered",
      "incident.acknowledged",
      "incident.escalated",
      "incident.resolved",
      "incident.reassigned",
      "incident.annotated",
      "incident.unacknowledged",
      "incident.delegated",
      "incident.priority_updated",
      "incident.reopened"          
    ]
    priorities = [data.pagerduty_priority.p5.id]

  }
}

Don't forget to export user level token to env
export PAGERDUTY_USER_TOKEN="user_level_token"

Have a try. But it's a pain to use user level token indeed.

@Glen-Moonpig
Copy link

@m1n9o Do you need to set the PAGERDUTY_USER_TOKEN environment variable? Is it not possible to just input the user token to the provider?
In my terraform I have 2 providers, 1 with user token and 1 with normal api token.

provider "pagerduty" {
  token = "api_level_token"
}

provider "pagerduty" {
  alias = "user"
  token = "user_level_token"
}

resource "pagerduty_slack_connection" "foo" {
   provider = pagerduty.user
   ...
}

@m1n9o
Copy link

m1n9o commented Oct 21, 2021

@Glen-Moonpig I dont think so, I tried. It is only valid when PAGERDUTY_USER_TOKEN is set. Look here.

@Glen-Moonpig
Copy link

@Glen-Moonpig I dont think so, I tried. It is only valid when PAGERDUTY_USER_TOKEN is set. Look here.

Oh wow that is not good. We use module inputs for everything, no environment variables.

@m1n9o
Copy link

m1n9o commented Oct 22, 2021

@Glen-Moonpig I dont think so, I tried. It is only valid when PAGERDUTY_USER_TOKEN is set. Look here.

Oh wow that is not good. We use module inputs for everything, no environment variables.

Yeah it's not good, but I think you can just export it in where terraform executed.
Actually, I wonder if they will improve this in near future or not.

@0x91
Copy link

0x91 commented Oct 25, 2021

Grabbing an environment variable directly in the provider instead allowing the token to be taken as an input is really unfriendly and against the established design patterns for terraform providers.

@heimweh @stmcallister Can this be fixed? I'm happy to submit a PR for this

@0x91
Copy link

0x91 commented Oct 25, 2021

actually I can see this is being addressed in: https://github.com/PagerDuty/terraform-provider-pagerduty/pull/384/files

Can you please also clarify the need for a separate User Token?

@jbfavre
Copy link
Contributor

jbfavre commented Oct 25, 2021

@0x91 User level API Token is mandatory to manage Slack connections.
You cannot use a global API Token for that because PagerDuty needs a valid Slack authentication and this is done at user level

@rabidscorpio
Copy link

@jbfavre Can you explain why it has to be done at the user level? I've seen situations like this at other organizations where they backed off of requiring user level authentication specifically because people want to be able to manage configuration in an automated way without having to create a ghost user. Requiring a pagerduty user level api key means I have to waste a user just to be able to automate slack configuration. If I use a user-level key for an actual user and that user leaves the organization or has a security breach then I have to reset the pagerduty-slack connection and potentially (?) all of the services that use that connection.

This seems extremely short-sighted, especially in light of the fact that IaC is becoming so much more common now. Can Pagerduty please rethink this use case.

@jbfavre
Copy link
Contributor

jbfavre commented Nov 22, 2021

@rabidscorpio not sure I'm the best candidate to do so, but I'll try anyway 😉
When trying to manually setup a Slack connection, you need to have both your Slack and PagerDuty accounts linked.
Which means PagerDuty got a OAuth token from Slack, on your behalf.
AFAIK, this is why you need a user token on terraform side: PagerDuty needs to be able to determine which OAuth token to use.

Now, why not using this OAuth token as an account token ?
I don't know exactly, but since it's a user token on Slack side, I think PagerDuty has to follow this workflow as well.
If not, there's a chances things will break whenever a user leaves the organisation.

But, again, I'm not a PagerDuty developer, so I might be wrong in my analysis.

Best,
JB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unable to use terraform-provider-pagerduty to create a Slack extension
7 participants