Skip to content

Remove Workflow from PSSessionType #9618

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
iSazonov merged 4 commits into from
May 21, 2019
Merged

Remove Workflow from PSSessionType #9618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ccf420
Remove Worhflow from PSSessionType
iSazonov May 16, 2019
65e75e2
Revert Workflow value
iSazonov May 16, 2019
92ed694
Remove PSSessionType and IsWorkflowConfigurationType()
iSazonov May 17, 2019
1506958
Remove unneeded conditional block
iSazonov May 18, 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
97 changes: 6 additions & 91 deletions src/System.Management.Automation/engine/remoting/commands/CustomShellCommands.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,26 +341,6 @@ function Register-PSSessionConfiguration
[ValidateSet("x86", "amd64")]
public string ProcessorArchitecture { get; set; }

/// <summary>
/// SessionType.
/// </summary>
/// Only want this on non assemblyName parameter set, since assembly decides the sessiontype.
[Parameter(ParameterSetName = PSSessionConfigurationCommandBase.NameParameterSetName)]
public PSSessionType SessionType
{
get
{
return sessionType;
}

set
{
sessionType = value;
}
}

internal PSSessionType sessionType = PSSessionType.DefaultRemoteShell;

#endregion

#region Constructors
Expand Down Expand Up @@ -648,7 +628,7 @@ protected override void ProcessRecord()
protected override void EndProcessing()
{
System.Management.Automation.Tracing.Tracer tracer = new System.Management.Automation.Tracing.Tracer();
tracer.EndpointRegistered(this.Name, this.sessionType.ToString(), WindowsIdentity.GetCurrent().Name);
tracer.EndpointRegistered(this.Name, WindowsIdentity.GetCurrent().Name);
}

#endregion
Expand Down Expand Up @@ -821,28 +801,6 @@ private string ConstructPluginContent(out string srcConfigFilePath, out string d
StringBuilder initParameters = new StringBuilder();

bool assemblyAndTypeTokensSet = false;
if (sessionType == PSSessionType.Workflow)
Copy link
Contributor

@PaulHigin PaulHigin May 16, 2019

Choose a reason for hiding this comment

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

We can also remove this method:
private bool IsWorkflowConfigurationType(System.Management.Automation.PowerShell ps)
on line: 3898

Copy link
Collaborator Author

@iSazonov iSazonov May 17, 2019

Choose a reason for hiding this comment

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

I'd want to keep the PR as small as possible for fast review and for avoiding breaking something.
We will remove the method in follow PRs.

Copy link
Contributor

@PaulHigin PaulHigin May 17, 2019

Choose a reason for hiding this comment

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

I don't see the point in doing a partial removal. Workflows are no longer supported and so we might as well remove everything associated with it. Register-PSSessionConfiguration affects the local machine and configures endpoints for PSCore6, which no longer supports workflows. I don't see how removing everything workflow breaks functionality.

Copy link
Collaborator Author

@iSazonov iSazonov May 17, 2019

Choose a reason for hiding this comment

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

PSSessionType and IsWorkflowConfigurationType() removed.

{
initParameters.Append(string.Format(CultureInfo.InvariantCulture,
initParamFormat,
ConfigurationDataFromXML.ENDPOINTCONFIGURATIONTYPE,
sessionType,
Environment.NewLine));

initParameters.Append(string.Format(CultureInfo.InvariantCulture,
initParamFormat,
ConfigurationDataFromXML.ASSEMBLYTOKEN,
ConfigurationDataFromXML.WORKFLOWCOREASSEMBLY,
Environment.NewLine));

initParameters.Append(string.Format(CultureInfo.InvariantCulture,
initParamFormat,
ConfigurationDataFromXML.SHELLCONFIGTYPETOKEN,
ConfigurationDataFromXML.WORKFLOWCORETYPENAME,
Environment.NewLine));

assemblyAndTypeTokensSet = true;
}

// DISC endpoint
if (Path != null)
Expand Down Expand Up @@ -1169,7 +1127,7 @@ private string ConstructPluginContent(out string srcConfigFilePath, out string d
tempValue);
}

if (sessionType == PSSessionType.Workflow && !isUseSharedProcessSpecified)
if (!isUseSharedProcessSpecified)
{
UseSharedProcess = true;
}
Expand Down Expand Up @@ -1214,13 +1172,6 @@ private string ConstructPluginContent(out string srcConfigFilePath, out string d

StringBuilder sessionConfigurationData = new StringBuilder();

if (sessionType == PSSessionType.Workflow)
{
List<object> modifiedModulePath = new List<object>(modulesToImport ?? Array.Empty<string>());
modifiedModulePath.Insert(0, ConfigurationDataFromXML.PSWORKFLOWMODULE);
modulesToImport = modifiedModulePath.ToArray();
}

if (modulesToImport != null && modulesToImport.Length > 0)
{
sessionConfigurationData.Append(string.Format(CultureInfo.InvariantCulture,
Expand Down Expand Up @@ -1262,7 +1213,7 @@ private string ConstructPluginContent(out string srcConfigFilePath, out string d
transportOption = transportOption.Clone() as PSTransportOption;
}

transportOption.LoadFromDefaults(sessionType, true);
transportOption.LoadFromDefaults(true);

// If useSharedHost is set to false, we need to set hostIdleTimeout to 0 as well, else WS-Man throws error
if (isUseSharedProcessSpecified && !UseSharedProcess)
Expand Down Expand Up @@ -3783,23 +3734,20 @@ private void SetSessionConfigurationTypeOptions()
StringBuilder sessionConfigurationData = new StringBuilder();

string modulePathParameter = null;
string unsetModulePathStr = string.Empty;
bool unsetModulePath = false;
if (modulePathSpecified)
{
bool isWorkflowConfiguration = IsWorkflowConfigurationType(ps);
if (modulesToImport == null ||
modulesToImport.Length == 0 ||
(modulesToImport.Length == 1 && modulesToImport[0] is string && ((string)modulesToImport[0]).Equals(string.Empty, StringComparison.OrdinalIgnoreCase)))
{
unsetModulePath = true;
unsetModulePathStr = isWorkflowConfiguration ? ConfigurationDataFromXML.PSWORKFLOWMODULE : string.Empty;
}
else
{
modulePathParameter = PSSessionConfigurationCommandUtilities.GetModulePathAsString(this.modulesToImport).Trim();
// Add the built-in module path if it's a workflow config
if (!string.IsNullOrEmpty(modulePathParameter) && isWorkflowConfiguration)
if (!string.IsNullOrEmpty(modulePathParameter))
{
List<object> modifiedModulePath = new List<object>(modulesToImport);
modifiedModulePath.Insert(0, ConfigurationDataFromXML.PSWORKFLOWMODULE);
Expand All @@ -3825,7 +3773,7 @@ private void SetSessionConfigurationTypeOptions()
{
sessionConfigurationData.Append(string.Format(CultureInfo.InvariantCulture,
initParamFormat,
PSSessionConfigurationData.ModulesToImportToken, unsetModulePathStr));
PSSessionConfigurationData.ModulesToImportToken, string.Empty));
}
// unsetModulePath is false AND modulePathParameter is not empty.
// 1. modulePathSpecified is false. In this case, modulePathParameter will be the original module path.
Expand Down Expand Up @@ -3891,30 +3839,6 @@ protected override void EndProcessing()

#region Private Methods

/// <summary>
/// Check if the current configuration is a workflow endpoint.
/// </summary>
/// <returns></returns>
private bool IsWorkflowConfigurationType(System.Management.Automation.PowerShell ps)
{
// Get the AssemblyName
ps.AddScript(string.Format(CultureInfo.InvariantCulture, getAssemblyNameDataFormat, CodeGeneration.EscapeSingleQuotedStringContent(Name)));
Collection<PSObject> psObjectCollection = ps.Invoke(new object[] { Name }) as Collection<PSObject>;
if (psObjectCollection == null || psObjectCollection.Count != 1)
{
Dbg.Assert(false, "This should never happen. ps.Invoke always return a Collection<PSObject>");
}

if (psObjectCollection[0] == null)
{
// Not workflow endpoint, no assembly name
return false;
}

string assemblyNameOfCurrentConfiguration = psObjectCollection[0].BaseObject.ToString();
return assemblyNameOfCurrentConfiguration.Equals(ConfigurationDataFromXML.WORKFLOWCOREASSEMBLY, StringComparison.OrdinalIgnoreCase);
}

private PSObject ConstructPropertiesForUpdate()
{
PSObject result = new PSObject();
Expand Down Expand Up @@ -4000,14 +3924,6 @@ private PSObject ConstructPropertiesForUpdate()
{
using (System.Management.Automation.PowerShell ps = System.Management.Automation.PowerShell.Create())
{
bool isWorkflowConfiguration = IsWorkflowConfigurationType(ps);
if (!string.IsNullOrEmpty(modulePathParameter) && isWorkflowConfiguration)
{
List<object> modifiedModulePath = new List<object>(modulesToImport);
modifiedModulePath.Insert(0, ConfigurationDataFromXML.PSWORKFLOWMODULE);
modulePathParameter = PSSessionConfigurationCommandUtilities.GetModulePathAsString(modifiedModulePath.ToArray()).Trim();
}

// Get the SessionConfigurationDataFormat
ps.AddScript(string.Format(CultureInfo.InvariantCulture, getSessionConfigurationDataSbFormat, CodeGeneration.EscapeSingleQuotedStringContent(Name)));
Collection<PSObject> psObjectCollection = ps.Invoke(new object[] { Name }) as Collection<PSObject>;
Expand Down Expand Up @@ -4045,10 +3961,9 @@ private PSObject ConstructPropertiesForUpdate()
// ModulesToImport exist in the pssessionConfigurationData
if (scd.ModulesToImportInternal != null && scd.ModulesToImportInternal.Count != 0)
{
string unsetModulePathStr = isWorkflowConfiguration ? ConfigurationDataFromXML.PSWORKFLOWMODULE : string.Empty;
sessionConfigurationData.Append(string.Format(CultureInfo.InvariantCulture,
initParamFormat,
PSSessionConfigurationData.ModulesToImportToken, unsetModulePathStr));
PSSessionConfigurationData.ModulesToImportToken, string.Empty));
if (!string.IsNullOrEmpty(privateData))
{
sessionConfigurationData.Append(string.Format(CultureInfo.InvariantCulture, privateDataFormat, privateData));
Expand Down
9 changes: 2 additions & 7 deletions ....Management.Automation/engine/remoting/commands/NewPSSessionConfigurationOptionCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ public class WSManConfigurationOption : PSTransportOption

private const string AttribProcessIdleTimeout = "ProcessIdleTimeoutSec";
internal static readonly int? DefaultProcessIdleTimeout_ForPSRemoting = 0; // in seconds
internal static readonly int? DefaultProcessIdleTimeout_ForWorkflow = 1209600; // in seconds
private int? _processIdleTimeoutSec = null;

internal const string AttribMaxIdleTimeout = "MaxIdleTimeoutms";
Expand Down Expand Up @@ -68,9 +67,8 @@ internal WSManConfigurationOption()
/// <summary>
/// LoadFromDefaults.
/// </summary>
/// <param name="sessionType"></param>
/// <param name="keepAssigned"></param>
protected internal override void LoadFromDefaults(PSSessionType sessionType, bool keepAssigned)
protected internal override void LoadFromDefaults(bool keepAssigned)
{
if (!keepAssigned || !_outputBufferingMode.HasValue)
{
Expand All @@ -79,10 +77,7 @@ protected internal override void LoadFromDefaults(PSSessionType sessionType, boo

if (!keepAssigned || !_processIdleTimeoutSec.HasValue)
{
_processIdleTimeoutSec
= sessionType == PSSessionType.Workflow
? DefaultProcessIdleTimeout_ForWorkflow
: DefaultProcessIdleTimeout_ForPSRemoting;
_processIdleTimeoutSec = DefaultProcessIdleTimeout_ForPSRemoting;
}

if (!keepAssigned || !_maxIdleTimeoutSec.HasValue)
Expand Down
8 changes: 1 addition & 7 deletions src/System.Management.Automation/engine/remoting/common/PSSessionConfigurationTypeOption.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,13 @@ internal virtual Hashtable ConstructQuotasAsHashtable()
throw new NotImplementedException();
}

internal void LoadFromDefaults(PSSessionType sessionType)
{
LoadFromDefaults(sessionType, false);
}

/// <summary>
/// Sets all the values to default values.
/// If keepAssigned is true only those values are set
/// which are unassigned.
/// </summary>
/// <param name="sessionType"></param>
/// <param name="keepAssigned"></param>
protected internal virtual void LoadFromDefaults(PSSessionType sessionType, bool keepAssigned)
protected internal virtual void LoadFromDefaults(bool keepAssigned)
{
throw new NotImplementedException();
}
Expand Down
56 changes: 0 additions & 56 deletions src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,6 @@ public enum AuthenticationMechanism
Kerberos = 0x6,
}

/// <summary>
/// Specifies the type of session configuration that
/// should be used for creating a connection info.
/// </summary>
public enum PSSessionType
{
/// <summary>
/// Default PowerShell remoting
/// endpoint.
/// </summary>
DefaultRemoteShell = 0,

/// <summary>
/// Default Workflow endpoint.
Copy link
Contributor

@PaulHigin PaulHigin May 16, 2019

Choose a reason for hiding this comment

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

I feel we should remove this. Is there any reason to keep it?

Copy link
Collaborator Author

@iSazonov iSazonov May 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Register-PSSessionConfiguration cmdlet has PSSessionType parameter and I want get confirmation that we can make the breaking change:

  • remove the value
  • or remove the type at all

I'd remove the PSSessionType type because we removed Workflow support and parser error will be ok.

Thoughts?

/cc @lzybkr

/// </summary>
Workflow = 1,
}

/// <summary>
/// Specify the type of access mode that should be
/// used when creating a session configuration.
Expand Down Expand Up @@ -1503,44 +1485,6 @@ internal void SetDisconnectedExpiresOnToNow()
}

#endregion Internal members

#region V3 Extensions

private const string DefaultM3PShellName = "Microsoft.PowerShell.Workflow";
private const string DefaultM3PEndpoint = Remoting.Client.WSManNativeApi.ResourceURIPrefix + DefaultM3PShellName;

/// <summary>
/// Constructor that constructs the configuration name from its type.
/// </summary>
/// <param name="configurationType">Type of configuration to construct.</param>
public WSManConnectionInfo(PSSessionType configurationType) : this()
{
ComputerName = string.Empty;
switch (configurationType)
{
case PSSessionType.DefaultRemoteShell:
{
// it is already the default
}

break;

case PSSessionType.Workflow:
{
ShellUri = DefaultM3PEndpoint;
}

break;
default:
{
Diagnostics.Assert(false, "Unknown value for PSSessionType");
}

break;
}
}

#endregion V3 Extensions
}

/// <summary>
Expand Down
2 changes: 0 additions & 2 deletions src/System.Management.Automation/engine/remoting/fanin/InitialSessionStateProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ internal class ConfigurationDataFromXML
internal const string MAXPSVERSIONTOKEN = "MaxPSVersion";
internal const string MODULESTOIMPORT = "ModulesToImport";
internal const string HOSTMODE = "hostmode";
internal const string ENDPOINTCONFIGURATIONTYPE = "sessiontype";
internal const string WORKFLOWCOREASSEMBLY = "Microsoft.PowerShell.Workflow.ServiceCore, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL";
Copy link
Contributor

@PaulHigin PaulHigin May 16, 2019

Choose a reason for hiding this comment

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

We can remove this (WORKFLOWCOREASSEMBLY) and all code accessing it.

Copy link
Collaborator Author

@iSazonov iSazonov May 17, 2019

Choose a reason for hiding this comment

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

Will do in follow PRs.

Copy link
Contributor

@PaulHigin PaulHigin May 17, 2019

Choose a reason for hiding this comment

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

@iSazonov Thanks for doing this! It is great to get this cleaned up.

internal const string WORKFLOWCORETYPENAME = "Microsoft.PowerShell.Workflow.PSWorkflowSessionConfiguration";
internal const string PSWORKFLOWMODULE = "%windir%\\system32\\windowspowershell\\v1.0\\Modules\\PSWorkflow";
internal const string CONFIGFILEPATH = "configfilepath";
internal const string CONFIGFILEPATH_CamelCase = "ConfigFilePath";
Expand Down
4 changes: 2 additions & 2 deletions src/System.Management.Automation/utils/tracing/TracingGen.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,9 @@ public void EndpointModified(string endpointName, string modifiedBy)
/// EndpointRegistered (EventId: 0xb041/45121)
/// </summary>
[EtwEvent(0xb041)]
public void EndpointRegistered(string endpointName, string endpointType, string registeredBy)
public void EndpointRegistered(string endpointName, string registeredBy)
{
WriteEvent(M3PEndpointRegisteredEvent, endpointName, endpointType, registeredBy);
WriteEvent(M3PEndpointRegisteredEvent, endpointName, registeredBy);
}

/// <summary>
Expand Down