Move gitlab_project_push_rules to gitlab_project.push_rules

[armsnyder]

Supersedes #416 #407 #269 #246
Fixes #352

See the discussion in #416 for some background.

The gitlab_project_push_rules resource has caused a lot of confusion because in GitLab EE, push rules are created implicitly when projects are created. This created a situation where if you wanted to use gitlab_project_push_rules you would always have to import it, and you could not create a project with push rules in the same terraform apply.

The solution in the PR is to remove the gitlab_project_push_rules resource, and add a push_rules attribute to the gitlab_project resource and data source.

I took care to add tests for adding, updating, and removing push rules in both EE and CE.

Note that push_rules is Optional+Computed, meaning that if the user sets one of the arguments, then removes the block entirely, it will not be removed from the upstream GitLab.

[roidelapluie]

Can we just say EE maybe? We don't give that level of details elsewhere IIRC.

Also can we link to upstream doc?

[armsnyder]

I linked to the upstream doc in the resource doc, where push_rules is an argument. This one is the data source doc. Do you still want me to add it here as well?

I searched our docs for "EE" and the only other instance of it I found was in service_github

requires either EE (self-hosted) or Silver and above (GitLab.com).

So in that case it's equally specific. I'm not sure if there are other EE features in the provider that we simply do not mention are EE only? In that case I can remove the mention of EE in the doc.

@nicholasklick You mentioned people were asking for EE features in this provider. Do you think it makes sense for us to document which fields are EE-only, or can we leave that out for simplicity?

[armsnyder]

In my latest push I removed mention of "EE" in the docs change.

[armsnyder]

I also added the doc link to both the data source and the resource. 😉

[roidelapluie]

This is duplicated?

[armsnyder]

Nope, this page is the resource. The earlier one was the data source.

[roidelapluie]

Suggested change

	var errProjectPushRulesUnsupported = errors.New("Project push rules are not supported in your version of GitLab")
	var errProjectPushRulesUnsupported = errors.New("project push rules are not supported in your version of GitLab")

[armsnyder]

In other providers, and in the Hashicorp docs, I have seen them use capitalized errors for errors that are returned from the CRUD functions, because they are user-facing. I moved this error back to inline to ease the confusion. Let me know if you still want me to change it.

[roidelapluie]

an we document like this:

https://registry.terraform.io/providers/hashicorp/kubernetes/latest/docs/resources/api_service

with "Nested blocks"

[armsnyder]

@roidelapluie So you're aware, I changed push_rules to be Optional+Computed. This allowed me to remove the DiffSuppressFunc. It also fixed a minor warning in the TF debug logs when using push_rules:

2020/09/19 12:23:26 [WARN] Provider "registry.terraform.io/-/gitlab" produced an unexpected new value for gitlab_project.this, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .push_rules: block count changed from 0 to 1

The side effect of O+C is if a user removes their push_rules block it will not remove the push rules from upstream. However I found multiple examples of O+C on nested blocks in the aws-terraform-provider.

[roidelapluie]

Awesome!