Add support for Azure DevOps RunPipelines service endpoint #184

zhmurko · 2020-09-11T08:37:03Z

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 pull request introduces a new resource azuredevops_serviceendpoint_devops that allows to manage Azure DevOps Service Connection. This connection is used to trigger subsequent builds from Azure Pipelines with task: RunPipelines@1 yaml definition.

Fix for Issue Number: #182

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

Yes
No

Does this introduce a breaking change?

Yes
No

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

Other information

ghost · 2020-09-11T08:37:17Z

All CLA requirements met.

zhmurko · 2020-09-11T10:18:43Z

build has failed by 60 minutes timeout. need a rebuild or increase azdo job timeout.

xuzhang3 · 2020-09-14T09:22:23Z

@zhmurko Is 'azdoapi' a new devops extension developed by your team? I cannot find an extension type azdoapi . If you means Azure Repos/Team Foundation Server, the type should be externaltfs

Error message:

--- FAIL: TestAccServiceEndpointAzureDevOps_PersonalTokenBasic (24.82s)
    testing.go:673: Step 0 error: errors during apply:
        
        Error: Error creating service endpoint in Azure DevOps: Unable to find service connection type 'azdoapi' using authentication scheme 'Token'.
        
          on /tmp/tf-test450287256/main.tf line 10:
          (source code not available)

zhmurko · 2020-09-14T10:26:44Z

hello @xuzhang3 thank you for looking into this issue.
I was pretty sure that's generally pre-installed type of Service Connection, but it turns out that it's one of Extensions installed from Marketplace manually: CSE-DevOps.RunPipelines
The "azdoapi" is a type of "Azure DevOps Service Connection" as shown on the screenshot below:

The value was picked up from API call to https://dev.azure.com/my-org-name/_apis/serviceendpoint/endpoints on save action via UI on Azure DevOps Cloud portal.
We use this service connection to trigger subsequent builds from YAML pipelines and really desire to create such endpoints with Terraform code altogether with the infrastructure.

xuzhang3 · 2020-09-15T09:44:10Z

website/docs/r/serviceendpoint_devops.html.markdown

+Azure DevOps Service Endpoint can be imported using the serviceendpoint id (resourceId in your browser's url bar), e.g.
+
+```sh
+ terraform import azuredevops_serviceendpoint_devops.serviceendpoint db0541e6-ae9f-474d-ab83-1f7913839080


Suggested change

terraform import azuredevops_serviceendpoint_devops.serviceendpoint db0541e6-ae9f-474d-ab83-1f7913839080

terraform import azuredevops_serviceendpoint_devops.serviceendpoint projectID/00000000-0000-0000-0000-000000000000`

[x] updated this documentation page

xuzhang3 · 2020-09-15T09:49:34Z

website/docs/r/serviceendpoint_devops.html.markdown

+
+## Import
+
+Azure DevOps Service Endpoint can be imported using the serviceendpoint id (resourceId in your browser's url bar), e.g.


Suggested change

Azure DevOps Service Endpoint can be imported using the serviceendpoint id (resourceId in your browser's url bar), e.g.

Azure DevOps Service Endpoint can be imported using the `project id`, `service connection id` , e.g.

[x] updated this documentation page

xuzhang3 · 2020-09-15T09:54:20Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+}
+
+// Convert internal Terraform data structure to an AzDO data structure:
+//


Unused code lines can be removed

[x] removed the commented examples

xuzhang3 · 2020-09-15T09:54:41Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+}
+
+// Convert AzDO data structure to internal Terraform data structure
+//


Unused code lines can be removed.

[x] removed the commented examples

xuzhang3 · 2020-09-15T09:55:32Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+// ResourceServiceEndpointAzureDevOps schema and implementation for Azure DevOps service endpoint resource
+func ResourceServiceEndpointAzureDevOps() *schema.Resource {
+	r := genBaseServiceEndpointResource(flattenServiceEndpointAzureDevOps, expandServiceEndpointAzureDevOps)
+	r.Schema["organization"] = &schema.Schema{


Can we rename organization to organization_name or org_name ?

[x] renamed organization to organization_name and updated the documentation page

xuzhang3 · 2020-09-15T10:00:35Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+		MinItems:      1,
+		MaxItems:      1,
+		Elem:          authPersonal,
+		ConflictsWith: []string{},


Conflict schema can be removed.

[x] removed ConflictsWith

xuzhang3 · 2020-09-15T10:02:48Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+	authPersonal.Schema[patHashKey] = patHashSchema
+	r.Schema["auth_personal"] = &schema.Schema{
+		Type:          schema.TypeSet,
+		Optional:      true,


If I skip the auth_personal configuration, after execute terraform apply, I got error:

Error: Error updating service endpoint in Azure DevOps: Unable to find service connection type 'azdoapi' using authentication scheme 'InstallationToken'. on main.tf line 5, in resource "azuredevops_serviceendpoint_devops" "serviceendpoint": 5: resource "azuredevops_serviceendpoint_devops" "serviceendpoint" {

added a unit test for default settings with empty auth_personal personal block

xuzhang3 · 2020-09-15T10:05:43Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+)
+
+const (
+	azdoPersonalAccessToken = "personal_access_token"


Duplicate with personalAccessToken

[x] removed this constant value

xuzhang3 · 2020-09-15T10:11:08Z

website/docs/r/serviceendpoint_devops.html.markdown

+
+```hcl
+resource "azuredevops_project" "project" {
+  project_name       = "Sample Project"


project_name has been renamed to name in the latest master

[x] renamed project_name to name in project definition in this example

xuzhang3 · 2020-09-15T10:14:16Z

azuredevops/internal/acceptancetests/resource_serviceendpoint_devops_test.go

+
+// validates that an apply followed by another apply (i.e., resource update) will be reflected in AzDO and the
+// underlying terraform state.
+func TestAccServiceEndpointAzureDevOps_CreateAndUpdate(t *testing.T) {


This test case is duplicate with TestAccServiceEndpointAzureDevOps_PersonalTokenUpdate

[x] removed this test case and added a test case for default settings

zhmurko · 2020-09-16T09:24:01Z

@xuzhang3 thank you for suggestions, I'm going to review and implement them soon

zhmurko · 2020-09-22T08:33:33Z

hi @xuzhang3
please review the updated pull request, all your suggestions are implemented.

zhmurko · 2020-09-22T08:36:57Z

and one question, does it worth to rename this resource to azuredevops_serviceendpoint_runpipeline to avoid confusions? in the UI this service connection is called just as a "Azure DevOps Service Connection", that may lead to miscommunication.

xuzhang3 · 2020-09-23T08:57:37Z

and one question, does it worth to rename this resource to azuredevops_serviceendpoint_runpipelines to avoid confusions? in the UI this service connection is called just as a "Azure DevOps Service Connection", that may lead to miscommunication.

I would suggest rename with azuredevops_serviceendpoint_pipelinerunner or azuredevops_serviceendpoint_pipelinedelegator, this extension is kind like the admin of another pipeline.

zhmurko · 2020-09-23T09:03:38Z

so I will refactor the code and docs with azuredevops_serviceendpoint_pipelinerunner resource name.

xuzhang3

Few changes requested but otherwise looks good. Can we rename function name prefix azdo to another name like pipelineRunner or something else, it's a bit odd with prefix azdo

xuzhang3 · 2020-09-23T09:14:07Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+}
+
+func azdoPersonalAccessTokenField() *schema.Resource {
+	field_name := "personal_access_token"


variable name style should follow camel style

xuzhang3 · 2020-09-23T09:15:18Z

azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_devops.go

+}
+
+// Convert internal Terraform data structure to an AzDO data structure:
+func expandServiceEndpointAzureDevOps(d *schema.ResourceData) (*serviceendpoint.ServiceEndpoint, *string, error) {


function expandServiceEndpointAzureDevOps never throw an error, I would suggest remove the error from the return parameter.

* rename files to *_serviceendpoint_runpipeline.* * update docs * update acc-tests * update package serviceendpoint * update package serviceendpoint tests * register in provider * update testutils tempalates * update provider test * improve docs

zhmurko · 2020-12-07T08:29:21Z

hi @xuzhang3
the refactoring has been completed. The build now fails again on Unable to find service connection type 'azdoapi' message, and I believe it's related to disabled/removed plugin RunPipelines from AzureDevOps. Could you please enable it so acceptance test can pass.

xuzhang3 · 2021-01-05T09:26:32Z

LGTM

zhmurko added 10 commits September 10, 2020 13:11

tests have passed

66d9c3f

register azuredevops_serviceendpoint_devops resource

1cb5714

add acceptance tests

db55347

field organization for serviceUrl and releaseUrl

39eadd1

add azuredevops_serviceendpoint_devops

abec134

fix provider_test

5eef163

fix unit tests

45931ea

pass lint

95a72e3

add notice to CHANGELONG.md

b30fc8d

add a documentation page

ad3b3a9

fix acctest

1962f06

xuzhang3 linked an issue Sep 11, 2020 that may be closed by this pull request

[Feature request] Add support for service endpoint for Azure DevOps #182

Closed

xuzhang3 added new-resource new-feature labels Sep 11, 2020

Merge branch 'master' into feature/add-serviceendpoint-azuredevops

418c44f

xuzhang3 self-assigned this Sep 15, 2020

xuzhang3 reviewed Sep 15, 2020

View reviewed changes

zhmurko added 8 commits September 21, 2020 14:27

update docs: add link to the extenition on Marketplace

b304590

organization_name

501b247

project_name -> name

632d391

make fmt

ad6e5f5

rewrite test helpers

4fb00dc

acceptance tests have passed

d16af53

acctest for empty auth_personal {}

5c0d958

fmt

87f7c4c

zhmurko added 2 commits September 22, 2020 09:56

retry CI build

25a65d0

unit test for empty auth_personal {}

717553f

xuzhang3 self-requested a review September 22, 2020 09:11

xuzhang3 reviewed Sep 23, 2020

View reviewed changes

xuzhang3 added the waiting-response label Nov 25, 2020

zhmurko changed the title ~~Add support for Azure DevOps Pipelines service endpoint~~ Add support for Azure DevOps RunPipelines service endpoint Dec 5, 2020

zhmurko added 4 commits December 5, 2020 14:43

remove a return error from func expandServiceEndpointRunPipeline

501446a

rename prefix "azdo" to "rp"

4ad3243

variable name should follow camel style

a5feb80

type expandFunc requires to return an error variable

d18c372

document update

f7f58d3

xuzhang3 merged commit 21fd222 into microsoft:master Jan 5, 2021

	terraform import azuredevops_serviceendpoint_devops.serviceendpoint db0541e6-ae9f-474d-ab83-1f7913839080
	terraform import azuredevops_serviceendpoint_devops.serviceendpoint projectID/00000000-0000-0000-0000-000000000000`


		## Import

		Azure DevOps Service Endpoint can be imported using the serviceendpoint id (resourceId in your browser's url bar), e.g.

Add support for Azure DevOps RunPipelines service endpoint #184

Add support for Azure DevOps RunPipelines service endpoint #184

Conversation

zhmurko commented Sep 11, 2020

All Submissions:

What about the current behavior has changed?

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

Does this introduce a breaking change?

Other information

ghost commented Sep 11, 2020 • edited by ghost

zhmurko commented Sep 11, 2020

xuzhang3 commented Sep 14, 2020

zhmurko commented Sep 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhmurko commented Sep 16, 2020

zhmurko commented Sep 22, 2020

zhmurko commented Sep 22, 2020 • edited

xuzhang3 commented Sep 23, 2020

zhmurko commented Sep 23, 2020

xuzhang3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhmurko commented Dec 7, 2020

xuzhang3 commented Jan 5, 2021

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

ghost commented Sep 11, 2020 •

edited by ghost

zhmurko commented Sep 22, 2020 •

edited