From 8288b5bb76d01c7fb88c51672bfb5d33e077d2d8 Mon Sep 17 00:00:00 2001 From: Jan Jansky Date: Thu, 8 Oct 2020 14:26:03 +0200 Subject: [PATCH] [policy] Fix failure conditions with upload The logic for determining if an archive should be uploaded to the Customer Portal was too strict, ease it to now properly only block on a missing case number since username and passwords may now be provided via env vars. Fixes an issue whereby we ignore a user-provided FTP directory. Adds a timeout and a timeout handler for FTP connections, rather than letting the connection attempt continue indefinitely. Second, adds exception handling for an edge case where the connection to the FTP server fails, but does not generate an exception from the ftplib module. Additionally, correct the type-ing of the error numbers being checked so that we actually match them. Caling "sos report --upload --case-id=123 --batch" should fallback to uploading to FTP server as the upload user is unknown and can't be prompted in batch mode. Related: #2276 Related: #2245 Resolves: #2265 Signed-off-by: Jan Jansky Signed-off-by: Jake Hunsaker --- sos/policies/__init__.py | 19 +++++++++++++------ sos/policies/redhat.py | 21 +++++++++++++++++---- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/sos/policies/__init__.py b/sos/policies/__init__.py index ed3f0cc..a22c277 100644 --- a/sos/policies/__init__.py +++ b/sos/policies/__init__.py @@ -972,7 +972,8 @@ class LinuxPolicy(Policy): """Should be overridden by policies to determine if a user needs to be provided or not """ - if not self.upload_user and not self._upload_user: + if not self.get_upload_password() and (self.get_upload_user() != + self._upload_user): msg = "Please provide upload user for %s: " % self.get_upload_url() self.upload_user = input(_(msg)) @@ -1029,7 +1030,8 @@ class LinuxPolicy(Policy): """ self.upload_archive = archive - self.upload_url = self.get_upload_url() + if not self.upload_url: + self.upload_url = self.get_upload_url() if not self.upload_url: raise Exception("No upload destination provided by policy or by " "--upload-url") @@ -1187,18 +1189,23 @@ class LinuxPolicy(Policy): password = self.get_upload_password() if not directory: - directory = self._upload_directory + directory = self.upload_directory or self._upload_directory try: - session = ftplib.FTP(url, user, password) + session = ftplib.FTP(url, user, password, timeout=15) + if not session: + raise Exception("connection failed, did you set a user and " + "password?") session.cwd(directory) + except socket.timeout: + raise Exception("timeout hit while connecting to %s" % url) except socket.gaierror: raise Exception("unable to connect to %s" % url) except ftplib.error_perm as err: errno = str(err).split()[0] - if errno == 503: + if errno == '503': raise Exception("could not login as '%s'" % user) - if errno == 550: + if errno == '550': raise Exception("could not set upload directory to %s" % directory) diff --git a/sos/policies/redhat.py b/sos/policies/redhat.py index 9fbe743..3412f44 100644 --- a/sos/policies/redhat.py +++ b/sos/policies/redhat.py @@ -312,15 +312,28 @@ support representative. "Enter your Red Hat Customer Portal username (empty to use " "public dropbox): ") ) + if not self.upload_user: + self.upload_url = RH_FTP_HOST + self.upload_user = self._upload_user + + def _upload_user_set(self): + user = self.get_upload_user() + return user and (user != 'anonymous') def get_upload_url(self): + if self.upload_url: + return self.upload_url if self.commons['cmdlineopts'].upload_url: return self.commons['cmdlineopts'].upload_url - if (not self.case_id or not self.upload_user or not - self.upload_password): - # Cannot use the RHCP. Use anonymous dropbox + # anonymous FTP server should be used as fallback when either: + # - case id is not set, or + # - upload user isn't set AND batch mode prevents to prompt for it + if (not self.case_id) or \ + ((not self._upload_user_set()) and + self.commons['cmdlineopts'].batch): self.upload_user = self._upload_user - self.upload_directory = self._upload_directory + if self.upload_directory is None: + self.upload_directory = self._upload_directory self.upload_password = None return RH_FTP_HOST else: -- 1.8.3.1