Skip to content

Commit 8ba3a26

Browse files
deigadeiga
committed
Address review comments
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
1 parent b12a7ca commit 8ba3a26

2 files changed

Lines changed: 77 additions & 27 deletions

File tree

‎github/resource_github_repository_vulnerability_alerts.go‎

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package github
33
import (
44
"context"
55
"fmt"
6+
"strconv"
67

8+
"github.com/hashicorp/terraform-plugin-log/tflog"
79
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
810
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
911
)
@@ -19,13 +21,18 @@ func resourceGithubRepositoryVulnerabilityAlerts() *schema.Resource {
1921
},
2022

2123
Schema: map[string]*schema.Schema{
22-
"repository_name": {
24+
"repository": {
2325
Type: schema.TypeString,
2426
Required: true,
2527
ForceNew: true,
2628
Description: "The repository name to configure vulnerability alerts for.",
2729
},
28-
"repository_owner": {
30+
"repository_id": {
31+
Type: schema.TypeInt,
32+
Computed: true,
33+
Description: "The ID of the repository to configure vulnerability alerts for.",
34+
},
35+
"owner": {
2936
Type: schema.TypeString,
3037
Required: true,
3138
ForceNew: true,
@@ -38,15 +45,17 @@ func resourceGithubRepositoryVulnerabilityAlerts() *schema.Resource {
3845
Description: "Whether vulnerability alerts are enabled for the repository.",
3946
},
4047
},
48+
49+
CustomizeDiff: diffRepository,
4150
}
4251
}
4352

4453
func resourceGithubRepositoryVulnerabilityAlertsCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
4554
meta := m.(*Owner)
4655
client := meta.v3client
4756

48-
owner := d.Get("repository_owner").(string)
49-
repoName := d.Get("repository_name").(string)
57+
owner := d.Get("owner").(string)
58+
repoName := d.Get("repository").(string)
5059

5160
vulnerabilityAlertsEnabled := d.Get("enabled").(bool)
5261
if vulnerabilityAlertsEnabled {
@@ -60,8 +69,15 @@ func resourceGithubRepositoryVulnerabilityAlertsCreate(ctx context.Context, d *s
6069
return diag.FromErr(err)
6170
}
6271
}
72+
repo, _, err := client.Repositories.Get(ctx, owner, repoName)
73+
if err != nil {
74+
return diag.FromErr(err)
75+
}
76+
if err = d.Set("repository_id", repo.GetID()); err != nil {
77+
return diag.FromErr(err)
78+
}
6379

64-
id, err := buildID(owner, repoName)
80+
id, err := buildID(owner, strconv.Itoa(int(repo.GetID())))
6581
if err != nil {
6682
return diag.FromErr(err)
6783
}
@@ -74,8 +90,8 @@ func resourceGithubRepositoryVulnerabilityAlertsRead(ctx context.Context, d *sch
7490
meta := m.(*Owner)
7591
client := meta.v3client
7692

77-
owner := d.Get("repository_owner").(string)
78-
repoName := d.Get("repository_name").(string)
93+
owner := d.Get("owner").(string)
94+
repoName := d.Get("repository").(string)
7995
vulnerabilityAlertsEnabled, _, err := client.Repositories.GetVulnerabilityAlerts(ctx, owner, repoName)
8096
if err != nil {
8197
return diag.Errorf("error reading repository vulnerability alerts: %s", err.Error())
@@ -91,8 +107,8 @@ func resourceGithubRepositoryVulnerabilityAlertsUpdate(ctx context.Context, d *s
91107
meta := m.(*Owner)
92108
client := meta.v3client
93109

94-
owner := d.Get("repository_owner").(string)
95-
repoName := d.Get("repository_name").(string)
110+
owner := d.Get("owner").(string)
111+
repoName := d.Get("repository").(string)
96112

97113
vulnerabilityAlertsEnabled := d.Get("enabled").(bool)
98114
if vulnerabilityAlertsEnabled {
@@ -114,8 +130,8 @@ func resourceGithubRepositoryVulnerabilityAlertsDelete(ctx context.Context, d *s
114130
meta := m.(*Owner)
115131
client := meta.v3client
116132

117-
owner := d.Get("repository_owner").(string)
118-
repoName := d.Get("repository_name").(string)
133+
owner := d.Get("owner").(string)
134+
repoName := d.Get("repository").(string)
119135
_, err := client.Repositories.DisableVulnerabilityAlerts(ctx, owner, repoName)
120136
if err != nil {
121137
return diag.FromErr(handleArchivedRepoDelete(err, "repository vulnerability alerts", d.Id(), owner, repoName))
@@ -124,22 +140,35 @@ func resourceGithubRepositoryVulnerabilityAlertsDelete(ctx context.Context, d *s
124140
return nil
125141
}
126142

127-
func resourceGithubRepositoryVulnerabilityAlertsImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
143+
func resourceGithubRepositoryVulnerabilityAlertsImport(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) {
144+
tflog.Debug(ctx, "Importing repository vulnerability alerts", map[string]any{"id": d.Id()})
128145
repoOwner, repoName, err := parseID2(d.Id())
129146
if err != nil {
130-
return nil, fmt.Errorf("invalid ID specified: supplied ID must be written as <repository_owner>/<repository_name>. Original error: %w", err)
147+
return nil, fmt.Errorf("invalid ID specified: supplied ID must be written as <repository_owner>:<repository_name>. Original error: %w", err)
131148
}
132-
if err := d.Set("repository_owner", repoOwner); err != nil {
149+
if err := d.Set("owner", repoOwner); err != nil {
133150
return nil, err
134151
}
135-
if err := d.Set("repository_name", repoName); err != nil {
152+
if err := d.Set("repository", repoName); err != nil {
136153
return nil, err
137154
}
138155

139-
id, err := buildID(repoOwner, repoName)
156+
meta := m.(*Owner)
157+
client := meta.v3client
158+
159+
repo, _, err := client.Repositories.Get(ctx, repoOwner, repoName)
160+
if err != nil {
161+
return nil, err
162+
}
163+
if err = d.Set("repository_id", repo.GetID()); err != nil {
164+
return nil, err
165+
}
166+
167+
id, err := buildID(repoOwner, strconv.Itoa(int(repo.GetID())))
140168
if err != nil {
141169
return nil, err
142170
}
171+
143172
d.SetId(id)
144173

145174
return []*schema.ResourceData{d}, nil

‎github/resource_github_repository_vulnerability_alerts_test.go‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
88
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
9+
"github.com/hashicorp/terraform-plugin-testing/terraform"
910
)
1011

1112
func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
@@ -21,8 +22,8 @@ func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
2122
}
2223
2324
resource "github_repository_vulnerability_alerts" "test" {
24-
repository_name = github_repository.test.name
25-
repository_owner = "%s"
25+
repository = github_repository.test.name
26+
owner = "%s"
2627
enabled = true
2728
}
2829
`, repoName, testAccConf.owner)
@@ -35,7 +36,7 @@ func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
3536
Config: config,
3637
Check: resource.ComposeTestCheckFunc(
3738
resource.TestCheckResourceAttr("github_repository_vulnerability_alerts.test", "enabled", "true"),
38-
resource.TestCheckResourceAttr("github_repository_vulnerability_alerts.test", "repository_name", repoName),
39+
resource.TestCheckResourceAttr("github_repository_vulnerability_alerts.test", "repository", repoName),
3940
),
4041
},
4142
},
@@ -54,8 +55,8 @@ func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
5455
}
5556
5657
resource "github_repository_vulnerability_alerts" "test" {
57-
repository_name = github_repository.test.name
58-
repository_owner = "%s"
58+
repository = github_repository.test.name
59+
owner = "%s"
5960
enabled = false
6061
}
6162
`, repoName, testAccConf.owner)
@@ -88,8 +89,8 @@ func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
8889
}
8990
9091
resource "github_repository_vulnerability_alerts" "test" {
91-
repository_name = github_repository.test.name
92-
repository_owner = "%s"
92+
repository = github_repository.test.name
93+
owner = "%s"
9394
enabled = %t
9495
}
9596
`
@@ -128,8 +129,8 @@ func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
128129
}
129130
130131
resource "github_repository_vulnerability_alerts" "test" {
131-
repository_name = github_repository.test.name
132-
repository_owner = "%s"
132+
repository = github_repository.test.name
133+
owner = "%s"
133134
enabled = %t
134135
}
135136
`
@@ -166,8 +167,8 @@ func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
166167
}
167168
168169
resource "github_repository_vulnerability_alerts" "test" {
169-
repository_name = github_repository.test.name
170-
repository_owner = "%s"
170+
repository = github_repository.test.name
171+
owner = "%s"
171172
enabled = true
172173
}
173174
`, repoName, testAccConf.owner)
@@ -186,6 +187,26 @@ func TestAccGithubRepositoryVulnerabilityAlerts(t *testing.T) {
186187
ResourceName: "github_repository_vulnerability_alerts.test",
187188
ImportState: true,
188189
ImportStateVerify: true,
190+
ImportStateIdFunc: func(state *terraform.State) (string, error) {
191+
vulnerabilityAlerts := state.RootModule().Resources["github_repository_vulnerability_alerts.test"]
192+
if vulnerabilityAlerts == nil {
193+
return "", fmt.Errorf("github_repository_vulnerability_alerts.test not found in state")
194+
}
195+
vulnerabilityAlertsID := vulnerabilityAlerts.Primary.ID
196+
if vulnerabilityAlertsID == "" {
197+
return "", fmt.Errorf("github_repository_vulnerability_alerts.test does not have an id in state")
198+
}
199+
owner, found := vulnerabilityAlerts.Primary.Attributes["owner"]
200+
if !found {
201+
return "", fmt.Errorf("github_repository_vulnerability_alerts.test does not have an owner in state")
202+
}
203+
repository, found := vulnerabilityAlerts.Primary.Attributes["repository"]
204+
if !found {
205+
return "", fmt.Errorf("github_repository_vulnerability_alerts.test does not have a repository in state")
206+
}
207+
return fmt.Sprintf("%s:%s", owner, repository), nil
208+
},
209+
ImportStateVerifyIgnore: []string{"enabled"},
189210
},
190211
},
191212
})

0 commit comments

Comments
 (0)