From be91cd9fa5ccabc25bdf703d04fa19366333649a Mon Sep 17 00:00:00 2001 From: Matthew Westphall Date: Thu, 6 Jun 2024 16:21:56 -0500 Subject: [PATCH 1/5] Update automerge check to error on invalid institution ID --- src/webapp/automerge_check.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/webapp/automerge_check.py b/src/webapp/automerge_check.py index 7cc9e491c..77d621361 100755 --- a/src/webapp/automerge_check.py +++ b/src/webapp/automerge_check.py @@ -122,19 +122,26 @@ def main(args): errors += check_resource_contacts(BASE_SHA, rg_fname, resources_affected, contacts) - if any( re.match(br'^projects/.*\.yaml', fname) for fname in modified ): + updated_projects = [fname for fname in modified if re.match(br'^projects/.*\.yaml', fname)] + if updated_projects: orgs_base = get_organizations_at_version(base) orgs_new = get_organizations_at_version(head) orgs_added = orgs_new - orgs_base for org in sorted(orgs_added): errors += ["New Organization '%s' requires OSG approval" % org] + invalid_institutions = get_invalid_institution_ids(head, updated_projects) + errors += [ + f"Invalid InstitutionID in project(s) {invalid_institutions.join(', ')}.\n" + f"Please see https://topology-institutions.osg-htc.org for valid ID list." + ] else: orgs_added = None + invalid_institutions = None print_errors(errors) return ( RC.ALL_CHECKS_PASS if len(errors) == 0 else RC.OUT_OF_DATE_ONLY if len(errors) == 1 and not up_to_date - else RC.ORGS_ADDED if orgs_added + else RC.ORGS_ADDED if orgs_added or invalid_institutions else RC.DT_MOD_ERRORS if len(DTs) > 0 else RC.CONTACT_ERROR if not contacts else RC.NON_DT_ERRORS ) @@ -212,6 +219,11 @@ def get_organizations_at_version(sha): if re.search(br'.\.yaml$', fname) ] return set( p.get("Organization") for p in projects ) +def get_invalid_institution_ids(sha, fnames): + projects = { fname : parse_yaml_at_version(sha, fname, {}) for fname in fnames } + return [fname for fname, yaml in projects.items() if not yaml.get("InstitutionID", "").startswith("https://osg-htc.org/iid/")] + + def commit_is_merged(sha_a, sha_b): args = ['git', 'merge-base', '--is-ancestor', sha_a, sha_b] ret, out = runcmd(args, stderr=_devnull) From a178b15d45d66e95f1b8b44f31b8aae754f277e5 Mon Sep 17 00:00:00 2001 From: Matthew Westphall Date: Fri, 7 Jun 2024 13:37:56 -0500 Subject: [PATCH 2/5] check against prod list of institution ids when adding new institution --- src/webapp/automerge_check.py | 5 ++++- src/webhook_app.py | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/webapp/automerge_check.py b/src/webapp/automerge_check.py index 77d621361..d11fa0732 100755 --- a/src/webapp/automerge_check.py +++ b/src/webapp/automerge_check.py @@ -2,6 +2,7 @@ import collections import subprocess +import requests import stat import yaml import sys @@ -220,8 +221,10 @@ def get_organizations_at_version(sha): return set( p.get("Organization") for p in projects ) def get_invalid_institution_ids(sha, fnames): + valid_institutions = requests.get('https://topology-institutions.osg-htc.org/api/institution_ids').json() + institution_ids = [i['id'] for i in valid_institutions] projects = { fname : parse_yaml_at_version(sha, fname, {}) for fname in fnames } - return [fname for fname, yaml in projects.items() if not yaml.get("InstitutionID", "").startswith("https://osg-htc.org/iid/")] + return [fname for fname, yaml in projects.items() if not yaml.get("InstitutionID", "") in institution_ids] def commit_is_merged(sha_a, sha_b): diff --git a/src/webhook_app.py b/src/webhook_app.py index cb00417d9..1cdee7239 100755 --- a/src/webhook_app.py +++ b/src/webhook_app.py @@ -285,13 +285,14 @@ def pull_request_hook(): set_webhook_pr_state(pull_num, head_sha, webhook_state) # only comment on errors if DT files modified or contact unknown + osg_bot_msg = "No reported errors" if ret in reportable_errors: osg_bot_msg = webhook_status_messages.automerge_status_messages[ret] body = osg_bot_msg.format(**locals()) action = 'REQUEST_CHANGES' if ret in rejectable_errors else 'COMMENT' publish_pr_review(pull_num, body, action, head_sha) - return Response('Thank You') + return Response(f'Thank You: {osg_bot_msg}') def runcmd(cmd, input=None, **kw): From 8b672668228f14dcb2af71f47b2ad85e6b395dd3 Mon Sep 17 00:00:00 2001 From: Matthew Westphall Date: Fri, 7 Jun 2024 14:45:08 -0500 Subject: [PATCH 3/5] Use variable for institutions api url --- src/webapp/automerge_check.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/webapp/automerge_check.py b/src/webapp/automerge_check.py index d11fa0732..1101c609b 100755 --- a/src/webapp/automerge_check.py +++ b/src/webapp/automerge_check.py @@ -9,6 +9,8 @@ import os import re +from webapp.models import GlobalData + try: from urllib.request import urlopen except ImportError: @@ -21,6 +23,8 @@ import xml.etree.ElementTree as et +INSTITUTIONS_API = GlobalData().config.get('INSTITUTIONS_API') +INSTITUTIONS_TLD = INSTITUTIONS_API.replace('/api','') # direct users to homepage rather than api # NOTE: throughout this program, git shas are of type str, while paths and # filenames are of type bytes. The motivation behind this is to handle @@ -131,10 +135,11 @@ def main(args): for org in sorted(orgs_added): errors += ["New Organization '%s' requires OSG approval" % org] invalid_institutions = get_invalid_institution_ids(head, updated_projects) - errors += [ - f"Invalid InstitutionID in project(s) {invalid_institutions.join(', ')}.\n" - f"Please see https://topology-institutions.osg-htc.org for valid ID list." - ] + if invalid_institutions: + errors += [ + f"Unrecognized InstitutionID in project(s) {', '.join(invalid_institutions)}. " + f"See {INSTITUTIONS_TLD} for known ID list." + ] else: orgs_added = None invalid_institutions = None @@ -221,7 +226,7 @@ def get_organizations_at_version(sha): return set( p.get("Organization") for p in projects ) def get_invalid_institution_ids(sha, fnames): - valid_institutions = requests.get('https://topology-institutions.osg-htc.org/api/institution_ids').json() + valid_institutions = requests.get(f'{INSTITUTIONS_API}/institution_ids').json() institution_ids = [i['id'] for i in valid_institutions] projects = { fname : parse_yaml_at_version(sha, fname, {}) for fname in fnames } return [fname for fname, yaml in projects.items() if not yaml.get("InstitutionID", "") in institution_ids] From 9458e9be8aae9345fb5ac67a0df2de25e56b6fa1 Mon Sep 17 00:00:00 2001 From: Matthew Westphall Date: Fri, 7 Jun 2024 14:51:02 -0500 Subject: [PATCH 4/5] update new_org_rejected status message --- src/webapp/webhook_status_messages.py | 2 +- src/webhook_app.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/webapp/webhook_status_messages.py b/src/webapp/webhook_status_messages.py index 50e7c7726..b8f8f8d27 100644 --- a/src/webapp/webhook_status_messages.py +++ b/src/webapp/webhook_status_messages.py @@ -76,7 +76,7 @@ "Thank you for your pull request.\n" "\n" "Your [requested changes]({base_sha}...{head_sha}) add a new organization" - " to a project file; this requires manual review from OSG Staff.\n" + " or project ID to a project file; this requires manual review from OSG Staff.\n" "\n" "-- OSG-BOT :oncoming_police_car:\n" "\n" diff --git a/src/webhook_app.py b/src/webhook_app.py index 1cdee7239..b3cd4cfd8 100755 --- a/src/webhook_app.py +++ b/src/webhook_app.py @@ -292,7 +292,7 @@ def pull_request_hook(): action = 'REQUEST_CHANGES' if ret in rejectable_errors else 'COMMENT' publish_pr_review(pull_num, body, action, head_sha) - return Response(f'Thank You: {osg_bot_msg}') + return Response(f'Thank You') def runcmd(cmd, input=None, **kw): From cc2edfaea0802f781b000aaad7ad5ab83b198dab Mon Sep 17 00:00:00 2001 From: Matthew Westphall Date: Fri, 7 Jun 2024 15:31:39 -0500 Subject: [PATCH 5/5] remove variable for institution ID since relative imports don't work based on the script's invocation method --- src/webapp/automerge_check.py | 11 ++++------- src/webhook_app.py | 3 +-- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/webapp/automerge_check.py b/src/webapp/automerge_check.py index 1101c609b..969d4d103 100755 --- a/src/webapp/automerge_check.py +++ b/src/webapp/automerge_check.py @@ -9,8 +9,6 @@ import os import re -from webapp.models import GlobalData - try: from urllib.request import urlopen except ImportError: @@ -23,8 +21,7 @@ import xml.etree.ElementTree as et -INSTITUTIONS_API = GlobalData().config.get('INSTITUTIONS_API') -INSTITUTIONS_TLD = INSTITUTIONS_API.replace('/api','') # direct users to homepage rather than api +INSTITUTIONS_API = "https://topology-institutions.osg-htc.org" # NOTE: throughout this program, git shas are of type str, while paths and # filenames are of type bytes. The motivation behind this is to handle @@ -138,7 +135,7 @@ def main(args): if invalid_institutions: errors += [ f"Unrecognized InstitutionID in project(s) {', '.join(invalid_institutions)}. " - f"See {INSTITUTIONS_TLD} for known ID list." + f"See {INSTITUTIONS_API} for known ID list." ] else: orgs_added = None @@ -226,10 +223,10 @@ def get_organizations_at_version(sha): return set( p.get("Organization") for p in projects ) def get_invalid_institution_ids(sha, fnames): - valid_institutions = requests.get(f'{INSTITUTIONS_API}/institution_ids').json() + valid_institutions = requests.get(f'{INSTITUTIONS_API}/api/institution_ids').json() institution_ids = [i['id'] for i in valid_institutions] projects = { fname : parse_yaml_at_version(sha, fname, {}) for fname in fnames } - return [fname for fname, yaml in projects.items() if not yaml.get("InstitutionID", "") in institution_ids] + return [fname.decode() for fname, yaml in projects.items() if not yaml.get("InstitutionID", "") in institution_ids] def commit_is_merged(sha_a, sha_b): diff --git a/src/webhook_app.py b/src/webhook_app.py index b3cd4cfd8..cb00417d9 100755 --- a/src/webhook_app.py +++ b/src/webhook_app.py @@ -285,14 +285,13 @@ def pull_request_hook(): set_webhook_pr_state(pull_num, head_sha, webhook_state) # only comment on errors if DT files modified or contact unknown - osg_bot_msg = "No reported errors" if ret in reportable_errors: osg_bot_msg = webhook_status_messages.automerge_status_messages[ret] body = osg_bot_msg.format(**locals()) action = 'REQUEST_CHANGES' if ret in rejectable_errors else 'COMMENT' publish_pr_review(pull_num, body, action, head_sha) - return Response(f'Thank You') + return Response('Thank You') def runcmd(cmd, input=None, **kw):