new resource "azurerm_iot_security_solution" #10034

njuCZ · 2021-01-04T09:04:18Z

intro:

doc https://docs.microsoft.com/en-us/azure/defender-for-iot/
iot security solution is a unified security solution for identifying IoT/OT assets, vulnerabilities, and threats

for the recommendation schema,

if create a iot security solution with no recommendation specified, all recommendation config are auto enabled.
If we only pass few options in recommendation and make it enabled or disabled, all other config are auto enabled.

to handle such matter and avoid diff, I have made each recommendation config as a sub schema, please review and check whether it's ok

katbyte

@njuCZ - all the tests are failing with:

Error: listing keys for IoTHub "acctestIoTHub-210113185506263492" (Resource Group "acctestRG-security-210113185506263492"): devices.IotHubResourceClient#ListKeys: Failure sending request: StatusCode=409 -- Original Error: autorest/azure: Service returned an error. Status=<nil> <nil>

njuCZ · 2021-01-14T06:04:37Z

@katbyte I have rerun the acctest in my local and all could succeed. Had a look at your error message, it seems the error occurs when reading iothub resource. So May I ask could you run iothub acctest successfully?

katbyte · 2021-01-19T16:48:44Z

@njuCZ - all the tests are still failing with that error.

njuCZ · 2021-01-20T06:28:26Z

@katbyte I reproduce such problem when using a different location. After investigating, I found deleting a iot security solution will cause the iotHub state changed to Transitioning, any further operation such as listKeys or Delete will throw 409 conflict. So I have added wait for state func. Now the acctest for iothub and iotSecuritySolution in teamcity could pass. Could you have a review again?

katbyte

Thanks for the pr @njuCZ - i have some comments and questions about the schema, once addressed i'll continue reviewing the PR

azurerm/internal/services/securitycenter/iot_security_solution_resource.go

website/docs/r/iot_security_solution.html.markdown

katbyte · 2021-01-21T18:27:16Z

website/docs/r/iot_security_solution.html.markdown

+A `user_defined_resource` block supports the following:
+
+* `query` - (Required) Azure Resource Graph query which represents the security solution's user defined resources.
+
+* `subscription_ids` - (Required) A list of subscription Ids on which the user defined resources query should be executed..


I think we should improve the ux here, i'm not sure what this is doing exactly? a list of iot hubs to apply this to via a graph query?

here is the doc about user_defined_resource: https://docs.microsoft.com/en-us/azure/defender-for-iot/quickstart-configure-your-solution. Through this field, user could monitor the entire IoT solution in one dashboard, surfacing all of your IoT devices, IoT platforms and back-end resources in Azure. so it is not for auto find iothub, but for other related resources.

In the portal, the ux is:

user choose the sub or resourceGroup, the portal will convert into graph query and send to the backend. Should we do like the portal and dynamically build the graph query?

Maybe it would be best to rename these to query_for_resources & query_subscription_ids?

njuCZ · 2021-01-25T07:22:50Z

@katbyte I have renamed the fields, could you have a look again?

katbyte · 2021-01-25T08:11:33Z

azurerm/internal/services/securitycenter/iot_security_solution_resource.go

+			"user_defined_resource": {
+				Type:     schema.TypeList,
+				Optional: true,
+				Computed: true,
+				MaxItems: 1,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"query_for_resources": {
+							Type:     schema.TypeString,
+							Required: true,
+						},
+
+						"query_subscription_ids": {
+							Type:     schema.TypeSet,
+							Required: true,
+							Elem: &schema.Schema{
+								Type:         schema.TypeString,
+								ValidateFunc: validation.IsUUID,
+							},
+						},
+					},
+				},
+			},


I think we can move these up one level? not sure if user_defined_resource really adds any useful information/ux?

katbyte

Thanks @njuCZ - some doc fixes and this is good to merge

katbyte · 2021-01-27T22:58:49Z

website/docs/r/iot_security_solution.html.markdown

+
+A `recommendations_enabled` block supports the following:
+
+* `events_to_exportacr_authentication` - (Optional) Could Service Principal Authentication be used with ACR repository? Defaults to `true`.


We need to fix thes properties:

Suggested change

* `events_to_exportacr_authentication` - (Optional) Could Service Principal Authentication be used with ACR repository? Defaults to `true`.

* `acr_authentication` - (Optional) Is Principal Authentication enabled for the ACR repository? Defaults to `true`.

and so forth

oh, sorry for all the typo

website/docs/r/iot_security_solution.html.markdown

njuCZ · 2021-01-28T02:57:33Z

@katbyte thanks for your careful review, I have updated the doc

tombuildsstuff · 2021-01-28T12:10:58Z

azurerm/internal/services/securitycenter/iot_security_solution_resource.go

+				Type:             schema.TypeString,
+				Optional:         true,
+				ValidateFunc:     loganalyticsValidate.LogAnalyticsWorkspaceID,
+				DiffSuppressFunc: suppress.CaseDifference,


@njuCZ is there a Swagger bug tracking this API bug?

have created an issue to track: Azure/azure-rest-api-specs#12721

thanks @njuCZ :)

tombuildsstuff · 2021-01-28T12:11:01Z

azurerm/internal/services/securitycenter/iot_security_solution_resource.go

+					Type:         schema.TypeString,
+					ValidateFunc: iothubValidate.IotHubID,
+				},
+				Set: set.HashStringIgnoreCase,


@njuCZ is there a Swagger bug tracking this API bug?

above, I used the same issue to track both

ghost · 2021-01-28T18:12:09Z

This has been released in version 2.45.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.45.0"
}
# ... other configuration ...

ghost · 2021-02-27T14:50:19Z

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!

new resource "azurerm_iot_security_solution"

9f873e8

ghost added size/XXL documentation labels Jan 4, 2021

fix typo

d0c8eb5

katbyte requested changes Jan 14, 2021

View reviewed changes

add wait for state func for iotHub

74f6028

njuCZ force-pushed the iot_security_solution branch from fdcfa62 to 74f6028 Compare January 20, 2021 05:30

update

e5d04df

njuCZ requested a review from katbyte January 20, 2021 06:29

katbyte added this to the v2.44.0 milestone Jan 21, 2021

tombuildsstuff modified the milestones: v2.44.0, v2.45.0 Jan 21, 2021

katbyte requested changes Jan 21, 2021

View reviewed changes

njuCZ added 2 commits January 22, 2021 11:35

update

19c5189

format

8ac2e74

njuCZ requested a review from katbyte January 22, 2021 05:04

rename to query_for_resources and query_subscription_ids

6f71da5

katbyte requested changes Jan 25, 2021

View reviewed changes

move "query_for_resources" and "query_subscription_ids" to top level

abb3b17

katbyte approved these changes Jan 27, 2021

View reviewed changes

update doc

ff5cef6

katbyte merged commit c0349b7 into hashicorp:master Jan 28, 2021

katbyte added the service/security-center label Jan 28, 2021

katbyte added a commit that referenced this pull request Jan 28, 2021

CHANGELOG.md for #10034

516f9ff

tombuildsstuff reviewed Jan 28, 2021

View reviewed changes

ghost locked as resolved and limited conversation to collaborators Feb 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new resource "azurerm_iot_security_solution" #10034

new resource "azurerm_iot_security_solution" #10034

njuCZ commented Jan 4, 2021

katbyte left a comment

njuCZ commented Jan 14, 2021 •

edited

Loading

katbyte commented Jan 19, 2021

njuCZ commented Jan 20, 2021

katbyte left a comment

katbyte Jan 21, 2021

njuCZ Jan 22, 2021 •

edited

Loading

katbyte Jan 23, 2021

njuCZ Jan 25, 2021

njuCZ commented Jan 25, 2021

katbyte Jan 25, 2021

njuCZ Jan 25, 2021

katbyte left a comment

katbyte Jan 27, 2021

njuCZ Jan 28, 2021

njuCZ commented Jan 28, 2021

tombuildsstuff Jan 28, 2021

njuCZ Jan 29, 2021

tombuildsstuff Jan 29, 2021

tombuildsstuff Jan 28, 2021

njuCZ Jan 29, 2021

ghost commented Jan 28, 2021

ghost commented Feb 27, 2021


		A `recommendations_enabled` block supports the following:

		* `events_to_exportacr_authentication` - (Optional) Could Service Principal Authentication be used with ACR repository? Defaults to `true`.

	* `events_to_exportacr_authentication` - (Optional) Could Service Principal Authentication be used with ACR repository? Defaults to `true`.
	* `acr_authentication` - (Optional) Is Principal Authentication enabled for the ACR repository? Defaults to `true`.

new resource "azurerm_iot_security_solution" #10034

new resource "azurerm_iot_security_solution" #10034

Conversation

njuCZ commented Jan 4, 2021

katbyte left a comment

Choose a reason for hiding this comment

njuCZ commented Jan 14, 2021 • edited Loading

katbyte commented Jan 19, 2021

njuCZ commented Jan 20, 2021

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njuCZ Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njuCZ commented Jan 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njuCZ commented Jan 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jan 28, 2021

ghost commented Feb 27, 2021

njuCZ commented Jan 14, 2021 •

edited

Loading

njuCZ Jan 22, 2021 •

edited

Loading