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

New resource azuredevops_workitemquery_permissions #79

Merged
merged 14 commits into from
Jul 30, 2020

Conversation

tmeckel
Copy link
Contributor

@tmeckel tmeckel commented Jul 16, 2020

All Submissions:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My code follows the code style of this project.
  • I ran lint checks locally prior to submission.
  • Have you checked to ensure there aren't other open PRs for the same update/change?

What about the current behavior has changed?

This PR introduces a new resource azuredevops_workitemquery_permissions to manage permissions on Work Item Queries and their folders inside a Azure DevOps Project.

Does this introduce a change to go.mod, go.sum or vendor/?

  • Yes
  • No

Does this introduce a breaking change?

  • Yes
  • No

Any relevant logs, error output, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Other information

Copy link
Collaborator

@xuzhang3 xuzhang3 left a comment

Choose a reason for hiding this comment

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

@tmeckel I try run the example HCl config and failed.

$ terraform apply --auto-approve=true
azuredevops_project.project: Creating...
azuredevops_project.project: Still creating... [10s elapsed]
azuredevops_project.project: Still creating... [20s elapsed]
azuredevops_project.project: Creation complete after 20s [id=xxxxxx-xxxx-xxxx-xxxx-xxxxxx]
data.azuredevops_group.project-contributors: Refreshing state...
data.azuredevops_group.project-administrators: Refreshing state...
data.azuredevops_group.project-readers: Refreshing state...
azuredevops_workitemquery_permissions.wiq-project-permissions: Creating...
azuredevops_workitemquery_permissions.wiq-sharedqueries-permissions: Creating...
azuredevops_workitemquery_permissions.wiq-folder-permissions: Creating...
azuredevops_workitemquery_permissions.wiq-project-permissions: Creation complete after 1s [id=$/xxxxxxxxxx/vssgp.xxxxxxxx]

Error: VS403284: Invalid operation. Unable to set bits '64' in security namespace 'xxxx-xxxx-xxxx-xxxxx-xxxxxxxx' as it is reserved by the system.

  on main.tf line 34, in resource "azuredevops_workitemquery_permissions" "wiq-sharedqueries-permissions":
  34: resource "azuredevops_workitemquery_permissions" "wiq-sharedqueries-permissions" {



Error: Unable to find query [Team] in folder [Shared Queries] because it has no children

  on main.tf line 44, in resource "azuredevops_workitemquery_permissions" "wiq-folder-permissions":
  44: resource "azuredevops_workitemquery_permissions" "wiq-folder-permissions" {

@xuzhang3
Copy link
Collaborator

Hi @tmeckel Thanks for your contribute to this PR. I have several questions for this PR.

  1. How fullcontrol works? I can execute with terraform apply success but permission configures in UI portal not changed, my understanding is if Iset permission with full control, all permissions should show up with Allow.
  2. How can I verify project level permission configuration. Will this configure override the share query permission configure?
  3. Can you help add more acceptance test cases to cover more scenario, For example:
    • permission update ,
    • permission configure with replace=false,replace=true.

@tmeckel
Copy link
Contributor Author

tmeckel commented Jul 28, 2020

Hi @tmeckel Thanks for your contribute to this PR. I have several questions for this PR.

1. How `fullcontrol` works?  I can execute with `terraform apply ` success but permission configures in  UI portal not changed, my understanding is  if Iset permission with full control, all permissions should show up with `Allow`.

You have to separate the inner working of the various Security Namespaces to the functionality in UI portal, which is mostly streamlined for easy usage or hides (does not) use certain functions of Azure DevOps. So perhaps you should ask someone from the product group how the Full control permission is really working.

2. How can I verify `project level ` permission configuration.  Will this configure override the `share query` permission configure?

As I stated about how the UI implements (reflects) the Security Namespace settings, here you have an example that the UI team didn't implement any functions to manage the Work Item Query permissions on the project level. You can use an indirect approach though, because as usual the permissions from the project level is inherited down to any folder and query.

When you set the permissions for the Readers group to the following settings (taken from the sample in the documentation)

  permissions = {
    Read              = "Allow"
    Delete            = "Deny"
    Contribute        = "Deny"
    ManagePermissions = "Deny"
  }

you'll see the following permissions assign to this group for the Shared Queries folder

image

The Permission Read is explicitly assigned (as default) to the group, so that the text (inherited) is missing.

3. Can you help add more acceptance test cases to cover more scenario,  For example:
   
   * permission update ,

I updated the acceptance tests and added a test to check the update of permissions TestAccWorkItemQueryPermissions_UpdateProjectPermissions

   * permission configure with `replace=false`,`replace=true`.

The issue with replace=false and replace=true is the fact, that with replace=true you replace the complete ACL for specific security namespace, i.e. there will be only one entry left in ACL, namely the ACE for the principal you specified in the azuredevops_workitemquery_permissions resource. If you use replace=false the ACE of azuredevops_workitemquery_permissions will merged (added) to the existing ACL of the security namespace token. Because we only read back the ACE of the principal that's specified in the resource we do not have currently any code to check if the Azure DevOps REST API really has replaced or merged the new ACE.

@tmeckel tmeckel requested a review from xuzhang3 July 28, 2020 17:24
…ctPermissions in azuredevops/internal/acceptancetests/resource_workitemquery_permissions_test.go
@tmeckel tmeckel requested a review from xuzhang3 July 30, 2020 07:33
@xuzhang3
Copy link
Collaborator

@tmeckel LGTM . But I think we can remove the FullController definition in the document. Although the server side support 6 types of permissions, only four are displayed in the Web UI.

Read                            1 
Contribute                      2
Delete                          4
ManagePermissions               8

…e/docs/r/workitemquery_permissions.html.markdown
@tmeckel
Copy link
Contributor Author

tmeckel commented Jul 30, 2020

@tmeckel LGTM . But I think we can remove the FullController definition in the document. Although the server side support 6 types of permissions, only four are displayed in the Web UI.

Read                            1 
Contribute                      2
Delete                          4
ManagePermissions               8

I agree => removed with 9c9062d

@tmeckel tmeckel requested a review from xuzhang3 July 30, 2020 09:22
@xuzhang3 xuzhang3 merged commit 368de4c into microsoft:master Jul 30, 2020
@tmeckel tmeckel deleted the r_query_permissions branch July 30, 2020 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants