From a308471a65f49f839f956f7aa4ae39c0eeebe9a1 Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Wed, 11 Oct 2023 10:55:28 +0200 Subject: [PATCH] fix: alert configuration data source nil pointer with third party notifications (#1513) * fix: alert configuration data source nil pointer with third party notifications * modify all third party notifications to use fake credentials so acceptance tests are run in CI * extract dummy keys to common variables * remove usage of project id env variable in third party alert configuration tests * execute tests in parallel as they are not using same project --- ...e_mongodbatlas_alert_configuration_test.go | 24 ++-- ...source_mongodbatlas_alert_configuration.go | 9 +- ...e_mongodbatlas_alert_configuration_test.go | 121 ++++++++++-------- mongodbatlas/util/type_conversion.go | 9 ++ 4 files changed, 98 insertions(+), 65 deletions(-) diff --git a/mongodbatlas/fw_data_source_mongodbatlas_alert_configuration_test.go b/mongodbatlas/fw_data_source_mongodbatlas_alert_configuration_test.go index 152986c307..0760755fb0 100644 --- a/mongodbatlas/fw_data_source_mongodbatlas_alert_configuration_test.go +++ b/mongodbatlas/fw_data_source_mongodbatlas_alert_configuration_test.go @@ -102,21 +102,21 @@ func TestAccConfigDSAlertConfiguration_withOutput(t *testing.T) { } func TestAccConfigDSAlertConfiguration_withPagerDuty(t *testing.T) { - SkipTestExtCred(t) // Will skip because requires external credentials aka api key var ( alert = &matlas.AlertConfiguration{} dataSourceName = "data.mongodbatlas_alert_configuration.test" - projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID") - serviceKey = os.Getenv("PAGER_DUTY_SERVICE_KEY") + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + serviceKey = dummy32CharKey ) resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, + PreCheck: func() { testAccPreCheckBasic(t) }, ProtoV6ProviderFactories: testAccProviderV6Factories, CheckDestroy: testAccCheckMongoDBAtlasAlertConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccDSMongoDBAtlasAlertConfigurationConfigWithPagerDuty(projectID, serviceKey, true), + Config: testAccDSMongoDBAtlasAlertConfigurationConfigWithPagerDuty(orgID, projectName, serviceKey, true), Check: resource.ComposeTestCheckFunc( testAccCheckMongoDBAtlasAlertConfigurationExists(dataSourceName, alert), resource.TestCheckResourceAttrSet(dataSourceName, "project_id"), @@ -245,16 +245,20 @@ func testAccDSMongoDBAtlasAlertConfigurationWithOutputs(orgID, projectName, outp `, orgID, projectName, outputLabel) } -func testAccDSMongoDBAtlasAlertConfigurationConfigWithPagerDuty(projectID, serviceKey string, enabled bool) string { +func testAccDSMongoDBAtlasAlertConfigurationConfigWithPagerDuty(orgID, projectName, serviceKey string, enabled bool) string { return fmt.Sprintf(` +resource "mongodbatlas_project" "test" { + name = %[2]q + org_id = %[1]q +} resource "mongodbatlas_alert_configuration" "test" { - project_id = %[1]q + project_id = mongodbatlas_project.test.id event_type = "NO_PRIMARY" - enabled = "%[3]t" + enabled = "%[4]t" notification { type_name = "PAGER_DUTY" - service_key = %[2]q + service_key = %[3]q delay_min = 0 } } @@ -263,5 +267,5 @@ data "mongodbatlas_alert_configuration" "test" { project_id = "${mongodbatlas_alert_configuration.test.project_id}" alert_configuration_id = "${mongodbatlas_alert_configuration.test.id}" } - `, projectID, serviceKey, enabled) + `, orgID, projectName, serviceKey, enabled) } diff --git a/mongodbatlas/fw_resource_mongodbatlas_alert_configuration.go b/mongodbatlas/fw_resource_mongodbatlas_alert_configuration.go index 4262a566d6..7474ae9eab 100644 --- a/mongodbatlas/fw_resource_mongodbatlas_alert_configuration.go +++ b/mongodbatlas/fw_resource_mongodbatlas_alert_configuration.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas/framework/conversion" + "github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas/util" "github.com/mwielbut/pointy" "go.mongodb.org/atlas-sdk/v20231001001/admin" matlas "go.mongodb.org/atlas/mongodbatlas" @@ -744,9 +745,9 @@ func newTFNotificationModelListV2(n []admin.AlertsNotificationRootForGroup, curr Roles: value.Roles, ChannelName: conversion.StringPtrNullIfEmpty(value.ChannelName), DatadogRegion: conversion.StringPtrNullIfEmpty(value.DatadogRegion), - DelayMin: types.Int64Value(int64(*value.DelayMin)), + DelayMin: types.Int64PointerValue(util.IntPtrToInt64Ptr(value.DelayMin)), EmailAddress: conversion.StringPtrNullIfEmpty(value.EmailAddress), - IntervalMin: types.Int64Value(int64(*value.IntervalMin)), + IntervalMin: types.Int64PointerValue(util.IntPtrToInt64Ptr(value.IntervalMin)), MobileNumber: conversion.StringPtrNullIfEmpty(value.MobileNumber), OpsGenieRegion: conversion.StringPtrNullIfEmpty(value.OpsGenieRegion), TeamID: conversion.StringPtrNullIfEmpty(value.TeamId), @@ -804,8 +805,8 @@ func newTFNotificationModelListV2(n []admin.AlertsNotificationRootForGroup, curr newState.Username = conversion.StringPtrNullIfEmpty(value.Username) } - newState.IntervalMin = types.Int64Value(int64(*value.IntervalMin)) - newState.DelayMin = types.Int64Value(int64(*value.DelayMin)) + newState.IntervalMin = types.Int64PointerValue(util.IntPtrToInt64Ptr(value.IntervalMin)) + newState.DelayMin = types.Int64PointerValue(util.IntPtrToInt64Ptr(value.DelayMin)) newState.EmailEnabled = types.BoolValue(value.EmailEnabled != nil && *value.EmailEnabled) newState.SMSEnabled = types.BoolValue(value.SmsEnabled != nil && *value.SmsEnabled) diff --git a/mongodbatlas/fw_resource_mongodbatlas_alert_configuration_test.go b/mongodbatlas/fw_resource_mongodbatlas_alert_configuration_test.go index 26bf3926b4..bccfa3306a 100644 --- a/mongodbatlas/fw_resource_mongodbatlas_alert_configuration_test.go +++ b/mongodbatlas/fw_resource_mongodbatlas_alert_configuration_test.go @@ -364,13 +364,17 @@ func TestAccConfigRSAlertConfiguration_importConfigNotifications(t *testing.T) { }) } +// dummy keys used for credential values in third party notifications +const dummy32CharKey = "11111111111111111111111111111111" +const dummy36CharKey = "11111111-1111-1111-1111-111111111111" + // used for testing notification that does not define interval_min attribute func TestAccConfigRSAlertConfiguration_importPagerDuty(t *testing.T) { - SkipTestExtCred(t) // Will skip because requires external credentials aka api key var ( resourceName = "mongodbatlas_alert_configuration.test" - projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID") - serviceKey = os.Getenv("PAGER_DUTY_SERVICE_KEY") + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + serviceKey = dummy32CharKey alert = &matlas.AlertConfiguration{} ) @@ -380,7 +384,7 @@ func TestAccConfigRSAlertConfiguration_importPagerDuty(t *testing.T) { CheckDestroy: testAccCheckMongoDBAtlasAlertConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccMongoDBAtlasAlertConfigurationPagerDutyConfig(projectID, serviceKey, true), + Config: testAccMongoDBAtlasAlertConfigurationPagerDutyConfig(orgID, projectName, serviceKey, true), Check: resource.ComposeTestCheckFunc( testAccCheckMongoDBAtlasAlertConfigurationExists(resourceName, alert), resource.TestCheckResourceAttrSet(resourceName, "project_id"), @@ -398,23 +402,22 @@ func TestAccConfigRSAlertConfiguration_importPagerDuty(t *testing.T) { } func TestAccConfigRSAlertConfiguration_DataDog(t *testing.T) { - SkipTestExtCred(t) // Will skip because requires external credentials aka api key - SkipTest(t) // Will force skip if enabled var ( resourceName = "mongodbatlas_alert_configuration.test" - projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID") - ddAPIKey = os.Getenv("DD_API_KEY") + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + ddAPIKey = dummy32CharKey ddRegion = "US" alert = &matlas.AlertConfiguration{} ) - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheckBasic(t) }, ProtoV6ProviderFactories: testAccProviderV6Factories, CheckDestroy: testAccCheckMongoDBAtlasAlertConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccMongoDBAtlasAlertConfigurationConfigWithDataDog(projectID, ddAPIKey, ddRegion, true), + Config: testAccMongoDBAtlasAlertConfigurationConfigWithDataDog(orgID, projectName, ddAPIKey, ddRegion, true), Check: resource.ComposeTestCheckFunc( testAccCheckMongoDBAtlasAlertConfigurationExists(resourceName, alert), resource.TestCheckResourceAttrSet(resourceName, "project_id"), @@ -425,21 +428,21 @@ func TestAccConfigRSAlertConfiguration_DataDog(t *testing.T) { } func TestAccConfigRSAlertConfiguration_PagerDuty(t *testing.T) { - SkipTestExtCred(t) // Will skip because requires external credentials aka api key var ( resourceName = "mongodbatlas_alert_configuration.test" - projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID") - serviceKey = os.Getenv("PAGER_DUTY_SERVICE_KEY") + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + serviceKey = dummy32CharKey alert = &matlas.AlertConfiguration{} ) - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheckBasic(t) }, ProtoV6ProviderFactories: testAccProviderV6Factories, CheckDestroy: testAccCheckMongoDBAtlasAlertConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccMongoDBAtlasAlertConfigurationPagerDutyConfig(projectID, serviceKey, true), + Config: testAccMongoDBAtlasAlertConfigurationPagerDutyConfig(orgID, projectName, serviceKey, true), Check: resource.ComposeTestCheckFunc( testAccCheckMongoDBAtlasAlertConfigurationExists(resourceName, alert), resource.TestCheckResourceAttrSet(resourceName, "project_id"), @@ -450,21 +453,21 @@ func TestAccConfigRSAlertConfiguration_PagerDuty(t *testing.T) { } func TestAccConfigRSAlertConfiguration_OpsGenie(t *testing.T) { - SkipTestExtCred(t) // Will skip because requires external credentials aka api key var ( resourceName = "mongodbatlas_alert_configuration.test" - projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID") - apiKey = os.Getenv("OPS_GENIE_API_KEY") + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + apiKey = dummy36CharKey alert = &matlas.AlertConfiguration{} ) - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheckBasic(t) }, ProtoV6ProviderFactories: testAccProviderV6Factories, CheckDestroy: testAccCheckMongoDBAtlasAlertConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccMongoDBAtlasAlertConfigurationOpsGenieConfig(projectID, apiKey, true), + Config: testAccMongoDBAtlasAlertConfigurationOpsGenieConfig(orgID, projectName, apiKey, true), Check: resource.ComposeTestCheckFunc( testAccCheckMongoDBAtlasAlertConfigurationExists(resourceName, alert), resource.TestCheckResourceAttrSet(resourceName, "project_id"), @@ -475,21 +478,21 @@ func TestAccConfigRSAlertConfiguration_OpsGenie(t *testing.T) { } func TestAccConfigRSAlertConfiguration_VictorOps(t *testing.T) { - SkipTestExtCred(t) // Will skip because requires external credentials aka api key var ( resourceName = "mongodbatlas_alert_configuration.test" - projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID") - apiKey = os.Getenv("VICTOR_OPS_API_KEY") + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + apiKey = dummy36CharKey alert = &matlas.AlertConfiguration{} ) - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheckBasic(t) }, ProtoV6ProviderFactories: testAccProviderV6Factories, CheckDestroy: testAccCheckMongoDBAtlasAlertConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccMongoDBAtlasAlertConfigurationVictorOpsConfig(projectID, apiKey, true), + Config: testAccMongoDBAtlasAlertConfigurationVictorOpsConfig(orgID, projectName, apiKey, true), Check: resource.ComposeTestCheckFunc( testAccCheckMongoDBAtlasAlertConfigurationExists(resourceName, alert), resource.TestCheckResourceAttrSet(resourceName, "project_id"), @@ -777,19 +780,23 @@ func testAccMongoDBAtlasAlertConfigurationConfigWithThresholdUpdated(orgID, proj `, orgID, projectName, enabled, threshold) } -func testAccMongoDBAtlasAlertConfigurationConfigWithDataDog(projectID, dataDogAPIKey, dataDogRegion string, enabled bool) string { +func testAccMongoDBAtlasAlertConfigurationConfigWithDataDog(orgID, projectName, dataDogAPIKey, dataDogRegion string, enabled bool) string { return fmt.Sprintf(` +resource "mongodbatlas_project" "test" { + name = %[2]q + org_id = %[1]q +} resource "mongodbatlas_third_party_integration" "atlas_datadog" { - project_id = "%[1]s" + project_id = mongodbatlas_project.test.id type = "DATADOG" - api_key = "%[3]s" - region = "%[4]s" + api_key = "%[4]s" + region = "%[5]s" } resource "mongodbatlas_alert_configuration" "test" { - project_id = "%[1]s" + project_id = mongodbatlas_project.test.id event_type = "REPLICATION_OPLOG_WINDOW_RUNNING_OUT" - enabled = %t + enabled = %[3]t notification { type_name = "GROUP" @@ -820,57 +827,69 @@ resource "mongodbatlas_alert_configuration" "test" { units = "HOURS" } } - `, projectID, enabled, dataDogAPIKey, dataDogRegion) + `, orgID, projectName, enabled, dataDogAPIKey, dataDogRegion) } -func testAccMongoDBAtlasAlertConfigurationPagerDutyConfig(projectID, serviceKey string, enabled bool) string { +func testAccMongoDBAtlasAlertConfigurationPagerDutyConfig(orgID, projectName, serviceKey string, enabled bool) string { return fmt.Sprintf(` +resource "mongodbatlas_project" "test" { + name = %[2]q + org_id = %[1]q +} resource "mongodbatlas_alert_configuration" "test" { - project_id = %[1]q + project_id = mongodbatlas_project.test.id event_type = "NO_PRIMARY" - enabled = "%[3]t" + enabled = "%[4]t" notification { type_name = "PAGER_DUTY" - service_key = %[2]q + service_key = %[3]q delay_min = 0 } } - `, projectID, serviceKey, enabled) + `, orgID, projectName, serviceKey, enabled) } -func testAccMongoDBAtlasAlertConfigurationOpsGenieConfig(projectID, apiKey string, enabled bool) string { +func testAccMongoDBAtlasAlertConfigurationOpsGenieConfig(orgID, projectName, apiKey string, enabled bool) string { return fmt.Sprintf(` +resource "mongodbatlas_project" "test" { + name = %[2]q + org_id = %[1]q +} resource "mongodbatlas_alert_configuration" "test" { - project_id = %[1]q + project_id = mongodbatlas_project.test.id event_type = "NO_PRIMARY" - enabled = "%[3]t" + enabled = "%[4]t" notification { type_name = "OPS_GENIE" - ops_genie_api_key = %[2]q + ops_genie_api_key = %[3]q ops_genie_region = "US" delay_min = 0 } } - `, projectID, apiKey, enabled) + `, orgID, projectName, apiKey, enabled) } -func testAccMongoDBAtlasAlertConfigurationVictorOpsConfig(projectID, apiKey string, enabled bool) string { +func testAccMongoDBAtlasAlertConfigurationVictorOpsConfig(orgID, projectName, apiKey string, enabled bool) string { return fmt.Sprintf(` +resource "mongodbatlas_project" "test" { + name = %[2]q + org_id = %[1]q +} resource "mongodbatlas_alert_configuration" "test" { - project_id = %[1]q + project_id = mongodbatlas_project.test.id event_type = "NO_PRIMARY" - enabled = "%[3]t" + enabled = "%[4]t" notification { type_name = "VICTOR_OPS" - victor_ops_api_key = %[2]q + victor_ops_api_key = %[3]q victor_ops_routing_key = "testing" delay_min = 0 } } - `, projectID, apiKey, enabled) + `, orgID, projectName, apiKey, enabled) } func testAccMongoDBAtlasAlertConfigurationConfigEmptyMetricThresholdConfig(orgID, projectName string, enabled bool) string { diff --git a/mongodbatlas/util/type_conversion.go b/mongodbatlas/util/type_conversion.go index 273e5bf876..1690e5ffa4 100644 --- a/mongodbatlas/util/type_conversion.go +++ b/mongodbatlas/util/type_conversion.go @@ -28,6 +28,15 @@ func Int64PtrToIntPtr(i64 *int64) *int { return &i } +func IntPtrToInt64Ptr(i *int) *int64 { + if i == nil { + return nil + } + + i64 := int64(*i) + return &i64 +} + // IsStringPresent returns true if the string is non-empty. func IsStringPresent(strPtr *string) bool { return strPtr != nil && len(*strPtr) > 0