diff --git a/gitlab-runner-mock/src/api/request.rs b/gitlab-runner-mock/src/api/request.rs index 3afd191..5ed19f4 100644 --- a/gitlab-runner-mock/src/api/request.rs +++ b/gitlab-runner-mock/src/api/request.rs @@ -8,11 +8,9 @@ use wiremock::{Request, Respond}; use crate::{GitlabRunnerMock, MockJobState}; /* - jobs/id => 200 if ok; + jobs/id => 200 if ok; - 403 if cancelled - < job-status: canceled in header -} + 403 if job token is invalid or the job was cancelled */ #[derive(Default, Deserialize)] diff --git a/gitlab-runner-mock/src/api/trace.rs b/gitlab-runner-mock/src/api/trace.rs index 0d71f44..21a8bc7 100644 --- a/gitlab-runner-mock/src/api/trace.rs +++ b/gitlab-runner-mock/src/api/trace.rs @@ -53,19 +53,16 @@ impl Respond for JobTraceResponder { }; if let Some(job) = self.mock.get_job(id) { - if token != job.token() { + if token != job.token() || job.state() != MockJobState::Running { ResponseTemplate::new(StatusCode::FORBIDDEN) - } else if job.state() != MockJobState::Running { - ResponseTemplate::new(StatusCode::FORBIDDEN) - .insert_header("Job-Status", &*job.state().to_string()) } else { match job.append_log(request.body.clone(), start, end) { Ok(()) => ResponseTemplate::new(StatusCode::ACCEPTED) .insert_header( "X-GitLab-Trace-Update-Interval", - &*self.mock.update_interval().to_string(), + self.mock.update_interval().to_string(), ) - .insert_header("Job-Status", &*job.state().to_string()), + .insert_header("Job-Status", job.state().to_string()), Err(e) => ResponseTemplate::new(StatusCode::RANGE_NOT_SATISFIABLE) .set_body_string(format!("{e:?}")), } diff --git a/gitlab-runner-mock/src/api/update.rs b/gitlab-runner-mock/src/api/update.rs index cd8acf4..0113e70 100644 --- a/gitlab-runner-mock/src/api/update.rs +++ b/gitlab-runner-mock/src/api/update.rs @@ -35,29 +35,22 @@ impl Respond for JobUpdateResponder { .unwrap(); if let Some(job) = self.mock.get_job(id) { - if r.token != job.token() { + if r.token != job.token() || job.state() != MockJobState::Running { ResponseTemplate::new(StatusCode::FORBIDDEN) } else { - let r = match (job.state(), r.state) { - (MockJobState::Running, MockJobState::Success) => { - job.update_state(r.state); - ResponseTemplate::new(StatusCode::OK) - } - (MockJobState::Running, MockJobState::Failed) => { - job.update_state(r.state); - ResponseTemplate::new(StatusCode::OK) - } - (MockJobState::Running, MockJobState::Running) => { - job.update_state(r.state); - ResponseTemplate::new(StatusCode::OK) - } - (current_state, _) if current_state != MockJobState::Running => { - ResponseTemplate::new(StatusCode::FORBIDDEN) - } - _ => panic!("Invalid state change"), - }; + assert!( + matches!( + r.state, + MockJobState::Success | MockJobState::Failed | MockJobState::Running + ), + "Invalid state change from {} -> {}", + job.state(), + r.state, + ); - r.append_header("Job-Status", &*job.state().to_string()) + job.update_state(r.state); + ResponseTemplate::new(StatusCode::OK) + .append_header("Job-Status", job.state().to_string()) } } else { ResponseTemplate::new(StatusCode::NOT_FOUND) diff --git a/gitlab-runner/src/client.rs b/gitlab-runner/src/client.rs index 365583d..9925054 100644 --- a/gitlab-runner/src/client.rs +++ b/gitlab-runner/src/client.rs @@ -21,7 +21,6 @@ where } const GITLAB_TRACE_UPDATE_INTERVAL: &str = "X-GitLab-Trace-Update-Interval"; -const JOB_STATUS: &str = "Job-Status"; #[derive(Debug, Default, Clone, Serialize)] struct FeaturesInfo { @@ -250,8 +249,10 @@ impl JobResponse { pub enum Error { #[error("Unexpected reply code {0}")] UnexpectedStatus(StatusCode), - #[error("Job cancelled")] - JobCancelled, + // May occur if the job was cancelled, in which case the job token we're + // using gets invalidated. + #[error("Forbidden")] + Forbidden, #[error("Request failure {0}")] Request(#[from] reqwest::Error), #[error("Failed to write to destination {0}")] @@ -336,13 +337,6 @@ impl Client { } } - fn check_for_job_cancellation(&self, response: &reqwest::Response) -> Result<(), Error> { - match response.headers().get(JOB_STATUS) { - Some(header) if header == "canceled" => Err(Error::JobCancelled), - _ => Ok(()), - } - } - pub async fn update_job( &self, id: u64, @@ -359,8 +353,6 @@ impl Client { let r = self.client.put(url).json(&update).send().await?; - self.check_for_job_cancellation(&r)?; - let trace_update_interval = r .headers() .get(GITLAB_TRACE_UPDATE_INTERVAL) @@ -369,6 +361,7 @@ impl Client { StatusCode::OK => Ok(JobUpdateReply { trace_update_interval, }), + StatusCode::FORBIDDEN => Err(Error::Forbidden), _ => Err(Error::UnexpectedStatus(r.status())), } } @@ -406,8 +399,6 @@ impl Client { .send() .await?; - self.check_for_job_cancellation(&r)?; - let trace_update_interval = r .headers() .get(GITLAB_TRACE_UPDATE_INTERVAL) @@ -417,6 +408,7 @@ impl Client { StatusCode::ACCEPTED => Ok(TraceReply { trace_update_interval, }), + StatusCode::FORBIDDEN => Err(Error::Forbidden), _ => Err(Error::UnexpectedStatus(r.status())), } } diff --git a/gitlab-runner/src/run.rs b/gitlab-runner/src/run.rs index 4cbdb2a..519b3cc 100644 --- a/gitlab-runner/src/run.rs +++ b/gitlab-runner/src/run.rs @@ -248,7 +248,7 @@ impl Run { .await { Ok(_reply) => (), - Err(crate::client::Error::JobCancelled) => cancel_token.cancel(), + Err(crate::client::Error::Forbidden) => cancel_token.cancel(), Err(err) => warn!("Failed to update job status: {:?}", err), } } @@ -276,7 +276,7 @@ impl Run { self.log_offset += len; reply.trace_update_interval } - Err(crate::client::Error::JobCancelled) => { + Err(crate::client::Error::Forbidden) => { cancel_token.cancel(); None }