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.

Sign up for GitHub

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

Feat: stop requiring gitlab_project_id for gitlab templates #941

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Feat: stop requiring gitlab_project_id for gitlab templates #941

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe92aca
Feat: stop requiring gitlab_project_id for gitlab templates
TomerHeber Aug 25, 2024
16d2c03
Merge branch 'main' into feat-remove-gitlab_project_id-#940
TomerHeber Aug 26, 2024
a794e4b
text was modified
TomerHeber Aug 26, 2024
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
2 changes: 0 additions & 2 deletions client/environment_discovery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ type EnvironmentDiscoveryPutPayload struct {
TerragruntTfBinary string `json:"terragruntTfBinary,omitempty"`
IsTerragruntRunAll bool `json:"is_terragrunt_run_all"`
Type string `json:"type"`
GitlabProjectId int `json:"gitlabProjectId,omitempty"`
TokenId string `json:"tokenId,omitempty"`
SshKeys []TemplateSshKey `json:"sshKeys,omitempty"`
GithubInstallationId int `json:"githubInstallationId,omitempty"`
Expand All @@ -35,7 +34,6 @@ type EnvironmentDiscoveryPayload struct {
TerragruntTfBinary string `json:"terragruntTfBinary" tfschema:",omitempty"`
IsTerragruntRunAll bool `json:"is_terragrunt_run_all"`
Type string `json:"type"`
GitlabProjectId int `json:"gitlabProjectId"`
TokenId string `json:"tokenId"`
SshKeys []TemplateSshKey `json:"sshKeys" tfschema:"-"`
GithubInstallationId int `json:"githubInstallationId"`
Expand Down
4 changes: 2 additions & 2 deletions client/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (

const TERRAGRUNT = "terragrunt"
const OPENTOFU = "opentofu"
const TERRAFORM = "terraform"
const WORKFLOW = "workflow"

type TemplateRetryOn struct {
Times int `json:"times,omitempty"`
Expand Down Expand Up @@ -67,7 +69,6 @@ type Template struct {
IsGitlab bool `json:"isGitLab"`
TerragruntTfBinary string `json:"terragruntTfBinary" tfschema:",omitempty"`
TokenName string `json:"tokenName" tfschema:",omitempty"`
GitlabProjectId int `json:"gitlabProjectId" tfschema:",omitempty"`
AnsibleVersion string `json:"ansibleVersion" tfschema:",omitempty"`
}

Expand All @@ -83,7 +84,6 @@ type TemplateCreatePayload struct {
TokenName string `json:"tokenName,omitempty"`
TokenId string `json:"tokenId,omitempty"`
GithubInstallationId int `json:"githubInstallationId,omitempty"`
GitlabProjectId int `json:"gitlabProjectId,omitempty"`
Revision string `json:"revision"`
OrganizationId string `json:"organizationId"`
TerraformVersion string `json:"terraformVersion,omitempty"`
Expand Down
12 changes: 6 additions & 6 deletions env0/configuration_template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func getConfigurationTemplateSchema(templateType TemplateType) map[string]*schem
s := map[string]*schema.Schema{
"id": {
Type: schema.TypeString,
Description: fmt.Sprintf("id of the %s", text),
Description: "id of the " + text,
Computed: true,
},
"repository": {
Expand Down Expand Up @@ -59,10 +59,10 @@ func getConfigurationTemplateSchema(templateType TemplateType) map[string]*schem
},
},
"gitlab_project_id": {
Type: schema.TypeInt,
Description: "the project id of the relevant repository",
Optional: true,
RequiredWith: []string{"token_id"},
Type: schema.TypeInt,
Description: "the project id of the relevant repository (deprecated)",
Deprecated: "'repository' is used instead",
Copy link
Contributor

Choose a reason for hiding this comment

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

Text should be changed
We should use "project id is now auto-fetched from the repository URL"

Optional: true,
},
"github_installation_id": {
Type: schema.TypeInt,
Expand Down Expand Up @@ -106,7 +106,7 @@ func getConfigurationTemplateSchema(templateType TemplateType) map[string]*schem
},
"name": {
Type: schema.TypeString,
Description: fmt.Sprintf("name for the %s", text),
Description: "name for the " + text,
Required: true,
},
}
Expand Down
64 changes: 16 additions & 48 deletions env0/resource_environment_discovery_configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"strings"

"github.com/env0/terraform-provider-env0/client"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand Down Expand Up @@ -42,8 +41,8 @@ func resourceEnvironmentDiscoveryConfiguration() *schema.Resource {
"type": {
Type: schema.TypeString,
Description: "the infrastructure type use. Valid values: 'opentofu', 'terraform', 'terragrunt', 'workflow' (default: 'opentofu')",
Default: "opentofu",
ValidateDiagFunc: NewStringInValidator([]string{"opentofu", "terraform", "terragrunt", "workflow"}),
Default: client.OPENTOFU,
ValidateDiagFunc: NewStringInValidator([]string{client.OPENTOFU, client.TERRAFORM, client.TERRAGRUNT, client.WORKFLOW}),
Optional: true,
},
"environment_placement": {
Expand Down Expand Up @@ -87,8 +86,8 @@ func resourceEnvironmentDiscoveryConfiguration() *schema.Resource {
Type: schema.TypeString,
Optional: true,
Description: "The binary to use with Terragrunt. Valid values: 'opentofu' and 'terraform' (default: 'opentofu')",
ValidateDiagFunc: NewStringInValidator([]string{"opentofu", "terraform"}),
Default: "opentofu",
ValidateDiagFunc: NewStringInValidator([]string{client.OPENTOFU, client.TERRAFORM}),
Default: client.OPENTOFU,
},
"is_terragrunt_run_all": {
Type: schema.TypeBool,
Expand Down Expand Up @@ -143,10 +142,10 @@ func resourceEnvironmentDiscoveryConfiguration() *schema.Resource {
Optional: true,
},
"gitlab_project_id": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Type: schema.TypeInt,
Description: "gitlab project id",
Optional: true,
RequiredWith: []string{"token_id"},
Type: schema.TypeInt,
Description: "gitlab project id (deprecated)",
Optional: true,
Deprecated: "'repository' is used instead",
},
"is_azure_devops": {
Type: schema.TypeBool,
Expand Down Expand Up @@ -197,62 +196,31 @@ func discoveryValidatePutPayload(putPayload *client.EnvironmentDiscoveryPutPaylo
terragruntVersionSet := putPayload.TerragruntVersion != ""

switch putPayload.Type {
case "opentofu":
case client.OPENTOFU:
if !opentofuVersionSet {
return errors.New("'opentofu_version' not set")
}
case "terraform":
case client.TERRAFORM:
if !terraformVersionSet {
return errors.New("'terraform_version' not set")
}
case "terragrunt":
case client.TERRAGRUNT:
if !terragruntVersionSet {
return errors.New("'terragrunt_version' not set")
}

if putPayload.TerragruntTfBinary == "opentofu" && !opentofuVersionSet {
if putPayload.TerragruntTfBinary == client.OPENTOFU && !opentofuVersionSet {
return errors.New("'terragrunt_tf_binary' is set to 'opentofu', but 'opentofu_version' not set")
}

if putPayload.TerragruntTfBinary == "terraform" && !terraformVersionSet {
if putPayload.TerragruntTfBinary == client.TERRAFORM && !terraformVersionSet {
return errors.New("'terragrunt_tf_binary' is set to 'terraform', but 'terraform_version' not set")
}
case "workflow":
case client.WORKFLOW:
default:
return fmt.Errorf("unhandled type %s", putPayload.Type)
}

vcsCounter := 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed this because this may cause some issues. Not clear how to resolve.
Schema validation is a nice to have. Instead BE will error on this.

vcsEnabledAttributes := []string{}

if putPayload.GithubInstallationId != 0 {
vcsCounter++
vcsEnabledAttributes = append(vcsEnabledAttributes, "github_installation_id")
}

if putPayload.BitbucketClientKey != "" {
vcsCounter++
vcsEnabledAttributes = append(vcsEnabledAttributes, "bitbucket_client_key")
}

if putPayload.GitlabProjectId != 0 {
vcsCounter++
vcsEnabledAttributes = append(vcsEnabledAttributes, "gitlab_project_id")
}

if putPayload.IsAzureDevops {
vcsCounter++
vcsEnabledAttributes = append(vcsEnabledAttributes, "is_azure_devops")
}

if vcsCounter == 0 {
return errors.New("must set exactly one vcs, none were configured: github_installation_id, bitbucket_client_key, gitlab_project_id, or is_azure_devops")
}

if vcsCounter > 1 {
return fmt.Errorf("must set exactly one vcs, but more were configured: %s", strings.Join(vcsEnabledAttributes, ", "))
}

return nil
}

Expand All @@ -273,7 +241,7 @@ func resourceEnvironmentDiscoveryConfigurationPut(ctx context.Context, d *schema
return diag.Errorf("validation error: %s", err.Error())
}

if putPayload.Type != "terragrunt" {
if putPayload.Type != client.TERRAGRUNT {
// Remove the default terragrunt_tf_binary if terragrunt isn't used.
putPayload.TerragruntTfBinary = ""
}
Expand Down Expand Up @@ -347,7 +315,7 @@ func resourceEnvironmentDiscoveryConfigurationImport(ctx context.Context, d *sch
d.Set("project_id", projectId)

if _, ok := d.GetOk("terragrunt_tf_binary"); !ok {
d.Set("terragrunt_tf_binary", "opentofu")
d.Set("terragrunt_tf_binary", client.OPENTOFU)
}

return []*schema.ResourceData{d}, nil
Expand Down
43 changes: 0 additions & 43 deletions env0/resource_environment_discovery_configuration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ func TestUnitEnvironmentDiscoveryConfigurationResource(t *testing.T) {
TerraformVersion: "1.7.8",
EnvironmentPlacement: "topProject",
WorkspaceNaming: "default",
GitlabProjectId: 12345,
TokenId: "abcdefg",
}

Expand All @@ -324,7 +323,6 @@ func TestUnitEnvironmentDiscoveryConfigurationResource(t *testing.T) {
TerraformVersion: putPayload.TerraformVersion,
EnvironmentPlacement: putPayload.EnvironmentPlacement,
WorkspaceNaming: putPayload.WorkspaceNaming,
GitlabProjectId: putPayload.GitlabProjectId,
TokenId: putPayload.TokenId,
}

Expand All @@ -336,7 +334,6 @@ func TestUnitEnvironmentDiscoveryConfigurationResource(t *testing.T) {
"type": putPayload.Type,
"glob_pattern": putPayload.GlobPattern,
"repository": putPayload.Repository,
"gitlab_project_id": putPayload.GitlabProjectId,
"token_id": putPayload.TokenId,
"terraform_version": putPayload.TerraformVersion,
}),
Expand All @@ -347,7 +344,6 @@ func TestUnitEnvironmentDiscoveryConfigurationResource(t *testing.T) {
resource.TestCheckResourceAttr(accessor, "type", putPayload.Type),
resource.TestCheckResourceAttr(accessor, "environment_placement", putPayload.EnvironmentPlacement),
resource.TestCheckResourceAttr(accessor, "workspace_naming", putPayload.WorkspaceNaming),
resource.TestCheckResourceAttr(accessor, "gitlab_project_id", strconv.Itoa(putPayload.GitlabProjectId)),
resource.TestCheckResourceAttr(accessor, "token_id", putPayload.TokenId),
resource.TestCheckResourceAttr(accessor, "terraform_version", putPayload.TerraformVersion),
),
Expand Down Expand Up @@ -711,45 +707,6 @@ func TestUnitEnvironmentDiscoveryConfigurationResource(t *testing.T) {
runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {})
})

t.Run("error: no vcs set", func(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed relevant tests.

testCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"project_id": projectId,
"glob_pattern": "**",
"repository": "https://re.po",
"type": "workflow",
}),
ExpectError: regexp.MustCompile("must set exactly one vcs, none were configured"),
},
},
}

runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {})
})

t.Run("error: more than one vcs set", func(t *testing.T) {
testCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"project_id": projectId,
"glob_pattern": "**",
"repository": "https://re.po",
"type": "workflow",
"github_installation_id": 1234,
"gitlab_project_id": 5678,
"token_id": "1345",
}),
ExpectError: regexp.MustCompile("must set exactly one vcs, but more were configured: github_installation_id, gitlab_project_id"),
},
},
}

runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {})
})

t.Run("import", func(t *testing.T) {
putPayload := client.EnvironmentDiscoveryPutPayload{
GlobPattern: "**",
Expand Down
34 changes: 17 additions & 17 deletions env0/resource_template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
func getTemplateSchema(prefix string) map[string]*schema.Schema {
var allVCSAttributes = []string{
"token_id",
"gitlab_project_id",
"github_installation_id",
"bitbucket_client_key",
"is_gitlab_enterprise",
Expand All @@ -29,14 +28,14 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
}

var allowedTemplateTypes = []string{
"terraform",
"terragrunt",
client.TERRAFORM,
client.TERRAGRUNT,
"pulumi",
"k8s",
"workflow",
client.WORKFLOW,
"cloudformation",
"helm",
"opentofu",
client.OPENTOFU,
"ansible",
}

Expand All @@ -56,6 +55,7 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
if prefix != "" {
attr = prefix + attr
}

butAttrs = append(butAttrs, attr)
}
}
Expand All @@ -70,6 +70,7 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
if prefix != "" {
str = prefix + str
}

ret = append(ret, str)
}

Expand Down Expand Up @@ -101,7 +102,7 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
Type: schema.TypeString,
Description: fmt.Sprintf("template type (allowed values: %s)", strings.Join(allowedTemplateTypes, ", ")),
Optional: true,
Default: "terraform",
Default: client.TERRAFORM,
ValidateDiagFunc: NewStringInValidator(allowedTemplateTypes),
},
"revision": {
Expand Down Expand Up @@ -152,14 +153,13 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
Type: schema.TypeString,
Description: "the git token id to be used",
Optional: true,
ConflictsWith: allVCSAttributesBut("token_id", "gitlab_project_id", "is_azure_devops", "path"),
ConflictsWith: allVCSAttributesBut("token_id", "is_azure_devops", "path"),
},
"gitlab_project_id": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Type: schema.TypeInt,
Description: "the project id of the relevant repository",
Optional: true,
ConflictsWith: allVCSAttributesBut("token_id", "gitlab_project_id", "path"),
RequiredWith: requiredWith("token_id"),
Type: schema.TypeInt,
Deprecated: "'repository' is used instead",
Description: "the project id of the relevant repository (deprecated)",
Optional: true,
},
"terraform_version": {
Type: schema.TypeString,
Expand Down Expand Up @@ -214,7 +214,7 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
"is_terragrunt_run_all": {
Type: schema.TypeBool,
Optional: true,
Description: `true if this template should execute run-all commands on multiple modules (check https://terragrunt.gruntwork.io/docs/features/execute-terraform-commands-on-multiple-modules-at-once/#the-run-all-command for additional details). Can only be true with "terragrunt" template type and terragrunt version 0.28.1 and above`,
Description: "true if this template should execute run-all commands on multiple modules (check https://terragrunt.gruntwork.io/docs/features/execute-terraform-commands-on-multiple-modules-at-once/#the-run-all-command for additional details). Can only be true with 'terragrunt' template type and terragrunt version 0.28.1 and above",
Default: "false",
},
"is_azure_devops": {
Expand Down Expand Up @@ -243,7 +243,7 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
Type: schema.TypeString,
Optional: true,
Description: "the binary to use if the template type is 'terragrunt'. Valid values 'opentofu' and 'terraform'. For new templates defaults to 'opentofu'",
ValidateDiagFunc: NewStringInValidator([]string{"opentofu", "terraform"}),
ValidateDiagFunc: NewStringInValidator([]string{client.OPENTOFU, client.TERRAFORM}),
},
"token_name": {
Type: schema.TypeString,
Expand Down Expand Up @@ -411,16 +411,16 @@ func templateCreatePayloadFromParameters(prefix string, d *schema.ResourceData)
// If the user has set a value - use it.
if terragruntTfBinary := d.Get(terragruntTfBinaryKey).(string); terragruntTfBinary != "" {
payload.TerragruntTfBinary = terragruntTfBinary
} else if templateType.(string) == "terragrunt" && isNew {
payload.TerragruntTfBinary = "opentofu"
} else if templateType.(string) == client.TERRAGRUNT && isNew {
payload.TerragruntTfBinary = client.OPENTOFU
}
}

templateCreatePayloadRetryOnHelper(prefix, d, "deploy", &payload.Retry.OnDeploy)
templateCreatePayloadRetryOnHelper(prefix, d, "destroy", &payload.Retry.OnDestroy)

if err := payload.Invalidate(); err != nil {
return payload, diag.Errorf(err.Error())
return payload, diag.FromErr(err)
}

return payload, nil
Expand Down
Loading
Loading