From 9c121b1b7730fdef5b7dce52c121173fe9b548a1 Mon Sep 17 00:00:00 2001 From: Douglas Harcourt Parsons Date: Mon, 13 May 2024 11:50:59 +0100 Subject: [PATCH] Validate sensitive environment variable config when creating env vars Turns out the API elides non-sensitive values into being sensitive if the team has a sensitiveEnvironmentVariablePolicy set on it. To prevent this from causing provider issues when the `sensitive` field is explicitly set to `false`, warn at plan time that the resource being created does not match the team policy. Closes #183 --- client/client.go | 16 +++-- client/team.go | 11 +-- sweep/main.go | 11 +-- vercel/provider.go | 2 +- vercel/resource_project.go | 71 ++++++++++++++++++- .../resource_project_environment_variable.go | 51 ++++++++++++- ...ource_project_environment_variable_test.go | 19 +++++ vercel/resource_project_test.go | 2 +- .../resource_shared_environment_variable.go | 50 ++++++++++++- 9 files changed, 213 insertions(+), 20 deletions(-) diff --git a/client/client.go b/client/client.go index 47036b34..f1cff0ab 100644 --- a/client/client.go +++ b/client/client.go @@ -1,6 +1,7 @@ package client import ( + "context" "net/http" "time" ) @@ -9,7 +10,7 @@ import ( type Client struct { token string client *http.Client - _teamID string + team Team baseURL string } @@ -33,11 +34,18 @@ func New(token string) *Client { } } -func (c *Client) WithTeamID(teamID string) *Client { - c._teamID = teamID +func (c *Client) WithTeam(team Team) *Client { + c.team = team return c } +func (c *Client) Team(ctx context.Context, teamID string) (Team, error) { + if teamID != "" { + return c.GetTeam(ctx, teamID) + } + return c.team, nil +} + // teamID is a helper method to return one of two values based on specificity. // It will return an explicitly passed teamID if it is defined. If not defined, // it will fall back to the teamID configured on the client. @@ -45,5 +53,5 @@ func (c *Client) teamID(teamID string) string { if teamID != "" { return teamID } - return c._teamID + return c.team.ID } diff --git a/client/team.go b/client/team.go index 9ec61326..ee0f9db1 100644 --- a/client/team.go +++ b/client/team.go @@ -13,13 +13,14 @@ type TeamCreateRequest struct { Name string `json:"name"` } -// TeamResponse is the information returned by the vercel api when a team is created. -type TeamResponse struct { - ID string `json:"id"` +// Team is the information returned by the vercel api when a team is created. +type Team struct { + ID string `json:"id"` + SensitiveEnvironmentVariablePolicy *string `json:"sensitiveEnvironmentVariablePolicy"` } // CreateTeam creates a team within vercel. -func (c *Client) CreateTeam(ctx context.Context, request TeamCreateRequest) (r TeamResponse, err error) { +func (c *Client) CreateTeam(ctx context.Context, request TeamCreateRequest) (r Team, err error) { url := fmt.Sprintf("%s/v1/teams", c.baseURL) payload := string(mustMarshal(request)) @@ -51,7 +52,7 @@ func (c *Client) DeleteTeam(ctx context.Context, teamID string) error { } // GetTeam returns information about an existing team within vercel. -func (c *Client) GetTeam(ctx context.Context, idOrSlug string) (r TeamResponse, err error) { +func (c *Client) GetTeam(ctx context.Context, idOrSlug string) (r Team, err error) { url := fmt.Sprintf("%s/v2/teams/%s", c.baseURL, idOrSlug) tflog.Info(ctx, "getting team", map[string]interface{}{ "url": url, diff --git a/sweep/main.go b/sweep/main.go index 377ce671..eb29ebe1 100644 --- a/sweep/main.go +++ b/sweep/main.go @@ -26,6 +26,7 @@ func main() { //lintignore:R009 panic("VERCEL_TERRAFORM_TESTING_DOMAIN environment variable not set") } + clearDNS := os.Getenv("VERCEL_TERRAFORM_CLEAR_DNS") != "" ctx := context.Background() // delete both for the testing team, and for without a team @@ -34,10 +35,12 @@ func main() { //lintignore:R009 panic(err) } - err = deleteAllDNSRecords(ctx, c, domain, teamID) - if err != nil { - //lintignore:R009 - panic(err) + if clearDNS { + err = deleteAllDNSRecords(ctx, c, domain, teamID) + if err != nil { + //lintignore:R009 + panic(err) + } } err = deleteAllSharedEnvironmentVariables(ctx, c, teamID) if err != nil { diff --git a/vercel/provider.go b/vercel/provider.go index 8e6c2cb7..4d024486 100644 --- a/vercel/provider.go +++ b/vercel/provider.go @@ -154,7 +154,7 @@ func (p *vercelProvider) Configure(ctx context.Context, req provider.ConfigureRe ) return } - vercelClient = vercelClient.WithTeamID(res.ID) + vercelClient = vercelClient.WithTeam(res) } resp.DataSourceData = vercelClient diff --git a/vercel/resource_project.go b/vercel/resource_project.go index 570260f5..68762cbd 100644 --- a/vercel/resource_project.go +++ b/vercel/resource_project.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" @@ -28,6 +29,7 @@ var ( _ resource.Resource = &projectResource{} _ resource.ResourceWithConfigure = &projectResource{} _ resource.ResourceWithImportState = &projectResource{} + _ resource.ResourceWithModifyPlan = &projectResource{} ) func newProjectResource() resource.Resource { @@ -949,7 +951,8 @@ func convertResponseToProject(ctx context.Context, response client.ProjectRespon return Project{}, fmt.Errorf("error reading project environment variables: %s", err) } for _, p := range environment { - if p.Sensitive.ValueBool() && p.Key.ValueString() == e.Key && hasSameTarget(p, e.Target) { + if p.Key.ValueString() == e.Key && hasSameTarget(p, e.Target) { + value = p.Value break } @@ -1042,6 +1045,72 @@ func convertResponseToProject(ctx context.Context, response client.ProjectRespon }, nil } +func (r *projectResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + if req.Plan.Raw.IsNull() { + return + } + var config Project + diags := req.Plan.Get(ctx, &config) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + environment, err := config.environment(ctx) + if err != nil { + resp.Diagnostics.AddError( + "Error parsing project environment variables", + "Could not read environment variables, unexpected error: "+err.Error(), + ) + return + } + + // work out if there are any new env vars that are specifying sensitive = false + var nonSensitiveEnvVars []path.Path + for i, e := range environment { + if e.ID.ValueString() != "" { + continue + } + if e.Sensitive.IsUnknown() || e.Sensitive.IsNull() || e.Sensitive.ValueBool() { + continue + } + nonSensitiveEnvVars = append( + nonSensitiveEnvVars, + path.Root("environment"). + AtSetValue(config.Environment.Elements()[i]). + AtName("sensitive"), + ) + } + + if len(nonSensitiveEnvVars) == 0 { + return + } + + // if sensitive is explicitly set to `false`, then validate that an env var can be created with the given + // team sensitive environment variable policy. + team, err := r.client.Team(ctx, config.TeamID.ValueString()) + if err != nil { + resp.Diagnostics.AddError( + "Error validating project environment variable", + "Could not validate project environment variable, unexpected error: "+err.Error(), + ) + return + } + + if team.SensitiveEnvironmentVariablePolicy == nil || *team.SensitiveEnvironmentVariablePolicy != "on" { + // the policy isn't enabled + return + } + + for _, p := range nonSensitiveEnvVars { + resp.Diagnostics.AddAttributeError( + p, + "Project Invalid", + "This team has a policy that forces all environment variables to be sensitive. Please remove the `sensitive` field for your environment variables or set the `sensitive` field to `true` in your configuration.", + ) + } +} + // Create will create a project within Vercel by calling the Vercel API. // This is called automatically by the provider when a new resource should be created. func (r *projectResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { diff --git a/vercel/resource_project_environment_variable.go b/vercel/resource_project_environment_variable.go index d02362f4..55b58c3c 100644 --- a/vercel/resource_project_environment_variable.go +++ b/vercel/resource_project_environment_variable.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" @@ -17,8 +18,10 @@ import ( ) var ( - _ resource.Resource = &projectEnvironmentVariableResource{} - _ resource.ResourceWithConfigure = &projectEnvironmentVariableResource{} + _ resource.Resource = &projectEnvironmentVariableResource{} + _ resource.ResourceWithConfigure = &projectEnvironmentVariableResource{} + _ resource.ResourceWithImportState = &projectEnvironmentVariableResource{} + _ resource.ResourceWithModifyPlan = &projectEnvironmentVariableResource{} ) func newProjectEnvironmentVariableResource() resource.Resource { @@ -108,6 +111,7 @@ At this time you cannot use a Vercel Project resource with in-line ` + "`environ Description: "Whether the Environment Variable is sensitive or not.", Optional: true, Computed: true, + Validators: []validator.Bool{}, PlanModifiers: []planmodifier.Bool{boolplanmodifier.RequiresReplace()}, }, }, @@ -126,6 +130,49 @@ type ProjectEnvironmentVariable struct { Sensitive types.Bool `tfsdk:"sensitive"` } +func (r *projectEnvironmentVariableResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + if req.Plan.Raw.IsNull() { + return + } + var config ProjectEnvironmentVariable + diags := req.Config.Get(ctx, &config) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if config.ID.ValueString() != "" { + // The resource already exists, so this is okay. + return + } + if config.Sensitive.IsUnknown() || config.Sensitive.IsNull() || config.Sensitive.ValueBool() { + // Sensitive is either true, or computed, which is fine. + return + } + + // if sensitive is explicitly set to `false`, then validate that an env var can be created with the given + // team sensitive environment variable policy. + team, err := r.client.Team(ctx, config.TeamID.ValueString()) + if err != nil { + resp.Diagnostics.AddError( + "Error validating project environment variable", + "Could not validate project environment variable, unexpected error: "+err.Error(), + ) + return + } + + if team.SensitiveEnvironmentVariablePolicy == nil || *team.SensitiveEnvironmentVariablePolicy != "on" { + // the policy isn't enabled + return + } + + resp.Diagnostics.AddAttributeError( + path.Root("sensitive"), + "Project Environment Variable Invalid", + "This team has a policy that forces all environment variables to be sensitive. Please remove the `sensitive` field or set the `sensitive` field to `true` in your configuration.", + ) +} + func (e *ProjectEnvironmentVariable) toCreateEnvironmentVariableRequest() client.CreateEnvironmentVariableRequest { target := []string{} for _, t := range e.Target { diff --git a/vercel/resource_project_environment_variable_test.go b/vercel/resource_project_environment_variable_test.go index d52d8eab..c6c08379 100644 --- a/vercel/resource_project_environment_variable_test.go +++ b/vercel/resource_project_environment_variable_test.go @@ -78,6 +78,14 @@ func TestAcc_ProjectEnvironmentVariables(t *testing.T) { resource.TestCheckResourceAttr("vercel_project_environment_variable.example_sensitive", "value", "bar-sensitive"), resource.TestCheckTypeSetElemAttr("vercel_project_environment_variable.example_sensitive", "target.*", "production"), resource.TestCheckResourceAttr("vercel_project_environment_variable.example_sensitive", "sensitive", "true"), + + /* + testAccProjectEnvironmentVariableExists("vercel_project_environment_variable.example_not_sensitive", testTeam()), + resource.TestCheckResourceAttr("vercel_project_environment_variable.example_not_sensitive", "key", "foo_not_sensitive"), + resource.TestCheckResourceAttr("vercel_project_environment_variable.example_not_sensitive", "value", "bar-not-sensitive"), + resource.TestCheckTypeSetElemAttr("vercel_project_environment_variable.example_not_sensitive", "target.*", "production"), + resource.TestCheckResourceAttr("vercel_project_environment_variable.example_not_sensitive", "sensitive", "false"), + */ ), }, { @@ -179,6 +187,17 @@ resource "vercel_project_environment_variable" "example_sensitive" { target = ["production"] sensitive = true } + +/* +resource "vercel_project_environment_variable" "example_not_sensitive" { + project_id = vercel_project.example.id + %[3]s + key = "foo_not_sensitive" + value = "bar-not-sensitive" + target = ["production"] + sensitive = false +} +*/ `, projectName, testGithubRepo(), teamIDConfig()) } diff --git a/vercel/resource_project_test.go b/vercel/resource_project_test.go index 6c35d63b..8081205c 100644 --- a/vercel/resource_project_test.go +++ b/vercel/resource_project_test.go @@ -526,7 +526,7 @@ resource "vercel_project" "test_git" { git_repository = { type = "github" repo = "%s" - production_branch = "staging" + production_branch = "production" deploy_hooks = [ { ref = "main" diff --git a/vercel/resource_shared_environment_variable.go b/vercel/resource_shared_environment_variable.go index 1606cb62..8eb89f10 100644 --- a/vercel/resource_shared_environment_variable.go +++ b/vercel/resource_shared_environment_variable.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" @@ -19,8 +20,10 @@ import ( ) var ( - _ resource.Resource = &sharedEnvironmentVariableResource{} - _ resource.ResourceWithConfigure = &sharedEnvironmentVariableResource{} + _ resource.Resource = &sharedEnvironmentVariableResource{} + _ resource.ResourceWithConfigure = &sharedEnvironmentVariableResource{} + _ resource.ResourceWithImportState = &sharedEnvironmentVariableResource{} + _ resource.ResourceWithModifyPlan = &sharedEnvironmentVariableResource{} ) func newSharedEnvironmentVariableResource() resource.Resource { @@ -31,6 +34,49 @@ type sharedEnvironmentVariableResource struct { client *client.Client } +func (r *sharedEnvironmentVariableResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + if req.Plan.Raw.IsNull() { + return + } + var config SharedEnvironmentVariable + diags := req.Plan.Get(ctx, &config) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if config.ID.ValueString() != "" { + // The resource already exists, so this is okay. + return + } + if config.Sensitive.IsUnknown() || config.Sensitive.IsNull() || config.Sensitive.ValueBool() { + // Sensitive is either true, or computed, which is fine. + return + } + + // if sensitive is explicitly set to `false`, then validate that an env var can be created with the given + // team sensitive environment variable policy. + team, err := r.client.Team(ctx, config.TeamID.ValueString()) + if err != nil { + resp.Diagnostics.AddError( + "Error validating shared environment variable", + "Could not validate shared environment variable, unexpected error: "+err.Error(), + ) + return + } + + if team.SensitiveEnvironmentVariablePolicy == nil || *team.SensitiveEnvironmentVariablePolicy != "on" { + // the policy isn't enabled + return + } + + resp.Diagnostics.AddAttributeError( + path.Root("sensitive"), + "Shared Environment Variable Invalid", + "This team has a policy that forces all environment variables to be sensitive. Please remove the `sensitive` field or set the `sensitive` field to `true` in your configuration.", + ) +} + func (r *sharedEnvironmentVariableResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = req.ProviderTypeName + "_shared_environment_variable" }