fix: alert configuration data source nil pointer with third party notifications #1513
Conversation
…tance tests are run in CI
| @@ -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)), | |||
There was a problem hiding this comment.
I am not sure why you need Int64PointerValue and util.IntPtrToInt64Ptr.
There was a problem hiding this comment.
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.
mongodbatlas/fw_resource_mongodbatlas_alert_configuration_test.go
Outdated
Show resolved
Hide resolved
|
cloud-dev seems unstable with project creations, will wait some time and run CI again. |
|
| @@ -398,23 +402,22 @@ func TestAccConfigRSAlertConfiguration_importPagerDuty(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestAccConfigRSAlertConfiguration_DataDog(t *testing.T) { | |||
There was a problem hiding this comment.
similar to before, are we still testing Datadog integration?
There was a problem hiding this comment.
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.
| @@ -28,6 +28,15 @@ func Int64PtrToIntPtr(i64 *int64) *int { | |||
| return &i | |||
| } | |||
|
|
|||
| func IntPtrToInt64Ptr(i *int) *int64 { | |||
There was a problem hiding this comment.
this seems the inverse to admin.Int64PtrToIntPtr. , don't know if it makes sense to have it in the SDK
There was a problem hiding this comment.
this conversion only seems needed when using old sdk (that uses int type), so eventually will be removed.
Description
Jira ticket: INTMDB-1182
Bug found when validating fix for INTMDB-981.
This nil pointer error is encountered when using the alert configuration data source for a third party alert configuration notification that does not define the
interval_minattribute.An acceptance test reproducing this issue was defined but skipped in CI as it relies on external credentials. Was able to verify that dummy credentials can be used to successfully create third party alert notifications so adjusted the corresponding tests so they all run in our CI moving forward.
Type of change:
Required Checklist:
Further comments