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.

Sign up for GitHub

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

fix: alert configuration data source nil pointer with third party notifications #1513

Merged
merged 5 commits into from
Oct 11, 2023
Merged

fix: alert configuration data source nil pointer with third party notifications #1513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1dbe988
fix: alert configuration data source nil pointer with third party not…
AgustinBettati Oct 9, 2023
0a0e735
modify all third party notifications to use fake credentials so accep…
AgustinBettati Oct 6, 2023
18815e6
extract dummy keys to common variables
AgustinBettati Oct 9, 2023
74093aa
remove usage of project id env variable in third party alert configur…
AgustinBettati Oct 9, 2023
2220947
execute tests in parallel as they are not using same project
AgustinBettati Oct 9, 2023
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
24 changes: 14 additions & 10 deletions mongodbatlas/fw_data_source_mongodbatlas_alert_configuration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,21 @@ func TestAccConfigDSAlertConfiguration_withOutput(t *testing.T) {
}

func TestAccConfigDSAlertConfiguration_withPagerDuty(t *testing.T) {
AgustinBettati marked this conversation as resolved.
Show resolved Hide resolved
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"),
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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)
}
9 changes: 5 additions & 4 deletions mongodbatlas/fw_resource_mongodbatlas_alert_configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/v20230201006/admin"
matlas "go.mongodb.org/atlas/mongodbatlas"
Expand Down Expand Up @@ -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)),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why you need Int64PointerValue and util.IntPtrToInt64Ptr.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IntervalMin and DelayMin are of type *int, as in some case they may not be defined. Int64PointerValue and util.IntPtrToInt64Ptr are used to safely store the optional value into the model, without insecurely accessing the underlying value as done in *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),
Expand Down Expand Up @@ -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)

Expand Down
121 changes: 70 additions & 51 deletions mongodbatlas/fw_resource_mongodbatlas_alert_configuration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
)

Expand All @@ -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"),
Expand All @@ -398,23 +402,22 @@ func TestAccConfigRSAlertConfiguration_importPagerDuty(t *testing.T) {
}

func TestAccConfigRSAlertConfiguration_DataDog(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to before, are we still testing Datadog integration?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, terraform configurations being tested are the same. Change here is that api key is now a hardcoded dummy value, when testing in cloud-dev found that there is no need to pass a real api key, as long as the format is correct the alert notifications are created successfully.

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"),
Expand All @@ -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"),
Expand All @@ -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"),
Expand All @@ -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"),
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions mongodbatlas/util/type_conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ func Int64PtrToIntPtr(i64 *int64) *int {
return &i
}

func IntPtrToInt64Ptr(i *int) *int64 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems the inverse to admin.Int64PtrToIntPtr. , don't know if it makes sense to have it in the SDK

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this conversion only seems needed when using old sdk (that uses int type), so eventually will be removed.

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
Expand Down
Loading