SQL Auditing fixes #5052

ranisha2 · 2017-11-30T13:02:48Z

Fix for issue: Set-AzureRmSqlDatabaseAuditing does not work with AuditAction, Set-AzureRmSqlDatabaseAuditingPolicy works #5046
Blob auditing storage account key must be set from secondary storage account key when StorageKeyType 'Secondary' is provided.

Description

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

…into dev

Sync

- Blob auditing storage account key must be set from secondary storage account key when StorageKeyType 'Secondary' is provided. - Fixes for issue: #5046

yaakoviyun · 2017-11-30T13:07:16Z

src/ResourceManager/Sql/Commands.Sql/Auditing/Services/SqlAuditAdapter.cs

@@ -485,7 +485,7 @@ private BlobAuditingCreateOrUpdateParameters PolicizeBlobAuditingModel(BaseBlobA

        private static IList<string> ExtractAuditActionsAndGroups(BaseBlobAuditingPolicyModel model)
        {
-            var dbPolicyModel = model as DatabaseBlobAuditingPolicyModel;
+            var dbPolicyModel = model as DatabaseBlobAuditingSettingsModel;
            var actionsAndGroups = new List<string>();
            if (dbPolicyModel != null)


do we want to add a trace for
if (dbPolicyModel == null)

It's expected for server level

yaakoviyun · 2017-11-30T13:07:33Z

src/ResourceManager/Sql/Commands.Sql/Auditing/Services/SqlAuditAdapter.cs

@@ -470,7 +470,7 @@ private BlobAuditingCreateOrUpdateParameters PolicizeBlobAuditingModel(BaseBlobA
            if (!IgnoreStorage && (model.AuditState == AuditStateType.Enabled))
            {
                properties.StorageEndpoint = ExtractStorageAccountName(model, storageEndpointSuffix);
-                properties.StorageAccountAccessKey = ExtractStorageAccountKey(model.StorageAccountName);
+                properties.StorageAccountAccessKey = ExtractStorageAccountKey(model.StorageAccountName, model.StorageKeyType);


we need tests for this change

Added a check for AuditAction value. Regarding the storage account access key, the value is not returned thus it cannot be checked.

Update TestBlobAuditingOnDatabase tests to check for AuditAction value

cormacpayne

@ranisha2 please update the Sql change log to reflect the changes being made in this PR

ranisha2 · 2017-12-01T22:04:46Z

@cormacpayne updated the change log.

cormacpayne · 2017-12-04T05:57:22Z

On-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/230/

cormacpayne · 2017-12-04T05:57:51Z

@yaakoviyun Hey Yaakov, any additional comments?

yaakoviyun and others added 5 commits January 30, 2017 14:37

Merge remote-tracking branch 'refs/remotes/Azure/dev' into dev

e8e203a

Merge branch 'dev' of https://github.com/yaakoviyun/azure-powershell …

57b579a

…into dev

Merge remote-tracking branch 'refs/remotes/Azure/dev' into dev

efe2a51

Merge pull request #16 from Azure/preview

157b717

Sync

SQL Auditing fixes

d615c43

- Blob auditing storage account key must be set from secondary storage account key when StorageKeyType 'Secondary' is provided. - Fixes for issue: #5046

ranisha2 requested review from cormacpayne and yaakoviyun November 30, 2017 13:03

yaakoviyun suggested changes Nov 30, 2017

View reviewed changes

yaakoviyun previously approved these changes Nov 30, 2017

View reviewed changes

azuresdkci added the needs-review label Nov 30, 2017

azuresdkci assigned cormacpayne Nov 30, 2017

Update TestBlobAuditingOnDatabase tests to check for AuditAction value

03f85aa

Update TestBlobAuditingOnDatabase tests to check for AuditAction value

ranisha2 dismissed yaakoviyun’s stale review via 03f85aa November 30, 2017 15:06

cormacpayne suggested changes Nov 30, 2017

View reviewed changes

cormacpayne added needs-revision and removed needs-review labels Nov 30, 2017

cormacpayne assigned ranisha2 and unassigned cormacpayne Nov 30, 2017

twitchax and others added 2 commits November 30, 2017 14:37

Merge branch 'preview' into dev

974decd

Add fixes description to SQL change log

5077d12

maddieclayton assigned cormacpayne and unassigned ranisha2 Dec 2, 2017

maddieclayton added needs-review and removed needs-revision labels Dec 2, 2017

cormacpayne approved these changes Dec 4, 2017

View reviewed changes

cormacpayne added Waiting for On Demand and removed needs-review labels Dec 4, 2017

cormacpayne removed their assignment Dec 4, 2017

cormacpayne assigned yaakoviyun Dec 4, 2017

yaakoviyun approved these changes Dec 4, 2017

View reviewed changes

cormacpayne merged commit 9601aed into Azure:preview Dec 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SQL Auditing fixes #5052

SQL Auditing fixes #5052

ranisha2 commented Nov 30, 2017 •

edited by azuresdkci

Loading

yaakoviyun Nov 30, 2017

ranisha2 Nov 30, 2017

yaakoviyun Nov 30, 2017

ranisha2 Nov 30, 2017

cormacpayne left a comment

ranisha2 commented Dec 1, 2017

cormacpayne commented Dec 4, 2017

cormacpayne commented Dec 4, 2017

SQL Auditing fixes #5052

SQL Auditing fixes #5052

Conversation

ranisha2 commented Nov 30, 2017 • edited by azuresdkci Loading

Description

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

cormacpayne left a comment

Choose a reason for hiding this comment

ranisha2 commented Dec 1, 2017

cormacpayne commented Dec 4, 2017

cormacpayne commented Dec 4, 2017

ranisha2 commented Nov 30, 2017 •

edited by azuresdkci

Loading