Skip to content

Adding new Powershell cmdlets to support set/get/remove of Azure AD administrator on MI #10057

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

Merged
merged 21 commits into from
Oct 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
cc77be4
Adding new Powershell cmdlets to support set/get/remove of Azure AD a…
SanjaMalesevic Sep 18, 2019
a938639
Merge branch 'master' into addPSCommandsForAadAdmin
SanjaMalesevic Sep 18, 2019
e910ebc
Fix syntax error in this file. It has occurred during merge.
SanjaMalesevic Sep 18, 2019
1163863
Skip this test
SanjaMalesevic Sep 18, 2019
f711aff
Delete recorded test
SanjaMalesevic Sep 18, 2019
8de5d3f
Merge pull request #1 from Azure/master
SanjaMalesevic Sep 19, 2019
135c95e
Merge pull request #2 from SanjaMalesevic/addPSCommandsForAadAdmin
SanjaMalesevic Sep 19, 2019
84c7242
Add recorded file
SanjaMalesevic Sep 19, 2019
7785de3
Remove file from local history
SanjaMalesevic Sep 19, 2019
6963685
Add InputObject Parameter Set
SanjaMalesevic Sep 23, 2019
b726588
Merge branch 'addPSCommandsForAadAdmin' of https://github.com/SanjaMa…
SanjaMalesevic Sep 23, 2019
9a82ef7
Add ResourceId Parameter Set
SanjaMalesevic Sep 23, 2019
86a8be5
Set default parameter set for get, set and remove
SanjaMalesevic Sep 23, 2019
809c89f
Rename parameter Name into InstanceName
SanjaMalesevic Sep 24, 2019
b4a6a65
Code review fixes
SanjaMalesevic Sep 26, 2019
29f37e3
Enable test for AAD admin on MI
SanjaMalesevic Sep 27, 2019
b597bf9
Read TenantId from json during Playback mode
SanjaMalesevic Sep 27, 2019
2e16333
Renaming session record test folder
SanjaMalesevic Sep 27, 2019
4d68fed
Update tenant id in DefaultContext using value from json
SanjaMalesevic Sep 30, 2019
00c8452
Code review fixes 2
SanjaMalesevic Oct 1, 2019
1ce0a90
Adding a comment into a test
SanjaMalesevic Oct 4, 2019
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// ----------------------------------------------------------------------------------
//
// Copyright Microsoft Corporation
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.ScenarioTest.SqlTests;
using Microsoft.WindowsAzure.Commands.ScenarioTest;
using Xunit;
using Xunit.Abstractions;
using RestTestFramework = Microsoft.Rest.ClientRuntime.Azure.TestFramework;

namespace Microsoft.Azure.Commands.Sql.Test.ScenarioTests
{
public class ManagedInstanceActiveDirectoryAdministratorTests : SqlTestsBase
{
public ManagedInstanceActiveDirectoryAdministratorTests(ITestOutputHelper output) : base(output)
{
}

protected override void SetupManagementClients(RestTestFramework.MockContext context)
{
var newResourcesClient = GetResourcesClient(context);
var sqlClient = GetSqlClient(context);
var networkClient = GetNetworkClient(context);
var graphClient = GetGraphClientVersion1_6(context);
Helper.SetupSomeOfManagementClients(newResourcesClient,sqlClient, networkClient, graphClient);
}

[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
public void TestManagedInstanceActiveDirectoryAdministrator()
{
RunPowerShellTest("Test-ManagedInstanceActiveDirectoryAdministrator");
}
}
}


Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# ----------------------------------------------------------------------------------
#
# Copyright Microsoft Corporation
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
# http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# ----------------------------------------------------------------------------------

<#
.SYNOPSIS
Tests for managing Active Directory Administrator on managed instance
#>
function Test-ManagedInstanceActiveDirectoryAdministrator
{
# Setup
$rg = Create-ResourceGroupForTest
Copy link
Member

Choose a reason for hiding this comment

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

You should follow the pattern of cleaning up created resources as part of the test, using a try/finally after creating the resource group, where the ResourceGroup is removed in the finally is generally the pattern to follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

$vnetName = "cl_initial"
Copy link
Member

Choose a reason for hiding this comment

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

You should generate random names for these - this allows each test execution to be indpenendent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I check all other tests in SQL module, and they don't use random name for vnet. I suppose that the vnet's should stay deterministic so in the record mode the next test can create MI quickly. Also, I noticed that the vnet is never dropped.
So, if you agreee I would leave this as it is now.

Copy link
Member

Choose a reason for hiding this comment

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

There is an entire facility for creatingand recording random names used by essentially every other test project. As you have this now, anyone who runs two of your tests in the same subscription will fail. Please fix this.

$subnetName = "Cool"

# Setup VNET
$virtualNetwork1 = CreateAndGetVirtualNetworkForManagedInstance $vnetName $subnetName $rg.Location
$subnetId = $virtualNetwork1.Subnets.where({ $_.Name -eq $subnetName })[0].Id

$managedInstance = Create-ManagedInstanceForTest $rg $subnetId

# If there is a need to re-record this test, these values must be changed to correspond to existing group and user from Azure Active Directory related to current subscription.
$activeDirectoryGroup1 = "aadadmin"
$activeDirectoryGroup1ObjectId = "52b6d571-5ff9-4b8f-92de-4a5b1bcdbbef"
$activeDirectoryUser1 = "CL AAD Test User"
$activeDirectoryUser1ObjectId = "034bb7d9-ca26-4c6f-abe0-4aff74fdca50"

try
{
# Verify there is no Active Directory Administrator set
$activeDirectoryAdmin = Get-AzSqlInstanceActiveDirectoryAdministrator -ResourceGroupName $rg.ResourceGroupName -InstanceName $managedInstance.ManagedInstanceName

Assert-Null $activeDirectoryAdmin

# Set an Active Directory Administrator Group on Managed Instance
# This command uses the Graph API to check if there is a user/group for provided DisplayName and ObjectId. Graph authentication blocks test passes, so if you need to record this test again, you must provide real token in
# MockTokenAuthenticationFactory constructor and change SetAuthenticationFactory in EnvironmentSetupHelper.
$activeDirectoryAdmin1 = Set-AzSqlInstanceActiveDirectoryAdministrator -ResourceGroupName $rg.ResourceGroupName -InstanceName $managedInstance.ManagedInstanceName -DisplayName $activeDirectoryGroup1 -ObjectId $activeDirectoryGroup1ObjectId

Assert-NotNull $activeDirectoryAdmin1

# Verify the correct Active Directory Administrator is set
Assert-AreEqual $activeDirectoryAdmin1.DisplayName $activeDirectoryGroup1
Assert-AreEqual $activeDirectoryAdmin1.ObjectId $activeDirectoryGroup1ObjectId

# Get an Active Directory Administrator
$activeDirectoryAdmin2 = Get-AzSqlInstanceActiveDirectoryAdministrator -ResourceGroupName $rg.ResourceGroupName -InstanceName $managedInstance.ManagedInstanceName

Assert-AreEqual $activeDirectoryAdmin2.DisplayName $activeDirectoryGroup1
Assert-AreEqual $activeDirectoryAdmin2.ObjectId $activeDirectoryGroup1ObjectId

# Set an Active Directory Administrator User on Managed Instance
$activeDirectoryAdmin3 = Set-AzSqlInstanceActiveDirectoryAdministrator -ResourceGroupName $rg.ResourceGroupName -InstanceName $managedInstance.ManagedInstanceName -DisplayName $activeDirectoryUser1 -ObjectId $activeDirectoryUser1ObjectId

Assert-AreEqual $activeDirectoryAdmin3.DisplayName $activeDirectoryUser1
Assert-AreEqual $activeDirectoryAdmin3.ObjectId $activeDirectoryUser1ObjectId

# Remove an Active Directory Administrator User from Managed Instance
$activeDirectoryAdmin4 = Remove-AzSqlInstanceActiveDirectoryAdministrator -ResourceGroupName $rg.ResourceGroupName -InstanceName $managedInstance.ManagedInstanceName -Force

# Verify that Active Directory Administrator was deleted
$activeDirectoryAdmin5 = Get-AzSqlInstanceActiveDirectoryAdministrator -ResourceGroupName $rg.ResourceGroupName -InstanceName $managedInstance.ManagedInstanceName

Assert-Null $activeDirectoryAdmin5
}
finally
{
Remove-ResourceGroupForTest $rg
}
}
35 changes: 33 additions & 2 deletions src/Sql/Sql.Test/ScenarioTests/SqlTestsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,17 @@
using Microsoft.Azure.Management.OperationalInsights;
using SDKMonitor = Microsoft.Azure.Management.Monitor;
using CommonMonitor = Microsoft.Azure.Management.Monitor.Version2018_09_01;
using Microsoft.Azure.Graph.RBAC;
using Microsoft.Azure.Management.KeyVault;
using Microsoft.Azure.Graph.RBAC;
using Microsoft.Azure.Commands.Common.Authentication.Abstractions;

namespace Microsoft.Azure.Commands.ScenarioTest.SqlTests
{
public class SqlTestsBase : RMTestBase
{
protected EnvironmentSetupHelper Helper;
protected string[] resourceTypesToIgnoreApiVersion;
private const string TenantIdKey = "TenantId";

protected SqlTestsBase(ITestOutputHelper output)
{
Expand Down Expand Up @@ -84,8 +86,8 @@ protected void RunPowerShellTest(params string[] scripts)
// Enable undo functionality as well as mock recording
using (var context = MockContext.Start(callingClassType, mockName))
{
SetupManagementClients(context);
Helper.SetupEnvironment(AzureModule.AzureResourceManager);
SetupManagementClients(context);
Helper.SetupModules(AzureModule.AzureResourceManager,
"ScenarioTests\\Common.ps1",
"ScenarioTests\\" + GetType().Name + ".ps1",
Expand Down Expand Up @@ -140,6 +142,35 @@ protected GraphRbacManagementClient GetGraphClient(MockContext context)
return graphClient;
}

protected Microsoft.Azure.Graph.RBAC.Version1_6.GraphRbacManagementClient GetGraphClientVersion1_6(MockContext context)
{
Microsoft.Azure.Graph.RBAC.Version1_6.GraphRbacManagementClient graphClient = context.GetServiceClient<Microsoft.Azure.Graph.RBAC.Version1_6.GraphRbacManagementClient>(TestEnvironmentFactory.GetTestEnvironment());
graphClient.BaseUri = TestEnvironmentFactory.GetTestEnvironment().Endpoints.GraphUri;
string tenantId = null;

if (HttpMockServer.Mode == HttpRecorderMode.Record)
{
tenantId = TestEnvironmentFactory.GetTestEnvironment().Tenant;
HttpMockServer.Variables[TenantIdKey] = tenantId;
}
Copy link
Member

Choose a reason for hiding this comment

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

You want to store the tenant ID in HttpMockServer.Variables in this case, I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

else if (HttpMockServer.Mode == HttpRecorderMode.Playback)
{
if (HttpMockServer.Variables.ContainsKey(TenantIdKey))
{
tenantId = HttpMockServer.Variables[TenantIdKey];
}
}
graphClient.TenantID = tenantId;
if (AzureRmProfileProvider.Instance != null &&
AzureRmProfileProvider.Instance.Profile != null &&
AzureRmProfileProvider.Instance.Profile.DefaultContext != null &&
AzureRmProfileProvider.Instance.Profile.DefaultContext.Tenant != null)
{
AzureRmProfileProvider.Instance.Profile.DefaultContext.Tenant.Id = tenantId;
}
return graphClient;
}

protected KeyVaultManagementClient GetKeyVaultClient(MockContext context)
{
return context.GetServiceClient<KeyVaultManagementClient>(TestEnvironmentFactory.GetTestEnvironment());
Expand Down
Loading