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

Implement Terrafrom resource azuredevops_project_permissions #47

Closed
EliiseS opened this issue Jul 2, 2020 · 8 comments · Fixed by #18
Closed

Implement Terrafrom resource azuredevops_project_permissions #47

EliiseS opened this issue Jul 2, 2020 · 8 comments · Fixed by #18
Assignees

Comments

@EliiseS
Copy link
Member

EliiseS commented Jul 2, 2020


Migrated from #104
Originally created by @tmeckel on Mon, 14 Oct 2019 15:31:03 GMT


Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

As a developer, I want the ability to manage the permissions of a project in
Azure DevOps. Thus a new Terraform resource shall be provided.

New or Affected Resource(s)

  • azuredevops_project_permissions

Potential Terraform Configuration

data "azuredevops_project" "project" {
   # Exports: id
   project_name = "Test Project"
}

data "azuredevops_user" "lookup-user" {
   principalname_name  = "[email protected]"
}

resource "azuredevops_group" "project-group" {
    project_id = azuredevops_project.project.id
    name       = "Test Group"
}

resource "azuredevops_project_permissions" "perm-project-group" {
    # Security Namespace: 52d39943-cb85-4d7f-8fa8-c6baac873819

    # Build ACL token from project id (GUID) as follows
    # $PROJECT:vstfs:///Classification/TeamProject/<ProjectGUID>
    project_id = data.azuredevops_project.project.id

    principals = [
        data.azuredevops_user.lookup-user.id_descriptor
        azuredevops_group.project-group.id_descriptor
    ]

    permissions = {
        "Enumerate"            = "Allow"
        "Delete"               = "Deny"
        "Publish_test_results" = "NotSet"
    }
}

Acceptance Criteria

[] Include Unit tests for the new operation

[] Acceptance (integration) test to validate repo creation/modification,
following the patterns from the azuredevops_build_definition resource

References

@EliiseS
Copy link
Member Author

EliiseS commented Jul 2, 2020


Migrated from #104 (comment)
Originally created by @nmiodice on Fri, 25 Oct 2019 15:35:13 GMT


In other words, each non-READ API in AzDO should really only be used by a single terraform resource, otherwise you can run into unknown dependencies in terraform state.

@EliiseS
Copy link
Member Author

EliiseS commented Jul 2, 2020


Migrated from #104 (comment)
Originally created by @nmiodice on Fri, 25 Oct 2019 15:33:38 GMT


@tmeckel Have you considered removing the principals block? If we allow management of group membership through multiple resources, it can be really tricky to implement correctly due hidden dependencies that users of the provider may introduce.

Here is a similar example where there are 2 ways to configure permissions to Key Vault in the AzureRM provider -- it is easy for permissions to accidentally get deleted or unintentionally added when the two approaches are mixed. See the note at the top: https://www.terraform.io/docs/providers/azurerm/r/key_vault.html

I have a similar concern with regards to proposing multiple different ways to configure group membership.

@EliiseS
Copy link
Member Author

EliiseS commented Jul 2, 2020


Migrated from #104 (comment)
Originally created by @tmeckel on Fri, 25 Oct 2019 18:39:36 GMT


I copy @tiwood as reference here.

@EliiseS
Copy link
Member Author

EliiseS commented Jul 2, 2020


Migrated from #104 (comment)
Originally created by @tmeckel on Fri, 25 Oct 2019 18:38:14 GMT


@nmiodice I think you've been caught in a misunderstanding. The principals block is required to create the ACE entries for the security namespace 52d39943-cb85-4d7f-8fa8-c6baac873819.

If you look at the REST API Access Control Lists - Query you can see how security is managed inside DevOps (TFS). You create ACEs and assign them to a security namespace. The security namespace defines the available permission settings and the ACEs consist of the principals and their assigned allow and deny "actions" to the available permissions respectively.

So we simply can't remove the principals block from this resource, because, as mentioned, you need the references to the principals that should be included in the security/policy assignment. Thus the principals bloc do not define the membership to a group or something but defines an entitlement.

@EliiseS
Copy link
Member Author

EliiseS commented Jul 2, 2020


Migrated from #104 (comment)
Originally created by @tmeckel on Fri, 25 Oct 2019 19:12:04 GMT


@nmiodice Thanks for adding @rguthriemsft here as well!

@EliiseS
Copy link
Member Author

EliiseS commented Jul 2, 2020


Migrated from #104 (comment)
Originally created by @nmiodice on Fri, 25 Oct 2019 18:46:06 GMT


@tmeckel, thanks for pointing this out. I'm not very strong in this area of devops and suspect that you are correct in pointing out my mis-understanding.

Calling in our resident AzDO expert - @rguthriemsft

@EliiseS
Copy link
Member Author

EliiseS commented Jul 2, 2020

@tmeckel Can you comment on this issue so I can assign you back?

Context: isaacs/github#100 (comment)

@EliiseS EliiseS removed the moved label Jul 2, 2020
@tmeckel
Copy link
Contributor

tmeckel commented Jul 2, 2020

🥇

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

Successfully merging a pull request may close this issue.

2 participants