New Resource: azurerm_security_center_assessment_metadata #10124

neil-yechenwei · 2021-01-11T05:34:26Z

Currently, terraform doesn't support Security.AssessmentMetadata RP. So submitted this PR to implement it.

--- PASS: TestAccSecurityCenterAssessmentMetadata_basic (66.90s)
--- PASS: TestAccSecurityCenterAssessmentMetadata_complete (68.07s)
--- PASS: TestAccSecurityCenterAssessmentMetadata_update (125.80s)

The api in go sdk package:
https://github.com/Azure/azure-sdk-for-go/blob/master/services/preview/security/mgmt/v3.0/security/assessmentsmetadata.go

katbyte

We have some test failing:

------- Stdout: -------
=== RUN   TestAccSecurityCenterAssessmentMetadata_complete
=== PAUSE TestAccSecurityCenterAssessmentMetadata_complete
=== CONT  TestAccSecurityCenterAssessmentMetadata_complete
    testing.go:745: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.
        
        Error: Check failed: "azurerm_security_center_assessment_metadata.test" still exists
        
        State: <no state>
--- FAIL: TestAccSecurityCenterAssessmentMetadata_complete (43.00s)
FAIL

------- Stdout: -------
=== RUN   TestAccSecurityCenterAssessmentMetadata_update
=== PAUSE TestAccSecurityCenterAssessmentMetadata_update
=== CONT  TestAccSecurityCenterAssessmentMetadata_update
    testing.go:684: Step 0 error: Check failed: Check 1/1 error: "azurerm_security_center_assessment_metadata.test" did not exist
--- FAIL: TestAccSecurityCenterAssessmentMetadata_update (22.06s)
FAIL

neil-yechenwei · 2021-01-14T01:35:44Z

@katbyte , After rerun, seems all test cases passed on my local. Maybe rerun or clean-up test env can resolve your problem?

$ make testacc TEST=./azurerm/internal/services/securitycenter/ TESTARGS="-parallel 11 -test.run=TestAccSecurityCenterAssessmentMetadata_"
==> 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 ./azurerm/internal/services/securitycenter/ -v -parallel 11 -test.run=TestAccSecurityCenterAssessmentMetadata_ -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccSecurityCenterAssessmentMetadata_basic
=== PAUSE TestAccSecurityCenterAssessmentMetadata_basic
=== RUN TestAccSecurityCenterAssessmentMetadata_requiresImport
=== PAUSE TestAccSecurityCenterAssessmentMetadata_requiresImport
=== RUN TestAccSecurityCenterAssessmentMetadata_complete
=== PAUSE TestAccSecurityCenterAssessmentMetadata_complete
=== RUN TestAccSecurityCenterAssessmentMetadata_update
=== PAUSE TestAccSecurityCenterAssessmentMetadata_update
=== CONT TestAccSecurityCenterAssessmentMetadata_basic
=== CONT TestAccSecurityCenterAssessmentMetadata_update
=== CONT TestAccSecurityCenterAssessmentMetadata_complete
=== CONT TestAccSecurityCenterAssessmentMetadata_requiresImport
--- PASS: TestAccSecurityCenterAssessmentMetadata_basic (64.15s)
--- PASS: TestAccSecurityCenterAssessmentMetadata_complete (65.74s)
--- PASS: TestAccSecurityCenterAssessmentMetadata_requiresImport (79.13s)
--- PASS: TestAccSecurityCenterAssessmentMetadata_update (116.31s)
PASS
ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/securitycenter 121.073s

neil-yechenwei · 2021-01-19T00:23:05Z

@katbyte , may I ask whether the test cases are passed from your side?

neil-yechenwei · 2021-01-27T06:35:09Z

@katbyte , Seems test case can pass after rerun on CI.

tombuildsstuff

hi @neil-yechenwei

Thanks for this PR - I've taken a look through and left some comments inline - since several of these fields can't be (or don't make sense to be, in the case of the uuid) user specified, it'd be worth rethinking the design here (since we can generate/default values for these as necessary) - but if we can fix those up then we should be able to take another look.

Thanks!

tombuildsstuff · 2021-01-27T10:30:38Z

azurerm/internal/services/securitycenter/security_center_assessment_metadata_resource.go

+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: validation.IsUUID,
+			},


unless this UUID can represent a built-in value - to be honest there's not much point making this user configurable - instead we'd be better to make this a Computed-only field and generate a default value in the Create function here

tombuildsstuff · 2021-01-27T10:32:25Z

azurerm/internal/services/securitycenter/security_center_assessment_metadata_resource.go

+	}
+}
+
+func resourceArmSecurityCenterAssessmentMetadataCreateUpdate(d *schema.ResourceData, meta interface{}) error {


we'll need to split this Create and Update function here to be able to use ignore_changes on these fields (since d.GetOk won't capture that) - which also makes the uuid logic easier here if we're generating them

azurerm/internal/services/securitycenter/security_center_assessment_metadata_resource.go

tombuildsstuff · 2021-01-27T11:12:53Z

website/docs/r/security_center_assessment_metadata.html.markdown

+
+* `name` - (Required) The GUID as name which should be used for this Security Center Assessment Metadata. Changing this forces a new Security Center Assessment Metadata to be created.
+
+* `assessment_type` - (Required) The type of the Security Center Assessment which can be specified by user. Possible value is `CustomerManaged`.


if this is the only possible value, there's no point exposing this field - we may as well default it

website/docs/r/security_center_assessment_metadata.html.markdown

tombuildsstuff · 2021-01-27T11:14:53Z

website/docs/r/security_center_assessment_metadata.html.markdown

+
+* `threats` - (Optional) A list of the threat impacts for the Security Center Assessment. Possible values are `AccountBreach`, `DataExfiltration`, `DataSpillage`, `DenialOfService`, `ElevationOfPrivilege`, `MaliciousInsider`, `MissingCoverage` and `ThreatResistance`.
+
+* `user_impact` - (Optional) The user impact of the Security Center Assessment. Possible values are `Low`, `Moderate` and `High`.


is there a default value for this?

Based on previous testing, there is no default value when this property isn't specified.

website/docs/r/security_center_assessment_metadata.html.markdown

neil-yechenwei · 2021-02-02T12:24:45Z

@tombuildsstuff , thanks for your comments. I've updated code and provide my idea. Please have an another look.

katbyte

Thanks @neil-yechenwei - this LGTM 👍

ghost · 2021-02-26T01:44:26Z

This has been released in version 2.49.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.49.0"
}
# ... other configuration ...

ghost · 2021-03-25T14:50:06Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

neil-yechenwei added 6 commits December 31, 2020 15:06

New resource: azurerm_security_center_assessment_metadata

b490f76

remove unused partnerData property

3f20e36

update code

b29f306

update code

342fc43

Merge remote-tracking branch 'upstream/master' into iotsecuritysolution

45a88d4

update code

a105034

ghost added size/XL documentation labels Jan 11, 2021

katbyte requested changes Jan 14, 2021

View reviewed changes

neil-yechenwei requested a review from katbyte January 14, 2021 01:36

tombuildsstuff suggested changes Jan 27, 2021

View reviewed changes

tombuildsstuff added new-resource service/security-center waiting-response labels Jan 27, 2021

neil-yechenwei added 5 commits January 28, 2021 14:49

Update code

c2643c3

update code

1657798

fix lint issue

ef8daad

update code

bdfb029

Update code

2b5aef0

ghost removed the waiting-response label Feb 2, 2021

neil-yechenwei requested a review from tombuildsstuff February 2, 2021 12:26

neil-yechenwei added 3 commits February 20, 2021 12:09

update code

c44413d

Merge remote-tracking branch 'upstream/master' into iotsecuritysolution

8c215bb

update code

226cd4b

katbyte added this to the v2.49.0 milestone Feb 22, 2021

katbyte approved these changes Feb 22, 2021

View reviewed changes

katbyte merged commit afe0f3d into hashicorp:master Feb 22, 2021

katbyte added a commit that referenced this pull request Feb 22, 2021

CHANGELOG.md for #10124

c187e81

ghost locked as resolved and limited conversation to collaborators Mar 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: azurerm_security_center_assessment_metadata #10124

New Resource: azurerm_security_center_assessment_metadata #10124

neil-yechenwei commented Jan 11, 2021 •

edited

Loading

katbyte left a comment

neil-yechenwei commented Jan 14, 2021 •

edited

Loading

neil-yechenwei commented Jan 19, 2021

neil-yechenwei commented Jan 27, 2021 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Jan 27, 2021

neil-yechenwei Jan 28, 2021

tombuildsstuff Jan 27, 2021

neil-yechenwei Jan 28, 2021

tombuildsstuff Jan 27, 2021

neil-yechenwei Jan 28, 2021 •

edited

Loading

tombuildsstuff Jan 27, 2021

neil-yechenwei Jan 28, 2021

neil-yechenwei commented Feb 2, 2021 •

edited

Loading

katbyte left a comment

ghost commented Feb 26, 2021

ghost commented Mar 25, 2021


		* `name` - (Required) The GUID as name which should be used for this Security Center Assessment Metadata. Changing this forces a new Security Center Assessment Metadata to be created.

		* `assessment_type` - (Required) The type of the Security Center Assessment which can be specified by user. Possible value is `CustomerManaged`.


		* `threats` - (Optional) A list of the threat impacts for the Security Center Assessment. Possible values are `AccountBreach`, `DataExfiltration`, `DataSpillage`, `DenialOfService`, `ElevationOfPrivilege`, `MaliciousInsider`, `MissingCoverage` and `ThreatResistance`.

		* `user_impact` - (Optional) The user impact of the Security Center Assessment. Possible values are `Low`, `Moderate` and `High`.

New Resource: azurerm_security_center_assessment_metadata #10124

New Resource: azurerm_security_center_assessment_metadata #10124

Conversation

neil-yechenwei commented Jan 11, 2021 • edited Loading

katbyte left a comment

Choose a reason for hiding this comment

neil-yechenwei commented Jan 14, 2021 • edited Loading

neil-yechenwei commented Jan 19, 2021

neil-yechenwei commented Jan 27, 2021 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Jan 27, 2021

Choose a reason for hiding this comment

neil-yechenwei Jan 28, 2021

Choose a reason for hiding this comment

tombuildsstuff Jan 27, 2021

Choose a reason for hiding this comment

neil-yechenwei Jan 28, 2021

Choose a reason for hiding this comment

tombuildsstuff Jan 27, 2021

Choose a reason for hiding this comment

neil-yechenwei Jan 28, 2021 • edited Loading

Choose a reason for hiding this comment

tombuildsstuff Jan 27, 2021

Choose a reason for hiding this comment

neil-yechenwei Jan 28, 2021

Choose a reason for hiding this comment

neil-yechenwei commented Feb 2, 2021 • edited Loading

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Feb 26, 2021

ghost commented Mar 25, 2021

neil-yechenwei commented Jan 11, 2021 •

edited

Loading

neil-yechenwei commented Jan 14, 2021 •

edited

Loading

neil-yechenwei commented Jan 27, 2021 •

edited

Loading

neil-yechenwei Jan 28, 2021 •

edited

Loading

neil-yechenwei commented Feb 2, 2021 •

edited

Loading