PR#376: Retry the `Errata._errata_authorized_get()` call. - freshmaker

freshmaker

#376 Retry the `Errata._errata_authorized_get()` call.

Merged 5 years ago by jkaluza. Opened 5 years ago by jkaluza.

jkaluza/freshmaker errata-retry-kerberos into master

Retry the `Errata._errata_authorized_get()` call.

Jan Kaluza • 5 years ago

7abb059

freshmaker/errata.py

file modified

+9 -8

		`@@ -33,6 +33,7 @@`
		`FreshmakerManualRebuildEvent)`
		`from freshmaker import conf, log`
		`from freshmaker.bugzilla import BugzillaAPI`
		`+ from freshmaker.utils import retry`


		`class ErrataAdvisory(object):`
		`@@ -122,19 +123,19 @@`
		`xmlrpc_url = self.server_url + '/errata/xmlrpc.cgi'`
		`self.xmlrpc = ServerProxy(xmlrpc_url, transport=SafeCookieTransport())`

		`+ @retry(wait_on=(requests.exceptions.RequestException,), logger=log)`
		`def _errata_authorized_get(self, args, *kwargs):`
		`- r = requests.get(`
		`- *args,`
		`- auth=HTTPKerberosAuth(principal=conf.krb_auth_principal),`
		`- **kwargs)`
		`- if r.status_code == 401:`
		`- log.info("CCache file expired, removing it.")`
		`- os.unlink(conf.krb_auth_ccache_file)`
		`+ try:`
		`r = requests.get(`
		`*args,`
		`auth=HTTPKerberosAuth(principal=conf.krb_auth_principal),`
		`**kwargs)`
		`- r.raise_for_status()`
		`+ r.raise_for_status()`
		`+ except requests.exceptions.RequestException as e:`
		`+ if e.response and e.response.status_code == 401:`
		`+ log.info("CCache file probably expired, removing it.")`
		`+ os.unlink(conf.krb_auth_ccache_file)`
		`+ raise`
		`return r.json()`

		`def _errata_rest_get(self, endpoint):`

tests/test_errata.py

file modified

+49 -1

		`@@ -20,7 +20,9 @@`
		`# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`# SOFTWARE.`

		`- from mock import patch`
		`+ from mock import patch, MagicMock`
		`+ from requests_kerberos.exceptions import MutualAuthenticationError`
		`+ from requests.exceptions import HTTPError`

		`from freshmaker.errata import Errata, ErrataAdvisory`
		`from freshmaker.events import (`
		`@@ -352,3 +354,49 @@`
		`xmlrpc.get_advisory_cdn_docker_file_list.return_value = None`
		`repo_tags = self.errata.get_docker_repo_tags(28484)`
		`self.assertEqual(repo_tags, None)`
		`+`
		`+`
		`+ class TestErrataAuthorizedGet(helpers.FreshmakerTestCase):`
		`+ def setUp(self):`
		`+ super(TestErrataAuthorizedGet, self).setUp()`
		`+ self.errata = Errata("https://localhost/")`
		`+`
		`+ self.patcher = helpers.Patcher(`
		`+ 'freshmaker.errata.')`
		`+ self.requests_get = self.patcher.patch("requests.get")`
		`+ self.response = MagicMock()`
		`+ self.response.json.return_value = {"foo": "bar"}`
		`+ self.unlink = self.patcher.patch("os.unlink")`
		`+`
		`+ def tearDown(self):`
		`+ super(TestErrataAuthorizedGet, self).tearDown()`
		`+ self.patcher.unpatch_all()`
		`+`
		`+ def test_errata_authorized_get(self):`
		`+ self.requests_get.return_value = self.response`
		`+ data = self.errata._errata_authorized_get("http://localhost/test")`
		`+ self.assertEqual(data, {"foo": "bar"})`
		`+`
		`+ def test_errata_authorized_get_kerberos_exception(self):`
		`+ # Test that MutualAuthenticationError is retried.`
		`+ self.requests_get.side_effect = [MutualAuthenticationError, self.response]`
		`+`
		`+ data = self.errata._errata_authorized_get("http://localhost/test")`
		`+`
		`+ self.assertEqual(data, {"foo": "bar"})`
		`+ self.assertEqual(len(self.requests_get.mock_calls), 2)`
		`+`
		`+ def test_errata_authorized_get_kerberos_exception_401(self):`
		`+ # Test that 401 error response is retried with kerberos ccache file`
		`+ # removed.`
		`+ error_response = MagicMock()`
		`+ error_response.status_code = 401`
		`+ error_response.raise_for_status.side_effect = HTTPError(`
		`+ "Expected exception", response=error_response)`
		`+ self.requests_get.side_effect = [error_response, self.response]`
		`+`
		`+ data = self.errata._errata_authorized_get("http://localhost/test")`
		`+`
		`+ self.assertEqual(data, {"foo": "bar"})`
		`+ self.assertEqual(len(self.requests_get.mock_calls), 2)`
		`+ self.unlink.assert_called_once_with(helpers.AnyStringWith("freshmaker_cc"))`

jkaluza commented 5 years ago

For so far unknown reasons, Errata Tool sometimes replies with
HTTP 200 OK to authorization request (the body of that response
is not known right now). This leads to MutualAuthenticationError
exception in requests_kerberos, because it is not expected
response in this phase of Kerberos auth.

As simple workaround, it should be enough to catch that exception
and try retrying the Errata query again. This is what this commit
does.