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

[feature request] sepearete resource for team membership #65

Closed
JordanSussman opened this issue Jan 29, 2020 · 26 comments
Closed

[feature request] sepearete resource for team membership #65

JordanSussman opened this issue Jan 29, 2020 · 26 comments

Comments

@JordanSussman
Copy link

Terraform Version

v0.12.20

Affected Resource(s)

  • opsgenie_team

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "opsgenie_user" "first" {
  username  = "[email protected]"
  full_name = "name "
  role      = "User"
}

resource "opsgenie_user" "second" {
  username  = "[email protected]"
  full_name = "name "
  role      = "User"
}

resource "opsgenie_team" "test" {
  name        = "example"
  description = "This team deals with all the things"

  member {
    id   = "${opsgenie_user.first.id}"
    role = "admin"
  }

  member {
    id   = "${opsgenie_user.second.id}"
    role = "user"
  }
}

I'd like to propose a separate resource of opsgenie_team_membership instead of relying on utilizing opsgenie_team for membership. This change would allow us to manage initial team membership Terraform and some further changes done through the UI.

Currently, if you make changes through the UI for a team that is managed via Terraform it will attempt to delete users from the team. If we instead utilize opsgenie_team_membership to add members to the team it wouldn't need to delete membership at the team level if users are added via the UI.

Proposed Terraform Configuration Files

resource "opsgenie_user" "first" {
  username  = "[email protected]"
  full_name = "name "
  role      = "User"
}

resource "opsgenie_user" "second" {
  username  = "[email protected]"
  full_name = "name "
  role      = "User"
}

resource "opsgenie_team" "test" {
  name        = "example"
  description = "This team deals with all the things"
}

resource "opsgenie_team_membership" "first" {
    id   = "${opsgenie_user.first.id}"
    role = "admin"
    team = "${opsgenie_team.test.id}"
}

resource "opsgenie_team_membership" "second" {
    id   = "${opsgenie_user.second.id}"
    role = "admin"
    team = "${opsgenie_team.test.id}"
}
@ffahri
Copy link

ffahri commented Feb 3, 2020

@JordanSussman hey could you write what kind of changes make this

Currently, if you make changes through the UI for a team that is managed via Terraform it will attempt to delete users from the team

I think there is a bug in team resource. Thats shouldnt be a expected behaviour (deleting user)

@JordanSussman
Copy link
Author

Steps to reproduce the deletion of team membership:

  1. Apply the following terraform code

    resource "opsgenie_user" "first" {
      username  = "[email protected]"
      full_name = "name "
      role      = "User"
    }
    
    resource "opsgenie_user" "second" {
      username  = "[email protected]"
      full_name = "name "
      role      = "User"
    }
    
    resource "opsgenie_team" "test" {
      name        = "example"
      description = "This team deals with all the things"
    
      member {
          id   = "${opsgenie_user.first.id}"
          role = "admin"
      }
    }
  2. Navigate to app.opsgenie.com/teams/dashboard/<team id of test team>/members

  3. Manually add second user to the test team

  4. Run plan against the previously mentioned terraform file and notice that it wants to remove the second user from the team

@jaceq
Copy link
Contributor

jaceq commented Feb 5, 2020

But that sounds right, when you create team via TF it manages that resource in authoritative way, this means that if anything was 'clicked' additionally it will be reset back to state defined in Terraform.
I think that overall mixing of TF managed resources with manual additions beats the purpose of TF

@JordanSussman
Copy link
Author

JordanSussman commented Feb 9, 2020

But that sounds right, when you create team via TF it manages that resource in authoritative way, this means that if anything was 'clicked' additionally it will be reset back to state defined in Terraform.
I think that overall mixing of TF managed resources with manual additions beats the purpose of TF

I don't disagree that the behavior is expected, however I still want the provider to support this specific use case by providing the opsgenie_team_membership resource. Which would allow me to do something like the following:

resource "opsgenie_user" "first" {
  username  = "[email protected]"
  full_name = "name "
  role      = "User"
}

resource "opsgenie_user" "second" {
  username  = "[email protected]"
  full_name = "name "
  role      = "User"
}

resource "opsgenie_team" "test" {
  name        = "example"
  description = "This team deals with all the things"
  
  lifecycle {
    ignore_changes = [
      member,
    ]
  }
}

resource "opsgenie_team_membership" "first" {
    id   = "${opsgenie_user.first.id}"
    role = "admin"
    team = "${opsgenie_team.test.id}"
}

resource "opsgenie_team_membership" "second" {
    id   = "${opsgenie_user.second.id}"
    role = "admin"
    team = "${opsgenie_team.test.id}"
}

Quite a few other providers provide similar workflows by separateing the high level creation of a resource from the smaller details. For example: aws_volume_attachment.

I'm happy to contribute this functionality if the maintainers agree it would be accepted.

@arnisoph
Copy link
Contributor

I would very welcome a(ny) change that makes the provider able to ignore team membership. I want TF to create the team, and that's just it.

@arnisoph
Copy link
Contributor

arnisoph commented Feb 18, 2020

I've build a very ugly hack to workaround this -issue- behaviour (simply disable the member attribute at all). @ffahri could you share your opinion to @JordanSussman 's comment please? thx :)

@ffahri
Copy link

ffahri commented Feb 19, 2020

Hey @arnisoph @JordanSussman
If there are use cases for managing users via ui etc we should provide this functionality to provider.
However question is should we change current behaviour (which can be hard to migrate some of users not sure)

team {
user{}
user{}
}

or we introduce new resource for this just for creating and managing teams without looking members

team_only{}

or flag inside team resource such as below example

team {
manage_members=false
}

@arnisoph
Copy link
Contributor

I think having the optional flag "manage_members=false" (member{} is already optional anyway) and providing additional resources like team_membership{} would be the easiest for the users.

AWS and GCP already have a similar approach for some time:

https://www.terraform.io/docs/providers/google/r/google_project_iam.html#google_project_iam_member-1

@arnisoph
Copy link
Contributor

arnisoph commented Mar 3, 2020

@ffahri I could prepare PR for that, what do you think?

@JordanSussman
Copy link
Author

I think having the optional flag "manage_members=false" (member{} is already optional anyway) and providing additional resources like team_membership{} would be the easiest for the users.

AWS and GCP already have a similar approach for some time:

https://www.terraform.io/docs/providers/google/r/google_project_iam.html#google_project_iam_member-1

I agree with this approach compared to the other options suggested.

@arnisoph
Copy link
Contributor

arnisoph commented Mar 4, 2020

I started to prepare a PR that first adds sth like "manage_members=false" and later a separate PR that adds new resources to maintain team membership. all those changes should be non-breaking for our users.

arnisoph added a commit to arnisoph/terraform-provider-opsgenie that referenced this issue Mar 10, 2020
@arnisoph
Copy link
Contributor

arnisoph commented Mar 10, 2020

@JordanSussman you can find the first PR in #79 . It makes it possible to disable Terraform to maintain team membership. This is useful, if you apply changes via OpsGenie web UI. Would you like to try this PR out? I can provide you the binary if needed.

A second PR would add a new resource type for actual team membership.

arnisoph added a commit to arnisoph/terraform-provider-opsgenie that referenced this issue Mar 10, 2020
@arnisoph
Copy link
Contributor

@ffahri if this PR is fine and completed/merged I'd continue with the second PR for the new team_membership resource.

ffahri pushed a commit that referenced this issue Mar 19, 2020
* add optional setting to stop maintaing team members, refs #65

Signed-off-by: Arnold Bechtoldt <[email protected]>
@ffahri
Copy link

ffahri commented Mar 23, 2020

ignore_members flag is in 0.2.9 release. we are waiting for this release. We can this resource to v0.3.0 release. thanks @arnisoph

@ffahri
Copy link

ffahri commented Mar 23, 2020

@arnisoph for the second pr what is on your mind?
I dont think we should not change current member type for backward compatibilty.
Adding new field should be okey team_membership for this

@arnisoph
Copy link
Contributor

yes, let's keep backwards compatibility "for some time". I will prepare a second PR soon, too.

@mrajeshh
Copy link

mrajeshh commented Jul 2, 2020

we also have usecase to group multiple users to multiple teams, @arnisoph any update on team_membership resource

@arnisoph
Copy link
Contributor

arnisoph commented Jul 3, 2020

@mrajeshh sorry, I decided not to implement a resource such as team_membership. There's currently no need for that in my current team.

@arnisoph
Copy link
Contributor

Expect a PR soon.

arnisoph added a commit to arnisoph/terraform-provider-opsgenie that referenced this issue Aug 13, 2020
@arnisoph
Copy link
Contributor

Added PR #156 . I propose deprecating the members attribute in opsgenie_team to avoid confusions about the ignore_members attribute.

arnisoph added a commit to arnisoph/terraform-provider-opsgenie that referenced this issue Aug 20, 2020
arnisoph added a commit to arnisoph/terraform-provider-opsgenie that referenced this issue Aug 20, 2020
arnisoph added a commit to arnisoph/terraform-provider-opsgenie that referenced this issue Aug 20, 2020
@dreamiurg
Copy link

We have another reason to ask for a separate opsgenie_team_membership resource. Today, it is not possible to bulk-assign users to OpsGenie team using for_each (you simply can not have a for_each statement on the member {} section). In our case the list of members for each OpsGenie team comes from another provider, and we'd like to be able to do something like this:

resource "opsgenie_team" "engineers" {
  name        = "engineers"
}

resource "opsgenie_team_membership" "engineers_members" {
  for_each = local.engineer_member_ids # or similiar
  team_id   = opsgenie_team.engineers.id
    
  role = "user"
  user_id = each.key
}

@arnisoph
Copy link
Contributor

@dreamiurg it's almost done, but I have to touch it again... #167

arnisoph added a commit to arnisoph/terraform-provider-opsgenie that referenced this issue Oct 19, 2020
@slushysnowman
Copy link

slushysnowman commented Jun 14, 2023

We have another reason to ask for a separate opsgenie_team_membership resource. Today, it is not possible to bulk-assign users to OpsGenie team using for_each (you simply can not have a for_each statement on the member {} section). In our case the list of members for each OpsGenie team comes from another provider, and we'd like to be able to do something like this:

resource "opsgenie_team" "engineers" {
  name        = "engineers"
}

resource "opsgenie_team_membership" "engineers_members" {
  for_each = local.engineer_member_ids # or similiar
  team_id   = opsgenie_team.engineers.id
    
  role = "user"
  user_id = each.key
}

Bit late, but you actually can by using something like this:

  dynamic "member" {
    for_each = data.opsgenie_user.admins
    content {
      id   = member.value.id
      role = "admin"
    }
  }

@slushysnowman
Copy link

This would still be a great feature to have in place.

We are adopting a 'self-service' model where we want to assign team admins via automation and then allow them to assign team users themselves. But with the current way this works, it's basically impossible using the opsgenie provider.

@koushik-swaminathan
Copy link
Contributor

@JordanSussman a bit late, but looks like but this comment does not reproduce the bug for me. This is what I get once I follow the steps you have mentioned in the comment.
`No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.`
Can you try it out with v0.6.26 and let me know?

@koushik-swaminathan
Copy link
Contributor

Closing the issue as I was not able to reproduce the issue. Please feel free to raise a new issue if it is still persistent with steps to reproduce the bug.

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

No branches or pull requests

8 participants