Skip to content

Commit d019d30

Browse files
nebrassnickfloyd
andauthored
fix(resource/github_actions_environment_variable): handle existing variables on create (#2758)
The `github_actions_environment_variable` resource would fail with a 409 Conflict error if the environment variable already existed in GitHub but not in the Terraform state. This could happen if the variable was created manually in the GitHub UI. This commit changes the create function to handle this conflict. It now attempts to update the variable if it already exists, effectively performing an "upsert" operation. This makes the resource more resilient and avoids unexpected errors when managing existing resources. Fixes #2328 Co-authored-by: Nick Floyd <[email protected]>
1 parent f549d17 commit d019d30

File tree

3 files changed

+110
-31
lines changed

3 files changed

+110
-31
lines changed

github/data_source_github_repository_file_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,8 @@ func TestDataSourceGithubRepositoryFileRead(t *testing.T) {
294294
"commit_message": {Type: schema.TypeString},
295295
"content": {Type: schema.TypeString},
296296
"id": {Type: schema.TypeString},
297+
"sha": {Type: schema.TypeString},
298+
"ref": {Type: schema.TypeString},
297299
}
298300

299301
schema := schema.TestResourceDataRaw(t, testSchema, map[string]interface{}{
@@ -366,6 +368,8 @@ func TestDataSourceGithubRepositoryFileRead(t *testing.T) {
366368
"commit_message": {Type: schema.TypeString},
367369
"content": {Type: schema.TypeString},
368370
"id": {Type: schema.TypeString},
371+
"sha": {Type: schema.TypeString},
372+
"ref": {Type: schema.TypeString},
369373
}
370374

371375
schema := schema.TestResourceDataRaw(t, testSchema, map[string]interface{}{
@@ -408,8 +412,8 @@ func TestDataSourceGithubRepositoryFileRead(t *testing.T) {
408412
`, randomID)
409413

410414
check := resource.ComposeTestCheckFunc(
411-
resource.TestCheckNoResourceAttr(
412-
"data.github_repository_file.test", "id",
415+
resource.TestCheckResourceAttr(
416+
"data.github_repository_file.test", "id", "",
413417
),
414418
)
415419

github/resource_github_actions_environment_variable.go

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ import (
1212

1313
func resourceGithubActionsEnvironmentVariable() *schema.Resource {
1414
return &schema.Resource{
15-
Create: resourceGithubActionsEnvironmentVariableCreate,
15+
Create: resourceGithubActionsEnvironmentVariableCreateOrUpdate,
1616
Read: resourceGithubActionsEnvironmentVariableRead,
17-
Update: resourceGithubActionsEnvironmentVariableUpdate,
17+
Update: resourceGithubActionsEnvironmentVariableCreateOrUpdate,
1818
Delete: resourceGithubActionsEnvironmentVariableDelete,
1919
Importer: &schema.ResourceImporter{
2020
StateContext: schema.ImportStatePassthroughContext,
@@ -58,7 +58,7 @@ func resourceGithubActionsEnvironmentVariable() *schema.Resource {
5858
}
5959
}
6060

61-
func resourceGithubActionsEnvironmentVariableCreate(d *schema.ResourceData, meta interface{}) error {
61+
func resourceGithubActionsEnvironmentVariableCreateOrUpdate(d *schema.ResourceData, meta interface{}) error {
6262
client := meta.(*Owner).v3client
6363
owner := meta.(*Owner).name
6464
ctx := context.Background()
@@ -73,33 +73,22 @@ func resourceGithubActionsEnvironmentVariableCreate(d *schema.ResourceData, meta
7373
Value: d.Get("value").(string),
7474
}
7575

76+
// Try to create the variable first
7677
_, err := client.Actions.CreateEnvVariable(ctx, owner, repoName, escapedEnvName, variable)
7778
if err != nil {
78-
return err
79-
}
80-
81-
d.SetId(buildThreePartID(repoName, envName, name))
82-
return resourceGithubActionsEnvironmentVariableRead(d, meta)
83-
}
84-
85-
func resourceGithubActionsEnvironmentVariableUpdate(d *schema.ResourceData, meta interface{}) error {
86-
client := meta.(*Owner).v3client
87-
owner := meta.(*Owner).name
88-
ctx := context.Background()
89-
90-
repoName := d.Get("repository").(string)
91-
envName := d.Get("environment").(string)
92-
escapedEnvName := url.PathEscape(envName)
93-
name := d.Get("variable_name").(string)
94-
95-
variable := &github.ActionsVariable{
96-
Name: name,
97-
Value: d.Get("value").(string),
98-
}
99-
100-
_, err := client.Actions.UpdateEnvVariable(ctx, owner, repoName, escapedEnvName, variable)
101-
if err != nil {
102-
return err
79+
if ghErr, ok := err.(*github.ErrorResponse); ok {
80+
if ghErr.Response.StatusCode == http.StatusConflict {
81+
// Variable already exists, try to update instead
82+
_, err = client.Actions.UpdateEnvVariable(ctx, owner, repoName, escapedEnvName, variable)
83+
if err != nil {
84+
return err
85+
}
86+
} else {
87+
return err
88+
}
89+
} else {
90+
return err
91+
}
10392
}
10493

10594
d.SetId(buildThreePartID(repoName, envName, name))

github/resource_github_actions_environment_variable_test.go

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package github
22

33
import (
4+
"context"
45
"fmt"
6+
"net/url"
57
"strings"
68
"testing"
79

10+
"github.com/google/go-github/v66/github"
811
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
9-
1012
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
13+
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
1114
)
1215

1316
func TestAccGithubActionsEnvironmentVariable(t *testing.T) {
@@ -195,3 +198,86 @@ func TestAccGithubActionsEnvironmentVariable(t *testing.T) {
195198
})
196199
})
197200
}
201+
202+
func TestAccGithubActionsEnvironmentVariable_alreadyExists(t *testing.T) {
203+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
204+
repoName := fmt.Sprintf("tf-acc-test-%s", randomID)
205+
envName := "environment / test"
206+
varName := "test_variable"
207+
value := "my_variable_value"
208+
209+
config := fmt.Sprintf(`
210+
resource "github_repository" "test" {
211+
name = "%s"
212+
vulnerability_alerts = true
213+
}
214+
215+
resource "github_repository_environment" "test" {
216+
repository = github_repository.test.name
217+
environment = "%s"
218+
}
219+
220+
resource "github_actions_environment_variable" "variable" {
221+
repository = github_repository.test.name
222+
environment = github_repository_environment.test.environment
223+
variable_name = "%s"
224+
value = "%s"
225+
}
226+
`, repoName, envName, varName, value)
227+
228+
testCase := func(t *testing.T, mode string) {
229+
resource.Test(t, resource.TestCase{
230+
PreCheck: func() {
231+
skipUnlessMode(t, mode)
232+
},
233+
Providers: testAccProviders,
234+
Steps: []resource.TestStep{
235+
{
236+
// First, create the repository and environment.
237+
Config: fmt.Sprintf(`
238+
resource "github_repository" "test" {
239+
name = "%s"
240+
vulnerability_alerts = true
241+
}
242+
243+
resource "github_repository_environment" "test" {
244+
repository = github_repository.test.name
245+
environment = "%s"
246+
}
247+
`, repoName, envName),
248+
Check: resource.ComposeTestCheckFunc(
249+
func(s *terraform.State) error {
250+
// Now that the repo and env are created, create the variable using the API.
251+
client := testAccProvider.Meta().(*Owner).v3client
252+
owner := testAccProvider.Meta().(*Owner).name
253+
ctx := context.Background()
254+
escapedEnvName := url.PathEscape(envName)
255+
256+
variable := &github.ActionsVariable{
257+
Name: varName,
258+
Value: value,
259+
}
260+
_, err := client.Actions.CreateEnvVariable(ctx, owner, repoName, escapedEnvName, variable)
261+
return err
262+
},
263+
),
264+
},
265+
{
266+
// Now, run the full config. Terraform should detect the existing variable and "adopt" it.
267+
Config: config,
268+
Check: resource.ComposeTestCheckFunc(
269+
resource.TestCheckResourceAttr("github_actions_environment_variable.variable", "value", value),
270+
),
271+
},
272+
},
273+
})
274+
}
275+
276+
t.Run("with an individual account", func(t *testing.T) {
277+
testCase(t, individual)
278+
})
279+
280+
t.Run("with an organization account", func(t *testing.T) {
281+
testCase(t, organization)
282+
})
283+
}

0 commit comments

Comments
 (0)