Merge branch 'idfilemodel' into 'next'

new fields for IdentityFileModel

See merge request !53

.env.ci

-################
-# Environment variables when testing
-
-# When testing, there are a few variables that need to be modified to ensure
-# that we can connect to remote servers and run their code. These mainly
-# revolve around 3 main ideas:
-#
-# - how to connect to that server (hostname, port, user, password)
-#
-# - where to store the temporary files (directory, filename),
-#
-# - where to find the Python interpreter as well as what arguments to use
-#   (name, arguments).
-
-# The hostname of the server to connect to. This can be either an IP address
-# (like 192.168.0.1) or an actual hostname (like example.com).
-#
-# Type: String
-# Example(s):
-#   TEST_SERVER_HOSTNAME=192.168.0.1
-#   TEST_SERVER_HOSTNAME=example.com
-#
-TEST_SERVER_HOSTNAME=djrems
-
-# The SSH port of the server to connect to. This will almost always be port 22,
-# but it could be different.
-#
-# Type: Number
-# Example(s):
-#   TEST_SERVER_PORT=22
-#
-TEST_SERVER_PORT=22
-
-# The remote user's username. This user should have password login enabled.
-#
-# Type: String
-# Example(s):
-#   TEST_REMOTE_USER=johnsmith
-#
-TEST_REMOTE_USER=testuser
-
-# The remote user's password. This will be used for initial connections during
-# the tests as well as testing the functionality of the public key transfers.
-#
-# Type: String
-# Example(s):
-#   TEST_REMOTE_PASSWORD=p4ssw0rd
-#
-TEST_REMOTE_PASSWORD=p4ssw0rd
-
-# The remote directory to store scripts in. This directory should already exist
-# as the tests won't create it automatically.
-#
-# Also note that if this directory is used by other things, it could make the
-# tests fail. For example, if you use /tmp/ as the remote directory and a root
-# process writes something to /tmp/ while a test is running, the test will
-# likely error because it doesn't have read permissions on that file.
-#
-# Type: String
-# Example(s):
-#   TEST_REMOTE_DIRECTORY=/tmp/django-remote-submission/
-#
-TEST_REMOTE_DIRECTORY=/tmp/
-
-# The filename to store the scripts as. Most tests only need one script to run,
-# so this is the name of that script. As the tests are running Python scripts, it should probably end in .py.
-#
-# Type: String
-# Example(s):
-#   TEST_REMOTE_FILENAME=foobar.py
-#
-TEST_REMOTE_FILENAME=jobtest.py
-
-# The path to the Python interpreter to use. If the path is not absolute, the
-# $PATH variable will be searched to find the right executable. This can be
-# either a Python 2 or Python 3 interpreter. Additionally, there is the option
-# to use /usr/bin/env to find the full path to the executable.
-#
-# Type: String
-# Example(s):
-#   TEST_PYTHON_PATH=python3
-#   TEST_PYTHON_PATH=/usr/bin/python3
-#   TEST_PYTHON_PATH=/usr/bin/env
-#
-TEST_PYTHON_PATH=python
-
-# The comma-separated arguments to use when running the Python
-# interpreter. This should include -u to make Python run in line-buffered mode.
-#
-# If using /usr/bin/env as the path, this should also include the executable
-# name, like python3, in addition to the -u argument.
-#
-# Type: List of String
-# Example(s):
-#   TEST_PYTHON_ARGUMENTS=-u
-#   TEST_PYTHON_ARGUMENTS=python3,-u
-#
-TEST_PYTHON_ARGUMENTS=-u

.gitignore

@@ -125,3 +125,7 @@ dump.rdb
 #
 Pipfile
 Pipfile.lock
+
+# Docker
+docker-compose.yml
+Dockerfile

.gitlab-ci.yml

@@ -22,8 +22,14 @@ stages:
   # download docker-compose executable
   - curl -L "https://github.com/docker/compose/releases/download/1.29.2/docker-compose-$(uname -s)-$(uname -m)" -o ./docker-compose
   - chmod +x ./docker-compose
+  - \cp cfg/.env-ci .env
+  - \cp  cfg/Dockerfile-ci Dockerfile
+  - \cp cfg/docker-compose-ci.yml docker-compose.yml
 
 .docker-teardown: &docker-teardown
+  - ./docker-compose down -v
+  # remove containers, images, volumes, and networks created by this pipeline
+  - docker system prune -f -a --volumes
   - sudo chown -R gitlab-runner .  # allows "docker system prune" next time the pipeline runs
 
 .conda-setup: &conda-setup
@@ -50,7 +56,6 @@ dockerbuild:
   before_script:
     - *docker-setup
   script:
-    - mv .env.ci .env
     - docker build -t ${IMAGE_NAMETAG} .  # create image with default Dockerfile
     - docker push ${IMAGE_NAMETAG}  # upload to the Container Registry
   after_script:
@@ -66,9 +71,8 @@ test:
   script:
     - docker pull ${IMAGE_NAMETAG}  # pull from the Container Registry
     - docker tag ${IMAGE_NAMETAG} djrems  # retag the image
-    - mv .env.ci .env
-    # create a container where to run pytest of remote tests with coverage
-    - ./docker-compose up
+    - ./docker-compose up -d
+    - docker exec -t service_djrems bash -c "make test"
   after_script:
     - *docker-teardown
   artifacts:

Makefile

-.PHONY: clean-pyc clean-build condadev coverage dockerbuilddev dockercleanall docs test-noremote test-remote test help
 SHELL := bash
 .DEFAULT_GOAL := help
 define BROWSER_PYSCRIPT
@@ -13,6 +12,14 @@ endef
 export BROWSER_PYSCRIPT
 BROWSER := python -c "$$BROWSER_PYSCRIPT"
 
+# command to run docker compose. change this to be what you have installed
+DOCKER_COMPOSE := docker-compose
+
+# all the lines in a recipe are passed to a single invocation of the shell.
+#.ONESHELL:
+
+.PHONY: clean-pyc clean-build condadev coverage dockerbuilddev dockercleanall docs test-noremote test-remote test help
+
 help:
 	@perl -nle'print $& if m{^[a-zA-Z_-]+:.*?## .*$$}' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-25s\033[0m %s\n", $$1, $$2}'
 
@@ -71,7 +78,9 @@ sdist: clean ## package
 
 makemigrations: ## update migrations with new model changes
 	cd example && \
+	python -m virtualenv venv && \
 	source venv/bin/activate && \
+	python -m pip install -r requirements.txt
 	python manage.py makemigrations django_remote_submission
 
 CONDA_ACTIVATE=source $$(conda info --base)/etc/profile.d/conda.sh ; conda activate
@@ -81,9 +90,19 @@ condadev: ## create conda environment for developing, and install pre-commit hoo
 	mamba env update -y --name djrems --file ./conda.environment/environment-dev.yml
 	$(CONDA_ACTIVATE) djrems; python setup.py develop; pre-commit install
 
+dockercleanall:  ## remove "service_djrems" container, "djrems" image, and associated volumes
+	docker-compose down -v
+	docker image rm djrems
+
 dockerbuilddev:  ## create docker image for testing
-	/bin/cp .env.ci .env
-	docker build --network host -t djrems .
+	\cp cfg/.env-localdev .env
+	\cp cfg/Dockerfile-localdev Dockerfile
+	\cp cfg/docker-compose-localdev.yml docker-compose.yml
+	$(DOCKER_COMPOSE) build
+
+dockerup:  ## start the docker container running the application. Assumes the image has been built
+	$(DOCKER_COMPOSE) up -d
+	$(DOCKER_COMPOSE) logs --tail=0 --follow
 
-dockercleanall:  ## delete ALL containers, images, and volumes
-	docker system prune -f -a --volumes
+dockertest: ## run the tests with coverage in a docker container running the application
+	docker exec -t service_djrems bash -c "make test"

cfg/.env-ci

+TEST_SERVER_HOSTNAME=djrems
+TEST_SERVER_PORT=22
+TEST_REMOTE_USER=testuser
+TEST_REMOTE_PASSWORD=p4ssw0rd
+TEST_REMOTE_DIRECTORY=/tmp
+TEST_REMOTE_FILENAME=jobtest.py
+TEST_PYTHON_PATH=python
+TEST_PYTHON_ARGUMENTS=-u

cfg/.env-localdev

+TEST_SERVER_HOSTNAME=djrems
+TEST_SERVER_PORT=22
+TEST_REMOTE_USER=testuser
+TEST_REMOTE_PASSWORD=p4ssw0rd
+TEST_REMOTE_DIRECTORY=/tmp
+TEST_REMOTE_FILENAME=jobtest.py
+TEST_PYTHON_PATH=python
+TEST_PYTHON_ARGUMENTS=-u

cfg/Dockerfile-localdev

+# latest miniconda3 image
+FROM continuumio/miniconda3
+
+ENV PYTHONUNBUFFERED 1
+ENV CFLAGS "-std=c++11"
+ENV LANG en_US.UTF-8
+ENV LANGUAGE en_US.UTF-8
+ENV LC_COLLATE C
+ENV LC_CTYPE en_US.UTF-8
+
+WORKDIR /opt/django_remote_submission
+COPY . .
+
+
+# This line will throw error if using an old miniconda3 image.
+# To correct, one must build a miniconda3 image while piling all apt-get statements together into one
+# https://hub.docker.com/r/continuumio/miniconda3/dockerfile
+RUN apt-get update && apt-get install -y emacs iputils-ping less make mlocate net-tools openssh-client openssh-server vim
+
+RUN conda install --name base -c conda-forge mamba
+RUN mamba env update --name base --file conda.environment/environment-dev.yml
+
+# Will start conda environment when $HOME/.bashrc is invoked
+# Critical to prepend .bashrc, otherwise the conda environment
+# will not be activated when executing commands via remote SSH
+RUN echo "$(echo "conda activate base"; cat /etc/skel/.bashrc)" > /etc/skel/.bashrc
+RUN echo "$(echo "source /etc/profile.d/conda.sh"; cat /etc/skel/.bashrc)" > /etc/skel/.bashrc
+RUN echo "$(echo "# Activate Conda Environment base"; cat /etc/skel/.bashrc)" > /etc/skel/.bashrc

docker-compose.yml → cfg/docker-compose-ci.yml

 #use for local development
 
-version: '3'
+version: '3.4'
 
 services:
 
   djrems:
     restart: always
     image: djrems
+    container_name: service_djrems
     hostname: djrems
     expose:
       - "22"

cfg/docker-compose-localdev.yml

+#use for local development
+
+version: '3.4'
+
+services:
+
+  djrems:
+    restart: always
+    build:
+      # build using the host's network, required in certain environments when accessing the debian repositories
+      network: host
+      context: .
+    image: djrems
+    container_name: service_djrems
+    hostname: djrems
+    expose:
+      - "22"
+    env_file:
+     - .env
+    volumes:
+      - ./docker-entrypoint.sh:/usr/bin/docker-entrypoint.sh
+      - artifacts:/opt/django_remote_submission/artifacts
+      - type: bind
+        source: ./django_remote_submission
+        target: /opt/django_remote_submission/django_remote_submission
+      - type: bind
+        source: ./tests
+        target: /opt/django_remote_submission/tests
+    command: /usr/bin/docker-entrypoint.sh
+
+volumes:
+  artifacts:

django_remote_submission/migrations/0003_identityfilemodel.py

+# Generated by Django 3.2 on 2022-03-15 23:55
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('django_remote_submission', '0002_identityfilemodel'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='identityfilemodel',
+            name='recipient',
+        ),
+        migrations.AddField(
+            model_name='identityfilemodel',
+            name='custodian',
+            field=models.CharField(
+                help_text='unique identifier of the entity entrusted with the SSH key',
+                max_length=250,
+                null=True,
+                verbose_name='custodian of the key',
+            ),
+        ),
+        migrations.AddField(
+            model_name='identityfilemodel',
+            name='executor',
+            field=models.CharField(
+                help_text='username for the account in the remote worker carrying out the jobs',
+                max_length=250,
+                null=True,
+                verbose_name='jobs executor',
+            ),
+        ),
+    ]

django_remote_submission/models.py

@@ -25,7 +25,6 @@ import ast
 import uuid
 
 from django.db import models
-from django.contrib.auth import get_user_model
 from django.utils.translation import gettext_lazy as _
 from django.conf import settings
 from django.core.exceptions import ValidationError, ObjectDoesNotExist, MultipleObjectsReturned
@@ -36,6 +35,7 @@ from model_utils.models import TimeStampedModel
 
 # standard imports
 from pathlib import Path
+from typing import Optional
 
 
 # Thanks http://stackoverflow.com/a/7394475
@@ -466,54 +466,90 @@ class Result(TimeStampedModel):
         return '{self.remote_filename} <{self.job}>'.format(self=self)
 
 
+class IdentityFileModelError(Exception):
+    pass
+
+
 class IdentityFileModelManager(models.Manager):
     class MultipleIdentityFileError(MultipleObjectsReturned):
         pass
 
-    def create_from_username(self, username: str) -> "IdentityFile":
+    def create_from_custodian(self, custodian: str, executor: Optional[str] = None) -> "IdentityFile":
         r"""Instantiate an identity file object and save in the database by passing only a user name.
         Avoids creation when a record for `username` is found in the database. In such case, returns the found record.
-        :param username: login user name
+        :param custodian: login user name, or Django session. Will be associated to a private/public SSH key pair.
+        :param executor: user name in charge of opening an SSH tunnel between the app and worker servers.
         :raise MultipleObjectsReturned: when more than one identity-file is found in the database
         :return: instance of model IdentityFileModel
         """
-        recipient = get_user_model().objects.get(username=username)
+        if not executor:
+            executor = custodian
         try:
-            idf_record = self.get(recipient=recipient)
+            idf_record = self.get(custodian=custodian)
         except ObjectDoesNotExist:
             id_file = IdentityFile(persistent=True)
-            idf_record = super().create(recipient=recipient, private=str(id_file.private), public=str(id_file.public))
+            idf_record = super().create(
+                private=str(id_file.private), public=str(id_file.public), custodian=custodian, executor=executor
+            )
         except MultipleObjectsReturned:
             # this should not happen
-            raise self.MultipleIdentityFileError(f"More than one identity file stored for {recipient}")
+            raise self.MultipleIdentityFileError(f"More than one identity file stored for {custodian}")
         return idf_record
 
 
 class IdentityFileModel(TimeStampedModel):
     r"""
-    record =  IdentityFile.objects.create_from_username(username)
+    Holds the state for a communication channel between the webapp service and the remote-worker server.
+
+    The state is set by three actors: owner, custodian, and executor.
+    - owner: in the webapp service, the owner runs the webapp and creates all the SSH keys.
+        If the webapp service is containerized, the owner will be the root account of the
+        container running the webapp service.
+    - custodian: in the webap service, the entity (typically a Django session) entrusted with
+        a unique SSH key. There's only one custodian per SSH key and one SSH key per custodian.
+        Custodians are entities that need to stablish a communication channel with the
+        remote-worker server to execute jobs.
+        The SSH key should be deleted when the custodian ceases to exist, e.g., when an expired
+        Django session is cleared. Thus the lifetime of the custodian determines the lifetime of
+        the SSH key.
+    - executor: in the remote worker server, name of the user account that creates an SSH tunnel
+        between the webapp service and the remote-worker server. Public SSH keys created in
+        the webapp service by owner are stored in the authorized_keys file of the executor. An
+        executor can store more than one public key, meaning one executor can execute jobs
+        for more than one custodian.
+
+    record =  IdentityFile.objects.create_from_custodian(username)
     """
-    recipient = models.OneToOneField(
-        settings.AUTH_USER_MODEL,
-        models.PROTECT,
-        related_name="idfile",
-        verbose_name=_("User"),
-        help_text=_("The user receiving the public file"),
-    )
+
     private = models.CharField(
         _("private key file"), help_text=_("The path to the private SSH key file"), max_length=250
     )
+
     public = models.CharField(_("public key file"), help_text=_("The path to the public SSH key file"), max_length=250)
 
+    custodian = models.CharField(
+        _("custodian of the key"),
+        help_text=_("unique identifier of the entity entrusted with the SSH key"),
+        max_length=250,
+        null=True,
+    )
+
+    executor = models.CharField(
+        _("jobs executor"),
+        help_text=_("username for the account in the remote worker carrying out the jobs"),
+        max_length=250,
+        null=True,
+    )
     objects = IdentityFileModelManager()
 
     def __str__(self):
         r"""Convert model to string, e.g. ``"zzz IDF"``"""
-        return f"{self.recipient} IDF"
+        return f"{self.custodian} IDF"
 
     def save(self, *args, **kwargs):
-        r"""Override the save method to prevent updating the record, if extant"""
-        kwargs["force_insert"] = True
+        if IdentityFileModel.objects.filter(custodian=self.custodian):
+            raise IdentityFileModelError(f"An IdentityFile record for {self.custodian} already exists in the database")
+        kwargs["force_insert"] = True  # prevent updating the record, if extante
         return super().save(*args, **kwargs)
 
     def delete(self, *args, **kwargs):

docker-entrypoint.sh

@@ -13,9 +13,8 @@ fi
 # start the ssh daemon
 /etc/init.d/ssh start
 
-# run remote tests
-make test
-
-# hack preventing container from exiting
-# uncomment only for debugging purposes
-# tail -f /dev/null
+# check the deamon is up every 60 seconds
+while : ; do
+    sleep 60
+    [[ "$(/etc/init.d/ssh status)" = "sshd is running." ]] || break
+done

tests/unit/test_models.py

@@ -8,12 +8,19 @@ test_django-remote-submission
 Tests for `django-remote-submission` models module.
 """
 # package imports
-from django_remote_submission.models import ListField, IdentityFileModel, Interpreter, Log, Job, Result, Server
+from django_remote_submission.models import (
+    ListField,
+    IdentityFileModel,
+    IdentityFileModelError,
+    Interpreter,
+    Log,
+    Job,
+    Result,
+    Server,
+)
 from django_remote_submission.wrapper.remote import IdentityFile
 
 # third party imports
-from django.core.exceptions import ObjectDoesNotExist
-from django.db.utils import IntegrityError
 from django.contrib.auth import get_user_model
 import pytest
 
@@ -125,70 +132,79 @@ class TestListField:
 
 
 class TestIdentityFileModel:
-    r'''
+    r"""
     @pytest.mark.django_db
     def test_create(self, user):
         with pytest.raises(ObjectDoesNotExist) as e:
-            record = IdentityFileModel.objects.create_by_username("non-existing-user")
-    '''
+            record = IdentityFileModel.objects.create_by_custodian("non-existing-user")
+    """
 
     @pytest.mark.django_db
     def test_instantiate(self, user):
         id_file = IdentityFile(sshdir="/tmp")
-        record = IdentityFileModel(recipient=user, private=str(id_file.private), public=str(id_file.public))
-        assert record.recipient == user
+        username = user.username
+        record = IdentityFileModel(
+            private=str(id_file.private), public=str(id_file.public), custodian=username, executor=username
+        )
         assert os.path.exists(record.private)
         assert os.path.exists(record.public)
+        assert record.custodian == username
+        assert record.executor == username
 
     @pytest.mark.django_db
     def test_create(self, user):
         id_file = IdentityFile(sshdir="/tmp")
-        record = IdentityFileModel.objects.create(recipient=user,
-                                                  private=str(id_file.private),
-                                                  public=str(id_file.public))
-        assert record.recipient == user
+        username = user.username
+        record = IdentityFileModel.objects.create(
+            private=str(id_file.private), public=str(id_file.public), custodian=username, executor=username
+        )
         assert os.path.exists(record.private)
         assert os.path.exists(record.public)
+        assert record.custodian == username
+        assert record.executor == username
+
         record.delete()
         assert os.path.exists(record.private) is False
         assert os.path.exists(record.public) is False
 
     @pytest.mark.django_db(transaction=True)
-    def test_create_from_username(self, user):
-        # error if user does not exist
-        with pytest.raises(ObjectDoesNotExist) as e:
-            IdentityFileModel.objects.create_from_username("non-existing-user")
-        assert str(e.value) == "User matching query does not exist."
+    def test_create_from_custodian(self, user):
+        username = user.username
 
         # create an IdentityFileModel record for `user`
-        record = IdentityFileModel.objects.create_from_username(user.username)
-        assert record.recipient == user
-        assert Path(record.private).parent == Path.home() / ".ssh"
+        record_1 = IdentityFileModel.objects.create_from_custodian(username)
+        assert record_1.custodian == username
+        assert Path(record_1.private).parent == Path.home() / ".ssh"
+        [os.remove(file) for file in [record_1.private, record_1.public]]
 
         # return the newly created IdentityFileModel record, do not instantiate another one
-        record_2 = IdentityFileModel.objects.create_from_username(user.username)
-        assert record_2.id == record.id
+        record_2 = IdentityFileModel.objects.create_from_custodian(username)
+        assert record_2.id == record_1.id
 
-        # we screw up if we try to create & store two records for the same user
+        # we screw up if we try to save in the database a record that already exists
         id_file = IdentityFile(sshdir="/tmp")
-        with pytest.raises(IntegrityError) as e:
-            IdentityFileModel.objects.create(recipient=user, private=str(id_file.private), public=str(id_file.public))
-        assert str(e.value) == "UNIQUE constraint failed: django_remote_submission_identityfilemodel.recipient_id"
+        record = IdentityFileModel(
+            private=str(id_file.private), public=str(id_file.public), custodian=username, executor=username
+        )
+        with pytest.raises(IdentityFileModelError) as e:
+            record.save()
+        assert str(e.value) == f"An IdentityFile record for {username} already exists in the database"
         [os.remove(file) for file in [id_file.private, id_file.public]]
 
-        # same as before, but using method IdentityFileModel.save()
+        # same as before, but using Model.objects.create(), which calls Model.save()
         id_file = IdentityFile(sshdir="/tmp")
-        record = IdentityFileModel(recipient=user, private=str(id_file.private), public=str(id_file.public))
-        with pytest.raises(IntegrityError) as e:
-            record.save()
-        assert str(e.value) == "UNIQUE constraint failed: django_remote_submission_identityfilemodel.recipient_id"
+        with pytest.raises(IdentityFileModelError) as e:
+            IdentityFileModel.objects.create(
+                private=str(id_file.private), public=str(id_file.public), custodian=username, executor=username
+            )
+        assert str(e.value) == f"An IdentityFile record for {username} already exists in the database"
         [os.remove(file) for file in [id_file.private, id_file.public]]
 
     @pytest.mark.django_db(transaction=True)
     def test_delete(self, user):
         r"""delete the record, along with the SSH key files"""
         username = user.username
-        record = IdentityFileModel.objects.create_from_username(user.username)
+        record = IdentityFileModel.objects.create_from_custodian(user.username)
         record.delete()
         assert os.path.exists(record.private) is False
         assert os.path.exists(record.public) is False