From 138bfd66b47aad3ac928bc9549d32479b0210edb Mon Sep 17 00:00:00 2001 From: Evan Carlin Date: Fri, 23 Aug 2024 12:35:47 -0600 Subject: [PATCH 1/3] Fix #7222: Handle 'proxy-for' header (#7224) Some proxys set the remote ip in the Proxy-For. --- sirepo/http_util.py | 15 +++++++++++++++ sirepo/pkcli/job_supervisor.py | 3 ++- sirepo/request.py | 4 ++-- sirepo/uri_router.py | 10 +++------- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/sirepo/http_util.py b/sirepo/http_util.py index bf09b34e38..08d1d9d210 100644 --- a/sirepo/http_util.py +++ b/sirepo/http_util.py @@ -46,3 +46,18 @@ def parse_auth_header(headers): if m := _AUTH_HEADER_RE.search(h): return m.group(1) return None + + +def remote_ip(request): + """IP address of client from request. + + Tornado covers 'X-Real-Ip' and 'X-Forwared-For'. This adds addition + headers to check. + + Args: + request (tornado.httputil.HTTPServerRequest): Incoming request + Returns: + str: IP address of client + + """ + return request.headers.get("proxy-for", request.remote_ip) diff --git a/sirepo/pkcli/job_supervisor.py b/sirepo/pkcli/job_supervisor.py index 76046be29c..e2a6762ad6 100644 --- a/sirepo/pkcli/job_supervisor.py +++ b/sirepo/pkcli/job_supervisor.py @@ -15,6 +15,7 @@ import sirepo.events import sirepo.feature_config import sirepo.global_resources.api +import sirepo.http_util import sirepo.job import sirepo.job_driver import sirepo.job_supervisor @@ -97,7 +98,7 @@ def open(self): pkdlog( "uri={} remote_ip={} ", self.request.uri, - self.request.remote_ip, + sirepo.http_util.remote_ip(self.request), ) def sr_close(self): diff --git a/sirepo/request.py b/sirepo/request.py index df03b1eed9..f2a02b8549 100644 --- a/sirepo/request.py +++ b/sirepo/request.py @@ -11,9 +11,9 @@ import pykern.pkcompat import pykern.pkjson import sirepo.const +import sirepo.http_util import sirepo.quest import sirepo.util -import urllib.parse import user_agents @@ -186,7 +186,7 @@ def _parse_authorization(value): http_method=r.method, http_request_uri=r.full_url(), http_server_uri=f"{r.protocol}://{r.host}/", - remote_addr=r.remote_ip, + remote_addr=sirepo.http_util.remote_ip(r), ) def body_as_bytes(self): diff --git a/sirepo/uri_router.py b/sirepo/uri_router.py index a51487518a..7d90090b50 100644 --- a/sirepo/uri_router.py +++ b/sirepo/uri_router.py @@ -7,25 +7,21 @@ from pykern import pkcollections from pykern import pkconfig from pykern import pkinspect -from pykern import pkjson from pykern.pkcollections import PKDict from pykern.pkdebug import pkdc, pkdexc, pkdlog, pkdp, pkdformat import asyncio -import contextlib import importlib import inspect -import os -import pkgutil import re import sirepo.api_auth import sirepo.auth import sirepo.const import sirepo.events import sirepo.feature_config +import sirepo.http_util import sirepo.spa_session import sirepo.uri import sirepo.util -import urllib.parse #: prefix for api functions _FUNC_PREFIX = "api_" @@ -225,7 +221,7 @@ def open(self): self.__headers = PKDict(r.headers) self.cookie_state = self.__headers.get("Cookie") self.http_server_uri = f"{r.protocol}://{r.host}/" - self.remote_addr = r.remote_ip + self.remote_addr = sirepo.http_util.remote_ip(r) self.ws_id = ws_count self.sr_log(None, "open", fmt=" ip={}", args=[_remote_peer(r)]) @@ -354,7 +350,7 @@ def _remote_peer(request): # socket is not set on stream for websockets. if hasattr(c, "stream") and hasattr(c.stream, "socket"): return "{}:{}".format(*c.stream.socket.getpeername()) - return f"{request.remote_ip}:0" + return f"{sirepo.http_util.remote_ip(request)}:0" sirepo.modules.import_and_init("sirepo.server").init_tornado() s = httpserver.HTTPServer( From 9df44ef53908ced668eebf044b1d3d7679de8df6 Mon Sep 17 00:00:00 2001 From: Evan Carlin Date: Tue, 27 Aug 2024 10:54:37 -0600 Subject: [PATCH 2/3] Fix #7223: Support sending to smtp server without authentication. (#7225) --- sirepo/smtp.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/sirepo/smtp.py b/sirepo/smtp.py index f8b722f0c0..f5838fa4b8 100644 --- a/sirepo/smtp.py +++ b/sirepo/smtp.py @@ -1,11 +1,9 @@ -# -*- coding: utf-8 -*- """SMTP connection to send emails -:copyright: Copyright (c) 2018-2019 RadiaSoft LLC. All Rights Reserved. +:copyright: Copyright (c) 2018-2024 RadiaSoft LLC. All Rights Reserved. :license: http://www.apache.org/licenses/LICENSE-2.0.html """ -from __future__ import absolute_import, division, print_function from pykern.pkdebug import pkdp, pkdlog from pykern import pkconfig from pykern.pkcollections import PKDict @@ -68,11 +66,12 @@ def _send_directly(msg): s.send_message(msg) -def _send_with_auth(msg): +def _send_via_relay_server(msg): with smtplib.SMTP(_cfg.server, _cfg.port) as s: s.starttls() s.ehlo() - s.login(_cfg.user, _cfg.password) + if _cfg.user and _cfg.password: + s.login(_cfg.user, _cfg.password) s.send_message(msg) @@ -94,14 +93,18 @@ def _init(): _cfg.server = "not " + _DEV_SMTP_SERVER _SEND = _send_directly return - _SEND = _send_with_auth + _SEND = _send_via_relay_server if pkconfig.in_dev_mode(): if _cfg.server is None: _cfg.server = _DEV_SMTP_SERVER return - if _cfg.server is None or _cfg.user is None or _cfg.password is None: + if _cfg.server is None: pkconfig.raise_error( - f"server={_cfg.server}, user={_cfg.user}, and password={_cfg.password} must be defined", + f"server={_cfg.server} must be defined", + ) + if bool(_cfg.user) != bool(_cfg.password): + pkconfig.raise_error( + f"user={_cfg.user} and password={_cfg.password} must be both set or not" ) From fef0fd3a456b1f046fa37169f80f39b775a59abe Mon Sep 17 00:00:00 2001 From: Evan Carlin Date: Fri, 30 Aug 2024 11:28:05 -0600 Subject: [PATCH 3/3] Fix #7231: Raydata fixes for scan detailed status (#7232) --- .../static/html/raydata-run-analysis.html | 2 +- sirepo/package_data/static/js/raydata.js | 128 ++++++++---------- sirepo/raydata/analysis_driver/__init__.py | 17 ++- sirepo/raydata/analysis_driver/chx.py | 28 +++- 4 files changed, 93 insertions(+), 82 deletions(-) diff --git a/sirepo/package_data/static/html/raydata-run-analysis.html b/sirepo/package_data/static/html/raydata-run-analysis.html index 2277526d45..a105fdb167 100644 --- a/sirepo/package_data/static/html/raydata-run-analysis.html +++ b/sirepo/package_data/static/html/raydata-run-analysis.html @@ -5,7 +5,7 @@
-
+
diff --git a/sirepo/package_data/static/js/raydata.js b/sirepo/package_data/static/js/raydata.js index 1783445e65..4e0bb386ab 100644 --- a/sirepo/package_data/static/js/raydata.js +++ b/sirepo/package_data/static/js/raydata.js @@ -1198,83 +1198,73 @@ SIREPO.app.directive('scanDetail', function() { template: `
Scan Detail
-
-
Scan Id: {{ scan.rduid }}
-
Analysis Elapsed Time: {{ analysisElapsedTime() }} seconds
-
-
Current Consecutive Failures: {{ consecutiveFailures() }}
-
-
-
Most Recent Status
- 
{{ currentStatus() }}
-
-
-
Detailed Status File
- 
{{ detailedStatus() }}
+
+
Scan Id: {{ scan.rduid }}
+
Analysis Elapsed Time: {{ analysisElapsedTime() }} seconds
+
+
Current Status: {{ scan.status }}
+
+ Detailed Status: +
    +
  • + {{ stepName }} +
      +
    • + {{ key }}: {{ parseTime(stepInfo[key]) }} +
      • +
    • elapsed: {{ stepElapsed(stepInfo) }}
      • +
    • status: {{ stepStatus(stepInfo) }}
      • +
    +
    • +
+
+
-
-`, - controller: function($scope, columnsService, utilities) { - function failureInRun(run) { - let r = false; - for (const f of Object.values($scope.detailedStatusFile()[run])) { - if (f.status === 'failed') { - r = true; - } - } - return r; - } - - function getSortedRunIndexes() { - return Object.keys($scope.detailedStatusFile()).map((x) => parseInt(x)).sort(); - } + `, + controller: function($filter, $scope, columnsService, raydataService, utilities) { + $scope.latestDetailedStatus = null; - function mostRecentAnalysisDetails() { - return $scope.detailedStatusFile()? $scope.detailedStatusFile()[Math.max(...getSortedRunIndexes())] : ''; - } + function setLatestDetailedStatus() { + $scope.latestDetailedStatus = $scope.scan.detailed_status[Math.max(Object.keys($scope.scan.detailed_status))]; + } $scope.analysisElapsedTime = () => { return $scope.scan && $scope.scan.analysis_elapsed_time ? $scope.scan.analysis_elapsed_time : null; }; - $scope.consecutiveFailures = () => { - if (! $scope.detailedStatusFile()) { - return ''; - } - let r = 0; - for (const k of getSortedRunIndexes().reverse()) { - if (failureInRun(k)) { - r += 1; - } else { - return r; - } - } - - return r; - }; - - $scope.currentStatus = () => { - let r = ''; - for (const k of Object.keys(mostRecentAnalysisDetails())) { - r += k + ': ' + mostRecentAnalysisDetails()[k].status + '\n'; - } - return r; - }; - - $scope.detailedStatus = () => { - return utilities.objectToText($scope.detailedStatusFile()).replace( - /(start:|stop:)(\s*)(\d+\.?\d*)/gi, - (_, p1, p2, p3) => { - return p1 + p2 + (new Date(parseFloat(p3)*1000)).toString(); - } - ) - ; - }; - - $scope.detailedStatusFile = () => { - return $scope.scan && $scope.scan.detailed_status && Object.keys($scope.scan.detailed_status).length > 0 ? $scope.scan.detailed_status : null; - }; + $scope.isEmptyObject = (obj) => { + return $.isEmptyObject(obj); + }; + + $scope.parseTime = (unixTime) => { + return (new Date(unixTime * 1000)).toString(); + }; + + $scope.stepElapsed = (stepInfo) => { + if (stepInfo.start && stepInfo.stop) { + return $filter('date')((stepInfo.stop - stepInfo.start) * 1000, 'HH:mm:ss', 'UTC'); + } + return null; + }; + + $scope.stepStatus = (stepInfo) => { + function pendingIfRunningElseError(scanStatus) { + if (raydataService.ANALYSIS_STATUS_NON_STOPPED.includes(scanStatus)) { + return 'pending'; + } + return 'error'; + } + + // start and stop have been set so the notebook was + // able to manage setting the status itself. + if (stepInfo.start && stepInfo.stop) { + return stepInfo.status; + } + return pendingIfRunningElseError($scope.scan.status); + }; + + $scope.$watch('scan', setLatestDetailedStatus); }, }; }); diff --git a/sirepo/raydata/analysis_driver/__init__.py b/sirepo/raydata/analysis_driver/__init__.py index 828b1b5992..701469ef4e 100644 --- a/sirepo/raydata/analysis_driver/__init__.py +++ b/sirepo/raydata/analysis_driver/__init__.py @@ -39,6 +39,11 @@ def get_notebooks(self, *args, **kwargs): raise NotImplementedError("children must implement this method") def get_output(self): + def load_json(path): + d = pkjson.load_any(path) + # CHX json are double encoded so may need to load 2x + return pkjson.load_any(d) if isinstance(d, str) else d + res = PKDict() for e in [ PKDict( @@ -53,7 +58,7 @@ def get_output(self): PKDict( name="jsonFiles", file_type="json", - op=pkjson.load_any, + op=load_json, ), ]: res[e.name] = [ @@ -69,12 +74,14 @@ def get_output_dir(self): def get_papermill_args(self): res = [] - for n, v in [ - ["uid", self.rduid], - ["scan", self.rduid], + for a in [ + PKDict(name="uid", value=self.rduid), + PKDict(name="scan", value=self.rduid), *self._get_papermill_args(), ]: - res.extend(["-p", f"'{n}'", f"'{v}'"]) + res.extend( + ["-r" if a.get("raw_param") else "-p", f"'{a.name}'", f"'{a.value}'"] + ) res.extend(("--report-mode", "--log-output", "--progress-bar")) return res diff --git a/sirepo/raydata/analysis_driver/chx.py b/sirepo/raydata/analysis_driver/chx.py index ba5ef36dad..3175a42c70 100644 --- a/sirepo/raydata/analysis_driver/chx.py +++ b/sirepo/raydata/analysis_driver/chx.py @@ -21,10 +21,14 @@ def get_conda_env(self): def get_detailed_status_file(self, rduid): p = self.get_output_dir().join(f"progress_dict_{rduid}.json") - if os.path.exists(p): - with open(p, "r") as f: - return pkjson.load_any(f) - return PKDict() + if not p.check(): + return PKDict() + d = pkjson.load_any(p) + # The notebooks do json.dump(json.dumps(progress_dict), outfile) + # which double encodes the json object. So, we may + # need to decode it 2x. Be compliant either way in case this + # changes in the future. + return pkjson.load_any(d) if isinstance(d, str) else d def get_notebooks(self): return [ @@ -54,9 +58,19 @@ def get_output_dir(self): def _get_papermill_args(self, *args, **kwargs): return [ - ["run_two_time", True], - ["run_dose", False], - ["username", self._scan_metadata.get_start_field("user")], + # Cycle can look like 2024_2 which is converted to int by papermill unless raw_param=True + PKDict( + name="cycle", + value=self._scan_metadata.get_start_field("cycle"), + raw_param=True, + ), + # POSIT: Same as AutoRun_functions.get_process_id + PKDict(name="process_id", value=f"{self.rduid}_0"), + PKDict(name="username", value=self._scan_metadata.get_start_field("user")), + PKDict( + name="user_group", + value=self._scan_metadata.get_start_field("user_group", unchecked=True), + ), ]