Add new data source gitlab_project_membership

[tommyknows]

This PR adds a new data source gitlab_project_membership which can get all members of a project.

Similar to the group_membership, but targeting projects. Also allows filtering by access level.

[klausenbusk]

This sounds useful for: https://github.com/gitlabhq/terraform-provider-gitlab/issues/453#issuecomment-789781249.

@roidelapluie could you have a look?

[timofurrer]

Please rebase to latest master. Otherwise LGTM!

[github-actions]

This pull request has merge conflicts. Please rebase your branch onto master.

[timofurrer]

@tommyknows do you still want to work on this? I think it might be good candidate for the next release!

[tommyknows]

Sorry for the delay. While I don't need this anymore, I'll be happy to have a look at rebasing this. Sounds like there's a bit more to do here, I'll try to tackle this until Monday!

[github-actions]

Conflicts are resolved. Thank you! 😀

[tommyknows]

Not quite sure why the test is failing, but my assumption is that there's a user that's added by default to a project with the "maintainer" level?

(I expect that the terraform user that creates the test project is the Owner, so I'm not sure where that other user could be coming from; the group / user that contains this project, maybe?)

[armsnyder]

Something to note is that this new resource has an attribute access_level, which is not present on the upstream API. The API docs only mention a query attribute.

https://docs.gitlab.com/ee/api/members.html#list-all-members-of-a-group-or-project

If this query parameter acts as a url-encoded query string, then I think it would be alright to have attributes on the resource that are used to form up that query parameter. However, as the code is now, it is doing the filtering after the API call. This is out-of-scope for what a provider should be responsible for.

Can you update the code to set the Query field? (https://github.com/xanzy/go-gitlab/blob/ae600d99fa047c58bc2de89f27d060f542601ed5/project_members.go#L39)

Unfortunately the API docs don't have an example of how to set this field, but either way, we shouldn't be adding custom fields to resources. (With some exceptions, like configuring terraform behavior.)

[tommyknows]

we shouldn't be adding custom fields to resources. (With some exceptions, like configuring terraform behavior.)

Just FYI: the group membership data source also supports such an attribute and does the same filtering, which is why I have added it.

I'll be happy to remove it though.

However, I've also just seen why the test is failing:

Due to an issue, projects in personal namespaces don’t show owner (50) permission for owner.

(from the docs you linked :) )

Instead, they're given / shown maintainer permissions, which explains the test failures.

[armsnyder]

Thanks, I still think we should either remove the attribute or drive it using query instead of custom code, even though the code was copied from a similar resource.

Terraform users can always use Terraform functions to filter the output. So the filtering logic in the provider is doubly redundant. 😄

[armsnyder]

For reasoning, see: https://www.terraform.io/plugin/hashicorp-provider-design-principles#resource-and-attribute-schema-should-closely-match-the-underlying-api

[tommyknows]

I'm totally fine with that decision, just wanted to mention why I did it :-)

I'll remove the attribute then.

[tommyknows]

Alright, implemented your changes as suggested!

[armsnyder]

lgtm

Looking at ISOTime some more, it might be truncating time at midnight due to the custom unmarshal function.

@timofurrer Can you look and see if you agree with the use of RFC1113 here, regardless of this?

[timofurrer]

I think this is still relevant :) I've just rebased and pushed, let's see. Since it's been a while I'll give it another review once the tests pass.

[github-actions]

This pull request has merge conflicts. Please rebase your branch onto main.

[github-actions]

Marking this pull request as stale due to 30 days of inactivity. If this pull request receives no comments in the next 14 days it will be closed. Maintainers can also remove the stale label.

To help this pull request get reviewed, please check that it is rebased onto the latest and is passing automated checks. It also helps if you could reference an issue that the pull request resolves, and create one if it doesn't exist.

[github-actions]

Marking this pull request as stale due to 30 days of inactivity. If this pull request receives no comments in the next 14 days it will be closed. Maintainers can also remove the stale label.

To help this pull request get reviewed, please check that it is rebased onto the latest and is passing automated checks. It also helps if you could reference an issue that the pull request resolves, and create one if it doesn't exist.

[github-actions]

Conflicts are resolved. Thank you! 😀

[timofurrer]

LGTM 🎉

stale

[github-actions]

This functionality has been released in v3.17.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!