Skip to content

Commit 8a7e6d0

Browse files
melinathmelinath
authored
Fixed incorrectly-ignored read for google_compute_reservation.share_settings (#17709)
1 parent 3848083 commit 8a7e6d0

4 files changed

Lines changed: 306 additions & 1 deletion

File tree

‎mmv1/products/compute/Reservation.yaml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ collection_url_key: 'items'
4848
include_in_tgc_next: true
4949
cai_resource_kind: 'Reservation'
5050
custom_code:
51+
decoder: templates/terraform/decoders/compute_reservation_share_settings_decoder.go.tmpl
5152
tgc_decoder: 'templates/tgc_next/decoders/reservation.go.tmpl'
5253
constants: 'templates/terraform/constants/reservation.go.tmpl'
5354
update_encoder: 'templates/terraform/update_encoder/reservation.go.tmpl'
@@ -90,6 +91,8 @@ examples:
9091
billing_account: 'BILLING_ACCT'
9192
exclude_docs: true
9293
external_providers: ["time"]
94+
ignore_read_extra:
95+
- share_settings.0.project_map
9396
- name: 'shared_reservation_beta'
9497
primary_resource_id: 'gce_reservation'
9598
vars:
@@ -101,6 +104,8 @@ examples:
101104
exclude_docs: true
102105
min_version: 'beta'
103106
external_providers: ["time"]
107+
ignore_read_extra:
108+
- share_settings.0.projects
104109
virtual_fields:
105110
- name: 'block_names'
106111
type: Array
@@ -167,7 +172,6 @@ properties:
167172
type: NestedObject
168173
description: |
169174
The share setting for reservations.
170-
ignore_read: true
171175
default_from_api: true
172176
properties:
173177
- name: 'shareType'
@@ -184,6 +188,7 @@ properties:
184188
description: |
185189
A map of project number and project config. This is only valid when shareType's value is SPECIFIC_PROJECTS.
186190
key_name: 'id'
191+
diff_suppress_func: 'computeReservationProjectMapDiffSuppress'
187192
value_type:
188193
type: NestedObject
189194
properties:
@@ -195,6 +200,7 @@ properties:
195200
type: Array
196201
description: |
197202
List of project IDs with which the reservation is shared.
203+
diff_suppress_func: 'computeReservationProjectsDiffSuppress'
198204
item_type:
199205
type: String
200206
min_version: 'beta'

‎mmv1/templates/terraform/constants/reservation.go.tmpl‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,56 @@ var (
88
_ = url.Parse
99
_ = rmClient.NewClient
1010
)
11+
12+
{{- if ne $.TargetVersionName "ga" }}
13+
func computeReservationShareSettingsSuppress(d *schema.ResourceData, suppressMap bool) bool {
14+
conf := d.GetRawConfig()
15+
if conf.IsNull() || !conf.GetAttr("share_settings").IsKnown() || conf.GetAttr("share_settings").IsNull() {
16+
return false
17+
}
18+
19+
shareSettingsSlice := conf.GetAttr("share_settings").AsValueSlice()
20+
if len(shareSettingsSlice) == 0 || shareSettingsSlice[0].IsNull() {
21+
return false
22+
}
23+
24+
shareSettings := shareSettingsSlice[0]
25+
26+
hasProjectMap := false
27+
if shareSettings.Type().HasAttribute("project_map") {
28+
projMapAttr := shareSettings.GetAttr("project_map")
29+
if !projMapAttr.IsNull() && projMapAttr.IsKnown() {
30+
if projMapAttr.LengthInt() > 0 {
31+
hasProjectMap = true
32+
}
33+
}
34+
}
35+
36+
hasProjects := false
37+
if shareSettings.Type().HasAttribute("projects") {
38+
projectsAttr := shareSettings.GetAttr("projects")
39+
if !projectsAttr.IsNull() && projectsAttr.IsKnown() {
40+
if projectsAttr.LengthInt() > 0 {
41+
hasProjects = true
42+
}
43+
}
44+
}
45+
46+
if suppressMap {
47+
return !hasProjectMap && hasProjects
48+
}
49+
return hasProjectMap && !hasProjects
50+
}
51+
52+
func computeReservationProjectMapDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
53+
return computeReservationShareSettingsSuppress(d, true)
54+
}
55+
56+
func computeReservationProjectsDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
57+
return computeReservationShareSettingsSuppress(d, false)
58+
}
59+
{{- else }}
60+
func computeReservationProjectMapDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
61+
return false
62+
}
63+
{{- end }}

‎mmv1/templates/terraform/decoders/compute_reservation_share_settings_decoder.go.tmpl‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
{{- if not (contains $.ProductMetadata.Compiler "terraformgoogleconversion") }}
2+
if shareSettingsVal, ok := res["shareSettings"]; ok && shareSettingsVal != nil {
3+
shareSettings := shareSettingsVal.(map[string]interface{})
4+
5+
config := meta.(*transport_tpg.Config)
6+
userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent)
7+
if err != nil {
8+
return nil, err
9+
}
10+
11+
// 1. Gather all unique project identifier strings from config
12+
uniqueConfigProjects := make(map[string]bool)
13+
14+
// Config items from project_map
15+
configProjectMapItems := make([]map[string]interface{}, 0)
16+
if val, ok := d.GetOk("share_settings.0.project_map"); ok && val != nil {
17+
for _, raw := range val.(*schema.Set).List() {
18+
item := raw.(map[string]interface{})
19+
configProjectMapItems = append(configProjectMapItems, item)
20+
if id, ok := item["id"].(string); ok && id != "" {
21+
uniqueConfigProjects[id] = true
22+
}
23+
if projId, ok := item["project_id"].(string); ok && projId != "" {
24+
uniqueConfigProjects[projId] = true
25+
}
26+
}
27+
}
28+
29+
{{- if ne $.TargetVersionName "ga" }}
30+
// Projects from projects array
31+
configProjectsList := make([]string, 0)
32+
if val, ok := d.GetOk("share_settings.0.projects"); ok && val != nil {
33+
for _, raw := range val.([]interface{}) {
34+
if proj, ok := raw.(string); ok && proj != "" {
35+
configProjectsList = append(configProjectsList, proj)
36+
uniqueConfigProjects[proj] = true
37+
}
38+
}
39+
}
40+
{{- end }}
41+
42+
// 2. Resolve all unique project identifiers to project numbers
43+
projectToNumber := make(map[string]string)
44+
for p := range uniqueConfigProjects {
45+
_, err := strconv.Atoi(p)
46+
if err != nil {
47+
// Resolve to project number
48+
proj, err := rmClient.NewClient(config, userAgent).Projects.Get(p).Do()
49+
if err != nil {
50+
log.Printf("[WARN] Failed to retrieve project metadata for %s: %s", p, err)
51+
continue
52+
}
53+
projNumStr := strconv.FormatInt(proj.ProjectNumber, 10)
54+
projectToNumber[p] = projNumStr
55+
} else {
56+
projectToNumber[p] = p
57+
}
58+
}
59+
60+
// Helper function to get project number for any configured string
61+
getProjectNumber := func(p string) string {
62+
if num, ok := projectToNumber[p]; ok {
63+
return num
64+
}
65+
return p
66+
}
67+
68+
// 3. Build lookup maps based on project numbers
69+
configProjectMapItemsByNumber := make(map[string]map[string]interface{})
70+
for _, item := range configProjectMapItems {
71+
id := item["id"].(string)
72+
idNum := getProjectNumber(id)
73+
configProjectMapItemsByNumber[idNum] = item
74+
}
75+
76+
{{- if ne $.TargetVersionName "ga" }}
77+
configProjectsByNumber := make(map[string]string)
78+
for _, proj := range configProjectsList {
79+
projNum := getProjectNumber(proj)
80+
configProjectsByNumber[projNum] = proj
81+
}
82+
{{- end }}
83+
84+
// 4. Update API response projectMap using the lookup map
85+
if apiProjectMapVal, ok := shareSettings["projectMap"]; ok && apiProjectMapVal != nil {
86+
apiProjectMap := apiProjectMapVal.(map[string]interface{})
87+
resolvedProjectMap := make(map[string]interface{})
88+
for id, raw := range apiProjectMap {
89+
originalMapItem := raw.(map[string]interface{})
90+
91+
resolvedID := id
92+
resolvedProjID := originalMapItem["projectId"].(string)
93+
94+
// Look up in config using the API key (which is a project number)
95+
if cfgItem, ok := configProjectMapItemsByNumber[id]; ok {
96+
resolvedID = cfgItem["id"].(string)
97+
resolvedProjID = cfgItem["project_id"].(string)
98+
}
99+
100+
resolvedMapItem := make(map[string]interface{})
101+
for k, v := range originalMapItem {
102+
resolvedMapItem[k] = v
103+
}
104+
resolvedMapItem["projectId"] = resolvedProjID
105+
106+
resolvedProjectMap[resolvedID] = resolvedMapItem
107+
}
108+
shareSettings["projectMap"] = resolvedProjectMap
109+
}
110+
111+
{{- if ne $.TargetVersionName "ga" }}
112+
// 5. Update API response projects array
113+
if apiProjectsVal, ok := shareSettings["projects"]; ok && apiProjectsVal != nil {
114+
apiProjects := apiProjectsVal.([]interface{})
115+
resolvedProjects := make([]interface{}, 0, len(apiProjects))
116+
for _, raw := range apiProjects {
117+
projNumStr := raw.(string)
118+
resolvedProjID := projNumStr
119+
if cfgProj, ok := configProjectsByNumber[projNumStr]; ok {
120+
resolvedProjID = cfgProj
121+
}
122+
resolvedProjects = append(resolvedProjects, resolvedProjID)
123+
}
124+
shareSettings["projects"] = resolvedProjects
125+
}
126+
{{- end }}
127+
128+
res["shareSettings"] = shareSettings
129+
}
130+
{{- end }}
131+
132+
return res, nil

‎mmv1/third_party/terraform/services/compute/reservation_diff_suppress_test.go.tmpl‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package compute
2+
3+
{{- if ne $.TargetVersionName "ga" }}
4+
import (
5+
"reflect"
6+
"testing"
7+
"unsafe"
8+
9+
"github.com/hashicorp/go-cty/cty"
10+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
11+
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
12+
)
13+
14+
func setResourceDataConfig(d *schema.ResourceData, config *terraform.ResourceConfig) {
15+
val := reflect.ValueOf(d).Elem()
16+
configField := val.FieldByName("config")
17+
ptr := unsafe.Pointer(configField.UnsafeAddr())
18+
*(**terraform.ResourceConfig)(ptr) = config
19+
}
20+
21+
func TestReservationDiffSuppress(t *testing.T) {
22+
t.Parallel()
23+
24+
reservationSchema := ResourceComputeReservation().Schema
25+
26+
cases := map[string]struct {
27+
rawConfig cty.Value
28+
expectSuppressMap bool
29+
expectSuppressProjs bool
30+
}{
31+
"only_project_map_configured": {
32+
rawConfig: cty.ObjectVal(map[string]cty.Value{
33+
"share_settings": cty.ListVal([]cty.Value{
34+
cty.ObjectVal(map[string]cty.Value{
35+
"share_type": cty.StringVal("SPECIFIC_PROJECTS"),
36+
"project_map": cty.SetVal([]cty.Value{
37+
cty.ObjectVal(map[string]cty.Value{
38+
"id": cty.StringVal("my-project-id"),
39+
"project_id": cty.StringVal("my-project-id"),
40+
}),
41+
}),
42+
}),
43+
}),
44+
}),
45+
expectSuppressMap: false,
46+
expectSuppressProjs: true,
47+
},
48+
"only_projects_configured": {
49+
rawConfig: cty.ObjectVal(map[string]cty.Value{
50+
"share_settings": cty.ListVal([]cty.Value{
51+
cty.ObjectVal(map[string]cty.Value{
52+
"share_type": cty.StringVal("SPECIFIC_PROJECTS"),
53+
"projects": cty.ListVal([]cty.Value{
54+
cty.StringVal("my-project-id"),
55+
}),
56+
}),
57+
}),
58+
}),
59+
expectSuppressMap: true,
60+
expectSuppressProjs: false,
61+
},
62+
"both_configured": {
63+
rawConfig: cty.ObjectVal(map[string]cty.Value{
64+
"share_settings": cty.ListVal([]cty.Value{
65+
cty.ObjectVal(map[string]cty.Value{
66+
"share_type": cty.StringVal("SPECIFIC_PROJECTS"),
67+
"project_map": cty.SetVal([]cty.Value{
68+
cty.ObjectVal(map[string]cty.Value{
69+
"id": cty.StringVal("my-project-id"),
70+
"project_id": cty.StringVal("my-project-id"),
71+
}),
72+
}),
73+
"projects": cty.ListVal([]cty.Value{
74+
cty.StringVal("another-project-id"),
75+
}),
76+
}),
77+
}),
78+
}),
79+
expectSuppressMap: false,
80+
expectSuppressProjs: false,
81+
},
82+
"neither_configured": {
83+
rawConfig: cty.ObjectVal(map[string]cty.Value{
84+
"share_settings": cty.ListVal([]cty.Value{
85+
cty.ObjectVal(map[string]cty.Value{
86+
"share_type": cty.StringVal("LOCAL"),
87+
}),
88+
}),
89+
}),
90+
expectSuppressMap: false,
91+
expectSuppressProjs: false,
92+
},
93+
}
94+
95+
for tn, tc := range cases {
96+
d := schema.TestResourceDataRaw(t, reservationSchema, map[string]interface{}{})
97+
98+
resourceConfig := &terraform.ResourceConfig{
99+
CtyValue: tc.rawConfig,
100+
}
101+
setResourceDataConfig(d, resourceConfig)
102+
103+
suppressMap := computeReservationProjectMapDiffSuppress("", "", "", d)
104+
if suppressMap != tc.expectSuppressMap {
105+
t.Errorf("case %s failed: expected projectMap suppress to be %v, got %v", tn, tc.expectSuppressMap, suppressMap)
106+
}
107+
108+
suppressProjs := computeReservationProjectsDiffSuppress("", "", "", d)
109+
if suppressProjs != tc.expectSuppressProjs {
110+
t.Errorf("case %s failed: expected projects suppress to be %v, got %v", tn, tc.expectSuppressProjs, suppressProjs)
111+
}
112+
}
113+
}
114+
{{- end }}

0 commit comments

Comments
 (0)