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

TRUNK SSHNP vs. RELEASE v3.2.0 SSHNPD does not work with a new device name #251

Closed
JeremyTubongbanua opened this issue Jul 12, 2023 · 8 comments · Fixed by #250
Closed
Assignees
Labels
bug Something isn't working

Comments

@JeremyTubongbanua
Copy link
Member

JeremyTubongbanua commented Jul 12, 2023

Conclusion:

The bug was happening because sshnp wouldn't wait for sync to finish

Describe the bug

Summary
Attempted testing current trunk SSHNP against release 3.2.0 SSHNPD and test failed. Found out that release 3.2.0 SSHNPD is not properly sending the device name to trunk SSHNP.

However, if you are using trunk SSHNP vs. release 3.2.0 SSHNPD with a device name that was already previously shared, then it will work. On the other hand, using an entirely new device name that was not already shared will always crash.

Steps to reproduce

Run the sshnpd binary on release 3.2.0 with an entirely new device name that was never shared before. Examle: ~/.local/bin/sshnpd -a @smoothalligator -m @jeremy_0 -d 123four -s -u -v

Run the sshnp binary on the latest trunk. Example: ~/.local/bin/sshnp -f @jeremy_0 -t @smoothalligator -d 123four -h @tastelessbanana -s id_ed25519.pub -v

On sshnp, you will get

Device "123four" unknown, or username not shared

Expected behavior

TRUNK NP to work with v3.2.0 NPD

Screenshots

No response

Smartphones

No response

Were you using an atApplication when the bug was found?

No response

Additional context

The naming convention I've created is "sshnp-sshnpd-sshrvd". Example: trunk-release3.2.0-release3.3.0 is testing trunk sshnp and release3.2.0 sshnpd and release3.3.0 sshrvd

Testing trunk-release3.2.0-@rv_am (FAILED): release3.2.0 sshnpd broken
https://github.com/atsign-foundation/sshnoports/actions/runs/5533026008/jobs/10096208608?pr=250

Same test as above but with a device name already shared (PASSED):
https://gist.github.com/JeremyTubongbanua/8911aa1cb7709542d54e7746275e350e

Some additional tests I did that are somewhat related to what we are discussing:

Testing release3.2.0-trunk-@rv_am (PASSED): release 3.2.0 sshnp works against trunk sshnpd
https://github.com/atsign-foundation/sshnoports/actions/runs/5533026008/jobs/10096208827?pr=250
Description of this: Although TRUNK NP does not work with 3.2.0 NPD, the converse does work (3.2.0 NP with TRUNK NPD works).

Testing release3.2.0-release3.2.0-release3.2.0 (PASSED): https://gist.github.com/JeremyTubongbanua/2474f8e8aecabc965da63ca34314db4c

@JeremyTubongbanua JeremyTubongbanua added the bug Something isn't working label Jul 12, 2023
@JeremyTubongbanua
Copy link
Member Author

JeremyTubongbanua commented Jul 12, 2023

Summary:

  • Release 3.2.0 works, but only if the person strictly uses the binaries (sshnp and sshnpd) inside of release 3.2.0.
  • Release 3.2.0 SSHNPD has trouble sending a new device name to Current Trunk SSHNP.
  • The simple solution to ensure compatibility across versions (in my opinion) is to drop support for v3.2.0.

@XavierChanth
Copy link
Member

Related to #211 ?

@JeremyTubongbanua
Copy link
Member Author

Related to #211 ?

Nope, I think this is more of a notifications issue.

#211 addresses atSigns that don't exist yet. But the tests we're running are already using activated atSigns

@XavierChanth
Copy link
Member

XavierChanth commented Jul 12, 2023

Does the same issue occur with 3.1.2? or is it strictly an issue with 3.2.0?

I think we have a breaking change in 3.3.0+ (i.e. 3.3.0 should of been 4.0.0)

@gkc
Copy link
Contributor

gkc commented Jul 12, 2023

Looking at the logs, I think this might be an artefact of the tests. The last visible output log line from the sshnpd is

sshnpd  | SHOUT|2023-07-12 14:55:43.545657| sshnpd |Starting sync for : @8052simple 

The change from 3.2.0 to 3.3.0 was we removed the need for sync

sshnpd 3.2.0 will not be responsive until it has completed its sync

Manual testing of trunk / 3.3.0 sshnp vs 3.2.0 sshnpd should confirm

@JeremyTubongbanua
Copy link
Member Author

JeremyTubongbanua commented Jul 12, 2023

Looking at the logs, I think this might be an artefact of the tests. The last visible output log line from the sshnpd is

sshnpd  | SHOUT|2023-07-12 14:55:43.545657| sshnpd |Starting sync for : @8052simple 

The change from 3.2.0 to 3.3.0 was we removed the need for sync

sshnpd 3.2.0 will not be responsive until it has completed its sync

Manual testing of trunk / 3.3.0 sshnp vs 3.2.0 sshnpd should confirm

Yup that was right.. sync needed to finish first. Just did my manual testing and 3.3.0 sshnp works with 3.2.0 sshnpd

@XavierChanth
Copy link
Member

I was about to say the only thing that changed was removing sync... this makes a lot of sense. Good catch Gary!

@JeremyTubongbanua
Copy link
Member Author

Conclusion - nothing was wrong with 3.2.0, it was just my test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants