Skip to content

Commit a74f767

Browse files
mhodgsontimofurrer
mhodgson
authored and
timofurrer
committed
Updates from review notes
1 parent 695faa0 commit a74f767

File tree

3 files changed

+49
-103
lines changed

3 files changed

+49
-103
lines changed

‎internal/provider/access_level_helpers.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,6 @@ var validProjectEnvironmentStates = []string{
6565
"available", "stopped",
6666
}
6767

68-
var validGroupSamlLinkAccessLevelNames = []string{
69-
"Guest",
70-
"Reporter",
71-
"Developer",
72-
"Maintainer",
73-
"Owner",
74-
}
75-
7668
var accessLevelNameToValue = map[string]gitlab.AccessLevelValue{
7769
"no one": gitlab.NoPermissions,
7870
"minimal": gitlab.MinimalAccessPermissions,

‎internal/provider/resource_gitlab_group_saml_link.go

Lines changed: 35 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,21 @@ import (
44
"context"
55
"fmt"
66
"log"
7-
"strings"
87

98
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
109
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1110
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1211
gitlab "github.com/xanzy/go-gitlab"
1312
)
1413

14+
var validGroupSamlLinkAccessLevelNames = []string{
15+
"Guest",
16+
"Reporter",
17+
"Developer",
18+
"Maintainer",
19+
"Owner",
20+
}
21+
1522
var _ = registerResource("gitlab_group_saml_link", func() *schema.Resource {
1623
return &schema.Resource{
1724
Description: `The ` + "`gitlab_group_saml_link`" + ` resource allows to manage the lifecycle of an SAML integration with a group.
@@ -22,12 +29,12 @@ var _ = registerResource("gitlab_group_saml_link", func() *schema.Resource {
2229
ReadContext: resourceGitlabGroupSamlLinkRead,
2330
DeleteContext: resourceGitlabGroupSamlLinkDelete,
2431
Importer: &schema.ResourceImporter{
25-
StateContext: resourceGitlabGroupSamlLinkImporter,
32+
StateContext: schema.ImportStatePassthroughContext,
2633
},
2734

2835
Schema: map[string]*schema.Schema{
29-
"group_id": {
30-
Description: "The id of the GitLab group.",
36+
"group": {
37+
Description: "The ID or path of the group to add the SAML Group Link to.",
3138
Type: schema.TypeString,
3239
Required: true,
3340
ForceNew: true,
@@ -45,65 +52,53 @@ var _ = registerResource("gitlab_group_saml_link", func() *schema.Resource {
4552
Required: true,
4653
ForceNew: true,
4754
},
48-
"force": {
49-
Description: "If true, then delete and replace an existing SAML link if one exists.",
50-
Type: schema.TypeBool,
51-
Optional: true,
52-
Default: false,
53-
ForceNew: true,
54-
},
5555
},
5656
}
5757
})
5858

5959
func resourceGitlabGroupSamlLinkCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
6060
client := meta.(*gitlab.Client)
6161

62-
groupId := d.Get("group_id").(string)
62+
group := d.Get("group").(string)
6363
accessLevel := d.Get("access_level").(string)
6464
samlGroupName := d.Get("saml_group_name").(string)
65-
force := d.Get("force").(bool)
6665

6766
options := &gitlab.AddGroupSAMLLinkOptions{
68-
AccessLevel: &accessLevel,
69-
SamlGroupName: &samlGroupName,
70-
}
71-
72-
if force {
73-
if err := resourceGitlabGroupSamlLinkDelete(ctx, d, meta); err != nil {
74-
return err
75-
}
67+
AccessLevel: gitlab.String(accessLevel),
68+
SamlGroupName: gitlab.String(samlGroupName),
7669
}
7770

7871
log.Printf("[DEBUG] Create GitLab group SamlLink %s", d.Id())
79-
SamlLink, _, err := client.Groups.AddGroupSAMLLink(groupId, options, gitlab.WithContext(ctx))
72+
SamlLink, _, err := client.Groups.AddGroupSAMLLink(group, options, gitlab.WithContext(ctx))
8073
if err != nil {
8174
return diag.FromErr(err)
8275
}
8376

84-
d.SetId(buildTwoPartID(&groupId, &SamlLink.Name))
77+
d.SetId(buildTwoPartID(&group, &SamlLink.Name))
8578

8679
return resourceGitlabGroupSamlLinkRead(ctx, d, meta)
8780
}
8881

8982
func resourceGitlabGroupSamlLinkRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
9083
client := meta.(*gitlab.Client)
91-
groupId := d.Get("group_id").(string)
84+
group, samlGroupName, parse_err := parseTwoPartID(d.Id())
85+
if parse_err != nil {
86+
return diag.FromErr(parse_err)
87+
}
9288

9389
// Try to fetch all group links from GitLab
94-
log.Printf("[DEBUG] Read GitLab group SamlLinks %s", groupId)
95-
samlLinks, _, err := client.Groups.ListGroupSAMLLinks(groupId, nil, gitlab.WithContext(ctx))
90+
log.Printf("[DEBUG] Read GitLab group SamlLinks %s", group)
91+
samlLinks, _, err := client.Groups.ListGroupSAMLLinks(group, nil, gitlab.WithContext(ctx))
9692
if err != nil {
9793
return diag.FromErr(err)
9894
}
9995

100-
// If we got here and don't have links, assume GitLab is below version 12.8 and skip the check
10196
if samlLinks != nil {
102-
// Check if the LDAP link exists in the returned list of links
97+
// Check if the SAML link exists in the returned list of links
10398
found := false
10499
for _, samlLink := range samlLinks {
105-
if buildTwoPartID(&groupId, &samlLink.Name) == d.Id() {
106-
d.Set("group_id", groupId)
100+
if samlLink.Name == samlGroupName {
101+
d.Set("group", group)
107102
d.Set("access_level", samlLink.AccessLevel)
108103
d.Set("saml_group_name", samlLink.Name)
109104
found = true
@@ -112,8 +107,9 @@ func resourceGitlabGroupSamlLinkRead(ctx context.Context, d *schema.ResourceData
112107
}
113108

114109
if !found {
110+
log.Printf("[DEBUG] GitLab SAML Group Link %d, group ID %s not found, removing from state", samlGroupName, group)
115111
d.SetId("")
116-
return diag.Errorf("SamlLink %s does not exist.", d.Id())
112+
return nil
117113
}
118114
}
119115

@@ -122,42 +118,20 @@ func resourceGitlabGroupSamlLinkRead(ctx context.Context, d *schema.ResourceData
122118

123119
func resourceGitlabGroupSamlLinkDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
124120
client := meta.(*gitlab.Client)
125-
groupId := d.Get("group_id").(string)
126-
samlGroupName := d.Get("saml_group_name").(string)
121+
group, samlGroupName, parse_err := parseTwoPartID(d.Id())
122+
if parse_err != nil {
123+
return diag.FromErr(parse_err)
124+
}
127125

128126
log.Printf("[DEBUG] Delete GitLab group SamlLink %s", d.Id())
129-
_, err := client.Groups.DeleteGroupSAMLLink(groupId, samlGroupName, cn, gitlab.WithContext(ctx))
127+
_, err := client.Groups.DeleteGroupSAMLLink(group, samlGroupName, gitlab.WithContext(ctx))
130128
if err != nil {
131-
switch err.(type) { // nolint // TODO: Resolve this golangci-lint issue: S1034: assigning the result of this type assertion to a variable (switch err := err.(type)) could eliminate type assertions in switch cases (gosimple)
132-
case *gitlab.ErrorResponse:
133-
// Ignore SAML links that don't exist
134-
if strings.Contains(string(err.(*gitlab.ErrorResponse).Message), "Linked SAML group not found") { // nolint // TODO: Resolve this golangci-lint issue: S1034(related information): could eliminate this type assertion (gosimple)
135-
log.Printf("[WARNING] %s", err)
136-
} else {
137-
return diag.FromErr(err)
138-
}
139-
default:
129+
if is404(err) {
130+
log.Printf("[WARNING] %s", err)
131+
} else {
140132
return diag.FromErr(err)
141133
}
142134
}
143135

144136
return nil
145137
}
146-
147-
func resourceGitlabGroupSamlLinkImporter(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
148-
parts := strings.SplitN(d.Id(), ":", 2)
149-
if len(parts) != 2 {
150-
return nil, fmt.Errorf("invalid saml link import id (should be <group id>:<saml group name>): %s", d.Id())
151-
}
152-
153-
groupId, samlGroupName := parts[0], parts[1]
154-
d.SetId(buildTwoPartID(&groupId, &samlGroupName))
155-
d.Set("group_id", groupId)
156-
d.Set("force", false)
157-
158-
diag := resourceGitlabGroupSamlLinkRead(ctx, d, meta)
159-
if diag.HasError() {
160-
return nil, fmt.Errorf("%s", diag[0].Summary)
161-
}
162-
return []*schema.ResourceData{d}, nil
163-
}

‎internal/provider/resource_gitlab_group_saml_link_test.go

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,13 @@ func TestAccGitlabGroupSamlLink_basic(t *testing.T) {
3232
// Create a group SAML link as a developer (uses testAccGitlabGroupLdapSamlCreateConfig for Config)
3333
{
3434
SkipFunc: isRunningInCE,
35-
Config: testAccGitlabGroupSamlLinkCreateConfig(rInt, &testSamlLink),
35+
Config: fmt.Sprintf(`
36+
resource "gitlab_group_saml_link" "foo" {
37+
group_id = "%d"
38+
access_level = "Developer"
39+
saml_group_name = "%s"
40+
41+
}`, rInt, rInt, testSamlLink.Name),
3642
Check: resource.ComposeTestCheckFunc(
3743
testAccCheckGitlabGroupSamlLinkExists(resourceName, &samlLink)),
3844
},
@@ -49,7 +55,12 @@ func TestAccGitlabGroupSamlLink_basic(t *testing.T) {
4955
// Update the group SAML link to change the access level (uses testAccGitlabGroupSamlLinkUpdateConfig for Config)
5056
{
5157
SkipFunc: isRunningInCE,
52-
Config: testAccGitlabGroupSamlLinkUpdateConfig(rInt, &testSamlLink),
58+
Config: fmt.Sprintf(`
59+
resource "gitlab_group_saml_link" "foo" {
60+
group_id = "%d"
61+
access_level = "Maintainer"
62+
saml_group_name = "%s"
63+
}`, rInt, rInt, testSamlLink.Name),
5364
Check: resource.ComposeTestCheckFunc(
5465
testAccCheckGitlabGroupSamlLinkExists(resourceName, &samlLink)),
5566
},
@@ -155,35 +166,4 @@ func testAccGetGitlabGroupSamlLink(samlLink *gitlab.SAMLGroupLink, resourceState
155166
}
156167

157168
return nil
158-
}
159-
160-
func testAccGitlabGroupSamlLinkCreateConfig(rInt int, testSamlLink *gitlab.SAMLGroupLink) string {
161-
return fmt.Sprintf(`
162-
resource "gitlab_group" "foo" {
163-
name = "foo%d"
164-
path = "foo%d"
165-
description = "Terraform acceptance test - Group SAML Links 1"
166-
}
167-
168-
resource "gitlab_group_saml_link" "foo" {
169-
group_id = "${gitlab_group.foo.id}"
170-
access_level = "Developer"
171-
saml_group_name = "%s"
172-
173-
}`, rInt, rInt, testSamlLink.Name)
174-
}
175-
176-
func testAccGitlabGroupSamlLinkUpdateConfig(rInt int, testSamlLink *gitlab.SAMLGroupLink) string {
177-
return fmt.Sprintf(`
178-
resource "gitlab_group" "foo" {
179-
name = "foo%d"
180-
path = "foo%d"
181-
description = "Terraform acceptance test - Group SAML Links 2"
182-
}
183-
184-
resource "gitlab_group_saml_link" "foo" {
185-
group_id = "${gitlab_group.foo.id}"
186-
access_level = "Maintainer"
187-
saml_group_name = "%s"
188-
}`, rInt, rInt, testSamlLink.Name)
189-
}
169+
}

0 commit comments

Comments
 (0)