Skip to content

Commit bf4be67

Browse files
modular-magicianmodular-magician
authored
Support project numbers in monitored project (#8454) (#5955)
Signed-off-by: Modular Magician <[email protected]>
1 parent f42c9a2 commit bf4be67

File tree

3 files changed

+334
-2
lines changed

3 files changed

+334
-2
lines changed

‎.changelog/8454.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
monitoring: fixed an issue in `google_monitoring_monitored_project` where project numbers were not accepted for `name`
3+
```

‎google-beta/resource_monitoring_monitored_project_test.go‎

Lines changed: 293 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,293 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: MPL-2.0
3+
package google
4+
5+
import (
6+
"fmt"
7+
"testing"
8+
9+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
10+
"github.com/hashicorp/terraform-provider-google-beta/google-beta/acctest"
11+
"github.com/hashicorp/terraform-provider-google-beta/google-beta/envvar"
12+
"github.com/hashicorp/terraform-provider-google-beta/google-beta/services/monitoring"
13+
"github.com/hashicorp/terraform-provider-google-beta/google-beta/tpgresource"
14+
)
15+
16+
func TestAccMonitoringMonitoredProject_projectNumLongForm(t *testing.T) {
17+
t.Parallel()
18+
19+
context := map[string]interface{}{
20+
"org_id": envvar.GetTestOrgFromEnv(t),
21+
"project_id": envvar.GetTestProjectFromEnv(),
22+
"random_suffix": acctest.RandString(t, 10),
23+
}
24+
25+
acctest.VcrTest(t, resource.TestCase{
26+
PreCheck: func() { acctest.AccTestPreCheck(t) },
27+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
28+
CheckDestroy: testAccCheckMonitoringMonitoredProjectDestroyProducer(t),
29+
Steps: []resource.TestStep{
30+
{
31+
Config: testAccMonitoringMonitoredProject_projectNumLongForm(context),
32+
},
33+
{
34+
ResourceName: "google_monitoring_monitored_project.primary",
35+
ImportState: true,
36+
ImportStateVerify: true,
37+
ImportStateVerifyIgnore: []string{"metrics_scope"},
38+
},
39+
},
40+
})
41+
}
42+
43+
func TestAccMonitoringMonitoredProject_projectNumShortForm(t *testing.T) {
44+
t.Parallel()
45+
46+
context := map[string]interface{}{
47+
"org_id": envvar.GetTestOrgFromEnv(t),
48+
"project_id": envvar.GetTestProjectFromEnv(),
49+
"random_suffix": acctest.RandString(t, 10),
50+
}
51+
52+
acctest.VcrTest(t, resource.TestCase{
53+
PreCheck: func() { acctest.AccTestPreCheck(t) },
54+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
55+
CheckDestroy: testAccCheckMonitoringMonitoredProjectDestroyProducer(t),
56+
Steps: []resource.TestStep{
57+
{
58+
Config: testAccMonitoringMonitoredProject_projectNumShortForm(context),
59+
},
60+
{
61+
ResourceName: "google_monitoring_monitored_project.primary",
62+
ImportState: true,
63+
ImportStateVerify: true,
64+
ImportStateVerifyIgnore: []string{"metrics_scope"},
65+
},
66+
},
67+
})
68+
}
69+
70+
func testAccMonitoringMonitoredProject_projectNumLongForm(context map[string]interface{}) string {
71+
return acctest.Nprintf(`
72+
resource "google_monitoring_monitored_project" "primary" {
73+
metrics_scope = "%{project_id}"
74+
name = "locations/global/metricsScopes/%{project_id}/projects/${google_project.basic.number}"
75+
}
76+
77+
resource "google_project" "basic" {
78+
project_id = "tf-test-m-id%{random_suffix}"
79+
name = "tf-test-m-id%{random_suffix}-display"
80+
org_id = "%{org_id}"
81+
}
82+
`, context)
83+
}
84+
85+
func testAccMonitoringMonitoredProject_projectNumShortForm(context map[string]interface{}) string {
86+
return acctest.Nprintf(`
87+
resource "google_monitoring_monitored_project" "primary" {
88+
metrics_scope = "%{project_id}"
89+
name = "${google_project.basic.number}"
90+
}
91+
92+
resource "google_project" "basic" {
93+
project_id = "tf-test-m-id%{random_suffix}"
94+
name = "tf-test-m-id%{random_suffix}-display"
95+
org_id = "%{org_id}"
96+
}
97+
`, context)
98+
}
99+
100+
func TestUnitMonitoringMonitoredProject_nameDiffSuppress(t *testing.T) {
101+
for _, tc := range monitoringMonitoredProjectDiffSuppressTestCases {
102+
tc.Test(t)
103+
}
104+
}
105+
106+
type MonitoringMonitoredProjectDiffSuppressTestCase struct {
107+
Name string
108+
KeysToSuppress []string
109+
Before map[string]interface{}
110+
After map[string]interface{}
111+
}
112+
113+
var monitoringMonitoredProjectDiffSuppressTestCases = []MonitoringMonitoredProjectDiffSuppressTestCase{
114+
// Project Id -> project Id
115+
{
116+
Name: "short project id to long project id suppressed",
117+
KeysToSuppress: []string{"name"},
118+
Before: map[string]interface{}{
119+
"name": "sameId",
120+
},
121+
After: map[string]interface{}{
122+
"name": "locations/global/metricsScopes/projectId/projects/sameId",
123+
},
124+
},
125+
{
126+
Name: "long project id to short project id suppressed",
127+
KeysToSuppress: []string{"name"},
128+
Before: map[string]interface{}{
129+
"name": "locations/global/metricsScopes/projectId/projects/sameId",
130+
},
131+
After: map[string]interface{}{
132+
"name": "sameId",
133+
},
134+
},
135+
{
136+
Name: "short project id to long project id show diff",
137+
KeysToSuppress: []string{},
138+
Before: map[string]interface{}{
139+
"name": "oldId",
140+
},
141+
After: map[string]interface{}{
142+
"name": "locations/global/metricsScopes/projectId/projects/newId",
143+
},
144+
},
145+
{
146+
Name: "long project id to short project id show diff",
147+
KeysToSuppress: []string{},
148+
Before: map[string]interface{}{
149+
"name": "locations/global/metricsScopes/projectId/projects/oldId",
150+
},
151+
After: map[string]interface{}{
152+
"name": "newId",
153+
},
154+
},
155+
156+
// Project Num -> Project Num
157+
{
158+
Name: "short project num to long project num suppressed",
159+
KeysToSuppress: []string{"name"},
160+
Before: map[string]interface{}{
161+
"name": "000000000000",
162+
},
163+
After: map[string]interface{}{
164+
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
165+
},
166+
},
167+
{
168+
Name: "long project num to short project num suppressed",
169+
KeysToSuppress: []string{"name"},
170+
Before: map[string]interface{}{
171+
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
172+
},
173+
After: map[string]interface{}{
174+
"name": "000000000000",
175+
},
176+
},
177+
{
178+
Name: "short project num to long project num show diff",
179+
KeysToSuppress: []string{},
180+
Before: map[string]interface{}{
181+
"name": "000000000000",
182+
},
183+
After: map[string]interface{}{
184+
"name": "locations/global/metricsScopes/projectId/projects/111111111111",
185+
},
186+
},
187+
{
188+
Name: "long project num to short project num show diff",
189+
KeysToSuppress: []string{},
190+
Before: map[string]interface{}{
191+
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
192+
},
193+
After: map[string]interface{}{
194+
"name": "111111111111",
195+
},
196+
},
197+
198+
// Project id <--> Project num
199+
// Every variation of this should be suppressed. We cannot detect
200+
// if the project number matches the id within a diff suppress
201+
{
202+
Name: "short project id to long project num suppressed",
203+
KeysToSuppress: []string{"name"},
204+
Before: map[string]interface{}{
205+
"name": "oldId",
206+
},
207+
After: map[string]interface{}{
208+
"name": "locations/global/metricsScopes/projectId/projects/111111111111",
209+
},
210+
},
211+
{
212+
Name: "long project id to short project num suppressed",
213+
KeysToSuppress: []string{"name"},
214+
Before: map[string]interface{}{
215+
"name": "locations/global/metricsScopes/projectId/projects/oldId",
216+
},
217+
After: map[string]interface{}{
218+
"name": "111111111111",
219+
},
220+
},
221+
{
222+
Name: "short project num to long project id suppressed",
223+
KeysToSuppress: []string{"name"},
224+
Before: map[string]interface{}{
225+
"name": "000000000000",
226+
},
227+
After: map[string]interface{}{
228+
"name": "locations/global/metricsScopes/projectId/projects/newId",
229+
},
230+
},
231+
{
232+
Name: "long project num to short project id suppressed",
233+
KeysToSuppress: []string{"name"},
234+
Before: map[string]interface{}{
235+
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
236+
},
237+
After: map[string]interface{}{
238+
"name": "newId",
239+
},
240+
},
241+
242+
// Empty -> anything (resource creation)
243+
{
244+
Name: "empty name to anything shows diff",
245+
KeysToSuppress: []string{},
246+
Before: map[string]interface{}{
247+
"name": "",
248+
},
249+
After: map[string]interface{}{
250+
"name": "newId",
251+
},
252+
},
253+
}
254+
255+
func (tc *MonitoringMonitoredProjectDiffSuppressTestCase) Test(t *testing.T) {
256+
mockResourceDiff := &tpgresource.ResourceDiffMock{
257+
Before: tc.Before,
258+
After: tc.After,
259+
}
260+
261+
keySuppressionMap := map[string]bool{}
262+
for key := range tc.Before {
263+
keySuppressionMap[key] = false
264+
}
265+
for key := range tc.After {
266+
keySuppressionMap[key] = false
267+
}
268+
269+
for _, key := range tc.KeysToSuppress {
270+
keySuppressionMap[key] = true
271+
}
272+
273+
for key, tcSuppress := range keySuppressionMap {
274+
oldValue, ok := tc.Before[key]
275+
if !ok {
276+
oldValue = ""
277+
}
278+
newValue, ok := tc.After[key]
279+
if !ok {
280+
newValue = ""
281+
}
282+
suppressed := monitoring.ResourceMonitoringMonitoredProjectNameDiffSuppressFunc(key, fmt.Sprintf("%v", oldValue), fmt.Sprintf("%v", newValue), mockResourceDiff)
283+
if suppressed != tcSuppress {
284+
var expectation string
285+
if tcSuppress {
286+
expectation = "be"
287+
} else {
288+
expectation = "not be"
289+
}
290+
t.Errorf("Test %s: expected key `%s` to %s suppressed", tc.Name, key, expectation)
291+
}
292+
}
293+
}

‎google-beta/services/monitoring/resource_monitoring_monitored_project.go‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,33 @@ import (
3131
transport_tpg "github.com/hashicorp/terraform-provider-google-beta/google-beta/transport"
3232
)
3333

34+
func ResourceMonitoringMonitoredProjectNameDiffSuppressFunc(k, old, new string, d tpgresource.TerraformResourceDataChange) bool {
35+
// Don't suppress if values are empty strings
36+
if old == "" || new == "" {
37+
return false
38+
}
39+
40+
oldShort := tpgresource.GetResourceNameFromSelfLink(old)
41+
newShort := tpgresource.GetResourceNameFromSelfLink(new)
42+
43+
// Suppress if short names are equal
44+
if oldShort == newShort {
45+
return true
46+
}
47+
48+
_, isOldNumErr := tpgresource.StringToFixed64(oldShort)
49+
isOldNumber := isOldNumErr == nil
50+
_, isNewNumErr := tpgresource.StringToFixed64(newShort)
51+
isNewNumber := isNewNumErr == nil
52+
53+
// Suppress if comparing a project number to project id
54+
return isOldNumber != isNewNumber
55+
}
56+
57+
func resourceMonitoringMonitoredProjectNameDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
58+
return ResourceMonitoringMonitoredProjectNameDiffSuppressFunc(k, old, new, d)
59+
}
60+
3461
func ResourceMonitoringMonitoredProject() *schema.Resource {
3562
return &schema.Resource{
3663
Create: resourceMonitoringMonitoredProjectCreate,
@@ -68,7 +95,7 @@ func ResourceMonitoringMonitoredProject() *schema.Resource {
6895
Type: schema.TypeString,
6996
Required: true,
7097
ForceNew: true,
71-
DiffSuppressFunc: tpgresource.CompareResourceNames,
98+
DiffSuppressFunc: resourceMonitoringMonitoredProjectNameDiffSuppress,
7299
Description: `Immutable. The resource name of the 'MonitoredProject'. On input, the resource name includes the scoping project ID and monitored project ID. On output, it contains the equivalent project numbers. Example: 'locations/global/metricsScopes/{SCOPING_PROJECT_ID_OR_NUMBER}/projects/{MONITORED_PROJECT_ID_OR_NUMBER}'`,
73100
},
74101
"create_time": {
@@ -363,9 +390,18 @@ func resourceMonitoringMonitoredProjectFindNestedObjectInList(d *schema.Resource
363390
}
364391
func resourceMonitoringMonitoredProjectDecoder(d *schema.ResourceData, meta interface{}, res map[string]interface{}) (map[string]interface{}, error) {
365392
config := meta.(*transport_tpg.Config)
393+
394+
expectedName, _ := expandNestedMonitoringMonitoredProjectName(d.Get("name"), d, config)
395+
expectedFlattenedName := flattenNestedMonitoringMonitoredProjectName(expectedName, d, config)
396+
_, isNumErr := tpgresource.StringToFixed64(expectedFlattenedName.(string))
397+
expectProjectNumber := isNumErr == nil
398+
366399
name := res["name"].(string)
367400
name = tpgresource.GetResourceNameFromSelfLink(name)
368-
if name != "" {
401+
402+
if expectProjectNumber {
403+
res["name"] = name
404+
} else if name != "" {
369405
project, err := config.NewResourceManagerClient(config.UserAgent).Projects.Get(name).Do()
370406
if err != nil {
371407
return nil, err

0 commit comments

Comments
 (0)