Skip to content
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

feat: add config file reader for sshnp #306

Merged
merged 2 commits into from
Aug 9, 2023
Merged

feat: add config file reader for sshnp #306

merged 2 commits into from
Aug 9, 2023

Conversation

XavierChanth
Copy link
Member

@XavierChanth XavierChanth commented Aug 8, 2023

@CurtlyCritchlow this PR is for you

- What I did

I added a static method: SSHNPParams.getConfigFilesFromDirectory(); which allows you to read in the config files to an iterable of SSHNPParams objects. This method also accepts an optional directory argument, although it is likely unnecessary to provide as the default should be suitable for our usage.

SSHNPParams is the main object which is used to generate the SSHNP class via SSHNP.fromParams(params);

Example code:

import 'package:sshnoports/sshnp/sshnp.dart';
import 'package:sshnoports/sshnp/sshnp_params.dart';

void main() async {
  /// This is a new method which returns an Iterable of [SSHNPParams] based on the folder it reads
  /// By default it reads from ~/.sshnp/config which is defined in [getDefaultSshnpConfigDirectory()] (in common/utils.dart)
  Iterable<SSHNPParams> cfg = await SSHNPParams.getConfigFilesFromDirectory();

  /// Iterate through the list of SSHNPParams and print some of the info out
  for (var c in cfg) {
    print((c.clientAtSign, c.device, c.localPort));
  }

  /// Create an [SSHNP] object from the first [SSHNPParams] object in the list
  /// Then see bin/sshnp.dart for how to use this appropriately
  SSHNP sshnp = await SSHNP.fromParams(cfg.first);

  // PROGRAM OUTPUT (specific to my machine):
  // (@xavierchanth, playpen, 0)
  // (@xavierchanth, playpen, 2222)
}

- How I did it

- How to verify it

  1. Create your own config files in ~/.sshnp/config (see here for a template of this)
  2. Then run the sample code

- Description for the changelog
feat: add config file reader for sshnp

@XavierChanth
Copy link
Member Author

After adding the write functionality and testing, it duplicated the first config file as expected:

import 'package:sshnoports/common/utils.dart';
import 'package:sshnoports/sshnp/sshnp.dart';
import 'package:sshnoports/sshnp/sshnp_params.dart';

void main() async {
  /// This is a new method which returns an Iterable of [SSHNPParams] based on the folder it reads
  /// By default it reads from ~/.sshnp/config which is defined in [getDefaultSshnpConfigDirectory()] (in common/utils.dart)
  Iterable<SSHNPParams> cfg = await SSHNPParams.getConfigFilesFromDirectory();

  /// Iterate through the list of SSHNPParams and print some of the info out
  for (var c in cfg) {
    print((c.clientAtSign, c.device, c.localPort));
  }

  /// Create an [SSHNP] object from the first [SSHNPParams] object in the list
  /// Then see bin/sshnp.dart for how to use this appropriately
  SSHNP sshnp = await SSHNP.fromParams(cfg.first);

  /// Get the config dir
  var configDir = getDefaultSshnpConfigDirectory(getHomeDirectory()!);

  print('Rewriting first config file to sshnp_test.env');

  await cfg.first.toFile('$configDir/sshnp_test.env');

  print('Reading the files again');
  cfg = await SSHNPParams.getConfigFilesFromDirectory();

  for (var c in cfg) {
    print((c.clientAtSign, c.device, c.localPort));
  }
}

  // OUTPUT:
  // (@xavierchanth, playpen, 0)
  // (@xavierchanth, playpen, 2222)
  // Rewriting first config file to sshnp_test.env
  // Reading the files again
  // (@xavierchanth, playpen, 0)
  // (@xavierchanth, playpen, 0)
  // (@xavierchanth, playpen, 2222)

@XavierChanth
Copy link
Member Author

@CurtlyCritchlow in general the goal with the GUI should be to work with SSHNPParams whenever possible, only converting it to the SSHNP object when you want to actually run that particular "profile". In essence, one SSHNPParam object represents one "profile"

@XavierChanth
Copy link
Member Author

XavierChanth commented Aug 8, 2023

@gkc as for the failed tests it looks like we have a regression where new client is no longer backwards compatible with >= 3.2.0. I think because sshnpd >= 3.2.0 don't share a username. Or is it that there is an issue with those versions of the at_client... maybe you have better visibility as to where the issue is occurring.

@gkc
Copy link
Contributor

gkc commented Aug 8, 2023

@gkc as for the failed tests it looks like we have a regression where new client is no longer backwards compatible with >= 3.2.0. I think because sshnpd >= 3.2.0 don't share a username. Or is it that there is an issue with those versions of the at_client... maybe you have better visibility as to where the issue is occurring.

sshnpd does still share a username (via a notification) if that option is passed as an sshnpd command line argument.
versions 3.1.2 and 3.2.0 as I recall used a put and thus required a sync to complete (since the PutRequestOptions.useRemoteAtServer flag was not available back then).

So I think this is just a test race condition; maybe we need to increase the delay in the local, v3.1.2 and local, v3.2.0 tests? Note that a re-run succeeds

[EDIT] Looking at the logs, I can see that, in the runs that failed, sshnpd had not completed its sync

@XavierChanth
Copy link
Member Author

@gkc as for the failed tests it looks like we have a regression where new client is no longer backwards compatible with >= 3.2.0. I think because sshnpd >= 3.2.0 don't share a username. Or is it that there is an issue with those versions of the at_client... maybe you have better visibility as to where the issue is occurring.

sshnpd does still share a username (via a notification) if that option is passed as an sshnpd command line argument. versions 3.1.2 and 3.2.0 as I recall used a put and thus required a sync to complete (since the PutRequestOptions.useRemoteAtServer flag was not available back then).

So I think this is just a test race condition; maybe we need to increase the delay in the local, v3.1.2 and local, v3.2.0 tests? Note that a re-run succeeds

[EDIT] Looking at the logs, I can see that, in the runs that failed, sshnpd had not completed its sync

Ah, it happened twice in a row on me, I can increase sync based timeout from 30s -> 60s

@gkc gkc merged commit da72bcc into trunk Aug 9, 2023
@gkc gkc deleted the config-reader-gui branch August 9, 2023 10:05
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.

2 participants