Skip to content
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

New resource: azurerm_security_center_storage_defender #23242

Merged
merged 7 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/google/go-cmp v0.5.9
github.com/google/uuid v1.3.1
github.com/hashicorp/go-azure-helpers v0.59.0
github.com/hashicorp/go-azure-sdk v0.20230907.1113401
github.com/hashicorp/go-azure-sdk v0.20230911.1163300
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference these have to go into their own PR to avoid conflicts. So can you please revert this and also do a rebase since the go-azure-sdk has already been updated since the opening of this PR.

github.com/hashicorp/go-hclog v1.5.0
github.com/hashicorp/go-multierror v1.1.1
github.com/hashicorp/go-uuid v1.0.3
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brv
github.com/hashicorp/go-azure-helpers v0.12.0/go.mod h1:Zc3v4DNeX6PDdy7NljlYpnrdac1++qNW0I4U+ofGwpg=
github.com/hashicorp/go-azure-helpers v0.59.0 h1:E73yoPN2r1y0vvZ4dgAh8NRGuVTNXH3lZKRjT+4oNmA=
github.com/hashicorp/go-azure-helpers v0.59.0/go.mod h1:BQUQp5udwbJ8pnzl0wByCLVEEyPMAFpJ9vOREiCzObo=
github.com/hashicorp/go-azure-sdk v0.20230907.1113401 h1:rtikuZ4FzVZrJ4lbldIJk1oIa1+Ofnq76+Mv3muDbLs=
github.com/hashicorp/go-azure-sdk v0.20230907.1113401/go.mod h1:Rk63T4GsVOHb/WohiAX7F0tMEd8MIKV+g4aV0Jv4XEk=
github.com/hashicorp/go-azure-sdk v0.20230911.1163300 h1:NbSXrPwMtzFQoN+pBfku7y2akKabHvVX/kIQu1d+4TM=
github.com/hashicorp/go-azure-sdk v0.20230911.1163300/go.mod h1:Rk63T4GsVOHb/WohiAX7F0tMEd8MIKV+g4aV0Jv4XEk=
github.com/hashicorp/go-checkpoint v0.5.0 h1:MFYpPZCnQqQTE18jFwSII6eUQrD/oxMFp3mlgcqk5mU=
github.com/hashicorp/go-checkpoint v0.5.0/go.mod h1:7nfLNL10NsxqO4iWuW6tWW0HjZuDrwkBuEQsVcpCOgg=
github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
Expand Down
1 change: 1 addition & 0 deletions internal/provider/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ func SupportedTypedServices() []sdk.TypedServiceRegistration {
vmware.Registration{},
voiceservices.Registration{},
web.Registration{},
securitycenter.Registration{},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetical ordering

}
services = append(services, autoRegisteredTypedServices()...)
return services
Expand Down
6 changes: 6 additions & 0 deletions internal/services/securitycenter/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package client
import (
"github.com/Azure/azure-sdk-for-go/services/preview/security/mgmt/v3.0/security" // nolint: staticcheck
"github.com/hashicorp/go-azure-sdk/resource-manager/security/2021-06-01/assessmentsmetadata"
"github.com/hashicorp/go-azure-sdk/resource-manager/security/2022-12-01-preview/defenderforstorage"
pricings_v2023_01_01 "github.com/hashicorp/go-azure-sdk/resource-manager/security/2023-01-01/pricings"
"github.com/hashicorp/terraform-provider-azurerm/internal/common"
)
Expand All @@ -23,6 +24,7 @@ type Client struct {
SettingClient *security.SettingsClient
AutomationsClient *security.AutomationsClient
ServerVulnerabilityAssessmentClient *security.ServerVulnerabilityAssessmentClient
DefenderForStorageClient *defenderforstorage.DefenderForStorageClient
}

func NewClient(o *common.ClientOptions) *Client {
Expand Down Expand Up @@ -64,6 +66,9 @@ func NewClient(o *common.ClientOptions) *Client {
ServerVulnerabilityAssessmentClient := security.NewServerVulnerabilityAssessmentClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId, ascLocation)
o.ConfigureClient(&ServerVulnerabilityAssessmentClient.Client, o.ResourceManagerAuthorizer)

DefenderForStorageClient := defenderforstorage.NewDefenderForStorageClientWithBaseURI(o.ResourceManagerEndpoint)
o.ConfigureClient(&DefenderForStorageClient.Client, o.ResourceManagerAuthorizer)

return &Client{
AssessmentsClient: &AssessmentsClient,
AssessmentsMetadataClient: &AssessmentsMetadataClient,
Expand All @@ -77,5 +82,6 @@ func NewClient(o *common.ClientOptions) *Client {
SettingClient: &SettingClient,
AutomationsClient: &AutomationsClient,
ServerVulnerabilityAssessmentClient: &ServerVulnerabilityAssessmentClient,
DefenderForStorageClient: &DefenderForStorageClient,
}
}
11 changes: 11 additions & 0 deletions internal/services/securitycenter/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
type Registration struct{}

var _ sdk.UntypedServiceRegistrationWithAGitHubLabel = Registration{}
var _ sdk.TypedServiceRegistrationWithAGitHubLabel = Registration{}

func (r Registration) AssociatedGitHubLabel() string {
return "service/security-center"
Expand Down Expand Up @@ -51,3 +52,13 @@ func (r Registration) SupportedResources() map[string]*pluginsdk.Resource {
"azurerm_security_center_server_vulnerability_assessment_virtual_machine": resourceServerVulnerabilityAssessmentVirtualMachine(),
}
}

func (r Registration) DataSources() []sdk.DataSource {
return []sdk.DataSource{}
}

func (r Registration) Resources() []sdk.Resource {
return []sdk.Resource{
StorageDefenderResource{},
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,314 @@
package securitycenter

import (
"context"
"fmt"
"time"

"github.com/hashicorp/go-azure-helpers/lang/pointer"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonids"
"github.com/hashicorp/go-azure-sdk/resource-manager/security/2022-12-01-preview/defenderforstorage"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
)

type StorageDefenderResource struct{}

type StorageDefenderModel struct {
StorageAccountId string `tfschema:"storage_account_id"`
Enabled bool `tfschema:"enabled"`
OverrideSubscriptionSettings bool `tfschema:"override_subscription_settings_enabled"`
MalwareScanningOnUploadEnabled bool `tfschema:"malware_scanning_on_upload_enabled"`
MalwareScanningOnUploadCapPerMon int64 `tfschema:"malware_scanning_on_upload_cap_gb_per_month"`
SensitiveDataDiscoveryEnabled bool `tfschema:"sensitive_data_discovery_enabled"`
}

var _ sdk.ResourceWithUpdate = StorageDefenderResource{}

func (s StorageDefenderResource) IDValidationFunc() pluginsdk.SchemaValidateFunc {
return commonids.ValidateScopeID
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we extend this to parse the Scope part of the id as a storage account ID

}

func (s StorageDefenderResource) ModelObject() interface{} {
return &StorageDefenderModel{}
}

func (s StorageDefenderResource) ResourceType() string {
return "azurerm_security_center_storage_defender"
}

func (s StorageDefenderResource) Arguments() map[string]*schema.Schema {
return map[string]*schema.Schema{
"storage_account_id": {
Type: pluginsdk.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: commonids.ValidateStorageAccountID,
},

"enabled": {
Type: pluginsdk.TypeBool,
Required: true,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't expose the enabled field as explained in the contributor docs.

This field should be controlled purely by the creation of it enabled = true and the deletion of it enabled = false.


"override_subscription_settings_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
Default: false,
},

"malware_scanning_on_upload_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
Default: false,
},

"malware_scanning_on_upload_cap_gb_per_month": {
Type: pluginsdk.TypeInt,
Optional: true,
Default: -1,
ValidateFunc: func(i interface{}, s string) (warnings []string, errors []error) {
// it requires -1 or greater than 0
v, ok := i.(int)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %s to be integer", s))
return warnings, errors
}

if v == -1 {
return warnings, errors
}

return validation.IntAtLeast(-1)(i, s)
},
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does -1 mean in this case? Also what would 0 mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the service does not accept 0, and -1 means no limit on that cap. Do we need to map user input 0 to payload -1?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be more clearly/simply achieved with:

Suggested change
"malware_scanning_on_upload_cap_gb_per_month": {
Type: pluginsdk.TypeInt,
Optional: true,
Default: -1,
ValidateFunc: func(i interface{}, s string) (warnings []string, errors []error) {
// it requires -1 or greater than 0
v, ok := i.(int)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %s to be integer", s))
return warnings, errors
}
if v == -1 {
return warnings, errors
}
return validation.IntAtLeast(-1)(i, s)
},
},
"malware_scanning_on_upload_cap_gb_per_month": {
Type: pluginsdk.TypeInt,
Optional: true,
Default: -1,
ValidateFunc: validation.Any(
validation.IntAtLeast(1),
validation.IntInSlice([]int{-1}),
),
},


"sensitive_data_discovery_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
Default: false,
},
}
}

func (s StorageDefenderResource) Attributes() map[string]*schema.Schema {
return map[string]*schema.Schema{}
}

func (s StorageDefenderResource) Create() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 10 * time.Minute,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we bump this up to 30, with such short timeouts we run the risk of being affected by rate limiting by the API.

Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
var plan StorageDefenderModel
if err := metadata.Decode(&plan); err != nil {
return fmt.Errorf("decoding: %+v", err)
}

client := metadata.Client.SecurityCenter.DefenderForStorageClient

id := commonids.NewScopeID(plan.StorageAccountId)

resp, err := client.Get(ctx, id)
if err != nil {
if !response.WasNotFound(resp.HttpResponse) {
return fmt.Errorf("checking for existing %s: %+v", id, err)
}
}

if !response.WasNotFound(resp.HttpResponse) &&
resp.Model != nil && resp.Model.Properties != nil && resp.Model.Properties.IsEnabled != nil && *resp.Model.Properties.IsEnabled {
return tf.ImportAsExistsError(s.ResourceType(), id.ID())
}

input := defenderforstorage.DefenderForStorageSetting{
Properties: &defenderforstorage.DefenderForStorageSettingProperties{
IsEnabled: pointer.To(plan.Enabled),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be defaulted to true

OverrideSubscriptionLevelSettings: pointer.To(plan.OverrideSubscriptionSettings),
MalwareScanning: &defenderforstorage.MalwareScanningProperties{
OnUpload: &defenderforstorage.OnUploadProperties{
IsEnabled: pointer.To(plan.MalwareScanningOnUploadEnabled),
CapGBPerMonth: pointer.To(plan.MalwareScanningOnUploadCapPerMon),
},
},
SensitiveDataDiscovery: &defenderforstorage.SensitiveDataDiscoveryProperties{
IsEnabled: pointer.To(plan.SensitiveDataDiscoveryEnabled),
},
},
}

_, err = client.Create(ctx, id, input)
if err != nil {
return fmt.Errorf("creating: %+v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("creating: %+v", err)
return fmt.Errorf("creating %s: %+v", id, err)

}

metadata.SetID(id)
return nil
},
}
}

func (s StorageDefenderResource) Update() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 10 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
var plan StorageDefenderModel
if err := metadata.Decode(&plan); err != nil {
return fmt.Errorf("decoding: %+v", err)
}

client := metadata.Client.SecurityCenter.DefenderForStorageClient

id, err := commonids.ParseScopeID(metadata.ResourceData.Id())
if err != nil {
return fmt.Errorf("decoding %+v", err)
}

resp, err := client.Get(ctx, *id)
if err != nil {
return fmt.Errorf("retrieving %s: %+v", *id, err)
}

model := resp.Model
if model == nil {
return fmt.Errorf("retrieving %s: model was nil", *id)
}

prop := model.Properties
if prop == nil {
return fmt.Errorf("retrieving %s: properties was nil", *id)
}

if metadata.ResourceData.HasChange("enabled") {
prop.IsEnabled = pointer.To(plan.Enabled)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above this shouldn't be user configurable.

Suggested change
if metadata.ResourceData.HasChange("enabled") {
prop.IsEnabled = pointer.To(plan.Enabled)
}


if metadata.ResourceData.HasChange("override_subscription_settings_enabled") {
prop.OverrideSubscriptionLevelSettings = pointer.To(plan.OverrideSubscriptionSettings)
}

if prop.MalwareScanning == nil {
prop.MalwareScanning = &defenderforstorage.MalwareScanningProperties{}
}

if prop.MalwareScanning.OnUpload == nil {
prop.MalwareScanning.OnUpload = &defenderforstorage.OnUploadProperties{}
}

if metadata.ResourceData.HasChange("malware_scanning_on_upload_enabled") {
prop.MalwareScanning.OnUpload.IsEnabled = pointer.To(plan.MalwareScanningOnUploadEnabled)
}

if metadata.ResourceData.HasChange("malware_scanning_on_upload_cap_gb_per_month") {
prop.MalwareScanning.OnUpload.CapGBPerMonth = pointer.To(plan.MalwareScanningOnUploadCapPerMon)
}

if prop.SensitiveDataDiscovery == nil {
prop.SensitiveDataDiscovery = &defenderforstorage.SensitiveDataDiscoveryProperties{}
}

if metadata.ResourceData.HasChange("sensitive_data_discovery_enabled") {
prop.SensitiveDataDiscovery.IsEnabled = pointer.To(plan.SensitiveDataDiscoveryEnabled)
}

input := defenderforstorage.DefenderForStorageSetting{
Properties: prop,
}

_, err = client.Create(ctx, *id, input)
if err != nil {
return fmt.Errorf("updating: %+v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("updating: %+v", err)
return fmt.Errorf("updating %s: %+v", id, err)

}

return nil
},
}
}

func (s StorageDefenderResource) Read() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 5 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
client := metadata.Client.SecurityCenter.DefenderForStorageClient

id, err := commonids.ParseScopeID(metadata.ResourceData.Id())
if err != nil {
return fmt.Errorf("parsing %+v", err)
}

resp, err := client.Get(ctx, *id)
if err != nil {
if response.WasNotFound(resp.HttpResponse) {
return metadata.MarkAsGone(id)
}
return fmt.Errorf("reading %+v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("reading %+v", err)
return fmt.Errorf("reading %s: %+v", id, err)

}

state := StorageDefenderModel{
StorageAccountId: id.ID(),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a scoped ID, we need to parse the scope part of id as a storage account ID

Suggested change
state := StorageDefenderModel{
StorageAccountId: id.ID(),
}
storageAccountId, err := commonids.ParseStorageAccountID(id.Scope)
if err != nil {
return err
}
state := StorageDefenderModel{
StorageAccountId: storageAccountId.ID(),
}


if model := resp.Model; model != nil {
if prop := model.Properties; prop != nil {
state.Enabled = pointer.From(prop.IsEnabled)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we should check if it's disabled, and if so then mark the resource as gone.

state.OverrideSubscriptionSettings = pointer.From(prop.OverrideSubscriptionLevelSettings)

if ms := prop.MalwareScanning; ms != nil {
if onUpload := ms.OnUpload; onUpload != nil {
state.MalwareScanningOnUploadEnabled = pointer.From(onUpload.IsEnabled)
state.MalwareScanningOnUploadCapPerMon = pointer.From(onUpload.CapGBPerMonth)
}
}

if sdd := prop.SensitiveDataDiscovery; sdd != nil {
state.SensitiveDataDiscoveryEnabled = pointer.From(sdd.IsEnabled)
}
}
}

return metadata.Encode(&state)
},
}
}

func (s StorageDefenderResource) Delete() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 10 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
client := metadata.Client.SecurityCenter.DefenderForStorageClient

id, err := commonids.ParseScopeID(metadata.ResourceData.Id())
if err != nil {
return fmt.Errorf("parsing %+v", err)
}

resp, err := client.Get(ctx, *id)
if err != nil {
if !response.WasNotFound(resp.HttpResponse) {
return fmt.Errorf("reading %+v", err)
}
}
// if the resource has never been created, it returns 404.
// once created, it could only be set to disable.
if response.WasNotFound(resp.HttpResponse) {
return nil
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the resource doesn't exist (or in this case disabled) when the delete is run we should raise an error instead of returning nil

Suggested change
if err != nil {
if !response.WasNotFound(resp.HttpResponse) {
return fmt.Errorf("reading %+v", err)
}
}
// if the resource has never been created, it returns 404.
// once created, it could only be set to disable.
if response.WasNotFound(resp.HttpResponse) {
return nil
}
if err != nil {
return fmt.Errorf("retrieving %s: %+v", id, err)
}


input := defenderforstorage.DefenderForStorageSetting{
Properties: &defenderforstorage.DefenderForStorageSettingProperties{
IsEnabled: pointer.To(false),
},
}

_, err = client.Create(ctx, *id, input)
if err != nil {
return fmt.Errorf("deleting %+v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("deleting %+v", err)
return fmt.Errorf("deleting %s: %+v", id, err)

}

return nil
},
}
}
Loading