Skip to content

Commit ef1e07e

Browse files
dglsparsonsdglsparsons
authored
Fix a couple of bugs around env vars (#394)
- We don't log properly if every env var creation fails - We were accidentally using encrypted values from the API response in some instances when 2 env vars had the same name & target, but different git branches.
1 parent 2e3d334 commit ef1e07e

File tree

2 files changed

+135
-14
lines changed

2 files changed

+135
-14
lines changed

‎client/environment_variable.go‎

Lines changed: 121 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ package client
22

33
import (
44
"context"
5+
"encoding/json"
6+
"errors"
57
"fmt"
8+
"strings"
69

710
"github.com/hashicorp/terraform-plugin-log/tflog"
811
)
@@ -60,13 +63,55 @@ func (c *Client) CreateEnvironmentVariable(ctx context.Context, request CreateEn
6063
}
6164

6265
if err != nil {
66+
// Try to parse detailed failure information from the API error response
67+
var apiErr APIError
68+
if errors.As(err, &apiErr) && len(apiErr.RawMessage) > 0 {
69+
var parsed struct {
70+
Error struct {
71+
Code string `json:"code"`
72+
Message string `json:"message"`
73+
} `json:"error"`
74+
Failed []FailedItem `json:"failed"`
75+
}
76+
if json.Unmarshal(apiErr.RawMessage, &parsed) == nil && len(parsed.Failed) > 0 {
77+
// Prefer the first failed item for single create
78+
f := parsed.Failed[0]
79+
// If it's a conflict, try to locate the conflicting env ID for a better message
80+
if f.Error.Code == "ENV_CONFLICT" {
81+
envs, errList := c.GetEnvironmentVariables(ctx, request.ProjectID, request.TeamID)
82+
if errList == nil {
83+
id, found := findConflictingEnvID(request.TeamID, request.ProjectID, EnvConflictError{
84+
Key: derefString(f.Error.EnvVarKey),
85+
Target: f.Error.Target,
86+
GitBranch: f.Error.GitBranch,
87+
}, envs)
88+
if found {
89+
return e, fmt.Errorf("failed to create environment variable, %s, conflicting environment variable ID is %s", f.Error.Message, id)
90+
}
91+
}
92+
}
93+
// Fallback to the message returned by the API for clarity
94+
msg := f.Error.Message
95+
if f.Error.Link != nil && *f.Error.Link != "" {
96+
msg = fmt.Sprintf("%s (see %s)", msg, *f.Error.Link)
97+
}
98+
return e, fmt.Errorf("failed to create environment variable, %s", msg)
99+
}
100+
}
63101
return e, fmt.Errorf("%w - %s", err, payload)
64102
}
65103
response.Created.Value = request.EnvironmentVariable.Value
66104
response.Created.TeamID = c.TeamID(request.TeamID)
67105
return response.Created, err
68106
}
69107

108+
func derefString(p *string) string {
109+
if p == nil {
110+
return ""
111+
}
112+
return *p
113+
}
114+
70115
func overlaps(s []string, e []string) bool {
71116
set := make(map[string]struct{}, len(s))
72117
for _, a := range s {
@@ -159,37 +204,102 @@ func (c *Client) CreateEnvironmentVariables(ctx context.Context, request CreateE
159204
body: payload,
160205
}, &response)
161206
if err != nil {
207+
// Attempt to parse detailed failure reasons from API error body
208+
var apiErr APIError
209+
if errors.As(err, &apiErr) && len(apiErr.RawMessage) > 0 {
210+
var parsed struct {
211+
Error struct {
212+
Code string `json:"code"`
213+
Message string `json:"message"`
214+
} `json:"error"`
215+
Failed []FailedItem `json:"failed"`
216+
}
217+
if json.Unmarshal(apiErr.RawMessage, &parsed) == nil && len(parsed.Failed) > 0 {
218+
// Optionally fetch envs once to augment conflict errors with IDs
219+
var envs []EnvironmentVariable
220+
var listErr error
221+
needsList := false
222+
for _, f := range parsed.Failed {
223+
if f.Error.Code == "ENV_CONFLICT" {
224+
needsList = true
225+
break
226+
}
227+
}
228+
if needsList {
229+
envs, listErr = c.GetEnvironmentVariables(ctx, request.ProjectID, request.TeamID)
230+
}
231+
msgs := make([]string, 0, len(parsed.Failed))
232+
for _, f := range parsed.Failed {
233+
msg := f.Error.Message
234+
if f.Error.Code == "ENV_CONFLICT" && listErr == nil {
235+
id, found := findConflictingEnvID(request.TeamID, request.ProjectID, EnvConflictError{
236+
Key: derefString(f.Error.EnvVarKey),
237+
Target: f.Error.Target,
238+
GitBranch: f.Error.GitBranch,
239+
}, envs)
240+
if found {
241+
msg = fmt.Sprintf("%s, conflicting environment variable ID is %s", msg, id)
242+
}
243+
}
244+
if f.Error.Link != nil && *f.Error.Link != "" {
245+
msg = fmt.Sprintf("%s (see %s)", msg, *f.Error.Link)
246+
}
247+
msgs = append(msgs, msg)
248+
}
249+
// de-duplicate while preserving order
250+
seen := make(map[string]struct{}, len(msgs))
251+
uniq := make([]string, 0, len(msgs))
252+
for _, m := range msgs {
253+
if _, ok := seen[m]; !ok {
254+
seen[m] = struct{}{}
255+
uniq = append(uniq, m)
256+
}
257+
}
258+
return nil, fmt.Errorf("failed to create environment variables, %s", strings.Join(uniq, "; "))
259+
}
260+
}
162261
return nil, fmt.Errorf("%w - %s", err, payload)
163262
}
164263

165264
decrypted := false
166-
for i := 0; i < len(response.Created); i++ {
265+
for i := range response.Created {
167266
// When env vars are created, their values are encrypted
168267
response.Created[i].Decrypted = &decrypted
169268
}
170269

171270
if len(response.Failed) > 0 {
172-
envs, err := c.GetEnvironmentVariables(ctx, request.ProjectID, request.TeamID)
173-
if err != nil {
174-
return response.Created, fmt.Errorf("failed to create environment variables. error detecting conflicting environment variables: %w", err)
271+
envs, listErr := c.GetEnvironmentVariables(ctx, request.ProjectID, request.TeamID)
272+
if listErr != nil {
273+
return response.Created, fmt.Errorf("failed to create environment variables. error detecting conflicting environment variables: %w", listErr)
175274
}
275+
msgs := make([]string, 0, len(response.Failed))
176276
for _, failed := range response.Failed {
277+
msg := failed.Error.Message
177278
if failed.Error.Code == "ENV_CONFLICT" {
178279
id, found := findConflictingEnvID(request.TeamID, request.ProjectID, EnvConflictError{
179-
Key: *failed.Error.EnvVarKey,
280+
Key: derefString(failed.Error.EnvVarKey),
180281
Target: failed.Error.Target,
181282
GitBranch: failed.Error.GitBranch,
182283
}, envs)
183284
if found {
184-
err = fmt.Errorf("%w, conflicting environment variable ID is %s", err, id)
185-
} else {
186-
err = fmt.Errorf("failed to create environment variables, %s", failed.Error.Message)
285+
msg = fmt.Sprintf("%s, conflicting environment variable ID is %s", msg, id)
187286
}
188-
} else {
189-
err = fmt.Errorf("failed to create environment variables, %s", failed.Error.Message)
287+
}
288+
if failed.Error.Link != nil && *failed.Error.Link != "" {
289+
msg = fmt.Sprintf("%s (see %s)", msg, *failed.Error.Link)
290+
}
291+
msgs = append(msgs, msg)
292+
}
293+
// de-duplicate while preserving order
294+
seen := make(map[string]struct{}, len(msgs))
295+
uniq := make([]string, 0, len(msgs))
296+
for _, m := range msgs {
297+
if _, ok := seen[m]; !ok {
298+
seen[m] = struct{}{}
299+
uniq = append(uniq, m)
190300
}
191301
}
192-
return response.Created, err
302+
return response.Created, fmt.Errorf("failed to create environment variables, %s", strings.Join(uniq, "; "))
193303
}
194304

195305
return response.Created, err

‎vercel/resource_project_environment_variables.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ import (
2121
"github.com/vercel/terraform-provider-vercel/v3/client"
2222
)
2323

24+
func strPtrEqual(a, b *string) bool {
25+
if a == nil && b == nil {
26+
return true
27+
}
28+
if a == nil || b == nil {
29+
return false
30+
}
31+
return *a == *b
32+
}
33+
2434
var (
2535
_ resource.Resource = &projectEnvironmentVariablesResource{}
2636
_ resource.ResourceWithConfigure = &projectEnvironmentVariablesResource{}
@@ -330,7 +340,7 @@ func convertResponseToProjectEnvironmentVariables(
330340
if diags.HasError() {
331341
return ProjectEnvironmentVariables{}, diags
332342
}
333-
if p.Key.ValueString() == e.Key && isSameStringSet(target, e.Target) && isSameStringSet(customEnvironmentIDs, e.CustomEnvironmentIDs) {
343+
if p.Key.ValueString() == e.Key && isSameStringSet(target, e.Target) && isSameStringSet(customEnvironmentIDs, e.CustomEnvironmentIDs) && strPtrEqual(p.GitBranch.ValueStringPointer(), e.GitBranch) {
334344
value = p.Value
335345
break
336346
}
@@ -405,6 +415,7 @@ func (r *projectEnvironmentVariablesResource) Create(ctx context.Context, req re
405415
"Error creating project environment variables",
406416
"Could not create project environment variables, unexpected error: "+err.Error(),
407417
)
418+
return
408419
}
409420

410421
result, diags := convertResponseToProjectEnvironmentVariables(ctx, created, plan, nil)
@@ -490,7 +501,7 @@ func (r *projectEnvironmentVariablesResource) Read(ctx context.Context, req reso
490501
resp.Diagnostics.Append(diags...)
491502
return
492503
}
493-
if ee.Key.ValueString() == e.Key && isSameStringSet(target, e.Target) && isSameStringSet(customEnvironmentIDs, e.CustomEnvironmentIDs) {
504+
if ee.Key.ValueString() == e.Key && isSameStringSet(target, e.Target) && isSameStringSet(customEnvironmentIDs, e.CustomEnvironmentIDs) && strPtrEqual(ee.GitBranch.ValueStringPointer(), e.GitBranch) {
494505
toUse = append(toUse, e)
495506
}
496507
}
@@ -599,7 +610,7 @@ func (r *projectEnvironmentVariablesResource) Update(ctx context.Context, req re
599610
resp.Diagnostics.Append(diags...)
600611
return
601612
}
602-
if ee.Key.ValueString() == e.Key && isSameStringSet(target, e.Target) && isSameStringSet(customEnvironmentIDs, e.CustomEnvironmentIDs) {
613+
if ee.Key.ValueString() == e.Key && isSameStringSet(target, e.Target) && isSameStringSet(customEnvironmentIDs, e.CustomEnvironmentIDs) && strPtrEqual(ee.GitBranch.ValueStringPointer(), e.GitBranch) {
603614
if e.Decrypted != nil && !*e.Decrypted {
604615
continue // We don't know if it's value is encrypted.
605616
}

0 commit comments

Comments
 (0)