Skip to content

Commit e75ff95

Browse files
timofurrertimofurrer
authored
Merge pull request #881 from timofurrer/bugfix/R004-issue-436
Fix incompatible assignments for ResourceData.Set() reported by tfproviderlint. Closes #436
2 parents cab5d6d + 5cce4ff commit e75ff95

3 files changed

+54
-80
lines changed

‎internal/provider/data_source_gitlab_project_protected_branch.go

+15-31
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,10 @@ func dataSourceGitlabProjectProtectedBranchRead(ctx context.Context, d *schema.R
9898
return diag.Errorf("error getting protected branch (Project: %v / Name %v): %v", project, name, err)
9999
}
100100

101-
// lintignore:R004 // TODO: Resolve this tfproviderlint issue
102-
if err := d.Set("push_access_levels", convertBranchAccessDescriptionsToStateBranchAccessDescriptions(pb.PushAccessLevels)); err != nil {
101+
if err := d.Set("push_access_levels", flattenBranchAccessDescriptions(pb.PushAccessLevels)); err != nil {
103102
return diag.FromErr(err)
104103
}
105-
// lintignore:R004 // TODO: Resolve this tfproviderlint issue
106-
if err := d.Set("merge_access_levels", convertBranchAccessDescriptionsToStateBranchAccessDescriptions(pb.MergeAccessLevels)); err != nil {
104+
if err := d.Set("merge_access_levels", flattenBranchAccessDescriptions(pb.MergeAccessLevels)); err != nil {
107105
return diag.FromErr(err)
108106
}
109107
if err := d.Set("allow_force_push", pb.AllowForcePush); err != nil {
@@ -118,33 +116,19 @@ func dataSourceGitlabProjectProtectedBranchRead(ctx context.Context, d *schema.R
118116
return nil
119117
}
120118

121-
type stateBranchAccessDescription struct {
122-
AccessLevel string `json:"access_level" mapstructure:"access_level"`
123-
AccessLevelDescription string `json:"access_level_description" mapstructure:"access_level_description"`
124-
GroupID int `json:"group_id,omitempty" mapstructure:"group_id,omitempty"`
125-
UserID int `json:"user_id,omitempty" mapstructure:"user_id,omitempty"`
126-
}
127-
128-
func convertBranchAccessDescriptionsToStateBranchAccessDescriptions(descriptions []*gitlab.BranchAccessDescription) []stateBranchAccessDescription {
129-
result := make([]stateBranchAccessDescription, 0)
130-
119+
func flattenBranchAccessDescriptions(descriptions []*gitlab.BranchAccessDescription) (values []map[string]interface{}) {
131120
for _, description := range descriptions {
132-
result = append(result, convertBranchAccessDescriptionToStateBranchAccessDescription(description))
133-
}
134-
135-
return result
136-
}
137-
138-
func convertBranchAccessDescriptionToStateBranchAccessDescription(description *gitlab.BranchAccessDescription) stateBranchAccessDescription {
139-
stateDescription := stateBranchAccessDescription{
140-
AccessLevel: accessLevelValueToName[description.AccessLevel],
141-
AccessLevelDescription: description.AccessLevelDescription,
142-
}
143-
if description.UserID != 0 {
144-
stateDescription.UserID = description.UserID
145-
}
146-
if description.GroupID != 0 {
147-
stateDescription.GroupID = description.GroupID
121+
v := map[string]interface{}{
122+
"access_level": accessLevelValueToName[description.AccessLevel],
123+
"access_level_description": description.AccessLevelDescription,
124+
}
125+
if description.UserID != 0 {
126+
v["user_id"] = description.UserID
127+
}
128+
if description.GroupID != 0 {
129+
v["group_id"] = description.GroupID
130+
}
131+
values = append(values, v)
148132
}
149-
return stateDescription
133+
return values
150134
}

‎internal/provider/data_source_gitlab_project_protected_branches.go

+17-22
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,6 @@ var _ = registerDataSource("gitlab_project_protected_branches", func() *schema.R
5757
}
5858
})
5959

60-
type stateProtectedBranch struct {
61-
ID int `json:"id,omitempty" mapstructure:"id,omitempty"`
62-
Name string `json:"name,omitempty" mapstructure:"name,omitempty"`
63-
PushAccessLevels []stateBranchAccessDescription `json:"push_access_levels,omitempty" mapstructure:"push_access_levels,omitempty"`
64-
MergeAccessLevels []stateBranchAccessDescription `json:"merge_access_levels,omitempty" mapstructure:"merge_access_levels,omitempty"`
65-
AllowForcePush bool `json:"allow_force_push,omitempty" mapstructure:"allow_force_push,omitempty"`
66-
CodeOwnerApprovalRequired bool `json:"code_owner_approval_required,omitempty" mapstructure:"code_owner_approval_required,omitempty"`
67-
}
68-
6960
func dataSourceGitlabProjectProtectedBranchesRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
7061
client := meta.(*gitlab.Client)
7162

@@ -78,7 +69,7 @@ func dataSourceGitlabProjectProtectedBranchesRead(ctx context.Context, d *schema
7869
return diag.FromErr(err)
7970
}
8071

81-
allProtectedBranches := make([]stateProtectedBranch, 0)
72+
var allProtectedBranches []*gitlab.ProtectedBranch
8273
totalPages := -1
8374
opts := &gitlab.ListProtectedBranchesOptions{}
8475
for opts.Page = 0; opts.Page != totalPages; opts.Page++ {
@@ -88,20 +79,10 @@ func dataSourceGitlabProjectProtectedBranchesRead(ctx context.Context, d *schema
8879
return diag.FromErr(err)
8980
}
9081
totalPages = resp.TotalPages
91-
for _, pb := range pbs {
92-
allProtectedBranches = append(allProtectedBranches, stateProtectedBranch{
93-
ID: pb.ID,
94-
Name: pb.Name,
95-
PushAccessLevels: convertBranchAccessDescriptionsToStateBranchAccessDescriptions(pb.PushAccessLevels),
96-
MergeAccessLevels: convertBranchAccessDescriptionsToStateBranchAccessDescriptions(pb.MergeAccessLevels),
97-
AllowForcePush: pb.AllowForcePush,
98-
CodeOwnerApprovalRequired: pb.CodeOwnerApprovalRequired,
99-
})
100-
}
82+
allProtectedBranches = append(allProtectedBranches, pbs...)
10183
}
10284

103-
// lintignore:R004 // TODO: Resolve this tfproviderlint issue
104-
if err := d.Set("protected_branches", allProtectedBranches); err != nil {
85+
if err := d.Set("protected_branches", flattenProtectedBranches(allProtectedBranches)); err != nil {
10586
return diag.FromErr(err)
10687
}
10788

@@ -114,3 +95,17 @@ func dataSourceGitlabProjectProtectedBranchesRead(ctx context.Context, d *schema
11495

11596
return nil
11697
}
98+
99+
func flattenProtectedBranches(protectedBranches []*gitlab.ProtectedBranch) (values []map[string]interface{}) {
100+
for _, protectedBranch := range protectedBranches {
101+
values = append(values, map[string]interface{}{
102+
"id": protectedBranch.ID,
103+
"name": protectedBranch.Name,
104+
"push_access_levels": flattenBranchAccessDescriptions(protectedBranch.PushAccessLevels),
105+
"merge_access_levels": flattenBranchAccessDescriptions(protectedBranch.MergeAccessLevels),
106+
"allow_force_push": protectedBranch.AllowForcePush,
107+
"code_owner_approval_required": protectedBranch.CodeOwnerApprovalRequired,
108+
})
109+
}
110+
return values
111+
}

‎internal/provider/resource_gitlab_branch_protection.go

+22-27
Original file line numberDiff line numberDiff line change
@@ -169,30 +169,30 @@ func resourceGitlabBranchProtectionRead(ctx context.Context, d *schema.ResourceD
169169
d.Set("project", project)
170170
d.Set("branch", pb.Name)
171171

172-
pushAccessLevels := convertAllowedAccessLevelsToBranchAccessDescriptions(pb.PushAccessLevels)
173-
if len(pushAccessLevels) > 0 {
174-
if err := d.Set("push_access_level", pushAccessLevels[0].AccessLevel); err != nil {
172+
if pushAccessLevel, err := firstValidAccessLevel(pb.PushAccessLevels); err == nil {
173+
if err := d.Set("push_access_level", accessLevelValueToName[*pushAccessLevel]); err != nil {
175174
return diag.Errorf("error setting push_access_level: %v", err)
176175
}
176+
} else {
177+
return diag.Errorf("unable to get push_access_level: %v", err)
177178
}
178179

179-
mergeAccessLevels := convertAllowedAccessLevelsToBranchAccessDescriptions(pb.MergeAccessLevels)
180-
if len(mergeAccessLevels) > 0 {
181-
if err := d.Set("merge_access_level", mergeAccessLevels[0].AccessLevel); err != nil {
180+
if mergeAccessLevels, err := firstValidAccessLevel(pb.MergeAccessLevels); err == nil {
181+
if err := d.Set("merge_access_level", accessLevelValueToName[*mergeAccessLevels]); err != nil {
182182
return diag.Errorf("error setting merge_access_level: %v", err)
183183
}
184+
} else {
185+
return diag.Errorf("unable to get merge_access_level: %v", err)
184186
}
185187

186188
if err := d.Set("allow_force_push", pb.AllowForcePush); err != nil {
187189
return diag.Errorf("error setting allow_force_push: %v", err)
188190
}
189191

190-
// lintignore: R004 // TODO: Resolve this tfproviderlint issue
191-
if err := d.Set("allowed_to_push", convertAllowedToToBranchAccessDescriptions(pb.PushAccessLevels)); err != nil {
192+
if err := d.Set("allowed_to_push", flattenNonZeroBranchAccessDescriptions(pb.PushAccessLevels)); err != nil {
192193
return diag.Errorf("error setting allowed_to_push: %v", err)
193194
}
194-
// lintignore: R004 // TODO: Resolve this tfproviderlint issue
195-
if err := d.Set("allowed_to_merge", convertAllowedToToBranchAccessDescriptions(pb.MergeAccessLevels)); err != nil {
195+
if err := d.Set("allowed_to_merge", flattenNonZeroBranchAccessDescriptions(pb.MergeAccessLevels)); err != nil {
196196
return diag.Errorf("error setting allowed_to_merge: %v", err)
197197
}
198198

@@ -284,36 +284,31 @@ func schemaAllowedTo() *schema.Schema {
284284
}
285285
}
286286

287-
func convertAllowedAccessLevelsToBranchAccessDescriptions(descriptions []*gitlab.BranchAccessDescription) []stateBranchAccessDescription {
288-
result := make([]stateBranchAccessDescription, 0)
289-
287+
func firstValidAccessLevel(descriptions []*gitlab.BranchAccessDescription) (*gitlab.AccessLevelValue, error) {
290288
for _, description := range descriptions {
291289
if description.UserID != 0 || description.GroupID != 0 {
292290
continue
293291
}
294-
result = append(result, stateBranchAccessDescription{
295-
AccessLevel: accessLevelValueToName[description.AccessLevel],
296-
AccessLevelDescription: description.AccessLevelDescription,
297-
})
292+
return &description.AccessLevel, nil
298293
}
299294

300-
return result
295+
return nil, fmt.Errorf("no valid access level found")
301296
}
302297

303-
func convertAllowedToToBranchAccessDescriptions(descriptions []*gitlab.BranchAccessDescription) []stateBranchAccessDescription {
304-
result := make([]stateBranchAccessDescription, 0)
305-
298+
// flattenNonZeroBranchAccessDescriptions flattens the list of branch access descriptions for the tf state.
299+
// only descriptions with non-zero user id and group id are included in the tf state.
300+
func flattenNonZeroBranchAccessDescriptions(descriptions []*gitlab.BranchAccessDescription) (values []map[string]interface{}) {
306301
for _, description := range descriptions {
307302
if description.UserID == 0 && description.GroupID == 0 {
308303
continue
309304
}
310-
result = append(result, stateBranchAccessDescription{
311-
AccessLevel: accessLevelValueToName[description.AccessLevel],
312-
AccessLevelDescription: description.AccessLevelDescription,
313-
UserID: description.UserID,
314-
GroupID: description.GroupID,
305+
values = append(values, map[string]interface{}{
306+
"access_level": accessLevelValueToName[description.AccessLevel],
307+
"access_level_description": description.AccessLevelDescription,
308+
"user_id": description.UserID,
309+
"group_id": description.GroupID,
315310
})
316311
}
317312

318-
return result
313+
return values
319314
}

0 commit comments

Comments
 (0)