Merge pull request #278 from apel/release-3.3.1

tofu-rocketry · web-flow · commit 74e3d415650e · 2023-10-09T16:24:45.000+01:00
Release 3.3.1 to master
diff --git a/.github/workflows/build-pkgs.yml b/.github/workflows/build-pkgs.yml
@@ -21,7 +21,7 @@ jobs:
       run: |
         sudo apt-get update
         sudo apt-get install rpmlint
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
       with:
         # Get all branches and tags so the latest tag can be found for VERSION
         fetch-depth: 0
@@ -51,7 +51,7 @@ jobs:
       run: rpmlint ${{ steps.rpm.outputs.rpm_dir_path }}
 
     - name: Upload artifact
-      uses: actions/upload-artifact@v3.1.2
+      uses: actions/upload-artifact@v3.1.3
       with:
         name: Binary and Source RPMs
         path: |
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -34,7 +34,7 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml
@@ -30,7 +30,7 @@ jobs:
         # Login against a Docker registry
         # https://github.com/docker/login-action
         name: Login to ${{ env.REGISTRY }}
-        uses: docker/login-action@v2
+        uses: docker/login-action@v3
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}
@@ -40,15 +40,15 @@ jobs:
         # https://github.com/docker/metadata-action
         name: Extract Docker metadata
         id: meta
-        uses: docker/metadata-action@v4
+        uses: docker/metadata-action@v5
         with:
           images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
 
       -
         # Build and push Docker image
         # https://github.com/docker/build-push-action
         name: Build and push Docker image
-        uses: docker/build-push-action@v4.1.1
+        uses: docker/build-push-action@v5.0.0
         with:
           # Only push containers to the registry on GitHub pushes,
           # not pull requests. GitHub won't let a rogue PR create a container
diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml
@@ -8,10 +8,10 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['2.x', '3.x']
+        python-version: ['3.x']
     name: Python ${{ matrix.python-version }} test
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
     - name: Set up Python
       uses: actions/setup-python@v4
       with:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -15,15 +15,22 @@ repos:
   - id: check-added-large-files
   - id: check-merge-conflict
   - id: check-yaml
+  - id: debug-statements
   - id: end-of-file-fixer
   - id: mixed-line-ending
     name: Force line endings to LF
     args: ['--fix=lf']
   - id: trailing-whitespace
 
 - repo: https://github.com/pre-commit/pygrep-hooks
-  rev: v1.9.0
+  rev: v1.10.0
   hooks:
   - id: python-check-mock-methods
   - id: python-no-eval
   - id: python-no-log-warn
+  - id: python-use-type-annotations
+
+# Pre-commit CI config, see https://pre-commit.ci/
+ci:
+  autofix_prs: false
+  autoupdate_schedule: quarterly
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,14 @@
 Changelog for ssm
 =================
+* Mon Oct 09 2023 Adrian Coveney <adrian.coveney@stfc.ac.uk> - 3.3.0-1
+ - Added warning that BDII broker fetching will be deprecated in a future version.
+ - Added non-zero exit status if sender or receiver crash.
+ - Fixed SSM hanging if TCP connection drops by adding timeout.
+ - Fixed directory queue system picking up sub-directories.
+ - Fixed AMS messaging library dependency issue.
+ - Fixed documentation for running a containerised receiver.
+ - Fixed a few minor code issues.
+
 * Thu Jun 29 2023 Adrian Coveney <adrian.coveney@stfc.ac.uk> - 3.3.0-1
  - Added destination queue to the log during startup to aid troubleshooting.
  - Added check that the config file exists to allow for better error messages.
diff --git a/README.md b/README.md
@@ -10,20 +10,17 @@ using the [STOMP protocol](http://stomp.github.io/) or via the ARGO Messaging Se
 Messages are signed and may be encrypted during transit.
 Persistent queues should be used to guarantee delivery.
 
-SSM is written in Python. Packages are available for RHEL 6 and 7, and
- Ubuntu Trusty.
+SSM is written in Python. Packages are available for RHEL 7, and Ubuntu Trusty.
 
 For more information about SSM, see the [EGI wiki](https://wiki.egi.eu/wiki/APEL/SSM).
 
 ## Acknowledgements
 
 <span>
-  <img alt="STFC logo" src="https://github.com/GOCDB/gocdb/raw/dev/htdocs/images/UKRI_STF_Council-Logo_Horiz-RGB_crop.png" height="57" />
-  <img alt="EU flag" src="https://github.com/GOCDB/gocdb/raw/dev/htdocs/images/eu_flag_yellow_low_150.png" height="51" />
-  <img alt="EOSC-hub logo" src="https://github.com/GOCDB/gocdb/raw/dev/htdocs/images/eosc-hub-v-web_150.png" height="57" />
+  <img alt="STFC logo" src="https://github.com/GOCDB/gocdb/raw/a3df819/htdocs/images/logos/ukri_stfc.png" height="57" />
 </span>
 
-SSM is provided by [STFC](https://stfc.ukri.org/), a part of [UK Research and Innovation](https://www.ukri.org/), and is co-funded by the [EOSC-hub](https://www.eosc-hub.eu/) project (Horizon 2020) under Grant number 777536. Licensed under the [Apache 2 License](http://www.apache.org/licenses/LICENSE-2.0).
+SSM is provided by [STFC](https://stfc.ukri.org/), a part of [UK Research and Innovation](https://www.ukri.org/). Licensed under the [Apache 2 License](http://www.apache.org/licenses/LICENSE-2.0).
 
 ## Installing the RPM
 
@@ -40,7 +37,7 @@ The Python STOMP library (N.B. versions between 3.1.1 (inclusive) and 5.0.0
 
 The Python AMS library. This is only required if you want to use AMS. See here for details on obtaining an RPM: https://github.com/ARGOeu/argo-ams-library/
 
-The Python ldap library
+The Python ldap library (N.B. versions before 3.4.0 (exclusive) are currently supported)
 * `yum install python-ldap`
 
 Optionally, the Python dirq library (N.B. this is only required if your messages
@@ -211,8 +208,8 @@ add your messages using the `add` method.
  ```
  docker run \
      -d --entrypoint ssmreceive \
-     -v /path/to/downloaded/config/sender.cfg:/etc/apel/sender.cfg \
-     -v /path/to/read/messages:/var/spool/apel/outgoing \
+     -v /path/to/downloaded/config/receiver.cfg:/etc/apel/receiver.cfg \
+     -v /path/to/read/messages:/var/spool/apel/ \
      -v /path/to/dns/file:/etc/apel/dns \
      -v /etc/grid-security:/etc/grid-security \
      -v /path/to/persistently/log:/var/log/apel \
diff --git a/apel-ssm.spec b/apel-ssm.spec
@@ -4,7 +4,7 @@
 %endif
 
 Name:           apel-ssm
-Version:        3.3.0
+Version:        3.3.1
 %define releasenumber 1
 Release:        %{releasenumber}%{?dist}
 Summary:        Secure stomp messenger
@@ -100,6 +100,15 @@ rm -rf $RPM_BUILD_ROOT
 %doc %_defaultdocdir/%{name}
 
 %changelog
+* Mon Oct 09 2023 Adrian Coveney <adrian.coveney@stfc.ac.uk> - 3.3.0-1
+ - Added warning that BDII broker fetching will be deprecated in a future version.
+ - Added non-zero exit status if sender or receiver crash.
+ - Fixed SSM hanging if TCP connection drops by adding timeout.
+ - Fixed directory queue system picking up sub-directories.
+ - Fixed AMS messaging library dependency issue.
+ - Fixed documentation for running a containerised receiver.
+ - Fixed a few minor code issues.
+
 * Thu Jun 29 2023 Adrian Coveney <adrian.coveney@stfc.ac.uk> - 3.3.0-1
  - Added destination queue to the log during startup to aid troubleshooting.
  - Added check that the config file exists to allow for better error messages.
diff --git a/requirements.txt b/requirements.txt
@@ -1,6 +1,7 @@
 # Base requirements for ssm
 
 argo-ams-library
+certifi<2020.4.5.2  # Used by AMS (via requests), 2020.4.5.2 dropped support for Python 2
 stomp.py<5.0.0
 python-daemon<=2.3.0  # 2.3.1 dropped support for Python 2
 python-ldap<3.4.0  # python-ldap-3.4.0 dropped support for Python 2
diff --git a/scripts/ssm-build-deb.sh b/scripts/ssm-build-deb.sh
@@ -16,7 +16,7 @@
 
 set -eu
 
-TAG=3.3.0-1
+TAG=3.3.1-1
 
 SOURCE_DIR=~/debbuild/source
 BUILD_DIR=~/debbuild/build
diff --git a/scripts/ssm-build-rpm.sh b/scripts/ssm-build-rpm.sh
@@ -10,7 +10,7 @@
 rpmdev-setuptree
 
 RPMDIR=/home/rpmb/rpmbuild
-VERSION=3.3.0-1
+VERSION=3.3.1-1
 SSMDIR=apel-ssm-$VERSION
 
 # Remove old sources and RPMS
diff --git a/ssm/__init__.py b/ssm/__init__.py
@@ -19,7 +19,7 @@
 import logging
 import sys
 
-__version__ = (3, 3, 0)
+__version__ = (3, 3, 1)
 
 LOG_BREAK = '========================================'
 
diff --git a/ssm/agents.py b/ssm/agents.py
@@ -141,10 +141,6 @@ def get_ssm_args(protocol, cp, log):
     elif protocol == Ssm2.AMS_MESSAGING:
         # Then we are setting up an SSM to connect to a AMS.
 
-        # TODO: See if setting use_ssl directly in Ssm2 constructor is ok.
-        # 'use_ssl' isn't checked when using AMS (SSL is always used), but it
-        # is needed for the call to the Ssm2 constructor below.
-        use_ssl = None
         try:
             # We only need a hostname, not a port
             host = cp.get('broker', 'host')
@@ -254,6 +250,9 @@ def run_sender(protocol, brokers, project, token, cp, log):
         print('SSM failed to complete successfully.  See log file for details.')
         log.error('Unexpected exception in SSM: %s', e)
         log.error('Exception type: %s', e.__class__)
+        sender_failed = True
+    else:
+        sender_failed = False
 
     try:
         sender.close_connection()
@@ -263,6 +262,8 @@ def run_sender(protocol, brokers, project, token, cp, log):
 
     log.info('SSM has shut down.')
     log.info(LOG_BREAK)
+    if sender_failed:
+        sys.exit(1)
 
 
 def run_receiver(protocol, brokers, project, token, cp, log, dn_file):
@@ -348,15 +349,24 @@ def run_receiver(protocol, brokers, project, token, cp, log, dn_file):
         log.info('Received the shutdown signal: %s', e)
         ssm.shutdown()
         dc.close()
+        receiver_failed = True
     except Exception as e:
         log.error('Unexpected exception: %s', e)
         log.error('Exception type: %s', e.__class__)
         log.error('The SSM will exit.')
         ssm.shutdown()
         dc.close()
+        receiver_failed = True
+    # Currently won't run the else statement due to the while loop in the reciever
+    # Leaving here in case of future refactoring, but commented out so the unreachable
+    # code isn't flagged by tests
+    # else:
+    #   receiver_failed = False
 
     log.info('Receiving SSM has shut down.')
     log.info(LOG_BREAK)
+    if receiver_failed:
+        sys.exit(1)
 
 
 def get_dns(dn_file, log):
diff --git a/ssm/brokers.py b/ssm/brokers.py
@@ -43,6 +43,8 @@ class StompBrokerGetter(object):
     def __init__(self, bdii_url):
         """Set up the LDAP connection and strings which are re-used."""
         # Set up the LDAP connection
+        logging.warning('LDAP is deprecated and will be removed in an upcoming version, '
+                        'please set host locally in SSM config.')
         log.debug('Connecting to %s...', bdii_url)
         self._ldap_conn = ldap.initialize(bdii_url)
 
@@ -99,7 +101,7 @@ def _get_broker_details(self, service_type):
         return broker_details
 
     def _broker_in_network(self, broker_id, network):
-        """Check that a GlueServiceUniqueID is part of a specified netowrk."""
+        """Check that a GlueServiceUniqueID is part of a specified network."""
         ldap_filter = '(&(GlueServiceDataKey=cluster)(GlueChunkKey=GlueServiceUniqueID=%s))' \
             % broker_id
         attrs = [self._service_data_value_key]
diff --git a/ssm/message_directory.py b/ssm/message_directory.py
@@ -87,8 +87,9 @@ def _get_messages(self, sort_by_mtime=False):
         """
         try:
             # Get a list of files under self.directory_path
-            # in an arbitrary order.
-            file_name_list = os.listdir(self.directory_path)
+            # in an arbitrary order (ignoring directories).
+            file_name_list = [file for file in os.listdir(self.directory_path)
+                              if os.path.isfile(os.path.join(self.directory_path, file))]
 
             if sort_by_mtime:
                 # Working space to hold the unsorted messages
diff --git a/ssm/ssm2.py b/ssm/ssm2.py
@@ -390,8 +390,12 @@ def _send_msg_ams(self, text, msgid):
             message = AmsMessage(data=to_send,
                                  attributes={'empaid': msgid}).dict()
 
-            argo_response = self._ams.publish(self._dest, message, retry=3)
+            argo_response = self._ams.publish(self._dest, message, retry=3, timeout=10)
             return argo_response['messageIds'][0]
+        else:
+            # We ignore empty messages as there is no point sending them.
+            # (STOMP did require empty messages to keep the connection alive.)
+            return None
 
     def pull_msg_ams(self):
         """Pull 1 message from the AMS and acknowledge it."""
@@ -411,7 +415,8 @@ def pull_msg_ams(self):
 
         for msg_ack_id, msg in self._ams.pull_sub(self._listen,
                                                   messages_to_pull,
-                                                  retry=3):
+                                                  retry=3,
+                                                  timeout=10):
             # Get the AMS message id
             msgid = msg.get_msgid()
             # Get the SSM dirq id
@@ -440,7 +445,7 @@ def pull_msg_ams(self):
         # it can move the offset for the next subscription pull
         # (basically acknowledging pulled messages)
         if ackids:
-            self._ams.ack_sub(self._listen, ackids, retry=3)
+            self._ams.ack_sub(self._listen, ackids, retry=3, timeout=10)
 
     def send_ping(self):
         """Perform connection stay-alive steps.
diff --git a/test/test_crypto.py b/test/test_crypto.py
@@ -17,9 +17,7 @@
     verify_cert, \
     CryptoException
 
-# Set up logging - is this necessary?
 logging.basicConfig()
-log = logging.getLogger('SSM')
 
 
 class TestEncryptUtils(unittest.TestCase):
diff --git a/test/test_message_directory.py b/test/test_message_directory.py
@@ -14,6 +14,7 @@
 """This module contains test cases for the MessageDirectory class."""
 from __future__ import print_function
 
+import os
 import shutil
 import tempfile
 import time
@@ -27,7 +28,7 @@ class TestMessageDirectory(unittest.TestCase):
 
     def setUp(self):
         """Create a MessageDirectory class on top of a temporary directory."""
-        self.tmp_dir = tempfile.mkdtemp(prefix='message_directory')
+        self.tmp_dir = tempfile.mkdtemp(prefix='message_directory_')
         self.message_directory = MessageDirectory(self.tmp_dir)
 
     def test_add_and_get(self):
@@ -137,6 +138,19 @@ def test_remove(self):
         # Check the count method returns the expected value.
         self.assertEqual(self.message_directory.count(), 0)
 
+    def test_dir_in_dir(self):
+        """Check that directories inside the queue are being ignored."""
+        self.longMessage = True  # Include normal unittest output before custom message.
+
+        # Add a single test file (closing it to ensure this works on Unix)
+        handle, _path = tempfile.mkstemp(dir=self.tmp_dir)
+        os.close(handle)
+        # Add a directory (to ignore)
+        tempfile.mkdtemp(prefix='extra_directory_', dir=self.tmp_dir)
+
+        self.assertEqual(self.message_directory.count(), 1, "Expected just one file, "
+                         "but greater result implies that directory is being counted.")
+
     def tearDown(self):
         """Remove test directory and all contents."""
         try:
