feature: add project mirror resources to provider #358

lukegriffith · 2020-07-14T09:27:30Z

Implemented functionality for project mirrors, as per issue https://github.com/terraform-providers/terraform-provider-gitlab/issues/150

This was recently added to the go-gitlab library by myself and released in v0.33.0

xanzy/go-gitlab#879

armsnyder

General thing I noticed - you can use %d instead of %v for the mirror ID (int) throughout, in format strings.

Also, tests?

gitlab/resource_gitlab_project_mirror.go

armsnyder

lgtm

armsnyder

sorry, just realized again - please add a test :)

armsnyder · 2020-07-28T23:22:07Z

Can you add a test for the importer as well? You can copy an _import test from another resource without much change.

lukegriffith · 2020-07-28T23:34:10Z

Will do, late GMT so will do tomorrow. Thanks for reviewing and patience much appreciated.

lukegriffith · 2020-08-02T19:43:53Z

Having issues adding the importer at the moment, and quite make sense of the error either. Still working on this, any input would be great - im not too familiar with the import tests, or the workflow it runs. Trying to pin down where the call to gitlab is being made.

armsnyder · 2020-08-03T03:08:29Z

The importer test is fine. The problem occurs after the import, during the read. Your read function uses some attributes that are not set in your import function.

Check the docs for ImportStatePassthrough

https://www.terraform.io/docs/extend/resources/import.html#importer-state-function

Where possible, prefer using schema.ImportStatePassthrough as the Importer State function. This function requires the Read function to be able to refresh the entire resource with d.Id() ONLY. Sometimes it is possible to adjust the resource Read function to replace d.Get() use with d.Id() if they exactly match or add a function that parses the resource ID into the necessary attributes.

jeremad · 2020-08-08T03:44:38Z

You can now rebase on master, go-gitlab has been updated

lukegriffith · 2020-08-28T19:47:10Z

I've got the acceptance tests working, testing manually resource imports fine also.

lukegriffith · 2020-08-28T20:45:46Z

rebased and caused a lot of mess. Resouce is working as expected. Do you require the commit history to be cleared up any further or is this good?

gitlab/resource_gitlab_project_mirror.go

armsnyder · 2020-08-29T05:43:06Z

gitlab/resource_gitlab_project_mirror.go

+	d.Set("enabled", projectMirror.Enabled)
+	d.Set("only_protected_branches", projectMirror.OnlyProtectedBranches)
+	d.Set("keep_divergent_refs", projectMirror.KeepDivergentRefs)
+	d.SetId(buildTwoPartID(projectID, &mirrorID))


Nitpick, but this line is unnecessary since in all cases the ID will have already been set before calling this function.

armsnyder · 2020-08-29T05:43:58Z

gitlab/resource_gitlab_project_mirror_test.go

@@ -0,0 +1,213 @@
+package gitlab
+
+// TODO convert to handle project mirror.


I can't tell what this means.

armsnyder · 2020-08-29T05:46:02Z

gitlab/resource_gitlab_project_mirror_test.go

+				ResourceName:            "gitlab_project_mirror.foo",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"enabled", "mirror_id", "project", "url"},


Why was this needed? The import should result in a fully synced state, ideally.

gitlab/resource_gitlab_project_mirror_test.go

armsnyder · 2020-08-29T05:47:32Z

If you could squash this MR that could be good, because GitHub looks confused. Maybe it's just me but it's showing changes in the diff that aren't yours.

lukegriffith · 2020-08-29T10:21:04Z

That occured once I performed git rebase master and it interlaced the more recent commits.
I will tidy this up from master and address the more recent comments.

Re: import test. Testing manually the resource is imported fine using the terraform cli, the import test does not show any fields. I've copied the test from other resources unsure why this is causing problems as the functionality works.

…e test

lukegriffith · 2020-08-29T11:55:17Z

Import test success, I wasn't setting all properties

armsnyder · 2020-08-29T15:31:44Z

gitlab/resource_gitlab_project_mirror_test.go

+	}
+}
+
+func testAccCheckGitlabProjectMirrorDestroy(s *terraform.State) error {


Just an fyi, a "destroy" test should be testing the behavior of your own resource destroy function, i.e. checking that the mirror gets disabled. There's an open issue about this generally #205 so we can come back and do this later.

sure, I will not make any changes in this PR for this.

armsnyder · 2020-08-29T15:33:32Z

Can you add a new documentation page for this new resource?

lukegriffith · 2020-08-29T17:31:40Z

added docs page.

armsnyder · 2020-08-29T19:39:48Z

Looks good, but can you add the new doc page to the index (like so #391)

lukegriffith · 2020-08-29T20:10:44Z

ack I actually searched for an index, didn't realise it was the erb file. should be done.

armsnyder

Thanks!

lukegriffith · 2020-09-02T13:28:12Z

@armsnyder is there anything I can do to speed this up getting merged? I have some work in the next week that i'd be keen to use this resource for... unsure on the release cadence or the process?

armsnyder · 2020-09-02T15:16:41Z

I don't have merging powers, but @nicholasklick may be able to give an update.

lukegriffith · 2020-09-06T19:51:09Z

Hi @nicholasklick any timeframe on this getting merged?

ghost added size/XL dependencies labels Jul 14, 2020

lukegriffith changed the title ~~Mirror config~~ feature: add project mirror resources to provider Jul 15, 2020

armsnyder requested changes Jul 18, 2020

View reviewed changes

gitlab/resource_gitlab_project_mirror.go Outdated Show resolved Hide resolved

gitlab/resource_gitlab_project_mirror.go Outdated Show resolved Hide resolved

armsnyder reviewed Jul 28, 2020

View reviewed changes

gitlab/resource_gitlab_project_mirror.go Outdated Show resolved Hide resolved

armsnyder approved these changes Jul 28, 2020

View reviewed changes

armsnyder requested changes Jul 28, 2020

View reviewed changes

armsnyder reviewed Aug 29, 2020

View reviewed changes

gitlab/resource_gitlab_project_mirror.go Show resolved Hide resolved

armsnyder reviewed Aug 29, 2020

View reviewed changes

gitlab/resource_gitlab_project_mirror_test.go Show resolved Hide resolved

lukegriffith added 3 commits August 29, 2020 12:08

Adding project miorror and tests

240918b

responding to PR comments, removing import state verify for acceptanc…

ef98ecb

…e test

Setting all properties in SetToState

f763312

removed commented out ignore

c7aa1d4

armsnyder reviewed Aug 29, 2020

View reviewed changes

adding docs page

34de59d

adding to index

279b653

armsnyder approved these changes Aug 29, 2020

View reviewed changes

nicholasklick merged commit f60dc34 into gitlabhq:master Sep 7, 2020

github-actions bot locked and limited conversation to collaborators Nov 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add project mirror resources to provider #358

feature: add project mirror resources to provider #358

lukegriffith commented Jul 14, 2020 •

edited

Loading

armsnyder left a comment

armsnyder left a comment

armsnyder left a comment

armsnyder commented Jul 28, 2020

lukegriffith commented Jul 28, 2020

lukegriffith commented Aug 2, 2020

armsnyder commented Aug 3, 2020 •

edited

Loading

jeremad commented Aug 8, 2020

lukegriffith commented Aug 28, 2020

lukegriffith commented Aug 28, 2020

armsnyder Aug 29, 2020

armsnyder Aug 29, 2020

armsnyder Aug 29, 2020

armsnyder commented Aug 29, 2020

lukegriffith commented Aug 29, 2020

lukegriffith commented Aug 29, 2020

armsnyder Aug 29, 2020

lukegriffith Aug 29, 2020

armsnyder commented Aug 29, 2020

lukegriffith commented Aug 29, 2020

armsnyder commented Aug 29, 2020

lukegriffith commented Aug 29, 2020 •

edited

Loading

armsnyder left a comment

lukegriffith commented Sep 2, 2020

armsnyder commented Sep 2, 2020

lukegriffith commented Sep 6, 2020

		@@ -0,0 +1,213 @@
		package gitlab

		// TODO convert to handle project mirror.

feature: add project mirror resources to provider #358

feature: add project mirror resources to provider #358

Conversation

lukegriffith commented Jul 14, 2020 • edited Loading

armsnyder left a comment

Choose a reason for hiding this comment

armsnyder left a comment

Choose a reason for hiding this comment

armsnyder left a comment

Choose a reason for hiding this comment

armsnyder commented Jul 28, 2020

lukegriffith commented Jul 28, 2020

lukegriffith commented Aug 2, 2020

armsnyder commented Aug 3, 2020 • edited Loading

jeremad commented Aug 8, 2020

lukegriffith commented Aug 28, 2020

lukegriffith commented Aug 28, 2020

armsnyder Aug 29, 2020

Choose a reason for hiding this comment

armsnyder Aug 29, 2020

Choose a reason for hiding this comment

armsnyder Aug 29, 2020

Choose a reason for hiding this comment

armsnyder commented Aug 29, 2020

lukegriffith commented Aug 29, 2020

lukegriffith commented Aug 29, 2020

armsnyder Aug 29, 2020

Choose a reason for hiding this comment

lukegriffith Aug 29, 2020

Choose a reason for hiding this comment

armsnyder commented Aug 29, 2020

lukegriffith commented Aug 29, 2020

armsnyder commented Aug 29, 2020

lukegriffith commented Aug 29, 2020 • edited Loading

armsnyder left a comment

Choose a reason for hiding this comment

lukegriffith commented Sep 2, 2020

armsnyder commented Sep 2, 2020

lukegriffith commented Sep 6, 2020

lukegriffith commented Jul 14, 2020 •

edited

Loading

armsnyder commented Aug 3, 2020 •

edited

Loading

lukegriffith commented Aug 29, 2020 •

edited

Loading