Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: support more vcs-type fields in module creation #1006

Merged
merged 4 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion client/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ type Module struct {
GithubInstallationId *int `json:"githubInstallationId" tfschema:",omitempty"`
BitbucketClientKey *string `json:"bitbucketClientKey" tfschema:",omitempty"`
IsGitlab bool `json:"isGitLab"`
IsBitbucketServer bool `json:"isBitbucketServer"`
IsGitHubEnterprise bool `json:"isGitHubEnterprise" tfschema:"is_github_enterprise"`
IsGitLabEnterprise bool `json:"isGitLabEnterprise" tfschema:"is_gitlab_enterprise"`
SshKeys []ModuleSshKey `json:"sshkeys"`
Type string `json:"type"`
Id string `json:"id"`
Expand Down Expand Up @@ -43,7 +46,10 @@ type ModuleCreatePayload struct {
TokenName string `json:"tokenName,omitempty"`
GithubInstallationId *int `json:"githubInstallationId,omitempty"`
BitbucketClientKey string `json:"bitbucketClientKey,omitempty"`
IsGitlab *bool `json:"isGitLab,omitempty"`
IsGitlab bool `json:"isGitLab"`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default is false. Not sure why this was ever a pointer.

IsBitbucketServer bool `json:"isBitbucketServer"`
IsGitHubEnterprise bool `json:"isGitHubEnterprise" tfschema:"is_github_enterprise"`
IsGitLabEnterprise bool `json:"isGitLabEnterprise" tfschema:"is_gitlab_enterprise"`
SshKeys []ModuleSshKey `json:"sshkeys,omitempty"`
Path string `json:"path,omitempty"`
TagPrefix string `json:"tagPrefix,omitempty"`
Expand All @@ -70,6 +76,9 @@ type ModuleUpdatePayload struct {
GithubInstallationId *int `json:"githubInstallationId"`
BitbucketClientKey string `json:"bitbucketClientKey"`
IsGitlab bool `json:"isGitLab"`
IsBitbucketServer bool `json:"isBitbucketServer"`
IsGitHubEnterprise bool `json:"isGitHubEnterprise" tfschema:"is_github_enterprise"`
IsGitLabEnterprise bool `json:"isGitLabEnterprise" tfschema:"is_gitlab_enterprise"`
SshKeys []ModuleSshKey `json:"sshkeys"`
Path string `json:"path"`
TagPrefix string `json:"tagPrefix,omitempty"`
Expand Down
63 changes: 44 additions & 19 deletions env0/resource_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@ import (
)

func resourceModule() *schema.Resource {
vcsExactlyOneOf := []string{
Copy link
Collaborator Author

@TomerHeber TomerHeber Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using conflictsWith instead.
Due to this comment:
when isGitLabEnterprise is true the token_id and token_name should not be required

"token_id",
"github_installation_id",
"bitbucket_client_key",
}

return &schema.Resource{
CreateContext: resourceModuleCreate,
ReadContext: resourceModuleRead,
Expand Down Expand Up @@ -50,10 +44,10 @@ func resourceModule() *schema.Resource {
Optional: true,
},
"token_id": {
Type: schema.TypeString,
Description: "the git token id to be used",
Optional: true,
ExactlyOneOf: vcsExactlyOneOf,
Type: schema.TypeString,
Description: "the git token id to be used",
Optional: true,
ConflictsWith: []string{"github_installation_id", "bitbucket_client_key"},
},
"token_name": {
Type: schema.TypeString,
Expand All @@ -62,16 +56,16 @@ func resourceModule() *schema.Resource {
RequiredWith: []string{"token_id"},
},
"github_installation_id": {
Type: schema.TypeInt,
Description: "the env0 application installation id on the relevant Github repository",
Optional: true,
ExactlyOneOf: vcsExactlyOneOf,
Type: schema.TypeInt,
Description: "the env0 application installation id on the relevant Github repository",
Optional: true,
ConflictsWith: []string{"token_id", "bitbucket_client_key"},
},
"bitbucket_client_key": {
Type: schema.TypeString,
Description: "the client key used for integration with Bitbucket",
Optional: true,
ExactlyOneOf: vcsExactlyOneOf,
Type: schema.TypeString,
Description: "the client key used for integration with Bitbucket",
Optional: true,
ConflictsWith: []string{"token_id", "github_installation_id"},
},
"ssh_keys": {
Type: schema.TypeList,
Expand Down Expand Up @@ -119,6 +113,30 @@ func resourceModule() *schema.Resource {
Optional: true,
Default: false,
},
"is_gitlab": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not using conflictsWith for these as well?
GL can't be BBS for example, seems like a good validation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

Type: schema.TypeBool,
Description: "true if this module integrates with GitLab",
Optional: true,
Default: false,
},
"is_bitbucket_server": {
Type: schema.TypeBool,
Description: "true if this module integrates with Bitbucket Server",
Optional: true,
Default: false,
},
"is_github_enterprise": {
Type: schema.TypeBool,
Description: "true if this module integrates with GitHub Enterprise",
Optional: true,
Default: false,
},
"is_gitlab_enterprise": {
Type: schema.TypeBool,
Description: "true if this module integrates with GitLab Enterprise",
Optional: true,
Default: false,
},
},
}
}
Expand Down Expand Up @@ -153,6 +171,13 @@ func resourceModuleRead(ctx context.Context, d *schema.ResourceData, meta interf
return ResourceGetFailure(ctx, "module", d, err)
}

if module.IsDeleted {
tflog.Warn(ctx, "Drift Detected: Terraform will remove id from state", map[string]interface{}{"id": d.Id()})
d.SetId("")

return nil
}

if err := writeResourceData(module, d); err != nil {
return diag.Errorf("schema resource data serialization failed: %v", err)
}
Expand Down Expand Up @@ -200,7 +225,7 @@ func getModuleByName(name string, meta interface{}) (*client.Module, error) {
var foundModules []client.Module

for _, module := range modules {
if module.ModuleName == name {
if !module.IsDeleted && module.ModuleName == name {
foundModules = append(foundModules, module)
}
}
Expand Down
88 changes: 88 additions & 0 deletions env0/resource_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,4 +493,92 @@ func TestUnitModuleResource(t *testing.T) {
mock.EXPECT().ModuleDelete(module.Id).Times(1)
})
})

t.Run("Import By Name - Module Is Deleted", func(t *testing.T) {
deletedModule := module
deletedModule.IsDeleted = true

testCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"module_name": module.ModuleName,
"module_provider": module.ModuleProvider,
"repository": module.Repository,
"description": module.Description,
"token_id": module.TokenId,
"token_name": module.TokenName,
"path": module.Path,
"tag_prefix": module.TagPrefix,
"module_test_enabled": module.ModuleTestEnabled,
"run_tests_on_pull_request": module.RunTestsOnPullRequest,
"opentofu_version": module.OpentofuVersion,
}),
},
{
ResourceName: resourceNameImport,
ImportState: true,
ImportStateId: deletedModule.ModuleName,
ImportStateVerify: false,
ExpectError: regexp.MustCompile("module with name .* not found"),
},
},
}

runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {
mock.EXPECT().ModuleCreate(gomock.Any()).Times(1).Return(&module, nil)
mock.EXPECT().Module(module.Id).Times(1).Return(&module, nil)
mock.EXPECT().Modules().Times(1).Return([]client.Module{deletedModule}, nil)
mock.EXPECT().ModuleDelete(module.Id).Times(1)
})
})

t.Run("Detect Drift When Module Is Deleted", func(t *testing.T) {
deletedModule := module
deletedModule.IsDeleted = true

testCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"module_name": module.ModuleName,
"module_provider": module.ModuleProvider,
"repository": module.Repository,
"description": module.Description,
"token_id": module.TokenId,
"token_name": module.TokenName,
"path": module.Path,
"tag_prefix": module.TagPrefix,
"module_test_enabled": module.ModuleTestEnabled,
"run_tests_on_pull_request": module.RunTestsOnPullRequest,
"opentofu_version": module.OpentofuVersion,
}),
},
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"module_name": module.ModuleName,
"module_provider": module.ModuleProvider,
"repository": module.Repository,
"description": module.Description,
"token_id": module.TokenId,
"token_name": module.TokenName,
"path": module.Path,
"tag_prefix": module.TagPrefix,
"module_test_enabled": module.ModuleTestEnabled,
"run_tests_on_pull_request": module.RunTestsOnPullRequest,
"opentofu_version": module.OpentofuVersion,
}),
ExpectNonEmptyPlan: true,
PlanOnly: true,
},
},
}

runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {
mock.EXPECT().ModuleCreate(gomock.Any()).Times(1).Return(&module, nil)
mock.EXPECT().Module(module.Id).Times(1).Return(&module, nil)
mock.EXPECT().Module(module.Id).Times(2).Return(&deletedModule, nil)
mock.EXPECT().ModuleDelete(module.Id).Times(1)
})
})
}
Loading