Add SAML Group link resource #1215

mhodgson · 2022-08-16T17:04:04Z

Description

Closes #503, #1214

BLOCKED: Note this requires an update to the Gitlab Go Client. a PR for the required update is here: xanzy/go-gitlab#1527

PR Checklist Acknowledgement

I acknowledge that all of the following items are true, where applicable:
- Resource attributes match 1:1 the names and structure of the API resource in the GitLab API documentation.
- Examples are updated with:
  - A *.tf file for the resource/s with at least one usage example
  - A *.sh file for the resource/s with an import example (if applicable)
- New resources have at minimum a basic test with three steps:
  - Create the resource
  - Update the attributes
  - Import the resource
- No new //lintignore comments were copied from existing code. (Linter rules are meant to be enforced on new code.)

github-actions

Welcome @mhodgson 👋

It looks like this is your first submission to the Terraform GitLab Provider! If you haven’t already done so, please make sure you have checked out our CONTRIBUTING.md guide to make sure your contribution has all the necessary elements in place for a successful approval.

Thanks again, and welcome to the community! 😃

PatrickRice-KSC · 2022-08-16T17:40:18Z

nice @mhodgson - thanks for the submission! I've thrown a watch on the xanzy repo upstream to see when that's merged. @timofurrer - were you added a maintainer there for reviews? I know we'd talked about that in one of the community hours.

timofurrer

Hey @mhodgson 👋

Thanks for the contribution! I think it's going into the right direction 🎉

I've added a few suggestions and things to align with the other recently added resources and styles and a some nitpicks.

Back to you 🏓

internal/provider/resource_gitlab_group_saml_link.go

internal/provider/resource_gitlab_group_saml_link_test.go

internal/provider/access_level_helpers.go

timofurrer · 2022-08-16T17:44:11Z

nice @mhodgson - thanks for the submission! I've thrown a watch on the xanzy repo upstream to see when that's merged. @timofurrer - were you added a maintainer there for reviews? I know we'd talked about that in one of the community hours.

No, I haven't unfortunately. However, I've just given the change set here an initial review! @PatrickRice-KSC feel free to add your comments to and @mhodgson let us know if you need anything! I'm looking forward to have this resource in the provider 🎉

mhodgson · 2022-08-16T18:00:05Z

@timofurrer just as a general FYI, I mostly copied the gitlab_group_ldap_link existing resource and code since it is very similar. Looking through your comments now, thanks for the quick review!

timofurrer · 2022-08-16T18:10:14Z

@timofurrer just as a general FYI, I mostly copied the gitlab_group_ldap_link existing resource and code since it is very similar. Looking through your comments now, thanks for the quick review!

Yeah ... Unfortunately, some resources are quite old and desperately need some love. Sorry for the inconvenience 🙏

timofurrer · 2022-08-17T07:05:55Z

@mhodgson @PatrickRice-KSC see my comment here: xanzy/go-gitlab#1527 (comment)

I think if the upstream MR can get into 15.3 we can simplify this resource a bit and don't have to search for the proper saml group link in response from the list endpoint. If it doesn't make it - we can merge as-is I guess.

PatrickRice-KSC · 2022-08-20T03:21:38Z

@mhodgson - in case you didn't see the comment thread on the upstream gitlab issue, the API has been updated to accept the integer values instead of the string values to match other APIs 🙂

timofurrer · 2022-08-22T08:19:23Z

@mhodgson I've integrated the latest go-gitlab release in main (#1224). Any change for you to rebase and update this PR accordingly for a release tomorrow?

While working on gitlabhq/terraform-provider-gitlab#1215 I have noticed that the access level type introcued in xanzy#1527 is wrong.

timofurrer · 2022-08-23T06:53:17Z

@mhodgson I've added an additional fix in go-gitlab: xanzy/go-gitlab#1531

Could you please enable that maintainer of this repository can change the PR change set? I've added some cleanup things on a commit on this branch: https://github.com/gitlabhq/terraform-provider-gitlab/pull/new/feature/saml-group-links

mhodgson · 2022-08-23T13:41:36Z

@timofurrer do you know how I enable that? I don't see that permission.

timofurrer · 2022-08-23T13:50:25Z

@mhodgson it's this setting here: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

mhodgson · 2022-08-23T15:14:04Z

@timofurrer I'm very sorry, but I do not see anything that resembles what the document you link suggests.

mhodgson · 2022-08-23T15:19:50Z

@timofurrer I cherry-picked your commit.

timofurrer · 2022-08-23T18:04:32Z

@mhodgson you also need to rebase to latest main to get the go-gitlab update :)

mhodgson · 2022-08-23T18:54:08Z

@timofurrer how do we exclude < 15.3 from the acceptance test matrix for the tests for this resource?

timofurrer · 2022-08-23T18:59:55Z

@mhodgson Good point!

You can use this pattern:

terraform-provider-gitlab/internal/provider/data_source_gitlab_cluster_agent_test.go

Line 15 in 47e8ddd

testAccRequiresAtLeast(t, "14.10")

mhodgson · 2022-08-23T19:49:05Z

@timofurrer I think it's good to go now. Looks like it just needs approval from your review.

timofurrer · 2022-08-24T16:13:19Z

@mhodgson btw. something went wrong with your latest rebase I think (looking at the diff) 🤔

github-actions · 2022-08-24T16:20:51Z

Conflicts are resolved. Thank you! 😀

mhodgson · 2022-08-24T16:21:33Z

@timofurrer sorry, rebased against origin instead of upstream. Rookie mistake...

I wouldn't say this is super urgent, but not sure on timeline for next release. I'd like to try to get it in the next few weeks if possible.

timofurrer · 2022-08-24T16:23:00Z

@mhodgson AFAIK, this will get into a bug fix release 15.3.2 - not sure when it's released though. You may follow it a little here:

PatrickRice-KSC · 2022-08-24T16:53:33Z

@mhodgson Fair point! The reason to wait for me is to make sure it really works - which it didn't until my latest comments for the latest changes on SaaS (I know this has been very confusing and changed last second). However, we can test this manually if it's urgent for you and release it sooner.

I think it makes sense to push it with a bugfix version just because it's so widely used, and would be extremely nice to use within the provider (we plan to use it as soon as it's available too 🙂 ).

timofurrer · 2022-08-31T09:49:46Z

@mhodgson I've integrated the latest GitLab bugfix release in #1238 which includes the required changes in the SAML API.

Do you have time to rebase this PR today? I'd be ready for a release today with #1239 .

timofurrer · 2022-08-31T10:33:18Z

@mhodgson false alarm :'( I've just learned that apparently this didn't make it into 15.3.2 as was planned: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96117#note_1083680405

While working on gitlabhq/terraform-provider-gitlab#1215 I have noticed that the access level type introcued in xanzy#1527 is wrong.

timofurrer · 2022-09-05T16:34:04Z

I'm closing this in favor of #1243 - in the spirits of releasing today!

Thanks @mhodgson for the nice contribution 🎉

github-actions · 2022-09-05T18:06:16Z

This functionality has been released in v3.18.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!

github-actions bot added documentation provider resource Adds or modifies a resource tests size/L labels Aug 16, 2022

github-actions bot reviewed Aug 16, 2022

View reviewed changes

mhodgson mentioned this pull request Aug 16, 2022

Add Group SAML Links xanzy/go-gitlab#1527

Merged

PatrickRice-KSC added this to the v3.17.0 milestone Aug 16, 2022

timofurrer requested changes Aug 16, 2022

View reviewed changes

timofurrer assigned mhodgson Aug 16, 2022

timofurrer linked an issue Aug 16, 2022 that may be closed by this pull request

Request for GitLab support of SAML Group Links #1214

Closed

1 task

timofurrer added a commit to timofurrer/go-gitlab that referenced this pull request Aug 23, 2022

Fix SAML Group Links Access Level type

c6383e6

While working on gitlabhq/terraform-provider-gitlab#1215 I have noticed that the access level type introcued in xanzy#1527 is wrong.

timofurrer mentioned this pull request Aug 23, 2022

Fix SAML Group Links Access Level type xanzy/go-gitlab#1531

Merged

mhodgson force-pushed the feature/saml-group-links branch from 43fb2cf to 6a43bd9 Compare August 23, 2022 18:06

mhodgson and others added 9 commits August 24, 2022 16:20

Add SAML Group link resource

4911122

Add examples for Group SAML links

118db31

Fix comma error

056a648

Fix other formatting errors

b290a36

Updates from review notes

3cbc960

Update SAML link read to use new Get endpoint

6045e89

Various fixes in SAML group link resource

ce93dd1

Don't run acceptance tests on less than 15.3

ef998f2

Requested updates to SAML link resource

857a9c4

mhodgson force-pushed the feature/saml-group-links branch from 6d68d16 to 857a9c4 Compare August 24, 2022 16:20

github-actions bot removed the merge-conflict PR cannot be merged due to a merge conflict label Aug 24, 2022

github-actions bot removed the dependencies label Aug 24, 2022

This was referenced Aug 31, 2022

Draft changelog for 3.18.0 #1239

Merged

Upgrade to latest GitLab releases #1238

Merged

timofurrer removed the blocked We are currently blocked to implement this label Aug 31, 2022

gravis pushed a commit to gravis/go-gitlab that referenced this pull request Sep 1, 2022

Fix SAML Group Links Access Level type

3e8acaa

While working on gitlabhq/terraform-provider-gitlab#1215 I have noticed that the access level type introcued in xanzy#1527 is wrong.

gravis pushed a commit to gravis/go-gitlab that referenced this pull request Sep 1, 2022

Fix SAML Group Links Access Level type

d3055f8

While working on gitlabhq/terraform-provider-gitlab#1215 I have noticed that the access level type introcued in xanzy#1527 is wrong.

This was referenced Sep 5, 2022

Upgrade testing matrix to GitLab 15.3.3 to allow SAML tests #1242

Merged

resource/gitlab_group_saml_link: Add new resource #1243

Merged

timofurrer closed this Sep 5, 2022

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SAML Group link resource #1215

Add SAML Group link resource #1215

mhodgson commented Aug 16, 2022 •

edited

github-actions bot left a comment

PatrickRice-KSC commented Aug 16, 2022

timofurrer left a comment

timofurrer commented Aug 16, 2022

mhodgson commented Aug 16, 2022

timofurrer commented Aug 16, 2022

timofurrer commented Aug 17, 2022

PatrickRice-KSC commented Aug 20, 2022

timofurrer commented Aug 22, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 24, 2022

github-actions bot commented Aug 24, 2022

mhodgson commented Aug 24, 2022

timofurrer commented Aug 24, 2022

PatrickRice-KSC commented Aug 24, 2022 •

edited

timofurrer commented Aug 31, 2022

timofurrer commented Aug 31, 2022

timofurrer commented Sep 5, 2022

github-actions bot commented Sep 5, 2022

Add SAML Group link resource #1215

Add SAML Group link resource #1215

Conversation

mhodgson commented Aug 16, 2022 • edited

Description

PR Checklist Acknowledgement

github-actions bot left a comment

Choose a reason for hiding this comment

PatrickRice-KSC commented Aug 16, 2022

timofurrer left a comment

Choose a reason for hiding this comment

timofurrer commented Aug 16, 2022

mhodgson commented Aug 16, 2022

timofurrer commented Aug 16, 2022

timofurrer commented Aug 17, 2022

PatrickRice-KSC commented Aug 20, 2022

timofurrer commented Aug 22, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 23, 2022

mhodgson commented Aug 23, 2022

timofurrer commented Aug 24, 2022

github-actions bot commented Aug 24, 2022

mhodgson commented Aug 24, 2022

timofurrer commented Aug 24, 2022

PatrickRice-KSC commented Aug 24, 2022 • edited

timofurrer commented Aug 31, 2022

timofurrer commented Aug 31, 2022

timofurrer commented Sep 5, 2022

github-actions bot commented Sep 5, 2022

mhodgson commented Aug 16, 2022 •

edited

PatrickRice-KSC commented Aug 24, 2022 •

edited