PR#1535: Only allow cancelling module builds in the init, wait, and build states - fm-orchestrator

fm-orchestrator

#1535 Only allow cancelling module builds in the init, wait, and build states

Merged 4 years ago by mprahl. Opened 4 years ago by mprahl.

patch-state-restrictions into master

Only allow cancelling module builds in the init, wait, and build states

mprahl • 4 years ago

9d946af

module_build_service/views.py

file modified

+21 -8

		`@@ -206,15 +206,28 @@`
		`log.error("Invalid JSON submitted")`
		`raise ValidationError("Invalid JSON submitted")`

		`- if module.state == models.BUILD_STATES["failed"]:`
		`- raise Forbidden("You can't cancel a failed module")`
		`+ state = r["state"]`
		`+ valid_input_states = ("failed", str(models.BUILD_STATES["failed"]))`
		`+ if state not in valid_input_states:`
		`+ raise ValidationError(`
		`+ "An invalid state was submitted. Valid states values are: {}"`
		`+ .format(", ".join(valid_input_states))`
		`+ )`

		`- if r["state"] == "failed" or r["state"] == str(models.BUILD_STATES["failed"]):`
		`- module.transition(`
		`- db.session, conf, models.BUILD_STATES["failed"], "Canceled by %s." % username)`
		`- else:`
		`- log.error('The provided state change of "{}" is not supported'.format(r["state"]))`
		`- raise ValidationError("The provided state change is not supported")`
		`+ valid_states_to_cancel = ("build", "init", "wait")`
		`+ module_state_name = models.INVERSE_BUILD_STATES[module.state]`
		`+ if module_state_name not in valid_states_to_cancel:`
		`+ log.error(`
		`+ "The user %s attempted to cancel a build in the %s state",`
		`+ username, module_state_name,`
		`+ )`
		`+ raise ValidationError(`
		`+ "To cancel a module build, it must be in one of the following states: {}"`
		`+ .format(", ".join(valid_states_to_cancel))`
		`+ )`
		`+`
		`+ module.transition(`
		`+ db.session, conf, models.BUILD_STATES["failed"], "Canceled by %s." % username)`
		`db.session.add(module)`
		`db.session.commit()`

tests/test_views/test_views.py

file modified

+18 -9

		`@@ -1253,18 +1253,25 @@`
		`assert data["state"] == 4`
		`assert data["state_reason"] == "Canceled by some_other_user."`

		`+ @pytest.mark.parametrize("module_state", (BUILD_STATES["failed"], BUILD_STATES["ready"]))`
		`@patch("module_build_service.auth.get_user", return_value=other_user)`
		`- def test_cancel_build_already_failed(self, mocked_get_user):`
		`+ def test_cancel_build_in_invalid_state(self, mocked_get_user, module_state):`
		`module = ModuleBuild.get_by_id(db_session, 7)`
		`- module.state = 4`
		`+ module.state = module_state`
		`db_session.commit()`

		`rv = self.client.patch(`
		`"/module-build-service/1/module-builds/7", data=json.dumps({"state": "failed"}))`

		`- data = json.loads(rv.data)`
		`- assert data["status"] == 403`
		`- assert data["error"] == "Forbidden"`
		`+ assert rv.status_code == 400`
		`+ assert rv.json == {`
		`+ "error": "Bad Request",`
		`+ "message": (`
		`+ "To cancel a module build, it must be in one of the following states: "`
		`+ "build, init, wait"`
		`+ ),`
		`+ "status": 400,`
		`+ }`

		`@patch("module_build_service.auth.get_user", return_value=("sammy", set()))`
		`def test_cancel_build_unauthorized_no_groups(self, mocked_get_user):`
		`@@ -1328,11 +1335,13 @@`
		`def test_cancel_build_wrong_state(self, mocked_get_user):`
		`rv = self.client.patch(`
		`"/module-build-service/1/module-builds/7", data=json.dumps({"state": "some_state"}))`
		`- data = json.loads(rv.data)`

		`- assert data["status"] == 400`
		`- assert data["error"] == "Bad Request"`
		`- assert data["message"] == "The provided state change is not supported"`
		`+ assert rv.status_code == 400`
		`+ assert rv.json == {`
		`+ "error": "Bad Request",`
		`+ "message": "An invalid state was submitted. Valid states values are: failed, 4",`
		`+ "status": 400,`
		`+ }`

		`@patch("module_build_service.auth.get_user", return_value=user)`
		`def test_submit_build_unsupported_scm_scheme(self, mocked_get_user):`