Skip to content

Conversation

@vthiebaut10
Copy link
Owner

Description

  1. Updated the SDKs (Compute, Network, HybridConnectivity, HybridCompute)
  2. Refactored how we retrieve the Relay Information
    • Incorporated the changes in the new HybridConnectivity API to manage port configurations on control plane. Created logic to identify if ports are enabled in service configuration, and to update service configuration.
    • Removed the RelayInformationUtils file and move all the logic to SshBaseCmdlet to facilitate error handling and user prompts.
    • Removed the global variables for relay information (it wasn't very good practice).

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.");
Copy link

Choose a reason for hiding this comment

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

Suggested change
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"))

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?

}

ValidateSshProxy(proxyPath);
//ValidateSshProxy(proxyPath);

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;

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);

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants