Community
Participate
Working Groups
Here we will track discussions if merging the SSH and telnet shell configurations can be done.
There is duplication in having separate telnet and ssh properties files, particularly when a host other than the default needs to be specified.
Currently the telnet and ssh components register separate managed services to listen for their configurations. The purpose for this was to keep the two components completely independent. However, I think we could register one centralized managed service to listen for one configuration, where to be included both the ssh and telnet properties. Then the properties will be passed to the components. This will keep them independent and still will remove the duplication.
(In reply to comment #2) > Currently the telnet and ssh components register separate managed services to > listen for their configurations. > The purpose for this was to keep the two components completely independent. > > However, I think we could register one centralized managed service to listen > for one configuration, where to be included both the ssh and telnet properties. > Then the properties will be passed to the components. > This will keep them independent and still will remove the duplication. Sounds good to me.
Created attachment 209272 [details] Patch providing the functionality The patch merges the ssh and telnet configurations into one. It provides a class, ConsoleConfigurationConvertor, both in kernel and user regions, which wait for the merged configuration and split it into two configurations (the ones which the console is registered).
(In reply to comment #4) > Created attachment 209272 [details] > Patch providing the functionality > > The patch merges the ssh and telnet configurations into one. It provides a > class, ConsoleConfigurationConvertor, both in kernel and user regions, which > wait for the merged configuration and split it into two configurations (the > ones which the console is registered). Looking good. One comment: is there a good reason why telnet.host and ssh.host need to be separately specifiable? If not, it would be better to share a host property.
(In reply to comment #5) > (In reply to comment #4) > > Created attachment 209272 [details] > > Patch providing the functionality > > > > The patch merges the ssh and telnet configurations into one. It provides a > > class, ConsoleConfigurationConvertor, both in kernel and user regions, which > > wait for the merged configuration and split it into two configurations (the > > ones which the console is registered). > > Looking good. One comment: is there a good reason why telnet.host and ssh.host > need to be separately specifiable? If not, it would be better to share a host > property. Theoretically, it could be possible someone to want the ssh to be accessible on a particular network interface on the machine, different from localhost/127.0.0.1. That is why I made different properties. But I am not sure at all that we really have such a use case. Can you think of such a scenario? If not, it is OK to have on e property and I will update the code.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Created attachment 209272 [details] > > > Patch providing the functionality > > > > > > The patch merges the ssh and telnet configurations into one. It provides a > > > class, ConsoleConfigurationConvertor, both in kernel and user regions, which > > > wait for the merged configuration and split it into two configurations (the > > > ones which the console is registered). > > > > Looking good. One comment: is there a good reason why telnet.host and ssh.host > > need to be separately specifiable? If not, it would be better to share a host > > property. > > Theoretically, it could be possible someone to want the ssh to be accessible on > a particular network interface on the machine, different from > localhost/127.0.0.1. That is why I made different properties. But I am not sure > at all that we really have such a use case. Can you think of such a scenario? > If not, it is OK to have on e property and I will update the code. You're right. Someone might want to configure telnet for localhost for unsecure local access and ssh for some other host for secure remote access. Best to keep them separate therefore. Thanks!
I'm applying this change over that nano branch and plan to close the bug after that.
Fixed with commits: nano - 7e4f769 kernel - 062f3e8 AFAIK this still needs to be documented so I'm leaving the bug open. I found out that the console properties file doesn't accept new lines at the end of the file. If such are present the properties are discarded and the configuration isn't initialized. This has to be documented as well.
(In reply to comment #9) > I found out that the console properties file doesn't accept new lines at the > end of the file. If such are present the properties are discarded and the > configuration isn't initialized. > This has to be documented as well. It would be better to fix this behaviour so we don't have to document it and users won't hit it by accident. Please would you capture the problem in a new bug.
(In reply to comment #10) > (In reply to comment #9) > > I found out that the console properties file doesn't accept new lines at the > > end of the file. If such are present the properties are discarded and the > > configuration isn't initialized. > > This has to be documented as well. > > It would be better to fix this behaviour so we don't have to document it and > users won't hit it by accident. Please would you capture the problem in a new > bug. Only to mention that the merged console properties are not handled in any special way - exactly as any other deployed configuration.
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > I found out that the console properties file doesn't accept new lines at the > > > end of the file. If such are present the properties are discarded and the > > > configuration isn't initialized. > > > This has to be documented as well. > > > > It would be better to fix this behaviour so we don't have to document it and > > users won't hit it by accident. Please would you capture the problem in a new > > bug. > > Only to mention that the merged console properties are not handled in any > special way - exactly as any other deployed configuration. Thanks. Then it's even more important to log a bug on this. It may be that Equinox config admin needs fixing.
User guide updated with the new configuration file with commit http://git.eclipse.org/c/virgo/org.eclipse.virgo.documentation.git/commit/?h=364571-introduce-nano&id=7c2754553c8b132514b166fb63e49fed881bcca2
(In reply to comment #13) > User guide updated with the new configuration file with commit > http://git.eclipse.org/c/virgo/org.eclipse.virgo.documentation.git/commit/?h=364571-introduce-nano&id=7c2754553c8b132514b166fb63e49fed881bcca2 The update is fine except that: The telnet properties in the file are prefixed with <emphasis>.telnet</emphasis>, and the ssh properties are prefixed with <emphasis>.ssh</emphasis>. should presumably say: The telnet properties in the file are prefixed with <emphasis>telnet.</emphasis>, and the ssh properties are prefixed with <emphasis>ssh.</emphasis>. (unless you've changed each of these to be a suffix).
(In reply to comment #14) > (In reply to comment #13) > > User guide updated with the new configuration file with commit > > http://git.eclipse.org/c/virgo/org.eclipse.virgo.documentation.git/commit/?h=364571-introduce-nano&id=7c2754553c8b132514b166fb63e49fed881bcca2 > > The update is fine except that: > > The telnet properties in the file are prefixed with > <emphasis>.telnet</emphasis>, and the ssh properties are prefixed with > <emphasis>.ssh</emphasis>. > > should presumably say: > > The telnet properties in the file are prefixed with > <emphasis>telnet.</emphasis>, and the ssh properties are prefixed with > <emphasis>ssh.</emphasis>. > > (unless you've changed each of these to be a suffix). Thanks for the comment, Glyn - my mistake. I will amend the change.
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > User guide updated with the new configuration file with commit > > > http://git.eclipse.org/c/virgo/org.eclipse.virgo.documentation.git/commit/?h=364571-introduce-nano&id=7c2754553c8b132514b166fb63e49fed881bcca2 > > > > The update is fine except that: > > > > The telnet properties in the file are prefixed with > > <emphasis>.telnet</emphasis>, and the ssh properties are prefixed with > > <emphasis>.ssh</emphasis>. > > > > should presumably say: > > > > The telnet properties in the file are prefixed with > > <emphasis>telnet.</emphasis>, and the ssh properties are prefixed with > > <emphasis>ssh.</emphasis>. > > > > (unless you've changed each of these to be a suffix). > > Thanks for the comment, Glyn - my mistake. I will amend the change. Fixed with http://git.eclipse.org/c/virgo/org.eclipse.virgo.documentation.git/commit/?h=364571-introduce-nano&id=426be96d5a599830cfb006b7817df0057828942f
I couldn't reproduce this anymore. I believe it was related to bug 368741 which I think is fixed now. Since Lazar committed the documentation I think we are good to close this bug.
