Add git repository file #225

phillebaba · 2020-11-19T21:58:27Z

This change adds a new resource type called azuredevops_git_repository_file which allows users to add files to the repositories they have created. Behavior wise it mimic the file resource in the GitHub provider, so that people who have used it should feel like this is familiar.

The resource does not actually care about commit history but instead aims to make sure that the specified content in the specified file is present. If it is not it will update the content with a new commit.

Fixes #23

simongottschlag · 2020-11-21T09:29:53Z

@xuzhang3 anything else needed here to get this PR merged and released? 👍

xuzhang3 · 2020-11-21T09:35:53Z

@xuzhang3 anything else needed here to get this PR merged and released? 👍
I can help review and merge next week 😄. I need check the PR and the new feature, also the release pipeline will take at least 40min (AccTests and builds).

simongottschlag · 2020-11-21T09:37:50Z

I can help review and merge next week 😄. I need check the PR and the new feature, also the release pipeline will take at least 40min (AccTests and builds).

@xuzhang3 that's awesome! Thank you and have a great weekend 😁👍

xuzhang3 · 2020-11-23T05:51:21Z

@phillebaba Can we add acceptances and document for this new resource?
AccTest Example
Document Example

phillebaba · 2021-01-12T22:30:56Z

Sorry I had to prioritize other things before christmas. I have started working on the tests now as other things rely on this change. Do you think I need to mock the API to get things working @xuzhang3?

xuzhang3 · 2021-01-14T03:00:26Z

Hi @phillebaba , If you cannot create the resource or the test resource depends on the third part service then you can mock the API, otherwise you should create real test resources.
In this scenario, I would suggest create a test project and repo first then you create the repo file and commits, after all resources created the test should check that all the resource is created as expected.
Example test configuration:

resource "azuredevops_project" "project" {
  name       = "Sample Project"
  visibility         = "private"
  version_control    = "Git"
  work_item_template = "Agile"
}

resource "azuredevops_git_repository" "repo" {
  project_id = azuredevops_project.project.id
  name       = "Sample Empty Git Repository"
  initialization {
    init_type = "Clean"
  }
}

resource "azuredevops_git_repository_file" "repo_file" {
  repository_id = azuredevops_git_repository.repo.id
  file = "./test.go"
  content = "#Managed by Terraform"
  branch = ""
  commit_message = "init commit"
  overwrite_on_create = false
}

phillebaba · 2021-01-15T09:31:35Z

@xuzhang3 thanks for the help!

I added some tests that will verify the simple CRUD flow, could you have a look?

xuzhang3

Hi @phillebaba you need add xxx.markdown file for azuredevops_git_repository_file

azuredevops/internal/acceptancetests/resource_git_repository_file_test.go

xuzhang3 · 2021-01-25T08:01:23Z

azuredevops/internal/service/git/resource_git_repository_file.go

+			},
+			"commit_message": {
+				Type:        schema.TypeString,
+				Optional:    true,


I would suggest change commit_message to comment. Since this is a git push comment, can we remove the Computed: true, and change Optional to Required?

I am trying to follow the standards set forward by the GitHub Terraform provider which has a similar resource. I would prefer if we could try to make this resource as similar as possible.

@xuzhang3 and @phillebaba just wondering if you have any final decisions on whether commit_message will change to comment. The reason I ask is, we are forging ahead with a version of the provider in our environment. All other change decisions in this PR will not have a major effect on us in the future but the commit_message change could mean we may have to do a lot of Terraform state manipulation down the track if we choose to go with commit_message and the upstream chooses comment.

@russellmccloy-coles My suggestion is based on the git push api, service use comment as commit-message. I'm ok with commit_message.

In git command, it is called message git commit --message . In API, it is comment. If a simple word can represent the configuration meanings, I prefer the simple one.

azuredevops/internal/service/git/resource_git_repository_file.go

xuzhang3 · 2021-01-25T09:11:30Z

azuredevops/internal/service/git/resource_git_repository_file.go

+	if item != nil {
+		if !overwriteOnCreate {
+			return fmt.Errorf("Refusing to overwrite existing file. Configure `overwrite_on_create` to `true` to override.")
+		} else {
+			changeType = git.VersionControlChangeTypeValues.Edit
+		}
+	}


Suggested change

if item != nil {

if !overwriteOnCreate {

return fmt.Errorf("Refusing to overwrite existing file. Configure `overwrite_on_create` to `true` to override.")

} else {

changeType = git.VersionControlChangeTypeValues.Edit

}

}

if item != nil && !overwriteOnCreate {

return fmt.Errorf("Refusing to overwrite existing file. Configure `overwrite_on_create` to `true` to override.")

}

changeType = git.VersionControlChangeTypeValues.Edit

@xuzhang3 the logic you have suggested here is different to the original logic:

if fails the test TestAccGitRepoFile_CreateAndUpdate

changeType = git.VersionControlChangeTypeValues.Edit always gets run when line 161 is not run

Test result:

API server listening at: 127.0.0.1:42024 === RUN TestAccGitRepoFile_CreateAndUpdate testing.go:673: Step 0 error: errors during apply: Error: The path 'foo.txt' does not exist at commit '64fba23b7b92e632619b0758c1eeb059fa685425' Parameter name: newPush on /tmp/tf-test256550528/main.tf line 2: (source code not available) --- FAIL: TestAccGitRepoFile_CreateAndUpdate (15.91s) FAIL

Whereas in the original code if: item != nil is true no further code gets run

Yes you are right about this one. Idiomatic go seems to be about never using else statements so I will change this as the else statement is not required after the error is returned.

russellmccloy-coles · 2021-02-09T23:29:15Z

Hi @phillebaba , I'm keen to use this feature. Can I help with the PR? I'm happy to make the adjustments as per @xuzhang3 's request, and push it to another branch. Also happy to push to your branch, but I won't have permissions.

cheers
Russ

phillebaba · 2021-02-11T10:59:21Z

@russellmccloy-coles sorry this keeps falling to the bottom of my backlog and I end up picking it up when I have the time. Do you want contributor permissions to my fork in case I become slow at responding again? Just make sure that you have signed the CLA in that case.

@xuzhang3 the majority of the changes requested should be done now. I just need to run through the acceptance tests once more. I will try to be quick to respond from now on so that we can get this PR merged ASAP 😄

russellmccloy-coles · 2021-02-11T22:16:07Z

@russellmccloy-coles sorry this keeps falling to the bottom of my backlog and I end up picking it up when I have the time. Do you want contributor permissions to my fork in case I become slow at responding again? Just make sure that you have signed the CLA in that case.

@xuzhang3 the majority of the changes requested should be done now. I just need to run through the acceptance tests once more. I will try to be quick to respond from now on so that we can get this PR merged ASAP 😄

@phillebaba yes please... that would be great as we have this as our number one focus right now

russellmccloy-coles · 2021-02-15T20:14:06Z

@phillebaba yes please... that would be great as we have this as our number one focus right now

@phillebaba I know you are busy but any luck adding me as a contributor?

azuredevops/internal/service/git/resource_git_repository_file.go

xuzhang3

@phillebaba Two changes:

getCommits use branch in xxx format while the parameter in getLastCommitId is in refs/heads/xxx format
If this resource support import, terraform should be able find the resource though APIs

russellmccloy-coles · 2021-02-21T23:25:47Z

@phillebaba just incase it helps. I have dumped my changes which we now have running in our environment into this branch on your repository:
https://github.com/phillebaba/terraform-provider-azuredevops/tree/feature/russells_code_incase_it_helps_you
Feel free to ignore, just thought i't may help.

Notes:

new function - waitForFilePush
all CRUD functions now call this new method: waitForFilePush
some other minor stuff

xuzhang3 · 2021-03-03T03:05:46Z

@xuzhang3 have you had time to look at this?

terraform import not working

Signed-off-by: Philip Laine <[email protected]>

* update document * update error message * bug fix

xuzhang3

LGTM

drdamour · 2022-01-26T20:42:32Z

is this supposed to detect changes on the repo?
IE:
i use this to crate file X with content X with commit message X
i then modify file Y (no change to content X)
i run tf apply, and i get a change in my tf state

drdamour · 2022-01-26T20:43:58Z

and it's pushing a noop commit...kinda makes this worthless as i happen to enjoy commiting to my repos ;)

phillebaba · 2022-02-14T20:51:56Z

This has honestly been very hard to test as the git API for Azure DevOps is not really documented plus there are a couple of bugs which have turned up from some of my testing. Using a git lib like libgit2 is not really possible so the API is the only option. This is only an initial feature implementation, and if you find issues fixing those would be very helpful for everyone!

drdamour · 2022-02-15T16:12:07Z

@xuzhang3 maybe this resource needs an "expiermental" status or something equivalent to that then

phillebaba · 2022-02-16T08:05:34Z

Why not just fix the underlying bug instead?

xuzhang3 · 2022-03-08T09:03:21Z

@drdamour this resource only cover a minimal features like create/push etc. of the AzGit repository files. Can out be more specific what bug it is or features you want? You can create a new issue for feature requirement.

drdamour · 2022-03-08T14:46:12Z

@xuzhang3 well that's what i'm asking, what's the feature it's supposed to support? becaues it's not just create, everytime i run an apply it applys a commit, that seems wrong

xuzhang3 · 2022-03-09T06:50:55Z

@drdamour this is a bug, this resource get the last commit message from repo and treat it as local update.

drdamour · 2022-03-10T14:09:05Z

i went to open a bug, but saw you opened #557 thx

phillebaba changed the title ~~Add git repository File~~ Add git repository file Nov 19, 2020

simongottschlag mentioned this pull request Nov 20, 2020

Publish new release to Terraform Registry #197

Closed

xuzhang3 added new-resource waiting-response labels Nov 23, 2020

phillebaba mentioned this pull request Jan 12, 2021

Support for azure devops (git implementation) fluxcd/terraform-provider-flux#71

Closed

xuzhang3 closed this Jan 25, 2021

xuzhang3 reopened this Jan 25, 2021

xuzhang3 added size/XXL new-feature and removed waiting-response labels Jan 25, 2021

xuzhang3 self-assigned this Jan 25, 2021

xuzhang3 requested changes Jan 25, 2021

View reviewed changes

phillebaba force-pushed the feature/git-commit branch 4 times, most recently from cf65937 to d8d647e Compare February 11, 2021 10:56

xuzhang3 reviewed Feb 18, 2021

View reviewed changes

azuredevops/internal/service/git/resource_git_repository_file.go Outdated Show resolved Hide resolved

xuzhang3 requested changes Feb 18, 2021

View reviewed changes

Philip Laine and others added 11 commits September 2, 2021 10:38

Add repository file resource

e9df6a3

Fix api and retry errors

f5a9abf

Fix linter errors

fc2b08a

Fix failing test

d57fd31

Retry when doing multiple commits to same repo

1df162f

Remove comment

97e24cd

Add acceptance tests

a7e8b4f

docs: add documentation for resource azuredevops_git_repository_file

e532eb5

Fix review comments

4ace96e

Signed-off-by: Philip Laine <[email protected]>

return error

e8d01a2

Rename default branch back to master and fix smaller issues

42441b1

phillebaba force-pushed the feature/git-commit branch from f37ff39 to 42441b1 Compare September 2, 2021 08:39

Refactor and fix commit update errors

b1026be

phillebaba force-pushed the feature/git-commit branch from 7ad90a2 to b1026be Compare September 2, 2021 11:58

Philip Laine and others added 4 commits September 2, 2021 13:59

Fix return err

d6e05c0

Add retry to file delete

17a4876

git repository file:

607d2b5

* update document * update error message * bug fix

update git repository file doc

f457b13

xuzhang3 approved these changes Sep 3, 2021

View reviewed changes

xuzhang3 merged commit f851c63 into microsoft:master Sep 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add git repository file #225

Add git repository file #225

phillebaba commented Nov 19, 2020

simongottschlag commented Nov 21, 2020

xuzhang3 commented Nov 21, 2020

simongottschlag commented Nov 21, 2020

xuzhang3 commented Nov 23, 2020

phillebaba commented Jan 12, 2021 •

edited

Loading

xuzhang3 commented Jan 14, 2021

phillebaba commented Jan 15, 2021

xuzhang3 left a comment

xuzhang3 Jan 25, 2021

phillebaba Feb 11, 2021

russellmccloy-coles Feb 17, 2021

xuzhang3 Feb 18, 2021

xuzhang3 Feb 18, 2021

xuzhang3 Jan 25, 2021

russellmccloy-coles Feb 11, 2021 •

edited

Loading

phillebaba Feb 11, 2021

russellmccloy-coles commented Feb 9, 2021

phillebaba commented Feb 11, 2021

russellmccloy-coles commented Feb 11, 2021

russellmccloy-coles commented Feb 15, 2021

xuzhang3 left a comment

russellmccloy-coles commented Feb 21, 2021

xuzhang3 commented Mar 3, 2021

xuzhang3 left a comment

drdamour commented Jan 26, 2022

drdamour commented Jan 26, 2022

phillebaba commented Feb 14, 2022

drdamour commented Feb 15, 2022

phillebaba commented Feb 16, 2022

xuzhang3 commented Mar 8, 2022

drdamour commented Mar 8, 2022

xuzhang3 commented Mar 9, 2022

drdamour commented Mar 10, 2022

Add git repository file #225

Add git repository file #225

Conversation

phillebaba commented Nov 19, 2020

simongottschlag commented Nov 21, 2020

xuzhang3 commented Nov 21, 2020

simongottschlag commented Nov 21, 2020

xuzhang3 commented Nov 23, 2020

phillebaba commented Jan 12, 2021 • edited Loading

xuzhang3 commented Jan 14, 2021

phillebaba commented Jan 15, 2021

xuzhang3 left a comment

Choose a reason for hiding this comment

xuzhang3 Jan 25, 2021

Choose a reason for hiding this comment

phillebaba Feb 11, 2021

Choose a reason for hiding this comment

russellmccloy-coles Feb 17, 2021

Choose a reason for hiding this comment

xuzhang3 Feb 18, 2021

Choose a reason for hiding this comment

xuzhang3 Feb 18, 2021

Choose a reason for hiding this comment

xuzhang3 Jan 25, 2021

Choose a reason for hiding this comment

russellmccloy-coles Feb 11, 2021 • edited Loading

Choose a reason for hiding this comment

phillebaba Feb 11, 2021

Choose a reason for hiding this comment

russellmccloy-coles commented Feb 9, 2021

phillebaba commented Feb 11, 2021

russellmccloy-coles commented Feb 11, 2021

russellmccloy-coles commented Feb 15, 2021

xuzhang3 left a comment

Choose a reason for hiding this comment

russellmccloy-coles commented Feb 21, 2021

xuzhang3 commented Mar 3, 2021

xuzhang3 left a comment

Choose a reason for hiding this comment

drdamour commented Jan 26, 2022

drdamour commented Jan 26, 2022

phillebaba commented Feb 14, 2022

drdamour commented Feb 15, 2022

phillebaba commented Feb 16, 2022

xuzhang3 commented Mar 8, 2022

drdamour commented Mar 8, 2022

xuzhang3 commented Mar 9, 2022

drdamour commented Mar 10, 2022

phillebaba commented Jan 12, 2021 •

edited

Loading

russellmccloy-coles Feb 11, 2021 •

edited

Loading