New Resource azuredevops_service_principal_entitlement

[nyanhp]

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?

Added new resource azuredevops_service_principal_entitlement to handle service principal entitlements. The API threw errors when attempting to use a principal name to add the principal, so I only implemented the use of origin and origin_id.

Issue Number: #1025 #797 #889

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?

jhp@S1-0666-W:~/azdotest$ terraform apply

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # azuredevops_service_principal_entitlement.entitlement will be created
  + resource "azuredevops_service_principal_entitlement" "entitlement" {
      + account_license_type = "express"
      + descriptor           = (known after apply)
      + id                   = (known after apply)
      + licensing_source     = "account"
      + origin               = "aad"
      + origin_id            = "0f3bc8a7-70d6-471a-8224-1419cbfdf862"
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

azuredevops_service_principal_entitlement.entitlement: Creating...
azuredevops_service_principal_entitlement.entitlement: Creation complete after 2s [id=583501a3-3ea5-6e0d-8329-52c95ed986a1]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
jhp@S1-0666-W:~/azdotest$ terraform apply --destroy --auto-approve
azuredevops_service_principal_entitlement.entitlement: Refreshing state... [id=583501a3-3ea5-6e0d-8329-52c95ed986a1]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  - destroy

Terraform will perform the following actions:

  # azuredevops_service_principal_entitlement.entitlement will be destroyed
  - resource "azuredevops_service_principal_entitlement" "entitlement" {
      - account_license_type = "express" -> null
      - descriptor           = "aadsp.NTgzNTAxYTMtM2VhNS03ZTBkLTgzMjktNTJjOTVlZDk4NmEx" -> null
      - id                   = "583501a3-3ea5-6e0d-8329-52c95ed986a1" -> null
      - licensing_source     = "account" -> null
      - origin               = "aad" -> null
      - origin_id            = "0f3bc8a7-70d6-471a-8224-1419cbfdf862" -> null
    }

Plan: 0 to add, 0 to change, 1 to destroy.
azuredevops_service_principal_entitlement.entitlement: Destroying... [id=583501a3-3ea5-6e0d-8329-52c95ed986a1]
azuredevops_service_principal_entitlement.entitlement: Destruction complete after 1s

Apply complete! Resources: 0 added, 0 changed, 1 destroyed.
jhp@S1-0666-W:~/azdotest$

azuredevops_service_principal_entitlement.entitlement: Refreshing state... [id=583501a3-3ea5-6e0d-8329-52c95ed986a1]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # azuredevops_service_principal_entitlement.entitlement will be updated in-place
  ~ resource "azuredevops_service_principal_entitlement" "entitlement" {
      ~ account_license_type = "express" -> "Stakeholder"
        id                   = "583501a3-3ea5-6e0d-8329-52c95ed986a1"
        # (4 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
azuredevops_service_principal_entitlement.entitlement: Modifying... [id=583501a3-3ea5-6e0d-8329-52c95ed986a1]
azuredevops_service_principal_entitlement.entitlement: Modifications complete after 1s [id=583501a3-3ea5-6e0d-8329-52c95ed986a1]

Other information

[xuzhang3]

@nyanph can you add the AccTest for the new resources?

[xuzhang3]

any other possible values ​​besides the default value?

[nyanhp]

Not sure. The docs say AAD, AD, MSA, but I'm not sure if that list is exhaustive

[xuzhang3]

we can add AD, AAD, MSA to the doc

[xuzhang3]

Should check the servicePrincipalEntitlement.ServicePrincipal.Descriptor and servicePrincipalEntitlement.ServicePrincipal first before get the values to prevent potential nil exception

[nyanhp]

I'm not sure I understand the request. The check is done in l126, before the function is called. I can repeat the same check within the function and throw an error if that is the correct way to do it

[xuzhang3]

Service may not return the property due to permission. Therefore, you need to check this property before calling it with a pointer.

[xuzhang3]

Can you add some AccTest to cover the usage scenarios?
Refs:
https://developer.hashicorp.com/terraform/plugin/sdkv2/testing/acceptance-tests
https://github.com/microsoft/terraform-provider-azuredevops/blob/main/azuredevops/internal/acceptancetests/resource_group_entitlement_test.go

[xuzhang3]

Service may not return the property due to permission. Therefore, you need to check this property before calling it with a pointer.

[nyanhp]

Can you add some AccTest to cover the usage scenarios? Refs: https://developer.hashicorp.com/terraform/plugin/sdkv2/testing/acceptance-tests https://github.com/microsoft/terraform-provider-azuredevops/blob/main/azuredevops/internal/acceptancetests/resource_group_entitlement_test.go

I am not that good with Go to be honest, so I'm afraid I can't do that properly.

[radoslawcz]

Any update on this?

[smokedlinq]

@xuzhang3 how do we get @nyanhp some help to get the acceptance tests added? Does the test tenant / docs have information about what service principals and/or managed identity resource(s) could be used to test against?

[fredgate]

This is a very valuable pull request

[xuzhang3]

@smokedlinq @nyanhp Service principal Entitlement is a little bit special. SPN requires to be real resource and user/PAT should have permission to access Azure resource. We can using an existed SPN to test

AccTest Example:

func TestAccServicePrincipalEntitlement_basic(t *testing.T) {
	if os.Getenv("ARM_SPN_OBJECT_ID") == "" {
		t.Skip("ARM_SPN_OBJECT_ID needs to be set for testing")
	}
	tfNode := "azuredevops_service_principal_entitlement.test"
	originId := os.Getenv("ARM_SPN_OBJECT_ID") // enterprise app object ID
	resource.ParallelTest(t, resource.TestCase{
		Providers:    testutils.GetProviders(),
		CheckDestroy: checkServicePrincipalEntitlementDestroyed,
		Steps: []resource.TestStep{
			{
				Config: hclServiceServicePrincipalEntitlementBasic(originId),
				Check: resource.ComposeTestCheckFunc(
					checkServicePrincipalEntitlementExists(originId),
					resource.TestCheckResourceAttrSet(tfNode, "descriptor"),
				),
			},
		},
	})
}

func checkServicePrincipalEntitlementExists(expectedDisplayName string) resource.TestCheckFunc {
	return func(s *terraform.State) error {
		resource, ok := s.RootModule().Resources["azuredevops_service_principal_entitlement.test"]
		if !ok {
			return fmt.Errorf(" Did not find a servicePrincipalEntitlement in the TF state")
		}

		clients := testutils.GetProvider().Meta().(*client.AggregatedClient)
		id, err := uuid.Parse(resource.Primary.ID)
		if err != nil {
			return fmt.Errorf(" { Parsing servicePrincipalEntitlement ID, got %s: %v", resource.Primary.ID, err)
		}

		servicePrincipalEntitlement, err := clients.MemberEntitleManagementClient.GetServicePrincipalEntitlement(clients.Ctx, memberentitlementmanagement.GetServicePrincipalEntitlementArgs{
			ServicePrincipalId: &id,
		})

		if err != nil {
			return fmt.Errorf("servicePrincipalEntitlement with ID=%s cannot be found!. Error=%v", id, err)
		}

		if !strings.EqualFold(strings.ToLower(*servicePrincipalEntitlement.ServicePrincipal.OriginId), strings.ToLower(expectedDisplayName)) {
			return fmt.Errorf("sssssssss ")
		}

		return nil
	}
}

func checkServicePrincipalEntitlementDestroyed(s *terraform.State) error {
	clients := testutils.GetProvider().Meta().(*client.AggregatedClient)

	for _, resource := range s.RootModule().Resources {
		if resource.Type != "azuredevops_service_principal_entitlement" {
			continue
		}

		id, err := uuid.Parse(resource.Primary.ID)
		if err != nil {
			return fmt.Errorf(" Parsing servicePrincipalEntitlement ID, got %s: %v", resource.Primary.ID, err)
		}

		servicePrincipalEntitlement, err := clients.MemberEntitleManagementClient.GetServicePrincipalEntitlement(clients.Ctx, memberentitlementmanagement.GetServicePrincipalEntitlementArgs{
			ServicePrincipalId: &id,
		})

		if err != nil {
			if utils.ResponseWasNotFound(err) {
				return nil
			}
			return fmt.Errorf(" Get servicePrincipalEntitlement :  %+v", err)
		}

		if servicePrincipalEntitlement != nil && servicePrincipalEntitlement.ServicePrincipal != nil {
			return fmt.Errorf(" Service Principal Entitlement with ID: %s should not exist", id.String())
		}
	}

	return nil
}

func hclServiceServicePrincipalEntitlementBasic(originID string) string {
	return fmt.Sprintf(`
resource "azuredevops_service_principal_entitlement" "test" {
	origin_id = "%s"
}
`, originID)
}

[DenisBalan]

Any updates?

[JamesDLD]

Any update on this? This is a very valuable pull request.

[radoslawcz]

I am also waiting for this.

[maksimluzhenkov]

Same here guys!

[xuzhang3]

@nyanhp can I fork your branch since you do not time to update this PR ?

[nyanhp]

@nyanhp can I fork your branch since you do not time to update this PR ?

Sure, the fork is public. I have plenty of time, but I am not going to become professional enough with Go, a language I rarely use, just to write integration tests for a one-time PR