Trying to fix Issue #18971 #18999

sinc59 · 2022-10-26T11:30:43Z

A PR to trying to fix the issue #18971.

Add the name argument and the ability to add one or more ressources with non-hardcoded contact name.
The API REST accept one or more contacts and the name "default1" is not anymore required see documentation.

Testing on an azure account:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # azurerm_security_center_contact.example will be created
  + resource "azurerm_security_center_contact" "example" {
      + alert_notifications = true
      + alerts_to_admins    = true
      + email               = "default@test.fr"
      + id                  = (known after apply)
      + name                = "me"
    }

  # azurerm_security_center_contact.example2 will be created
  + resource "azurerm_security_center_contact" "example2" {
      + alert_notifications = true
      + alerts_to_admins    = true
      + email               = "test2@test.fr"
      + id                  = (known after apply)
      + name                = "test2"
    }

Plan: 2 to add, 0 to change, 0 to destroy.

Result (using the azure cli to list accounts):

]$ az security contact list 
AlertNotifications    AlertsToAdmins    Email                    Location     Name    Phone
--------------------  ----------------  -----------------------  -----------  ------  -------
On                    On                default@test.fr          West Europe  me
On                    On                test2@test.fr            West Europe  test2

Best regards,
Sébastien Caloone

….com:sinc59/terraform-provider-azurerm into issue_18971_security_center_contact_resource

stephybun

Thanks for this PR @sinc59, unfortunately it looks like we have a test failure:

------- Stdout: -------
=== RUN   TestAccSecurityCenterContact
=== RUN   TestAccSecurityCenterContact/contact
=== RUN   TestAccSecurityCenterContact/contact/update
testcase.go:117: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
(map[string]string) {
}
(map[string]string) (len=1) {
(string) (len=4) "name": (string) (len=12) "test-account"
}
=== RUN   TestAccSecurityCenterContact/contact/requiresImport
testcase.go:117: Step 2/2, expected an error but got none
=== RUN   TestAccSecurityCenterContact/contact/phoneOptional
testcase.go:117: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
(map[string]string) {
}
(map[string]string) (len=1) {
(string) (len=4) "name": (string) (len=12) "test-account"
}
=== RUN   TestAccSecurityCenterContact/contact/basic
testcase.go:117: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
(map[string]string) {
}
(map[string]string) (len=1) {
(string) (len=4) "name": (string) (len=12) "test-account"
}
--- FAIL: TestAccSecurityCenterContact (497.63s)
--- FAIL: TestAccSecurityCenterContact/contact (497.63s)
--- FAIL: TestAccSecurityCenterContact/contact/update (165.09s)
--- FAIL: TestAccSecurityCenterContact/contact/requiresImport (158.78s)
--- FAIL: TestAccSecurityCenterContact/contact/phoneOptional (86.93s)
--- FAIL: TestAccSecurityCenterContact/contact/basic (86.83s)
FAIL

sinc59 · 2022-10-27T21:04:52Z

HI @stephybun
I'va pushed a new commit to fix the tests failed but, i get a new error on the acctests command:

$ make acctests SERVICE='securitycenter' TESTARGS='-run=TestAccSecurityCenterContact' TESTTIMEOUT='60m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./internal/services/securitycenter -run=TestAccSecurityCenterContact -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccSecurityCenterContact
=== RUN   TestAccSecurityCenterContact/contact
=== RUN   TestAccSecurityCenterContact/contact/basic
=== RUN   TestAccSecurityCenterContact/contact/update
=== RUN   TestAccSecurityCenterContact/contact/requiresImport
    testcase.go:117: Step 2/2, expected an error but got none
=== RUN   TestAccSecurityCenterContact/contact/phoneOptional
--- FAIL: TestAccSecurityCenterContact (702.89s)
    --- FAIL: TestAccSecurityCenterContact/contact (702.89s)
        --- PASS: TestAccSecurityCenterContact/contact/basic (106.19s)
        --- PASS: TestAccSecurityCenterContact/contact/update (324.70s)
        --- FAIL: TestAccSecurityCenterContact/contact/requiresImport (159.76s)
        --- PASS: TestAccSecurityCenterContact/contact/phoneOptional (112.23s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter        702.926s
FAIL
make: *** [GNUmakefile:100 : acctests] Erreur 1

I don't know why i get this error, after a manual import of the ressource, i get the contact name in the state ressource.
Did you have an idea ?
(I'm beginner on terraform and the go language)

Sébastien

katbyte

Thanks @sinc59 - looks like we have a test failure (by not failing)

------- Stdout: -------
=== RUN   TestAccSecurityCenterContact
=== RUN   TestAccSecurityCenterContact/contact
=== RUN   TestAccSecurityCenterContact/contact/basic
=== RUN   TestAccSecurityCenterContact/contact/update
=== RUN   TestAccSecurityCenterContact/contact/requiresImport
    testcase.go:117: Step 2/2, expected an error but got none
=== RUN   TestAccSecurityCenterContact/contact/phoneOptional
--- FAIL: TestAccSecurityCenterContact (477.09s)
    --- FAIL: TestAccSecurityCenterContact/contact (477.09s)
        --- PASS: TestAccSecurityCenterContact/contact/basic (94.99s)
        --- PASS: TestAccSecurityCenterContact/contact/update (150.93s)
        --- FAIL: TestAccSecurityCenterContact/contact/requiresImport (146.30s)
        --- PASS: TestAccSecurityCenterContact/contact/phoneOptional (84.87s)
FAIL

katbyte · 2022-10-27T22:45:04Z

internal/services/securitycenter/security_center_contact_resource.go

@@ -42,6 +36,13 @@ func resourceSecurityCenterContact() *pluginsdk.Resource {
 		},

 		Schema: map[string]*pluginsdk.Schema{
+			"name": {


since we are adding this property we'll need to update the docs?

Absolutely, i have not yet run the scaffold website command.

the docs still need updating & we'll need to make this Optional/Defaulted in 3.x / make this Required in 4.0 since otherwise this is a breaking change

stephybun · 2022-10-28T13:06:48Z

@sinc59 the test that's currently failing is expecting an error from this line

terraform-provider-azurerm/internal/services/securitycenter/security_center_contact_resource.go

Line 89 in d1b0da6

return tf.ImportAsExistsError("azurerm_security_center_contact", id.ID())

So something is amiss within that block.

Also if my understanding is correct it's possible to create multiple contacts with custom names now? This means we can split the tests out instead of grouping them which will make it possible to run them individually and might help with debugging the issue.

sinc59 · 2022-10-28T17:19:47Z

Yes @stephybun we can set multiple contacts with custom names.
I don't understand why i have an error on the tf.ImportAsExistsError because the import of the resource works (within the terraform import command).

But it's certainly a misapprehension from my part.

Sébastien

stephybun · 2022-10-28T17:23:24Z

Just to clarify, the requiresImport test validates that if the resource already exists then tf.ImportAsExistsError will throw an error, so the test is expecting an error but isn't receiving one.

stephybun · 2022-11-03T08:05:50Z

Tests are passing now

tombuildsstuff

Left one comment inline but otherwise 👍

tombuildsstuff · 2022-11-03T08:10:49Z

internal/services/securitycenter/security_center_contact_resource.go

@@ -42,6 +36,13 @@ func resourceSecurityCenterContact() *pluginsdk.Resource {
 		},

 		Schema: map[string]*pluginsdk.Schema{
+			"name": {


the docs still need updating & we'll need to make this Optional/Defaulted in 3.x / make this Required in 4.0 since otherwise this is a breaking change

github-actions · 2022-11-04T00:20:38Z

This functionality has been released in v3.30.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-12-05T02:14:25Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Trying to fix Issue hashicorp#18971

ba9441d

github-actions bot added the service/security-center label Oct 26, 2022

Sébastien Caloone added 3 commits October 26, 2022 13:45

Fix fmt

be71eef

Adding name variable on azurerm_security_center_contact resource

b6a4d2b

Merge branch 'issue_18971_security_center_contact_resource' of github…

3f1c81a

….com:sinc59/terraform-provider-azurerm into issue_18971_security_center_contact_resource

stephybun requested changes Oct 27, 2022

View reviewed changes

github-actions bot added the waiting-response label Oct 27, 2022

Adding name in resourceSecurityCenterContactRead func

d1b0da6

github-actions bot removed the waiting-response label Oct 27, 2022

katbyte requested changes Oct 27, 2022

View reviewed changes

github-actions bot added the waiting-response label Oct 27, 2022

github-actions bot removed the waiting-response label Oct 28, 2022

katbyte assigned stephybun Nov 1, 2022

github-actions bot added the size/M label Nov 3, 2022

pass SecurityContactName into Get function instead of SubscriptionId

fbb9951

stephybun force-pushed the issue_18971_security_center_contact_resource branch from 8e221e0 to fbb9951 Compare November 3, 2022 07:56

tombuildsstuff approved these changes Nov 3, 2022

View reviewed changes

add features flag for name property and update docs

2285458

stephybun added the enhancement label Nov 3, 2022

github-actions bot added the documentation label Nov 3, 2022

terrafmt

f66170b

stephybun merged commit 36f6aaa into hashicorp:main Nov 3, 2022

github-actions bot added this to the v3.30.0 milestone Nov 3, 2022

stephybun added a commit that referenced this pull request Nov 3, 2022

Update for #18999

a1abccd

stephybun mentioned this pull request Nov 3, 2022

Cannot set multiple azurerm_security_center_contact ressource #18971

Closed

1 task

github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trying to fix Issue #18971 #18999

Trying to fix Issue #18971 #18999

sinc59 commented Oct 26, 2022

stephybun left a comment

sinc59 commented Oct 27, 2022

katbyte left a comment

katbyte Oct 27, 2022

sinc59 Oct 28, 2022

tombuildsstuff Nov 3, 2022

stephybun commented Oct 28, 2022 •

edited

Loading

sinc59 commented Oct 28, 2022

stephybun commented Oct 28, 2022

stephybun commented Nov 3, 2022

tombuildsstuff left a comment

tombuildsstuff Nov 3, 2022

github-actions bot commented Nov 4, 2022

github-actions bot commented Dec 5, 2022

Trying to fix Issue #18971 #18999

Trying to fix Issue #18971 #18999

Conversation

sinc59 commented Oct 26, 2022

stephybun left a comment

Choose a reason for hiding this comment

sinc59 commented Oct 27, 2022

katbyte left a comment

Choose a reason for hiding this comment

katbyte Oct 27, 2022

Choose a reason for hiding this comment

sinc59 Oct 28, 2022

Choose a reason for hiding this comment

tombuildsstuff Nov 3, 2022

Choose a reason for hiding this comment

stephybun commented Oct 28, 2022 • edited Loading

sinc59 commented Oct 28, 2022

stephybun commented Oct 28, 2022

stephybun commented Nov 3, 2022

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Nov 3, 2022

Choose a reason for hiding this comment

github-actions bot commented Nov 4, 2022

github-actions bot commented Dec 5, 2022

stephybun commented Oct 28, 2022 •

edited

Loading