Skip to content

Commit

Permalink
Linter fixes + small refactors
Browse files Browse the repository at this point in the history
  • Loading branch information
@Shocktrooper
Shocktrooper committed Mar 31, 2022
1 parent dfd36e5 commit 60d19c4
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 123 deletions.
2 changes: 1 addition & 1 deletion GNUmakefile
View file
Expand Up @@ -20,7 +20,7 @@ endif
test: ## Run unit tests.
go test $(TESTARGS) $(PROVIDER_SRC_DIR)

TFPROVIDERLINTX_CHECKS = -XAT001=false -XR003=false -XS002=false
TFPROVIDERLINTX_CHECKS = -XR003=false -XS002=false

fmt: tool-golangci-lint tool-tfproviderlintx tool-terraform tool-shfmt ## Format files and fix issues.
gofmt -w -s .
Expand Down
5 changes: 2 additions & 3 deletions internal/provider/resource_gitlab_group.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"net/http"
"strings"
"time"

Expand Down Expand Up @@ -251,9 +250,9 @@ func resourceGitlabGroupRead(ctx context.Context, d *schema.ResourceData, meta i
client := meta.(*gitlab.Client)
log.Printf("[DEBUG] read gitlab group %s", d.Id())

group, resp, err := client.Groups.GetGroup(d.Id(), nil, gitlab.WithContext(ctx))
group, _, err := client.Groups.GetGroup(d.Id(), nil, gitlab.WithContext(ctx))
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
if is404(err) {
log.Printf("[DEBUG] gitlab group %s not found so removing from state", d.Id())
d.SetId("")
return nil
Expand Down
5 changes: 2 additions & 3 deletions internal/provider/resource_gitlab_group_share_group.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"net/http"
"strconv"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand Down Expand Up @@ -93,9 +92,9 @@ func resourceGitlabGroupShareGroupRead(ctx context.Context, d *schema.ResourceDa
}

// Query main group
group, resp, err := client.Groups.GetGroup(groupId, nil, gitlab.WithContext(ctx))
group, _, err := client.Groups.GetGroup(groupId, nil, gitlab.WithContext(ctx))
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
if is404(err) {
log.Printf("[DEBUG] gitlab group %s not found so removing from state", groupId)
d.SetId("")
return nil
Expand Down
140 changes: 62 additions & 78 deletions internal/provider/resource_gitlab_group_share_group_test.go
View file
Expand Up @@ -4,82 +4,61 @@ import (
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
"github.com/xanzy/go-gitlab"
)

func TestAccGitlabGroupShareGroup_basic(t *testing.T) {
randName := acctest.RandomWithPrefix("acctest")
testAccCheck(t)

mainGroup := testAccCreateGroups(t, 1)[0]
sharedGroup := testAccCreateGroups(t, 1)[0]

// lintignore: AT001 // TODO: Resolve this tfproviderlint issue
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: providerFactories,
CheckDestroy: testAccCheckGitlabShareGroupDestroy,
Steps: []resource.TestStep{
// Share a new group with another group
{
Config: testAccGitlabGroupShareGroupConfig(
randName,
Config: testAccGitlabGroupShareGroupConfig(mainGroup.ID, sharedGroup.ID,
`
group_access = "guest"
expires_at = "2099-01-01"
`,
),
Check: testAccCheckGitlabGroupSharedWithGroup(randName, "2099-01-01", gitlab.GuestPermissions),
Check: testAccCheckGitlabGroupSharedWithGroup(mainGroup.Name, sharedGroup.Name, "2099-01-01", gitlab.GuestPermissions),
},
// Update the share group
{
Config: testAccGitlabGroupShareGroupConfig(randName, `group_access = "reporter"`),
Check: testAccCheckGitlabGroupSharedWithGroup(randName, "", gitlab.ReporterPermissions),
},
// Delete the gitlab_group_share_group resource
{
Config: testAccGitlabGroupShareGroupConfigDelete(randName),
Check: testAccCheckGitlabGroupIsNotShared(randName),
// Verify Import
ResourceName: "gitlab_group_share_group.test",
ImportState: true,
ImportStateVerify: true,
},
},
})
}

// lintignore: AT002 // TODO: Resolve this tfproviderlint issue
func TestAccGitlabGroupShareGroup_import(t *testing.T) {
randName := acctest.RandomWithPrefix("acctest")

resource.Test(t, resource.TestCase{
ProviderFactories: providerFactories,
PreCheck: func() { testAccPreCheck(t) },
CheckDestroy: testAccCheckGitlabGroupDestroy,
Steps: []resource.TestStep{
// Update the share group
{
// create shared groups
Config: testAccGitlabGroupShareGroupConfig(
randName,
`
group_access = "guest"
expires_at = "2099-03-03"
`,
),
Check: testAccCheckGitlabGroupSharedWithGroup(randName, "2099-03-03", gitlab.GuestPermissions),
Config: testAccGitlabGroupShareGroupConfig(mainGroup.ID, sharedGroup.ID, `group_access = "reporter"`),
Check: testAccCheckGitlabGroupSharedWithGroup(mainGroup.Name, sharedGroup.Name, "", gitlab.ReporterPermissions),
},
{
// Verify Import
ResourceName: "gitlab_group_share_group.test",
ImportState: true,
ImportStateVerify: true,
},
// Delete the gitlab_group_share_group resource
{
Config: testAccGitlabGroupShareGroupConfigDelete(),
Check: testAccCheckGitlabGroupIsNotShared(mainGroup.Name),
},
},
})
}

func testAccCheckGitlabGroupSharedWithGroup(
groupName string,
expireTime string,
accessLevel gitlab.AccessLevelValue,
) resource.TestCheckFunc {
func testAccCheckGitlabGroupSharedWithGroup(mainGroupName string, sharedGroupName string, expireTime string, accessLevel gitlab.AccessLevelValue) resource.TestCheckFunc {
return func(_ *terraform.State) error {
mainGroup, _, err := testGitlabClient.Groups.GetGroup(fmt.Sprintf("%s_main", groupName), nil)
mainGroup, _, err := testGitlabClient.Groups.GetGroup(mainGroupName, nil)
if err != nil {
return err
}
Expand All @@ -91,8 +70,8 @@ func testAccCheckGitlabGroupSharedWithGroup(

sharedGroup := mainGroup.SharedWithGroups[0]

if sharedGroup.GroupName != fmt.Sprintf("%s_share", groupName) {
return fmt.Errorf("group name was %s (wanted %s)", sharedGroup.GroupName, fmt.Sprintf("%s_share", groupName))
if sharedGroup.GroupName != sharedGroupName {
return fmt.Errorf("group name was %s (wanted %s)", sharedGroup.GroupName, sharedGroupName)
}

if gitlab.AccessLevelValue(sharedGroup.GroupAccessLevel) != accessLevel {
Expand All @@ -109,9 +88,9 @@ func testAccCheckGitlabGroupSharedWithGroup(
}
}

func testAccCheckGitlabGroupIsNotShared(groupName string) resource.TestCheckFunc {
func testAccCheckGitlabGroupIsNotShared(mainGroupName string) resource.TestCheckFunc {
return func(_ *terraform.State) error {
mainGroup, _, err := testGitlabClient.Groups.GetGroup(fmt.Sprintf("%s_main", groupName), nil)
mainGroup, _, err := testGitlabClient.Groups.GetGroup(mainGroupName, nil)
if err != nil {
return err
}
Expand All @@ -125,46 +104,51 @@ func testAccCheckGitlabGroupIsNotShared(groupName string) resource.TestCheckFunc
}
}

func testAccGitlabGroupShareGroupConfig(
randName string,
shareGroupSettings string,
) string {
return fmt.Sprintf(
`
resource "gitlab_group" "test_main" {
name = "%[1]s_main"
path = "%[1]s_main"
func testAccCheckGitlabShareGroupDestroy(s *terraform.State) error {
var groupId string
var sharedGroupId int
var err error

for _, rs := range s.RootModule().Resources {
if rs.Type == "gitlab_group_share_group" {
groupId, sharedGroupId, err = groupIdsFromId(rs.Primary.ID)
if err != nil {
return fmt.Errorf("[ERROR] cannot get Group ID and ShareGroupId from input: %v", rs.Primary.ID)
}

// Get Main Group
group, _, err := testGitlabClient.Groups.GetGroup(groupId, nil)
if err != nil {
return err
}

// Make sure that SharedWithGroups attribute on the main group does not contain the shared group id at all
for _, sharedGroup := range group.SharedWithGroups {
if sharedGroupId == sharedGroup.GroupID {
return fmt.Errorf("GitLab Group Share %d still exists", sharedGroupId)
}
}
}
}

resource "gitlab_group" "test_share" {
name = "%[1]s_share"
path = "%[1]s_share"
}
return nil
}

func testAccGitlabGroupShareGroupConfig(mainGroupId int, shareGroupId int, shareGroupSettings string) string {
return fmt.Sprintf(
`
resource "gitlab_group_share_group" "test" {
group_id = gitlab_group.test_main.id
share_group_id = gitlab_group.test_share.id
%[2]s
group_id = %[1]d
share_group_id = %[2]d
%[3]s
}
`,
randName,
mainGroupId,
shareGroupId,
shareGroupSettings,
)
}

func testAccGitlabGroupShareGroupConfigDelete(randName string) string {
return fmt.Sprintf(
`
resource "gitlab_group" "test_main" {
name = "%[1]s_main"
path = "%[1]s_main"
}
resource "gitlab_group" "test_share" {
name = "%[1]s_share"
path = "%[1]s_share"
}
`,
randName,
)
func testAccGitlabGroupShareGroupConfigDelete() string {
return ``
}
5 changes: 2 additions & 3 deletions internal/provider/resource_gitlab_group_test.go
View file
Expand Up @@ -2,7 +2,6 @@ package provider

import (
"fmt"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -284,8 +283,8 @@ func testAccCheckGitlabGroupDisappears(group *gitlab.Group) resource.TestCheckFu
// Fixes groups API async deletion issue
// https://github.com/gitlabhq/terraform-provider-gitlab/issues/319
for start := time.Now(); time.Since(start) < 15*time.Second; {
g, resp, err := testGitlabClient.Groups.GetGroup(group.ID, nil)
if resp != nil && resp.StatusCode == http.StatusNotFound {
g, _, err := testGitlabClient.Groups.GetGroup(group.ID, nil)
if is404(err) {
return nil
}
if g != nil && g.MarkedForDeletionOn != nil {
Expand Down
5 changes: 2 additions & 3 deletions internal/provider/resource_gitlab_instance_variable.go
View file
Expand Up @@ -3,7 +3,6 @@ package provider
import (
"context"
"log"
"net/http"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -96,9 +95,9 @@ func resourceGitlabInstanceVariableRead(ctx context.Context, d *schema.ResourceD

log.Printf("[DEBUG] read gitlab instance level CI variable %s", key)

v, resp, err := client.InstanceVariables.GetVariable(key, gitlab.WithContext(ctx))
v, _, err := client.InstanceVariables.GetVariable(key, gitlab.WithContext(ctx))
if err != nil {
if resp.StatusCode == http.StatusNotFound {
if is404(err) {
log.Printf("[DEBUG] gitlab instance level CI variable for %s not found so removing from state", d.Id())
d.SetId("")
return nil
Expand Down
25 changes: 24 additions & 1 deletion internal/provider/resource_gitlab_instance_variable_test.go
View file
Expand Up @@ -15,10 +15,10 @@ func TestAccGitlabInstanceVariable_basic(t *testing.T) {
var instanceVariable gitlab.InstanceVariable
rString := acctest.RandString(5)

// lintignore: AT001 // TODO: Resolve this tfproviderlint issue
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: providerFactories,
CheckDestroy: testAccCheckGitlabInstanceVariableDestroy,
Steps: []resource.TestStep{
// Create a variable with default options
{
Expand Down Expand Up @@ -120,6 +120,29 @@ func testAccCheckGitlabInstanceVariableExists(n string, instanceVariable *gitlab
}
}

func testAccCheckGitlabInstanceVariableDestroy(s *terraform.State) error {
var key string

for _, rs := range s.RootModule().Resources {
if rs.Type == "gitlab_instance_variable" {
key = rs.Primary.ID
}
}

iv, _, err := testGitlabClient.InstanceVariables.GetVariable(key)
if err == nil {
if iv != nil {
return fmt.Errorf("Instance Variable %s still exists", key)
}
} else {
if !is404(err) {
return err
}
}

return nil
}

type testAccGitlabInstanceVariableExpectedAttributes struct {
Key string
Value string
Expand Down
5 changes: 2 additions & 3 deletions internal/provider/resource_gitlab_project_freeze_period.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"net/http"
"strconv"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand Down Expand Up @@ -84,9 +83,9 @@ func resourceGitlabProjectFreezePeriodRead(ctx context.Context, d *schema.Resour

log.Printf("[DEBUG] read gitlab FreezePeriod %s/%d", projectID, freezePeriodID)

freezePeriod, resp, err := client.FreezePeriods.GetFreezePeriod(projectID, freezePeriodID, gitlab.WithContext(ctx))
freezePeriod, _, err := client.FreezePeriods.GetFreezePeriod(projectID, freezePeriodID, gitlab.WithContext(ctx))
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
if is404(err) {
log.Printf("[DEBUG] project freeze period for %s not found so removing it from state", d.Id())
d.SetId("")
return nil
Expand Down
5 changes: 2 additions & 3 deletions internal/provider/resource_gitlab_project_membership.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"net/http"
"strconv"
"strings"

Expand Down Expand Up @@ -85,9 +84,9 @@ func resourceGitlabProjectMembershipRead(ctx context.Context, d *schema.Resource
return diag.FromErr(err)
}

projectMember, resp, err := client.ProjectMembers.GetProjectMember(projectId, userId, gitlab.WithContext(ctx))
projectMember, _, err := client.ProjectMembers.GetProjectMember(projectId, userId, gitlab.WithContext(ctx))
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
if is404(err) {
log.Printf("[DEBUG] gitlab project membership for %s not found so removing from state", d.Id())
d.SetId("")
return nil
Expand Down

0 comments on commit 60d19c4

Please sign in to comment.