Skip to content

Don't NullRef during Get-AzureDeployment when the cloud service has extended configuration #4788

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.

Sign up for GitHub

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 5 commits into from
Nov 2, 2017
Merged

Don't NullRef during Get-AzureDeployment when the cloud service has extended configuration #4788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2bb4cb3
Don't NullRef when performing Get-AzureDeployment when the cloud serv…
spaoliello Oct 13, 2017
7fc96a1
Add tests
spaoliello Oct 21, 2017
4a7653f
Remove summy secrets from test
spaoliello Oct 22, 2017
027ec89
Updated test session record
spaoliello Oct 23, 2017
f9a7cf5
Remove spurious metadata
spaoliello Oct 25, 2017
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
9 changes: 9 additions & 0 deletions src/ServiceManagement/Common/Commands.ScenarioTest/Commands.ScenarioTest.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,12 @@
<None Include="Resources\DiagnosticsExtension\Files\diagnostics.wadcfgx">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Include="Resources\ServiceManagement\Files\dSMSTest.cscfg">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Include="Resources\ServiceManagement\Files\dSMSTest.cspkg">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Include="Resources\SqlIaaSExtension\SqlIaaSExtensionTests.ps1">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
Expand Down Expand Up @@ -302,6 +308,9 @@
<None Include="SessionRecords\Microsoft.WindowsAzure.Commands.ScenarioTest.ServiceManagementTests\RunAzurePlatformVMImageNegativeTest.json">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Include="SessionRecords\Microsoft.WindowsAzure.Commands.ScenarioTest.ServiceManagementTests\RunDSMSHostedServiceTest.json">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Include="SessionRecords\Microsoft.WindowsAzure.Commands.ScenarioTest.ServiceManagementTests\RunEnableAndDisableDataCollectionTests.json">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
Expand Down
23 changes: 23 additions & 0 deletions ...eManagement/Common/Commands.ScenarioTest/Resources/ServiceManagement/Files/dSMSTest.cscfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
**********************************************************************************************

This file was generated by a tool from the project file: ServiceConfiguration.Cloud.cscfg

Changes to this file may cause incorrect behavior and will be lost if the file is regenerated.

**********************************************************************************************
-->
<ServiceConfiguration serviceName="AzureCloudService1" xmlns="http://schemas.microsoft.com/ServiceHosting/2008/10/ServiceConfiguration" osFamily="5" osVersion="*" schemaVersion="2015-04.2.6">
<Role name="WorkerRole1">
<Instances count="1" />
<ConfigurationSettings>
<Setting name="Microsoft.WindowsAzure.Plugins.Diagnostics.ConnectionString" value="UseDevelopmentStorage=true" />
<Setting name="DummySetting" value="Foo" />
</ConfigurationSettings>
<Certificates>
<Certificate name="ThumbprintCert" thumbprint="{{PLACEHOLDER}}" thumbprintAlgorithm="sha1" />
<Certificate name="dSMSCert" sourceLocation="/genevacustomers/monitoring/prod/warmpath/test/auxweb/certificates/chained/portaldogfoodmdscert.pfx" />
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 relative sourceLocation works if you are using RDFE to perform deployments. It will automatically use the closest dSMS server location.
I tested this deployment with that format and it does work.

</Certificates>
</Role>
</ServiceConfiguration>
Binary file added ...eManagement/Common/Commands.ScenarioTest/Resources/ServiceManagement/Files/dSMSTest.cspkg
View file
Open in desktop
Binary file not shown.
67 changes: 67 additions & 0 deletions ...ement/Common/Commands.ScenarioTest/Resources/ServiceManagement/ServiceManagementTests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,73 @@ function Run-AutoGeneratedVirtualMachineCmdletTests
}
}

# Run dSMS Hosted Service test
function Run-DSMSHostedServiceTest
{
# Setup
$svcName = 'pstest' + (Get-CloudServiceName);
$location = Get-DefaultLocation;

$storageName = 'pstest' + (getAssetName);
New-AzureStorageAccount -StorageAccountName $storageName -Location $location;

# Associate the new storage account with the current subscription
Set-CurrentStorageAccountName $storageName;

$testMode = Get-ComputeTestMode;
if ($testMode.ToLower() -ne 'playback')
{
$cspkg = '.\Resources\ServiceManagement\Files\dSMSTest.cspkg';
}
else
{
$cspkg = "https://${storageName}.blob.azure.windows.net/blob/dSMSTest.cspkg";
}
$cscfg = "$TestOutputRoot\Resources\ServiceManagement\Files\dSMSTest.cscfg";
$cscfgChanged = "$TestOutputRoot\Resources\ServiceManagement\Files\dSMSTest-changed.cscfg";

# Create a temporary self-signed cert
$cert = New-SelfSignedCertificate -DnsName "example.local" -CertStoreLocation "Cert:\CurrentUser\My";
$certPath = "Cert:\CurrentUser\My\$($cert.Thumbprint)"
# Update the cscfg to use the new cert
(Get-Content $cscfg) | ForEach-Object {$_ -replace "\{\{PLACEHOLDER\}\}", $cert.Thumbprint} | Set-Content $cscfgChanged;

try
{
# Create Hosted Service
$result = New-AzureService -ServiceName $svcName -Location $location -Label $svcName -Description $svcName;

# Upload the certificate
Add-AzureCertificate -ServiceName $svcName -CertToDeploy $cert;

# Deploy to staging
$result = New-AzureDeployment -ServiceName $svcName -Package $cspkg -Configuration $cscfgChanged -Label $svcName -Slot Staging;

# Get Deployment
$deploy = Get-AzureDeployment -ServiceName $svcName -Slot Staging;
Copy link
Contributor

@vanbasten2323 vanbasten2323 Oct 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

So before the change, if the you have a line in cscfg like: , then you would get null ref error when you create a new deployment. Is my understanding correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. If you didn't supply a thumbprint attribute, it would NullRef when you perform Get-AzureDeployment

Copy link
Contributor

Choose a reason for hiding this comment

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

Then shouldn't the NullRef exception be thrown when you do New-AzureDeployment? The PR topic says the exception happens when you do Get-AzureDeployment

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 NullRef was only thrown when I ran Get-AzureDeployment. The test goes through CRUD to ensure it's not occurring anywhere else. I'd rather have extra coverage :)


# Make a change
$newConfig = $deploy.Configuration -replace 'Setting name="DummySetting" value="Foo"', 'Setting name="DummySetting" value="Bar"';
$newConfig | Set-Content $cscfgChanged;

# Update configuration
$result = Set-AzureDeployment -Config -ServiceName $svcName -Configuration $cscfgChanged -Slot Staging;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to get a new config and do a Set-AzureDeployment on the new config? I don't quite understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to test round-trip. Deploy (tests New) Get (tests Get) Update (tests Set).
I wanted to make sure all parts could handle config without thumbprints.

}
finally
{
# Cleanup
Cleanup-CloudService $svcName;
Cleanup-Storage $storageName;
if (Test-Path $cscfgChanged)
{
Remove-Item $cscfgChanged;
}
if (Test-Path $certPath)
{
Remove-Item $certPath
}
}
}

# Run New-AzureComputeArgumentList Cmdlet Tests Using Method Names
function Run-NewAzureComputeArgumentListTests
Expand Down
9 changes: 9 additions & 0 deletions src/ServiceManagement/Common/Commands.ScenarioTest/ServiceManagement/ScenarioTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ public void RunAutoGeneratedVirtualMachineCmdletTests()
this.RunPowerShellTest("Run-AutoGeneratedVirtualMachineCmdletTests");
}

[Fact]
[Trait(Category.Service, Category.ServiceManagement)]
[Trait(Category.AcceptanceType, Category.CheckIn)]
[Trait(Category.AcceptanceType, Category.BVT)]
public void RunDSMSHostedServiceTest()
{
this.RunPowerShellTest("Run-DSMSHostedServiceTest");
}

[Fact]
[Trait(Category.Service, Category.ServiceManagement)]
[Trait(Category.AcceptanceType, Category.CheckIn)]
Expand Down
2,210 changes: 2,210 additions & 0 deletions ...t.WindowsAzure.Commands.ScenarioTest.ServiceManagementTests/RunDSMSHostedServiceTest.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/ServiceManagement/Compute/Commands.ServiceManagement/Model/CertificateConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@ public class CertificateConfiguration
public string Thumbprint { get; set; }

public string ThumbprintAlgorithm { get; set; }

public string SourceLocation { get; set; }
}
}
19 changes: 15 additions & 4 deletions src/ServiceManagement/Compute/Commands.ServiceManagement/Model/RoleConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ public RoleConfiguration(XElement data)
{
var certificate = new CertificateConfiguration
{
Thumbprint = setting.Attribute("thumbprint").Value,
ThumbprintAlgorithm = setting.Attribute("thumbprintAlgorithm").Value
Thumbprint = setting.Attribute("thumbprint") == null ? null : setting.Attribute("thumbprint").Value,
ThumbprintAlgorithm = setting.Attribute("thumbprintAlgorithm") == null ? null : setting.Attribute("thumbprintAlgorithm").Value,
SourceLocation = setting.Attribute("sourceLocation") == null ? null : setting.Attribute("sourceLocation").Value,
};

this.Certificates.Add(setting.Attribute("name").Value, certificate);
Expand Down Expand Up @@ -104,8 +105,18 @@ internal XElement Serialize()
{
XElement certificateElement = new XElement(this.ns + "Certificate");
certificateElement.SetAttributeValue("name", certificate.Key);
certificateElement.SetAttributeValue("thumbprint", certificate.Value.Thumbprint);
certificateElement.SetAttributeValue("thumbprintAlgorithm", certificate.Value.ThumbprintAlgorithm);
if (certificate.Value.Thumbprint != null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could certificate.Value be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. It's a value driven by the foreach loop over existing elements.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant this.Certificates is of type Dictionary<string, CertificateConfiguration>();. and the for loop make sure that variable "certificate" is not null... I wonder if "certificate.Value' can be null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see what you're asking.
Certificates.value won't be null because in the deserializer where it's set up, an element to the Dictionary is only added after the CertificateConfiguration object is created using an Object Initializer (so it can't be null).
See lines 44-53.

{
certificateElement.SetAttributeValue("thumbprint", certificate.Value.Thumbprint);
}
if (certificate.Value.ThumbprintAlgorithm != null)
{
certificateElement.SetAttributeValue("thumbprintAlgorithm", certificate.Value.ThumbprintAlgorithm);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to define a constant string instead of using hard coded string literals?

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 was following the existing pattern of the file. There are no constants at all.

}
if (certificate.Value.SourceLocation != null)
{
certificateElement.SetAttributeValue("sourceLocation", certificate.Value.SourceLocation);
}
certificatesElement.Add(certificateElement);
}

Expand Down