Skip to content

Commit

Permalink
Improve error handling and bug fix (Azure#14033)
Browse files Browse the repository at this point in the history 
* Improve error handling and bug fix

* Change literal string to constant per change in Common assembly

* Fix build error

* Code refactoring
  • Loading branch information
@erich-wang
erich-wang authored Feb 2, 2021
1 parent 7cbd0d2 commit 6c87ccf
Show file tree
Hide file tree
Showing 25 changed files with 481 additions and 219 deletions.
9 changes: 6 additions & 3 deletions src/Aks/Aks.sln
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.27703.2035
# Visual Studio Version 16
VisualStudioVersion = 16.0.30804.86
MinimumVisualStudioVersion = 10.0.40219.1
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Aks", "Aks\Aks.csproj", "{8058D403-06E3-4BED-8924-D166CE303961}"
EndProject
Expand Down Expand Up @@ -39,6 +38,10 @@ Global
{142D7B0B-388A-4CEB-A228-7F6D423C5C2E}.Debug|Any CPU.Build.0 = Debug|Any CPU
{142D7B0B-388A-4CEB-A228-7F6D423C5C2E}.Release|Any CPU.ActiveCfg = Release|Any CPU
{142D7B0B-388A-4CEB-A228-7F6D423C5C2E}.Release|Any CPU.Build.0 = Release|Any CPU
{6BD6B80A-06AF-4B5B-9230-69CCFC6C8D64}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{6BD6B80A-06AF-4B5B-9230-69CCFC6C8D64}.Debug|Any CPU.Build.0 = Debug|Any CPU
{6BD6B80A-06AF-4B5B-9230-69CCFC6C8D64}.Release|Any CPU.ActiveCfg = Release|Any CPU
{6BD6B80A-06AF-4B5B-9230-69CCFC6C8D64}.Release|Any CPU.Build.0 = Release|Any CPU
{FF81DC73-B8EC-4082-8841-4FBF2B16E7CE}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{FF81DC73-B8EC-4082-8841-4FBF2B16E7CE}.Debug|Any CPU.Build.0 = Debug|Any CPU
{FF81DC73-B8EC-4082-8841-4FBF2B16E7CE}.Release|Any CPU.ActiveCfg = Release|Any CPU
Expand Down
3 changes: 3 additions & 0 deletions src/Aks/Aks/ChangeLog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
- Additional information about change #1
-->
## Upcoming Release
* Refined error messages of cmdlet failure.
* Upgraded exception handling to use Azure PowerShell related exceptions.
* Fixed the issue that user could not use provided service principal to create Kubernetes cluster. [#13938]

## Version 2.0.1
* Fixed the issue that user cannot use service principal to create a new Kubernetes cluster. [#13012]
Expand Down
4 changes: 4 additions & 0 deletions src/Aks/Aks/Commands/Constants.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,9 @@ public static class Constants
};
public const string AddOnNameMonitoring = "Monitoring";
public const string AddOnNameVirtualNode = "VirtualNode";

internal const string DotNetApiParameterResourceGroupName = "resourceGroupName";
internal const string DotNetApiParameterResourceName = "resourceName";
internal const string DotNetApiParameterAgentPoolName = "agentPoolName";
}
}
57 changes: 41 additions & 16 deletions src/Aks/Aks/Commands/CreateOrUpdateKubeBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
using Microsoft.Rest.Azure.OData;
using Microsoft.Azure.Management.Internal.Resources.Models;
using Microsoft.WindowsAzure.Commands.Common.CustomAttributes;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.WindowsAzure.Commands.Common;

namespace Microsoft.Azure.Commands.Aks
{
Expand Down Expand Up @@ -150,7 +152,7 @@ protected virtual ManagedCluster BuildNewCluster()
new ContainerServiceLinuxProfile(LinuxProfileAdminUserName,
new ContainerServiceSshConfiguration(pubKey));

var acsServicePrincipal = EnsureServicePrincipal(ServicePrincipalIdAndSecret?.UserName, ServicePrincipalIdAndSecret?.Password?.ToString());
var acsServicePrincipal = EnsureServicePrincipal(ServicePrincipalIdAndSecret?.UserName, ServicePrincipalIdAndSecret?.Password?.ConvertToString());

var spProfile = new ManagedClusterServicePrincipalProfile(
acsServicePrincipal.SpId,
Expand Down Expand Up @@ -218,8 +220,6 @@ protected void BeforeBuildNewCluster()
/// <exception cref="ArgumentException">The SSH key or file argument was null and there was no default pub key in path.</exception>
protected string GetSshKey(string sshKeyOrFile)
{
const string helpLink = "https://docs.microsoft.com/en-us/azure/virtual-machines/linux/mac-create-ssh-keys";

// SSH key was specified as either a file or as key data
if (!string.IsNullOrEmpty(SshKeyValue))
{
Expand All @@ -237,7 +237,8 @@ protected string GetSshKey(string sshKeyOrFile)
var path = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".ssh", "id_rsa.pub");
if (!AzureSession.Instance.DataStore.FileExists(path))
{
throw new ArgumentException(string.Format(Resources.CouldNotFindSshPublicKeyInError, path, helpLink));
var errorMessage = string.Format(Resources.CouldNotFindSshPublicKeyInError, path);
throw new AzPSArgumentException(errorMessage, nameof(SshKeyValue));
}

WriteVerbose(string.Format(Resources.FetchSshPublicKeyFromFile, path));
Expand All @@ -248,12 +249,22 @@ protected string GetSshKey(string sshKeyOrFile)

protected AcsServicePrincipal EnsureServicePrincipal(string spId = null, string clientSecret = null)
{
//If user specifies service principal, just use it directly and no need to save to disk
if(!string.IsNullOrEmpty(spId) && !string.IsNullOrEmpty(clientSecret))
{
return new AcsServicePrincipal()
{
SpId = spId,
ClientSecret = clientSecret
};
}

var acsServicePrincipal = LoadServicePrincipal();
if (acsServicePrincipal == null)
{
WriteVerbose(string.Format(
WriteWarning(string.Format(
Resources.NoServicePrincipalFoundCreatingANewServicePrincipal,
AcsSpFilePath));
AcsSpFilePath, DefaultContext.Subscription.Id));

// if nothing to load, make one
if (clientSecret == null)
Expand Down Expand Up @@ -296,14 +307,16 @@ private AcsServicePrincipal BuildServicePrincipal(string name, string url, strin

if (!success)
{
throw new CmdletInvocationException(Resources.CouldNotCreateAServicePrincipalWithTheRightPermissionsAreYouAnOwner);
throw new AzPSInvalidOperationException(
Resources.CouldNotCreateAServicePrincipalWithTheRightPermissionsAreYouAnOwner,
desensitizedMessage: Resources.CouldNotCreateAServicePrincipalWithTheRightPermissionsAreYouAnOwner);
}

AddSubscriptionRoleAssignment("Contributor", sp.ObjectId);
return new AcsServicePrincipal { SpId = app.AppId, ClientSecret = clientSecret, ObjectId = app.ObjectId };
}

protected void AddAcrRoleAssignment(string acrName, AcsServicePrincipal acsServicePrincipal)
protected void AddAcrRoleAssignment(string acrName, string acrParameterName, AcsServicePrincipal acsServicePrincipal)
{
string acrResourceId = null;
try
Expand All @@ -313,9 +326,12 @@ protected void AddAcrRoleAssignment(string acrName, AcsServicePrincipal acsServi
var acrObjects = RmClient.Resources.List(acrQuery);
acrResourceId = acrObjects.First().Id;
}
catch(Exception ex)
catch(Exception)
{
throw new CmdletInvocationException(string.Format(Resources.CouldNotFindSpecifiedAcr, acrName), ex);
throw new AzPSArgumentException(
string.Format(Resources.CouldNotFindSpecifiedAcr, acrName),
acrParameterName,
string.Format(Resources.CouldNotFindSpecifiedAcr, "*"));
}

var roleId = GetRoleId("acrpull", acrResourceId);
Expand All @@ -331,7 +347,10 @@ protected void AddAcrRoleAssignment(string acrName, AcsServicePrincipal acsServi
}
catch(Exception ex)
{
throw new CmdletInvocationException(string.Format(Resources.CouldNotFindObjectIdForServicePrincipal, acsServicePrincipal.SpId), ex);
throw new AzPSInvalidOperationException(
string.Format(Resources.CouldNotFindObjectIdForServicePrincipal, acsServicePrincipal.SpId),
ex,
string.Format(Resources.CouldNotFindObjectIdForServicePrincipal,"*"));
}
}
var success = RetryAction(() =>
Expand All @@ -342,8 +361,9 @@ protected void AddAcrRoleAssignment(string acrName, AcsServicePrincipal acsServi

if (!success)
{
throw new CmdletInvocationException(
Resources.CouldNotAddAcrRoleAssignment);
throw new AzPSInvalidOperationException(
Resources.CouldNotAddAcrRoleAssignment,
desensitizedMessage: Resources.CouldNotAddAcrRoleAssignment);
}
}

Expand Down Expand Up @@ -374,8 +394,9 @@ protected void AddSubscriptionRoleAssignment(string role, string appId)

if (!success)
{
throw new CmdletInvocationException(
Resources.CouldNotCreateAServicePrincipalWithTheRightPermissionsAreYouAnOwner);
throw new AzPSInvalidOperationException(
Resources.CouldNotAssignServicePrincipalWithSubsContributorPermission,
desensitizedMessage: Resources.CouldNotAssignServicePrincipalWithSubsContributorPermission);
}
}

Expand Down Expand Up @@ -407,7 +428,11 @@ protected bool RetryAction(Action action, string actionName = null)
protected AcsServicePrincipal LoadServicePrincipal()
{
var config = LoadServicePrincipals();
return config?[DefaultContext.Subscription.Id];
if(config?.ContainsKey(DefaultContext.Subscription.Id) == true)
{
return config[DefaultContext.Subscription.Id];
}
return null;
}

protected Dictionary<string, AcsServicePrincipal> LoadServicePrincipals()
Expand Down
17 changes: 8 additions & 9 deletions src/Aks/Aks/Commands/DisableAzureRmAddons.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,15 @@
// ----------------------------------------------------------------------------------


using System.Collections.Generic;
using System.Management.Automation;

using Microsoft.Azure.Commands.Aks.Models;
using Microsoft.Azure.Commands.Aks.Properties;
using Microsoft.Azure.Commands.Aks.Utils;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Management.ContainerService.Models;
using Microsoft.WindowsAzure.Commands.Utilities.Common;

using System;
using System.Collections.Generic;
using System.Management.Automation;
using System.Runtime.ExceptionServices;
using System.Text;

namespace Microsoft.Azure.Commands.Aks.Commands
{
[Cmdlet(VerbsLifecycle.Disable, ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "AksAddOn", DefaultParameterSetName = DefaultParamSet, SupportsShouldProcess = true)]
Expand All @@ -38,11 +35,13 @@ protected override IDictionary<string, ManagedClusterAddonProfile> UpdateAddonsP
string addonServiceName = Constants.AddOnUserReadNameToServiceNameMapper.GetValueOrDefault(addOn, null);
if (addonServiceName == null)
{
throw new ArgumentException(string.Format(Resources.AddonNotDefined, addOn));
var errorMessage = string.Format(Resources.AddonNotDefined, addOn, string.Join(",", Constants.AddOnUserReadNameToServiceNameMapper.Keys));
throw new AzPSArgumentException(errorMessage, nameof(Name), desensitizedMessage: errorMessage);
}
if (!addonProfiles.ContainsKey(addonServiceName))
{
throw new ArgumentException(string.Format(Resources.AddonIsNotInstalled, addOn));
var errorMessage = string.Format(Resources.AddonIsNotInstalled, addOn);
throw new AzPSArgumentException(errorMessage, nameof(Name), desensitizedMessage: errorMessage);
}
ManagedClusterAddonProfile addonProfile = addonProfiles[addonServiceName];
addonProfile.Config = null;
Expand Down
12 changes: 4 additions & 8 deletions src/Aks/Aks/Commands/EnableAzureRmAddons.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,12 @@
// ----------------------------------------------------------------------------------


using System.Collections.Generic;
using System.Management.Automation;

using Microsoft.Azure.Commands.Aks.Models;
using Microsoft.Azure.Commands.Aks.Properties;
using Microsoft.Azure.Commands.Aks.Utils;
using Microsoft.Azure.Management.ContainerService.Models;
using Microsoft.WindowsAzure.Commands.Utilities.Common;

using System;
using System.Collections.Generic;
using System.Management.Automation;
using System.Text;

namespace Microsoft.Azure.Commands.Aks.Commands
{
Expand All @@ -44,7 +40,7 @@ public class EnableAzureRmAddons : UpdateAddonsBase

protected override IDictionary<string, ManagedClusterAddonProfile> UpdateAddonsProfile(IDictionary<string, ManagedClusterAddonProfile> addonProfiles)
{
return AddonUtils.EnableAddonsProfile(addonProfiles, Name, WorkspaceResourceId, SubnetName);
return AddonUtils.EnableAddonsProfile(addonProfiles, Name, nameof(Name), WorkspaceResourceId, nameof(WorkspaceResourceId), SubnetName, nameof(SubnetName));
}
}
}
58 changes: 37 additions & 21 deletions src/Aks/Aks/Commands/GetAzureRmAks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Linq;
using System.Management.Automation;

using Microsoft.Azure.Commands.Aks.Models;
using Microsoft.Azure.Commands.Aks.Properties;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Commands.ResourceManager.Common.ArgumentCompleters;
using Microsoft.Azure.Management.ContainerService;
using Microsoft.Azure.Management.Internal.Resources.Utilities.Models;
using Microsoft.Rest;
using Microsoft.WindowsAzure.Commands.Common.CustomAttributes;

namespace Microsoft.Azure.Commands.Aks
Expand Down Expand Up @@ -77,28 +79,42 @@ public override void ExecuteCmdlet()

RunCmdLet(() =>
{
switch (ParameterSetName)
try
{
case NameParameterSet:
var kubeCluster = Client.ManagedClusters.Get(ResourceGroupName, Name);
WriteObject(PSMapper.Instance.Map<PSKubernetesCluster>(kubeCluster), true);
break;
case IdParameterSet:
var resource = new ResourceIdentifier(Id);
var idCluster = Client.ManagedClusters.Get(resource.ResourceGroupName, resource.ResourceName);
WriteObject(PSMapper.Instance.Map<PSKubernetesCluster>(idCluster), true);
break;
case ResourceGroupParameterSet:
var kubeClusterList = string.IsNullOrEmpty(ResourceGroupName)
? ListPaged(() => Client.ManagedClusters.List(),
nextPageLink => Client.ManagedClusters.ListNext(nextPageLink))
: ListPaged(() => Client.ManagedClusters.ListByResourceGroup(ResourceGroupName),
nextPageLink => Client.ManagedClusters.ListNext(nextPageLink));
switch (ParameterSetName)
{
case NameParameterSet:
var kubeCluster = Client.ManagedClusters.Get(ResourceGroupName, Name);
WriteObject(PSMapper.Instance.Map<PSKubernetesCluster>(kubeCluster), true);
break;
case IdParameterSet:
var resource = new ResourceIdentifier(Id);
var idCluster = Client.ManagedClusters.Get(resource.ResourceGroupName, resource.ResourceName);
WriteObject(PSMapper.Instance.Map<PSKubernetesCluster>(idCluster), true);
break;
case ResourceGroupParameterSet:
var kubeClusterList = string.IsNullOrEmpty(ResourceGroupName)
? ListPaged(() => Client.ManagedClusters.List(),
nextPageLink => Client.ManagedClusters.ListNext(nextPageLink))
: ListPaged(() => Client.ManagedClusters.ListByResourceGroup(ResourceGroupName),
nextPageLink => Client.ManagedClusters.ListNext(nextPageLink));
WriteObject(kubeClusterList.Select(PSMapper.Instance.Map<PSKubernetesCluster>), true);
break;
default:
throw new ArgumentException(Resources.ParameterSetError);
WriteObject(kubeClusterList.Select(PSMapper.Instance.Map<PSKubernetesCluster>), true);
break;
default:
throw new AzPSArgumentException(Resources.ParameterSetError, "InvalidParameterSet", null, Resources.ParameterSetError);
}
}
catch (ValidationException e)
{
var sdkApiParameterMap = new Dictionary<string, CmdletParameterNameValuePair>()
{
{ Constants.DotNetApiParameterResourceGroupName, new CmdletParameterNameValuePair(nameof(ResourceGroupName), ResourceGroupName) },
{ Constants.DotNetApiParameterResourceName, new CmdletParameterNameValuePair(nameof(Name), Name) },
};
if (!HandleValidationException(e, sdkApiParameterMap))
throw;
}
});
}
Expand Down
Loading

0 comments on commit 6c87ccf

Please sign in to comment.