diff --git a/.github/workflows/build-pkgs.yml b/.github/workflows/build-pkgs.yml index a48e35ea..43589e31 100644 --- 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 index 29501c12..37232599 100644 --- 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 index 6190ce31..59f06ebc 100644 --- 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,7 +40,7 @@ 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 }} @@ -48,7 +48,7 @@ jobs: # 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 index f7f89548..6d58f556 100644 --- 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 index f80092a2..86c0ad80 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -15,6 +15,7 @@ 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 @@ -22,8 +23,14 @@ repos: - 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 index b0a161aa..84efd27c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,14 @@ Changelog for ssm ================= +* Mon Oct 09 2023 Adrian Coveney - 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 - 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 index 47762b75..7cf98716 100644 --- 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 - STFC logo - EU flag - EOSC-hub logo + STFC logo -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 index 5be15b28..5333bdc0 100644 --- 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 - 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 - 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 index 23c85f7a..e2c93c97 100644 --- 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 index 1a4db00c..a8e9aee1 100755 --- 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 index e99d3725..b71ce358 100644 --- 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 index 74f42008..738a7e51 100644 --- 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 index 3d6ef885..07ebeed7 100644 --- 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 index 598824e0..28826c34 100644 --- 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 index b7a70c65..62dad40d 100644 --- 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 index 72099ad8..9cbc28e2 100644 --- 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 index db13fdbb..0dcb4001 100644 --- 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 index af78bdc7..2e3d8541 100644 --- 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: