-
Notifications
You must be signed in to change notification settings - Fork 15
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
refactor noports code for modularity, readability and testability #196
Comments
This is in progress but is slow going. sshnp is almost complete after about 6 hours effort with a couple of hours remaining. sshnpd will take less time, maybe 4 hours, and sshrvd is simpler again, estimate 3 hours . About 1.5 days effort to complete the refactoring I guess Then a round of reviews and testing, probably will combine to around 1.5 days |
I use 2 of my own docker containers. I compile two sets of binaries in each container ( Video (2 minuts) of me reproducing the bug: https://drive.google.com/file/d/1wRjl1Bt4VS9W5FdL2BiueqR0wBK7Yqhz/view?usp=sharing What I use to test: https://github.com/JeremyTubongbanua/sshnp_docker sshnp refactored (error)
sshnp trunk (working)
|
More context: The Dockerfile Base image I build FROM dart:latest AS buildimage
RUN mkdir -p /app/bin /app/binrefactor /app/repo
RUN apt-get update
RUN apt-get install -y git
WORKDIR /app/repo
RUN git clone https://github.com/atsign-foundation/sshnoports .
RUN git fetch origin ; \
git checkout gkc-refactor-sshnp ; \
dart pub get ; \
dart compile exe /app/repo/bin/sshnp.dart -o /app/binrefactor/sshnp ; \
dart compile exe /app/repo/bin/sshrv.dart -o /app/binrefactor/sshrv ; \
dart compile exe /app/repo/bin/sshnpd.dart -o /app/binrefactor/sshnpd ; \
dart compile exe /app/repo/bin/sshrvd.dart -o /app/binrefactor/sshrvd ; \
dart compile exe /app/repo/bin/activate_cli.dart -o /app/binrefactor/at_activate ;
RUN git fetch origin ; \
git checkout trunk ; \
dart pub get ; \
dart compile exe /app/repo/bin/sshnp.dart -o /app/bin/sshnp ; \
dart compile exe /app/repo/bin/sshrv.dart -o /app/bin/sshrv ; \
dart compile exe /app/repo/bin/sshnpd.dart -o /app/bin/sshnpd ; \
dart compile exe /app/repo/bin/sshrvd.dart -o /app/bin/sshrvd ; \
dart compile exe /app/repo/bin/activate_cli.dart -o /app/bin/at_activate ;
FROM ubuntu:latest
# FROM dart:latest
ENV USER=atsign
ENV HOMEDIR=/$USER
ENV USER_ID=1024
ENV GROUP_ID=1024
# install necessities
RUN apt-get update ; \
apt-get install -y sudo wget curl openssh-server nano vim iproute2 nmap tmux git;
# set up user
RUN groupadd --gid ${GROUP_ID} ${USER} ; \
useradd --system --shell /bin/bash --home ${HOMEDIR} --uid ${USER_ID} --gid ${GROUP_ID} ${USER} ; \
usermod -aG sudo ${USER} ;
# set up other files/folders, sshnp/sshnpd needs access to ~/.ssh, ~/.sshnp and especially the ~/.ssh/authorized_keys file
RUN mkdir -p ${HOMEDIR}/.sshnp ${HOMEDIR}/.ssh ; \
touch ${HOMEDIR}/.ssh/authorized_keys ; \
chown -R ${USER}:${USER} ${HOMEDIR} ;
# disable sudo password, requires vim
RUN ex +"%s/^%sudo.*$/%sudo ALL=(ALL:ALL) NOPASSWD:ALL/g" -scwq! /etc/sudoers ;
# "PasswordAuthentication no" in /etc/ssh/sshd_config
RUN sed -E -i 's|^#?(PasswordAuthentication)\s.*|\1 no|' /etc/ssh/sshd_config
# "ListenAddress 127.0.0.1" in /etc/ssh/sshd_config
RUN sed -i 's/#ListenAddress 0.0.0.0/ListenAddress 127.0.0.1/g' /etc/ssh/sshd_config
# copy binaries from buildimage
RUN mkdir -p ${HOMEDIR}/.local/bin ${HOMEDIR}/.local/binrefactor ${HOMEDIR}/repo ;
COPY --from=buildimage --chown=${USER}:${USER} /app/bin/* ${HOMEDIR}/.local/bin
COPY --from=buildimage --chown=${USER}:${USER} /app/binrefactor/* ${HOMEDIR}/.local/binrefactor
WORKDIR ${HOMEDIR}
# RUN echo '${USER}:123' | sudo chpasswd
USER ${USER}
ENTRYPOINT sudo service ssh start && bash Then the Dockerfile image that uses the base image FROM jeremy_demo-base
COPY keys/*.atKeys ${HOMEDIR}/.atsign/keys/ sshnp commands I ransudo ssh-keygen -A ; ssh-keygen -o -a 100 -t ed25519 -f ~/.ssh/id_ed25519 -C "john@example.com"
~/.local/bin/sshnp -f @jeremy_0 -t @smoothalligator -d docker -h @rv_am -s id_ed25519.pub -v
~/.local/binrefactor/sshnp -f @jeremy_0 -t @smoothalligator -d docker -h @rv_am -s id_ed25519.pub -v sshnpd commands I ran~/.local/bin/sshnpd -a @smoothalligator -m @jeremy_0 -d docker -s -u -v
~/.local/binrefactor/sshnpd -a @smoothalligator -m @jeremy_0 -d docker -s -u -v |
Thanks @JeremyTubongbanua - could you possibly run a REPL for all three atSigns, then run your test with the trunk binaries and email me the REPL outputs? |
@JeremyTubongbanua Think I've figured out what the problem was, have pushed a couple of commits (fix one, fix two) |
working @gkc :)
|
@JeremyTubongbanua Saw this in your logs above, |
Sorry for late reply, my container wasn't working all day yesterday and I realized it was because I didn't start sshd D: I can confirm that it's working good for me as of commit 3c6c31d |
sshrv needs no refactoring, it's a very short program Although sshrvd is also a short program, it needs a little bit of refactoring just so it's similar to sshnp and sshnpd. Estimating 2SP for this Dropping priority to P1 as sshnp and sshnpd were the big ones and their refactoring is complete |
@gkc I can take on refactoring sshrvd I'd also like to do a little more refactoring of sshnp and sshnpd to make some of the shared logic more visible |
@XavierChanth sgtm |
I think we can call this a wrap |
Is your feature request related to a problem? Please describe.
Single long main methods are difficult to follow and impossible to unit test
Describe the solution you'd like
Code refactored for modularity, readability and testability
NO FUNCTIONAL CHANGES WHATSOEVER, just refactoring
The text was updated successfully, but these errors were encountered: