[Bug - Fix] azuredevops_build_definition - skip_first_run to work for all repo types

[arkoc]

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?

Issue Number: #862

The code is using clients.GitReposClient which I bealive is working only with DevOps Git Repos.
When I pass repoId in GitHub format (gitOrg/repoName), it's throwing error "Page not found".

As far as I can see from the code, we need the branch information for passing commitId to version property in RunPipeline request. After few tests in DevOps UI and with Rest API directly, it looks like version is not required. So we can ommit that which will allow to remove the code with GitReposClient which was causing the code to work with only Azure DevOps Repos.

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

[arkoc]

@arkoc the command you issued was incorrect. Please try again.

Examples are:

@microsoft-github-policy-service agree

and

@microsoft-github-policy-service agree company="your company"

@microsoft-github-policy-service agree

[xuzhang3]

@arkoc I think we could keep branch checks (pre-checks) of the Azure DevOps Git repo to ensure the pipeline can be triggered successfully. For other repository types we can ignore this check.

[arkoc]

@arkoc I think we could keep branch checks (pre-checks) of the Azure DevOps Git repo to ensure the pipeline can be triggered successfully. For other repository types we can ignore this check.

If the build definition was succesffully created, can't we safely trigger the same build definition with the same branch name?
We are using the same input which is used for creating build definition. I bealive that any pre-checks are redundant here.

[xuzhang3]

@arkoc if the repository not initialized and there is not branch exist, pipeline run will not be triggered even with skip_first_run enabled. This is potential bug/issue and can be prevented with pre-checking

[arkoc]

@arkoc if the repository not initialized and there is not branch exist, pipeline run will not be triggered even with skip_first_run enabled. This is potential bug/issue and can be prevented with pre-checking

Can you create a build definition without initializing repository / branch? I bealive the valid git connection to the repo/branch is required for creating the build definition (pipeline). Am I missing something?

[xuzhang3]

@arkoc It is possible to such a build definition:
Example:

resource "azuredevops_project" "example" {
  name               = "Example Project"
  visibility         = "private"
  version_control    = "Git"
  work_item_template = "Agile"
}

resource "azuredevops_git_repository" "example" {
  project_id = azuredevops_project.example.id
  name       = "Example Empty Git Repository"
  initialization {
    init_type = "Uninitialized"
  }
}

resource "azuredevops_build_definition" "example" {
  project_id = azuredevops_project.example.id
  name       = "Example Build Definition"
  path       = "\\ExampleFolder"

  ci_trigger {
    use_yaml = false
  }

  repository {
    repo_type   = "TfsGit"
    repo_id     = azuredevops_git_repository.example.id
    branch_name = azuredevops_git_repository.example.default_branch
    yml_path    = "azure-pipelines.yml"
  }

  schedules {
    branch_filter {
      include = ["main"]
      exclude = ["test", "regression"]
    }
    days_to_build              = ["Wed", "Sun"]
    schedule_only_with_changes = true
    start_hours                = 10
    start_minutes              = 59
    time_zone                  = "(UTC) Coordinated Universal Time"
  }

  features {
    skip_first_run = true
  }
}

[arkoc]

Even if you add checks for repo and branch, run for this pipeline is going to still fail. The mentioned checks are not going to prevent it.

[xuzhang3]

An error/hit message should pop up to the user indicating that the running pipeline will not be triggered.

[arkoc]

This problem is not that repo or branch doesn't exists, the problem is that this pipleine doesn't have any .yaml with it. So it will not be tirggered.

Right now the integration is fully broken for non tfs repos. It's not working for anyone. This suggested fix, can make it work for everyone, except cases where build pipleine doesnt have any instructions with it (is empty so nothing to trigger).

How you suggest to continue? I suggest that, we can alwas trigger the pipele at that stage, if that errors out, we should just show "warning or hint" that run didn't work, beacause ultimately the resource was created.

[xuzhang3]

@arkoc Sounds reasonable. Can you add the hit message if the pipeline cannot be triggered?

[arkoc]

@arkoc Sounds reasonable. Can you add the hit message if the pipeline cannot be triggered?

Added. I am first time using Go, as well as implementing anything in Terraform Plugins. Is the diag.Diagnostics right choice for this problem?

[xuzhang3]

=== RUN   TestAccBuildDefinition_DataSource
=== PAUSE TestAccBuildDefinition_DataSource
=== RUN   TestAccBuildDefinition_with_path_DataSource
=== PAUSE TestAccBuildDefinition_with_path_DataSource
=== RUN   TestAccBuildDefinition_Basic
=== PAUSE TestAccBuildDefinition_Basic
=== RUN   TestAccBuildDefinition_PathUpdate
=== PAUSE TestAccBuildDefinition_PathUpdate
=== RUN   TestAccBuildDefinition_WithVariables
=== PAUSE TestAccBuildDefinition_WithVariables
=== RUN   TestAccBuildDefinition_Schedules
=== PAUSE TestAccBuildDefinition_Schedules
=== CONT  TestAccBuildDefinition_DataSource
=== CONT  TestAccBuildDefinition_PathUpdate
=== CONT  TestAccBuildDefinition_Schedules
=== CONT  TestAccBuildDefinition_WithVariables
=== CONT  TestAccBuildDefinition_Basic
=== CONT  TestAccBuildDefinition_with_path_DataSource
--- PASS: TestAccBuildDefinition_with_path_DataSource (42.05s)
--- PASS: TestAccBuildDefinition_Schedules (45.57s)
--- PASS: TestAccBuildDefinition_DataSource (47.50s)
--- PASS: TestAccBuildDefinition_Basic (50.20s)
--- PASS: TestAccBuildDefinition_PathUpdate (62.58s)
--- PASS: TestAccBuildDefinition_WithVariables (76.31s)
PASS
ok      github.com/microsoft/terraform-provider-azuredevops/azuredevops/internal/acceptancetests        84.407s

[xuzhang3]

@arkoc LGTM