Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new git_integration service to the provider to support managing Dataverse Git integration backed by Azure DevOps, including an environment-level repo binding and a solution-level branch/folder binding.
Changes:
- Introduces
powerplatform_environment_git_integrationto manage Dataversesourcecontrolconfiguration(and root branch behavior in Environment scope). - Introduces
powerplatform_solution_git_branchto manage Dataversesourcecontrolbranchconfigurationfor a solution partition. - Registers both resources in the provider, and adds unit tests, examples, and generated docs.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/services/git_integration/api_git_integration.go | Implements Dataverse Web API client operations for git selector endpoints + source control configuration/branch lifecycle and polling. |
| internal/services/git_integration/dto.go | Adds DTOs for git selector endpoints, source control entities, and org/solution updates. |
| internal/services/git_integration/models.go | Adds Terraform resource models plus helper constants and conversion helpers. |
| internal/services/git_integration/resource_environment_git_integration.go | Implements powerplatform_environment_git_integration schema + CRUD + import. |
| internal/services/git_integration/resource_solution_git_branch.go | Implements powerplatform_solution_git_branch schema + CRUD + import. |
| internal/services/git_integration/validation.go | Adds remote validation helpers (selector endpoint validation, scope checks, duplicate binding checks). |
| internal/services/git_integration/resource_environment_git_integration_test.go | Unit tests for env git integration lifecycle, scope behavior, import, and edge cases. |
| internal/services/git_integration/resource_solution_git_branch_test.go | Unit tests for solution branch binding lifecycle, validation, and import. |
| internal/services/git_integration/tests/shared/get_environment_00000000-0000-0000-0000-000000000001.json | Shared BAP environment fixture used by unit tests. |
| internal/services/git_integration/tests/resource/environment_git_integration/get_sourcecontrolconfiguration_1.json | Fixture for environment git integration reads. |
| internal/services/git_integration/tests/resource/solution_git_branch/get_sourcecontrolbranchconfiguration_1.json | Fixture for solution git branch read (initial state). |
| internal/services/git_integration/tests/resource/solution_git_branch/get_sourcecontrolbranchconfiguration_2.json | Fixture for solution git branch read (updated state). |
| internal/provider/provider.go | Registers the two new resources in the provider. |
| internal/provider/provider_test.go | Verifies provider exposes the new resources. |
| examples/resources/powerplatform_environment_git_integration/resource.tf | Example usage for environment git integration. |
| examples/resources/powerplatform_environment_git_integration/variables.tf | Example variables for environment git integration. |
| examples/resources/powerplatform_environment_git_integration/outputs.tf | Example outputs for environment git integration. |
| examples/resources/powerplatform_environment_git_integration/import.sh | Example import command for environment git integration. |
| examples/resources/powerplatform_solution_git_branch/resource.tf | Example usage wiring environment + solution + git integration + solution branch binding. |
| examples/resources/powerplatform_solution_git_branch/variables.tf | Example variables for solution git branch. |
| examples/resources/powerplatform_solution_git_branch/outputs.tf | Example outputs for solution git branch. |
| examples/resources/powerplatform_solution_git_branch/import.sh | Example import command for solution git branch. |
| docs/resources/environment_git_integration.md | Generated docs for powerplatform_environment_git_integration. |
| docs/resources/solution_git_branch.md | Generated docs for powerplatform_solution_git_branch. |
|
@mawasile hopefully this looks okay. Let me know if you need any changes or clarifications. |
AdamCoulterOz
force-pushed
the
codex/git-integration-design
branch
from
6cd3e5c to
a1586c1
Compare
…to codex/git-integration-design
| func normalizeSolutionID(environmentID, configuredValue string) (string, error) { | ||
| if configuredValue == "" { | ||
| return "", errors.New("the `solution_id` attribute must not be empty") | ||
| } | ||
|
|
||
| environmentPrefix, solutionID, found := strings.Cut(configuredValue, "_") | ||
| if !found { | ||
| return "", fmt.Errorf("the `solution_id` value `%s` must be the `id` exported by `powerplatform_solution` for environment `%s`", configuredValue, environmentID) | ||
| } | ||
|
|
||
| if environmentPrefix != environmentID { | ||
| return "", fmt.Errorf("the `solution_id` value `%s` uses environment `%s`, but this resource is targeting environment `%s`", configuredValue, environmentPrefix, environmentID) | ||
| } | ||
|
|
||
| return solutionID, nil |
There was a problem hiding this comment.
normalizeSolutionID only checks for the <environmentId>_<solutionGuid> delimiter, but it never validates that either segment is actually a GUID. Since the extracted solutionID is later interpolated into OData filters and composite-key URLs, non-GUID input can lead to malformed requests or injection-style issues. Consider validating both environmentPrefix and solutionID against the repo’s GUID regex (and returning a clear Invalid solution_id error) before returning solutionID.
There was a problem hiding this comment.
normalizeSolutionID now rejects non-GUID environment and solution segments before they ever reach OData URLs.
| func (c *client) DeleteSolutionGitBranch(ctx context.Context, environmentID, branchID, configurationID, partitionID string) error { | ||
| existingBranch, err := c.lookupAnySolutionGitBranchByPartition(ctx, environmentID, configurationID, partitionID) | ||
| if err != nil { | ||
| if errors.Is(err, customerrors.ErrObjectNotFound) { | ||
| return nil | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| if existingBranch.StatusCode == sourceControlBranchConfigurationStatusInactive { | ||
| return c.waitForSolutionGitBranchRemoval(ctx, environmentID, branchID, configurationID, partitionID) | ||
| } | ||
|
|
||
| environmentHost, err := c.EnvironmentClient.GetEnvironmentHostById(ctx, environmentID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| apiURL := helpers.BuildDataverseApiUrl(environmentHost, buildSourceControlBranchConfigurationCompositeKeyPath(branchID, partitionID), nil) | ||
| headers := http.Header{} | ||
| headers.Set("If-Match", "*") | ||
| resp, err := c.Api.Execute(ctx, nil, http.MethodPatch, apiURL, headers, disableSourceControlBranchConfigurationDto{ | ||
| StatusCode: sourceControlBranchConfigurationStatusInactive, | ||
| }, []int{http.StatusNoContent, http.StatusNotFound, http.StatusForbidden}, nil) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if err := c.Api.HandleForbiddenResponse(resp); err != nil { | ||
| return err | ||
| } | ||
| if resp.HttpResponse.StatusCode == http.StatusNotFound { | ||
| return nil | ||
| } | ||
|
|
||
| return c.waitForSolutionGitBranchRemoval(ctx, environmentID, branchID, configurationID, partitionID) | ||
| } |
There was a problem hiding this comment.
DeleteSolutionGitBranch looks up the existing branch by (configurationID, partitionID), but then performs the disable PATCH using the branchID argument (typically state ID). If the remote row was recreated and the ID drifted, the PATCH can 404, and waitForSolutionGitBranchRemoval will continue to see the existing row and potentially loop until timeout. Use existingBranch.ID (from the lookup) for the PATCH and subsequent removal wait, or explicitly handle ID mismatches by retrying with the discovered ID.
There was a problem hiding this comment.
DeleteSolutionGitBranch now patches and waits on the branch row Dataverse actually returns from the lookup, instead of trusting a possibly stale state ID.
| if err := r.GitIntegrationClient.EnableEnvironmentScopeSolutions(ctx, plan.EnvironmentID.ValueString()); err != nil { | ||
| resp.Diagnostics.AddError(fmt.Sprintf("Client error when enabling environment-scoped solutions for %s", r.FullTypeName()), err.Error()) | ||
| return | ||
| } |
There was a problem hiding this comment.
In Environment scope, the resource proactively enables solutions via EnableEnvironmentScopeSolutions, but the resource does not record which solutions it changed. As a result, Delete cannot reliably revert solution.enabledforsourcecontrolintegration to its prior value, which contradicts the PR description and leaves lasting side-effects after destroy. Consider tracking the set of solution IDs that were enabled by this resource in state (or another durable mechanism) and reverting them on Delete (and potentially when switching away from Environment scope).
| if err := r.GitIntegrationClient.EnableEnvironmentScopeSolutions(ctx, plan.EnvironmentID.ValueString()); err != nil { | |
| resp.Diagnostics.AddError(fmt.Sprintf("Client error when enabling environment-scoped solutions for %s", r.FullTypeName()), err.Error()) | |
| return | |
| } | |
| tflog.Debug(ctx, "Skipping automatic enablement of environment-scoped solutions because the resource does not durably track which solutions it changed and therefore cannot safely revert them on destroy", map[string]interface{}{ | |
| "environment_id": plan.EnvironmentID.ValueString(), | |
| "resource_type": r.FullTypeName(), | |
| }) |
There was a problem hiding this comment.
This is by design, because this is how the make portal works. The intended behaviour is that once "environment" scoped source control is enabled, all unmanaged solutions in that environment lazy bind on first navigation to the git page for them (as an automatic side effect). We don't need to keep track of which ones we've activated, because they all automatically get deactivated when the single environment binding is removed again. They cannot be connected mutually exclusively from each other.
| func (c *client) DeleteSolutionGitBranch(ctx context.Context, environmentID, branchID, configurationID, partitionID string) error { | ||
| existingBranch, err := c.lookupAnySolutionGitBranchByPartition(ctx, environmentID, configurationID, partitionID) | ||
| if err != nil { | ||
| if errors.Is(err, customerrors.ErrObjectNotFound) { | ||
| return nil | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| if existingBranch.StatusCode == sourceControlBranchConfigurationStatusInactive { | ||
| return c.waitForSolutionGitBranchRemoval(ctx, environmentID, branchID, configurationID, partitionID) | ||
| } | ||
|
|
||
| environmentHost, err := c.EnvironmentClient.GetEnvironmentHostById(ctx, environmentID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| apiURL := helpers.BuildDataverseApiUrl(environmentHost, buildSourceControlBranchConfigurationCompositeKeyPath(branchID, partitionID), nil) | ||
| headers := http.Header{} | ||
| headers.Set("If-Match", "*") | ||
| resp, err := c.Api.Execute(ctx, nil, http.MethodPatch, apiURL, headers, disableSourceControlBranchConfigurationDto{ | ||
| StatusCode: sourceControlBranchConfigurationStatusInactive, | ||
| }, []int{http.StatusNoContent, http.StatusNotFound, http.StatusForbidden}, nil) |
There was a problem hiding this comment.
DeleteSolutionGitBranch looks up the current branch row (existingBranch), but the disable PATCH still uses the branchID argument (state ID) to build the composite-key URL. If the row was recreated and the state ID is stale, the PATCH can 404 while the row still exists, and waitForSolutionGitBranchRemoval can loop until timeout. Use existingBranch.ID (and pass that same discovered ID into the subsequent wait) once the lookup succeeds.
There was a problem hiding this comment.
delete now uses the discovered live branch ID, not stale state
| if environmentPrefix != environmentID { | ||
| return "", fmt.Errorf("the `solution_id` value `%s` uses environment `%s`, but this resource is targeting environment `%s`", configuredValue, environmentPrefix, environmentID) | ||
| } |
There was a problem hiding this comment.
In normalizeSolutionID, the environment prefix and the configured environment_id are compared using a raw string inequality. Since GUIDs are case-insensitive and can appear in different canonical forms, this can reject otherwise valid solution_id values (e.g., mixed-case environment IDs). Consider normalizing both values (e.g., parse both as UUIDs and compare their canonical .String() values, or use strings.EqualFold) before returning an error.
There was a problem hiding this comment.
solution_id now compares parsed UUIDs canonically, so mixed-case GUIDs are accepted and invalid environment_id fails loudly.
| case "SolutionScope": | ||
| return scopeSolution | ||
| default: | ||
| return "" |
There was a problem hiding this comment.
GetSourceControlIntegrationScope can return an empty string when the org setting is missing or unrecognized (sourceControlIntegrationScopeFromOrgDbValue default case). That empty value is then written into Terraform state as scope, even though the schema/validators only support Environment or Solution, which can cause perpetual diffs or confusing behavior. Consider treating an unknown/missing value as Solution (Dataverse default) or returning an explicit error diagnostic when the scope cannot be determined.
| return "" | |
| return scopeSolution |
There was a problem hiding this comment.
GetSourceControlIntegrationScope now errors if Dataverse returns an unknown or missing scope instead of writing "" into state.
| projectName := types.StringNull() | ||
| if dto.ProjectName != "" { | ||
| projectName = types.StringValue(dto.ProjectName) | ||
| } | ||
|
|
||
| return EnvironmentGitIntegrationResourceModel{ | ||
| ID: types.StringValue(dto.ID), | ||
| EnvironmentID: types.StringValue(environmentID), | ||
| GitProvider: types.StringValue(gitProviderFromInt(dto.GitProvider)), | ||
| Scope: types.StringValue(scope), | ||
| OrganizationName: types.StringValue(dto.OrganizationName), | ||
| ProjectName: projectName, |
There was a problem hiding this comment.
convertSourceControlConfigurationDtoToModel sets ProjectName to types.StringNull() when the API returns an empty projectname. Since project_name is a required schema attribute, writing null into state can lead to invalid state and/or perpetual diffs after Read/Import. Prefer always setting a concrete string value in state (even if empty), or treat an empty API value as an error and surface a diagnostic so required attributes are never stored as null.
| projectName := types.StringNull() | |
| if dto.ProjectName != "" { | |
| projectName = types.StringValue(dto.ProjectName) | |
| } | |
| return EnvironmentGitIntegrationResourceModel{ | |
| ID: types.StringValue(dto.ID), | |
| EnvironmentID: types.StringValue(environmentID), | |
| GitProvider: types.StringValue(gitProviderFromInt(dto.GitProvider)), | |
| Scope: types.StringValue(scope), | |
| OrganizationName: types.StringValue(dto.OrganizationName), | |
| ProjectName: projectName, | |
| return EnvironmentGitIntegrationResourceModel{ | |
| ID: types.StringValue(dto.ID), | |
| EnvironmentID: types.StringValue(environmentID), | |
| GitProvider: types.StringValue(gitProviderFromInt(dto.GitProvider)), | |
| Scope: types.StringValue(scope), | |
| OrganizationName: types.StringValue(dto.OrganizationName), | |
| ProjectName: types.StringValue(dto.ProjectName), |
There was a problem hiding this comment.
convertSourceControlConfigurationDtoToModel now always writes a concrete project_name string to state, never null.
| func (c *client) EnsureSolutionScopeRootBranch(ctx context.Context, environmentID, configurationID, organizationName, projectName, repositoryName string) error { | ||
| defaultBranch, err := c.GetGitRepositoryDefaultBranch(ctx, environmentID, organizationName, projectName, repositoryName) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| existingBranch, err := c.lookupAnySolutionGitBranchByPartition(ctx, environmentID, configurationID, rootPartitionID) | ||
| if err == nil { |
There was a problem hiding this comment.
EnsureSolutionScopeRootBranch is invoked for both Solution-scope and Environment-scope flows (e.g., EnvironmentGitIntegrationResource calls it when scope == "Environment"). The current name suggests it is only relevant to Solution scope, which makes the code harder to follow. Consider renaming it to something scope-agnostic (e.g., EnsureRootBranchConfiguration) to reflect actual usage.
There was a problem hiding this comment.
renamed EnsureSolutionScopeRootBranch to EnsureRootBranchConfiguration so the helper name matches actual use in both Solution and Environment scope flows.
| func gitProviderFromInt(value int) string { | ||
| switch value { | ||
| case 0: | ||
| return gitProviderAzureDevOps | ||
| default: | ||
| return "" | ||
| } | ||
| } |
There was a problem hiding this comment.
gitProviderFromInt returns an empty string for any Dataverse gitprovider value other than 0. Because git_provider is a required attribute and the resource only supports Azure DevOps, writing "" into state on Read/Import will lead to confusing diffs and hides the real problem (an unsupported remote configuration). Consider surfacing an explicit diagnostic/error when the API returns an unknown provider value, rather than mapping it to an empty string.
mawasile
left a comment
mawasile
left a comment
There was a problem hiding this comment.
Don't we need data sources to read existing integrations?
|
|
||
| resource "powerplatform_environment_git_integration" "example" { | ||
| environment_id = powerplatform_environment.example.id | ||
| git_provider = var.git_provider |
There was a problem hiding this comment.
because .tf files are build directly into docs and therefore ends up as terraform registry documentation, it is better when examples do not have var. values and values are explicit in the .tf files
| @@ -0,0 +1,53 @@ | |||
| variable "environment_display_name" { | |||
There was a problem hiding this comment.
as above, lets move those values inline into .tf file
| use_cli = true | ||
| } | ||
|
|
||
| # Known limitation: Dataverse Git integration currently works only with delegated |
There was a problem hiding this comment.
additionally those new resources that don't support non-interactive should be mentioned in preview section of the README.md file
| MarkdownDescription: "Git provider for the repository binding. Supported value is `AzureDevOps`.", | ||
| Required: true, | ||
| Validators: []validator.String{ | ||
| stringvalidator.OneOf(gitProviderAzureDevOps), |
There was a problem hiding this comment.
if the only supported value if Azure Dev Ops, then there is not need to expose that attribute to the user, we can expose it later if more endpoints would be added
| tflog.Debug(ctx, fmt.Sprintf("METADATA: %s", resp.TypeName)) | ||
| } | ||
|
|
||
| func (r *EnvironmentGitIntegrationResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { |
There was a problem hiding this comment.
in environment scope UI allows to pick branch and git folder, in solution scope root git folder. Why we don't expose those values as attributes?
| defer exitContext() | ||
|
|
||
| resp.Schema = schema.Schema{ | ||
| MarkdownDescription: "Manages a solution-level Dataverse Git branch binding. This maps to the documented `sourcecontrolbranchconfiguration` Dataverse table and links a solution partition to a branch and folder beneath an environment Git integration.\n\nKnown limitation: the underlying Power Platform Git integration bootstrap currently requires delegated user principal authentication with Azure DevOps access. Service principal, app-only, and OIDC pipeline identities are not currently supported by the backing Dataverse Git integration flow.", |
There was a problem hiding this comment.
resource should be added to README.md file, to preview section
| @@ -0,0 +1,3 @@ | |||
| # Solution Git branch resources can be imported using environment_id/git_integration_id/solution_id | |||
| # The final segment can be either the raw Dataverse solution id or the provider-formatted powerplatform_solution.id | |||
| terraform import powerplatform_solution_git_branch.example 00000000-0000-0000-0000-000000000000/11111111-1111-1111-1111-111111111111/00000000-0000-0000-0000-000000000000_22222222-2222-2222-2222-222222222222 | |||
There was a problem hiding this comment.
Shouldn't it look like that?
| terraform import powerplatform_solution_git_branch.example 00000000-0000-0000-0000-000000000000/11111111-1111-1111-1111-111111111111/00000000-0000-0000-0000-000000000000_22222222-2222-2222-2222-222222222222 | |
| terraform import powerplatform_solution_git_branch.example 00000000-0000-0000-0000-000000000000/11111111-1111-1111-1111-111111111111/22222222-2222-2222-2222-222222222222 |
| # user principal authentication that also has Azure DevOps repository access. | ||
| # Service principal, app-only, and OIDC pipeline identities are not supported. | ||
|
|
||
| resource "local_file" "solution_settings_file" { |
There was a problem hiding this comment.
I think we can simplify this .tf example and skip solution import without solution_settings_file
|
|
||
| environment_id = powerplatform_environment.example.id | ||
| git_provider = var.git_provider | ||
| scope = var.scope |
There was a problem hiding this comment.
like in other .tf file, we can do the variable inline, so that the example is more readable in docs
|
|
||
| httpmock.RegisterResponder("POST", "https://00000000-0000-0000-0000-000000000001.crm4.dynamics.com/api/data/v9.0/sourcecontrolbranchconfigurations", | ||
| func(req *http.Request) (*http.Response, error) { | ||
| bodyBytes, err := io.ReadAll(req.Body) |
There was a problem hiding this comment.
returning .json files will be simpler to manage
AdamCoulterOz
changed the title
Summary
This PR adds first-class Dataverse Git integration support to the provider for Azure DevOps-backed environments and solutions.
It introduces:
powerplatform_environment_git_integrationpowerplatform_solution_git_branchRelated Issue
Closes #1104
What’s Included
internal/services/git_integrationpowerplatform_environment_git_integrationpowerplatform_solution_git_branchprovider.goandprovider_test.goBehavior Notes
powerplatform_environment_git_integrationThis resource manages the environment-level Dataverse repository binding.
environment_idscope = "Solution"andscope = "Environment"organization_name,project_name, andrepository_nameagainst Dataverse Git selector endpointsSourceControlIntegrationScopebefore creating the parent Git configuration, matching the Maker UI sequencepowerplatform_solution_git_branchThis resource manages a solution-level Dataverse branch/folder binding.
solution_idinstead of exposingpartition_idpartitionId, matching the Maker UI / Web API behaviorpartitionIdquery resultEnvironmentScopeEnvironmentScopebehavior is different fromSolutionScope, and this PR mirrors the UI-backed behavior validated live:sourcecontrolconfigurationsourcecontrolbranchconfigurationforpartitionId = 00000000-0000-0000-0000-000000000000solution.enabledforsourcecontrolintegration = trueValidation
Has been run locally:
make precommitHas also been manually live-validated against real Dataverse environments for:
SolutionScopeEnvironmentScopeAcceptance Tests
CI-backed acceptance tests are not included in this PR.
Reason:
TestAcccoverage in CI, maintainers will need upstream-managed Azure DevOps org/project/repo/branch access for the acceptance-test identityThis can be added later once an upstream Azure DevOps acceptance fixture is available.
Known Separate Follow-Ups
These are not introduced by this PR, but were encountered during live validation and are tracked separately: