From 5ca61aac6aff775bccb2b96ed38ed1e52f239ef7 Mon Sep 17 00:00:00 2001 From: Amith Murthy Date: Fri, 4 Sep 2026 11:40:51 -0700 Subject: [PATCH 1/5] adding lsstsci facility for grouper gid workflow --- modules/coactd.py | 11 ++++---- tests/test_repo_registration_gid.py | 40 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/modules/coactd.py b/modules/coactd.py index 53aedad..17734fc 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -408,6 +408,9 @@ class RepoRegistration(Registration): 'RepoUpdateFeature' ] + # facilities whose repos get a POSIX group minted in Grouper at creation time + GROUPER_FACILITIES = ('cryoem', 'lsstsci') + REPO_USERS_GQL = gql(""" query getRepoUsers ( $repo: RepoInput! ) { repo( filter: $repo ) { @@ -532,17 +535,15 @@ def do_new_repo( if repo_allocation_end_delta is None: repo_allocation_end_delta = pdl.duration(years=5) - # For CryoEM repos (ct* / ce*), create a POSIX group via Grouper + # for qualifying facilities, create a POSIX group via Grouper repo_gid = None grouper_name = "" - uses_grouper = ( - facility.lower() == 'cryoem' - ) + uses_grouper = facility.lower() in self.GROUPER_FACILITIES if uses_grouper: grouper_name = f"sdf-{facility.lower()}-{repo.lower()}" try: if not self.grouper_password_file: - raise ValueError("Grouper password file must be provided for CryoEM ct/ce repos") + raise ValueError(f"Grouper password file must be provided for {facility} repos") grouper_kwargs = dict( grouper_name=grouper_name, state="present", diff --git a/tests/test_repo_registration_gid.py b/tests/test_repo_registration_gid.py index e3eea4e..0c1eda9 100644 --- a/tests/test_repo_registration_gid.py +++ b/tests/test_repo_registration_gid.py @@ -188,6 +188,46 @@ def test_all_cryoem_repos_use_grouper(self, repo_registration, mock_ansible_runn # Should create both slurm and posixgroup features (4 back_channel calls: user query + repoUpsert + 2 features) assert repo_registration.back_channel.execute.call_count == 4 + def test_lsstsci_repos_use_grouper(self, repo_registration, mock_ansible_runner): + """Test that lsstsci repos use grouper and pass the gid through to add_repo.yaml.""" + # Setup + repo_registration.run_playbook.return_value = mock_ansible_runner + repo_registration.extract_grouper_values = Mock(return_value=('67890', 'sdf-lsstsci-dp1')) + repo_registration.back_channel.execute.side_effect = [ + {'repo': None}, # Query returns null (repo not found) + {'repoUpsert': {'Id': 'repo-789'}}, + {'repoUpsertFeature': {'Id': 'feature-slurm'}}, + {'repoUpsertFeature': {'Id': 'feature-posix'}} + ] + + # Execute + result = repo_registration.do_new_repo( + repo='dp1', + facility='lsstsci', + principal='test-user' + ) + + # Verify + assert result is True + repo_registration.run_playbook.assert_any_call( + 'coact/grouper.yml', + grouper_name='sdf-lsstsci-dp1', + state='present', + grouper_description='POSIX group for lsstsci dp1 repository access', + grouper_password_file='/tmp/test-grouper-password', + ) + # the gid drives the repo directory ownership in ansible-role-s3df-repo + repo_registration.run_playbook.assert_any_call( + 'coact/add_repo.yaml', + facility='lsstsci', + repo='dp1', + repo_principal='test-user', + repo_users=['test-user'], + gidNumber='67890', + groupName='sdf-lsstsci-dp1' + ) + assert repo_registration.back_channel.execute.call_count == 4 + class TestRepoIdempotency: """Test idempotency fixes for NewRepo request workflow.""" From 889fa3dfa9f4535bb91ea3e163f1047ef90e992b Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Mon, 31 Aug 2026 10:25:06 -0700 Subject: [PATCH 2/5] update CI/CD workflow to use specific versions of actions --- .github/workflows/ci-cd.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci-cd.yaml b/.github/workflows/ci-cd.yaml index 21db93e..acaff56 100644 --- a/.github/workflows/ci-cd.yaml +++ b/.github/workflows/ci-cd.yaml @@ -15,16 +15,16 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@11f9893b081a58869d3b5fccaea48c9e9e46f990 # v8.3.2 with: version: "0.11.7" enable-cache: true - name: Set up Python - uses: actions/setup-python@v6 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version-file: "pyproject.toml" From b61551041de0eb9ce1b2c49a61a7da9e6e726f93 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Mon, 31 Aug 2026 10:32:47 -0700 Subject: [PATCH 3/5] sdf-ansible not available in CI --- tests/test_s3df_posixgroup_ldif.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_s3df_posixgroup_ldif.py b/tests/test_s3df_posixgroup_ldif.py index 45a32a6..2de1de9 100644 --- a/tests/test_s3df_posixgroup_ldif.py +++ b/tests/test_s3df_posixgroup_ldif.py @@ -7,6 +7,8 @@ import os from unittest.mock import Mock +import pytest + # Mock the ansible modules imported by s3df_posixgroup sys.modules['ansible.module_utils.basic'] = Mock() sys.modules['ansible.module_utils.common.text.converters'] = Mock() @@ -17,6 +19,13 @@ if LIBRARY_PATH not in sys.path: sys.path.insert(0, LIBRARY_PATH) +# sdf-ansible not available in CI +if not os.path.isfile(os.path.join(LIBRARY_PATH, 's3df_posixgroup.py')): + pytest.skip( + "sdf-ansible submodule not checked out; run `git submodule update --init`", + allow_module_level=True, + ) + from s3df_posixgroup import _change_membership_template # noqa: E402 DN = "cn=sdf-cryoem-cd10,ou=Group,dc=sdf,dc=slac,dc=stanford,dc=edu" From d6a59759495c28790cc3787c6afe1418f1682234 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Mon, 31 Aug 2026 10:23:44 -0700 Subject: [PATCH 4/5] fix dated tests --- tests/test_slurm_node_memory.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_slurm_node_memory.py b/tests/test_slurm_node_memory.py index 1946f22..73a4239 100644 --- a/tests/test_slurm_node_memory.py +++ b/tests/test_slurm_node_memory.py @@ -22,35 +22,35 @@ def setup_method(self): def test_parse_slurm_nodelist_single(self): """Test parsing a single node name.""" - result = self.importer.parse_slurm_nodelist("sdfmilan0271") - assert result == ["sdfmilan0271"] + result = self.importer.parse_slurm_nodelist("sdfmilan271") + assert result == ["sdfmilan271"] def test_parse_slurm_nodelist_range(self): """Test parsing a SLURM node range.""" result = self.importer.parse_slurm_nodelist("sdfmilan[269-272]") - expected = ["sdfmilan0269", "sdfmilan0270", "sdfmilan0271", "sdfmilan0272"] + expected = ["sdfmilan269", "sdfmilan270", "sdfmilan271", "sdfmilan272"] assert result == expected def test_parse_slurm_nodelist_list(self): """Test parsing a comma-separated list of nodes.""" result = self.importer.parse_slurm_nodelist("sdfmilan[006,011,027]") - expected = ["sdfmilan0006", "sdfmilan0011", "sdfmilan0027"] + expected = ["sdfmilan006", "sdfmilan011", "sdfmilan027"] assert result == expected def test_parse_slurm_nodelist_mixed(self): """Test parsing a mixed range and list.""" result = self.importer.parse_slurm_nodelist("sdfmilan[001-003,010,020-022]") expected = [ - "sdfmilan0001", "sdfmilan0002", "sdfmilan0003", - "sdfmilan0010", - "sdfmilan0020", "sdfmilan0021", "sdfmilan0022" + "sdfmilan001", "sdfmilan002", "sdfmilan003", + "sdfmilan010", + "sdfmilan020", "sdfmilan021", "sdfmilan022" ] assert result == expected def test_parse_slurm_nodelist_different_prefix(self): """Test parsing with different node prefix.""" result = self.importer.parse_slurm_nodelist("sdfrome[001-003]") - expected = ["sdfrome0001", "sdfrome0002", "sdfrome0003"] + expected = ["sdfrome001", "sdfrome002", "sdfrome003"] assert result == expected def test_parse_slurm_nodelist_unparseable(self): From 8a93a7c14fce8ceab53a0fea353532c94c7a3940 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 4 Sep 2026 15:04:27 -0700 Subject: [PATCH 5/5] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index 774348c..490aed2 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 774348c0098b70ad802274abe4603dc6c83fb2b1 +Subproject commit 490aed26e7659fd68f308bfeb7b2cf07880f9bce