-
Notifications
You must be signed in to change notification settings - Fork 189
feat: Adds support for security contact in mongodbatlas_organization
resource & data sources
#3263
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
Conversation
@@ -38,42 +51,18 @@ func TestAccConfigRSOrganization_Basic(t *testing.T) { | |||
Steps: []resource.TestStep{ | |||
{ | |||
Config: configBasic(orgOwnerID, name, description, roleName, false, nil), | |||
Check: resource.ComposeAggregateTestCheckFunc( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored checks into common functions
APIx bot: a message has been sent to Docs Slack channel |
.changelog/3263.txt
Outdated
@@ -0,0 +1,11 @@ | |||
```release-note:enhancement | |||
resource/mongodbatlas_organization: Adds new `security_contact` attribute` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 'new' is redundant and can be dropped in all notes. also there is a wrong tick at the end of all messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
RestrictEmployeeAccess: conversion.Pointer(false), | ||
GenAIFeaturesEnabled: conversion.Pointer(true), | ||
} | ||
) | ||
|
||
func TestAccConfigRSOrganization_Basic(t *testing.T) { | ||
acc.SkipTestForCI(t) // affects the org |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess it can't run in CI yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it needs paying organizations to create new ones & dev is unstable when deleting orgs at this point as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
"multi_factor_auth_required": strconv.FormatBool(*settings.MultiFactorAuthRequired), | ||
"restrict_employee_access": strconv.FormatBool(*settings.RestrictEmployeeAccess), | ||
"gen_ai_features_enabled": strconv.FormatBool(*settings.GenAIFeaturesEnabled), | ||
"security_contact": settings.GetSecurityContact(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprises me a bit that we check the empty string ""
and it works. Would expect it to be null
in the state when it doesn't exist. 🤔 probably because it is SDKv2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes my understanding is that it's due to SDKv2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. Thank you for refactoring the test too!
Description
Adds support for security contact in
mongodbatlas_organization
resource & data sourcesLink to any related issue(s): CLOUDP-286563
Type of change:
Required Checklist:
Further comments