-
Notifications
You must be signed in to change notification settings - Fork 0
SSHArc GA #5
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
base: main
Are you sure you want to change the base?
SSHArc GA #5
Conversation
| if (exception.Body.Error.Code == "AuthorizationFailed") | ||
| { | ||
| //throw new AzPSApplicationException("You do not have authorization to create this endpoint. Only Contributor/Owner."); | ||
| throw new AzPSCloudException($"Client is not authrorized to create or update Service Configuration in the endpoint for {Name} in the Resource Group {ResourceGroupName}. This is an operation that must be performed by an account with Owner or Contributor role to allow SSH connections to the specified port {Port}. For more information see: https://aka.ms/ssharc/allow-ports."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| throw new AzPSCloudException($"Client is not authrorized to create or update Service Configuration in the endpoint for {Name} in the Resource Group {ResourceGroupName}. This is an operation that must be performed by an account with Owner or Contributor role to allow SSH connections to the specified port {Port}. For more information see: https://aka.ms/ssharc/allow-ports."); | |
| throw new AzPSCloudException($"Client is not authorized to create or update Service Configuration in the endpoint for {Name} in the Resource Group {ResourceGroupName}. This is an operation that must be performed by an account with Owner or Contributor role to allow SSH connections to the specified port {Port}. For more information see: https://aka.ms/ssharc/allow-ports."); |
| // attempt to create it if the user has permission | ||
| CreateServiceConfiguration(); | ||
| } | ||
| else if (exception.Body.Error.Code.Equals("ResourceNotFound")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know that these exception properties are always non-null? Should we use the pattern:
else if (exception.Body?.Error?.Code?.Equals("ResourceNotFound"))Also should the string compare be case insensitive?
src/Ssh/Ssh/Common/SshBaseCmdlet.cs
Outdated
| } | ||
|
|
||
| ValidateSshProxy(proxyPath); | ||
| //ValidateSshProxy(proxyPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove commented out code.
| if (result.Port != Int32.Parse(Port)) | ||
| { | ||
| return false; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove white space.
| private void CreateServiceConfiguration() | ||
| { | ||
| System.Collections.ObjectModel.Collection<ChoiceDescription> choices = new System.Collections.ObjectModel.Collection<ChoiceDescription> { new ChoiceDescription("N"), new ChoiceDescription("Y") }; | ||
| int userChoice = this.Host.UI.PromptForChoice($"The port {Port} that you are trying to connect to is not allowed for SSH connections for this resource. Would you like to update the current Service Confugration in the endpoint to allow connections to port {Port}?", "You must have owner or contributor roles in this resource to be able to change the service configuration. For more information see: http://aka.ms/ssharc/allow-ports", choices, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be easier to read using parameter list names, and multiple lines:
int userChoice = Host.UI.PromptForChoice(
caption: $"The port...",
message: "You must have owner...",
choices: choices,
defaultChoices: 0);Also PowerShell coding guidelines recommends not using the this. keyword when referencing class properties or methods.
Description