Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support project numbers in monitored project #15305

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/8454.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
monitoring: fixed an issue in `google_monitoring_monitored_project` where project numbers were not accepted for `name`
```
293 changes: 293 additions & 0 deletions google/resource_monitoring_monitored_project_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
package google

import (
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-provider-google/google/acctest"
"github.com/hashicorp/terraform-provider-google/google/envvar"
"github.com/hashicorp/terraform-provider-google/google/services/monitoring"
"github.com/hashicorp/terraform-provider-google/google/tpgresource"
)

func TestAccMonitoringMonitoredProject_projectNumLongForm(t *testing.T) {
t.Parallel()

context := map[string]interface{}{
"org_id": envvar.GetTestOrgFromEnv(t),
"project_id": envvar.GetTestProjectFromEnv(),
"random_suffix": acctest.RandString(t, 10),
}

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckMonitoringMonitoredProjectDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccMonitoringMonitoredProject_projectNumLongForm(context),
},
{
ResourceName: "google_monitoring_monitored_project.primary",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"metrics_scope"},
},
},
})
}

func TestAccMonitoringMonitoredProject_projectNumShortForm(t *testing.T) {
t.Parallel()

context := map[string]interface{}{
"org_id": envvar.GetTestOrgFromEnv(t),
"project_id": envvar.GetTestProjectFromEnv(),
"random_suffix": acctest.RandString(t, 10),
}

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckMonitoringMonitoredProjectDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccMonitoringMonitoredProject_projectNumShortForm(context),
},
{
ResourceName: "google_monitoring_monitored_project.primary",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"metrics_scope"},
},
},
})
}

func testAccMonitoringMonitoredProject_projectNumLongForm(context map[string]interface{}) string {
return acctest.Nprintf(`
resource "google_monitoring_monitored_project" "primary" {
metrics_scope = "%{project_id}"
name = "locations/global/metricsScopes/%{project_id}/projects/${google_project.basic.number}"
}

resource "google_project" "basic" {
project_id = "tf-test-m-id%{random_suffix}"
name = "tf-test-m-id%{random_suffix}-display"
org_id = "%{org_id}"
}
`, context)
}

func testAccMonitoringMonitoredProject_projectNumShortForm(context map[string]interface{}) string {
return acctest.Nprintf(`
resource "google_monitoring_monitored_project" "primary" {
metrics_scope = "%{project_id}"
name = "${google_project.basic.number}"
}

resource "google_project" "basic" {
project_id = "tf-test-m-id%{random_suffix}"
name = "tf-test-m-id%{random_suffix}-display"
org_id = "%{org_id}"
}
`, context)
}

func TestUnitMonitoringMonitoredProject_nameDiffSuppress(t *testing.T) {
for _, tc := range monitoringMonitoredProjectDiffSuppressTestCases {
tc.Test(t)
}
}

type MonitoringMonitoredProjectDiffSuppressTestCase struct {
Name string
KeysToSuppress []string
Before map[string]interface{}
After map[string]interface{}
}

var monitoringMonitoredProjectDiffSuppressTestCases = []MonitoringMonitoredProjectDiffSuppressTestCase{
// Project Id -> project Id
{
Name: "short project id to long project id suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "sameId",
},
After: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/sameId",
},
},
{
Name: "long project id to short project id suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/sameId",
},
After: map[string]interface{}{
"name": "sameId",
},
},
{
Name: "short project id to long project id show diff",
KeysToSuppress: []string{},
Before: map[string]interface{}{
"name": "oldId",
},
After: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/newId",
},
},
{
Name: "long project id to short project id show diff",
KeysToSuppress: []string{},
Before: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/oldId",
},
After: map[string]interface{}{
"name": "newId",
},
},

// Project Num -> Project Num
{
Name: "short project num to long project num suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "000000000000",
},
After: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
},
},
{
Name: "long project num to short project num suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
},
After: map[string]interface{}{
"name": "000000000000",
},
},
{
Name: "short project num to long project num show diff",
KeysToSuppress: []string{},
Before: map[string]interface{}{
"name": "000000000000",
},
After: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/111111111111",
},
},
{
Name: "long project num to short project num show diff",
KeysToSuppress: []string{},
Before: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
},
After: map[string]interface{}{
"name": "111111111111",
},
},

// Project id <--> Project num
// Every variation of this should be suppressed. We cannot detect
// if the project number matches the id within a diff suppress
{
Name: "short project id to long project num suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "oldId",
},
After: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/111111111111",
},
},
{
Name: "long project id to short project num suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/oldId",
},
After: map[string]interface{}{
"name": "111111111111",
},
},
{
Name: "short project num to long project id suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "000000000000",
},
After: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/newId",
},
},
{
Name: "long project num to short project id suppressed",
KeysToSuppress: []string{"name"},
Before: map[string]interface{}{
"name": "locations/global/metricsScopes/projectId/projects/000000000000",
},
After: map[string]interface{}{
"name": "newId",
},
},

// Empty -> anything (resource creation)
{
Name: "empty name to anything shows diff",
KeysToSuppress: []string{},
Before: map[string]interface{}{
"name": "",
},
After: map[string]interface{}{
"name": "newId",
},
},
}

func (tc *MonitoringMonitoredProjectDiffSuppressTestCase) Test(t *testing.T) {
mockResourceDiff := &tpgresource.ResourceDiffMock{
Before: tc.Before,
After: tc.After,
}

keySuppressionMap := map[string]bool{}
for key := range tc.Before {
keySuppressionMap[key] = false
}
for key := range tc.After {
keySuppressionMap[key] = false
}

for _, key := range tc.KeysToSuppress {
keySuppressionMap[key] = true
}

for key, tcSuppress := range keySuppressionMap {
oldValue, ok := tc.Before[key]
if !ok {
oldValue = ""
}
newValue, ok := tc.After[key]
if !ok {
newValue = ""
}
suppressed := monitoring.ResourceMonitoringMonitoredProjectNameDiffSuppressFunc(key, fmt.Sprintf("%v", oldValue), fmt.Sprintf("%v", newValue), mockResourceDiff)
if suppressed != tcSuppress {
var expectation string
if tcSuppress {
expectation = "be"
} else {
expectation = "not be"
}
t.Errorf("Test %s: expected key `%s` to %s suppressed", tc.Name, key, expectation)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,33 @@ import (
transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
)

func ResourceMonitoringMonitoredProjectNameDiffSuppressFunc(k, old, new string, d tpgresource.TerraformResourceDataChange) bool {
// Don't suppress if values are empty strings
if old == "" || new == "" {
return false
}

oldShort := tpgresource.GetResourceNameFromSelfLink(old)
newShort := tpgresource.GetResourceNameFromSelfLink(new)

// Suppress if short names are equal
if oldShort == newShort {
return true
}

_, isOldNumErr := tpgresource.StringToFixed64(oldShort)
isOldNumber := isOldNumErr == nil
_, isNewNumErr := tpgresource.StringToFixed64(newShort)
isNewNumber := isNewNumErr == nil

// Suppress if comparing a project number to project id
return isOldNumber != isNewNumber
}

func resourceMonitoringMonitoredProjectNameDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
return ResourceMonitoringMonitoredProjectNameDiffSuppressFunc(k, old, new, d)
}

func ResourceMonitoringMonitoredProject() *schema.Resource {
return &schema.Resource{
Create: resourceMonitoringMonitoredProjectCreate,
Expand Down Expand Up @@ -68,7 +95,7 @@ func ResourceMonitoringMonitoredProject() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: tpgresource.CompareResourceNames,
DiffSuppressFunc: resourceMonitoringMonitoredProjectNameDiffSuppress,
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}'`,
},
"create_time": {
Expand Down Expand Up @@ -363,9 +390,18 @@ func resourceMonitoringMonitoredProjectFindNestedObjectInList(d *schema.Resource
}
func resourceMonitoringMonitoredProjectDecoder(d *schema.ResourceData, meta interface{}, res map[string]interface{}) (map[string]interface{}, error) {
config := meta.(*transport_tpg.Config)

expectedName, _ := expandNestedMonitoringMonitoredProjectName(d.Get("name"), d, config)
expectedFlattenedName := flattenNestedMonitoringMonitoredProjectName(expectedName, d, config)
_, isNumErr := tpgresource.StringToFixed64(expectedFlattenedName.(string))
expectProjectNumber := isNumErr == nil

name := res["name"].(string)
name = tpgresource.GetResourceNameFromSelfLink(name)
if name != "" {

if expectProjectNumber {
res["name"] = name
} else if name != "" {
project, err := config.NewResourceManagerClient(config.UserAgent).Projects.Get(name).Do()
if err != nil {
return nil, err
Expand Down