Skip to content

Commit 6cd4797

Browse files
committed
github: generate GitHub token closer to time of use
1 parent 4188ee1 commit 6cd4797

File tree

1 file changed

+63
-16
lines changed

1 file changed

+63
-16
lines changed

github/server/src/variety/basic.rs

Lines changed: 63 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2021 Oxide Computer Company
2+
* Copyright 2023 Oxide Computer Company
33
*/
44

55
use crate::{App, FlushOut, FlushState};
@@ -66,6 +66,9 @@ struct BasicPrivate {
6666
job_outputs: Vec<BasicOutput>,
6767
#[serde(default)]
6868
job_outputs_extra: usize,
69+
70+
#[serde(default)]
71+
extra_repo_ids: Vec<i64>,
6972
}
7073

7174
#[derive(Debug, Serialize, Deserialize)]
@@ -311,17 +314,65 @@ pub(crate) async fn run(
311314
* to update our state.
312315
*/
313316
let bt = b.job_get(jid).await?.into_inner();
317+
let running = bt.state == "running";
318+
let complete = bt.state == "completed" || bt.state == "failed";
314319
let new_state = Some(bt.state);
315-
let complete = if let Some(state) = new_state.as_deref() {
316-
state == "completed" || state == "failed"
317-
} else {
318-
false
319-
};
320320
if new_state != p.job_state {
321321
cr.flushed = false;
322322
p.job_state = new_state;
323323
}
324324

325+
if running {
326+
let store = b.job_store_get_all(jid).await?.into_inner();
327+
328+
if !store.contains_key("GITHUB_TOKEN") {
329+
/*
330+
* As has become something of a theme, the GitHub API with which
331+
* applications can generate an ephemeral credential for access
332+
* to private repositories presents considerable opportunity for
333+
* improvement. One cannot specify a retention period, so the
334+
* tokens just expire after around one hour. If this is not
335+
* long enough for you, well, whose fault is that anyway?
336+
*
337+
* In order to provide the absolute freshest token that we can,
338+
* we must generate the token only once the job begins running:
339+
* we cannot control how long a job spends in the "queued"
340+
* state, as it depends on how busy the system is; we also
341+
* cannot control how long the job spends in the "waiting"
342+
* state, as it depends on how long its dependencies take to
343+
* complete execution before it.
344+
*/
345+
let token = app
346+
.temp_access_token(
347+
cs.install,
348+
&repo,
349+
Some(&p.extra_repo_ids),
350+
)
351+
.await?;
352+
353+
/*
354+
* Place the generated token in the property store for the job.
355+
* This store can be updated while the job is running. Values
356+
* are obtained through the "bmat" control program inside the
357+
* job. Timing is important but not critical: the job will wait
358+
* for the value to appear before continuing.
359+
*/
360+
b.job_store_put(
361+
jid,
362+
"GITHUB_TOKEN",
363+
&buildomat_client::types::JobStoreValue {
364+
/*
365+
* Mark this value as a secret so that it will not be
366+
* included in diagnostic output.
367+
*/
368+
secret: true,
369+
value: token,
370+
},
371+
)
372+
.await?;
373+
}
374+
}
375+
325376
/*
326377
* We don't want to overwhelm GitHub with requests to update the screen,
327378
* so we will only update our "tail -f" view of build output at most
@@ -534,7 +585,6 @@ pub(crate) async fn run(
534585
* whether the repository for the check run or one of the other
535586
* repositories to which the check needs access.
536587
*/
537-
let mut extras = Vec::new();
538588
if !c.access_repos.is_empty() {
539589
/*
540590
* First, make sure this job is authorised by a member of the
@@ -565,8 +615,8 @@ pub(crate) async fn run(
565615
let msg = if let Some((owner, name)) = dep.split_once("/") {
566616
match gh.repos().get(owner, name).await {
567617
Ok(fr) => {
568-
if !extras.contains(&fr.id) {
569-
extras.push(fr.id);
618+
if !p.extra_repo_ids.contains(&fr.id) {
619+
p.extra_repo_ids.push(fr.id);
570620
}
571621
continue;
572622
}
@@ -608,9 +658,6 @@ pub(crate) async fn run(
608658
}
609659
}
610660

611-
let token =
612-
app.temp_access_token(cs.install, &repo, Some(&extras)).await?;
613-
614661
/*
615662
* Create a series of tasks to configure the build environment
616663
* before handing control to the user program.
@@ -689,8 +736,6 @@ pub(crate) async fn run(
689736
});
690737
}
691738

692-
buildenv.insert("GITHUB_TOKEN".into(), token.clone());
693-
694739
/*
695740
* Write the temporary access token which gives brief read-only access
696741
* to only this (potentially private) repository into the ~/.netrc file.
@@ -711,8 +756,12 @@ pub(crate) async fn run(
711756
workdir: Some("/home/build".into()),
712757
script: "\
713758
#!/bin/bash\n\
759+
\n\
714760
set -o errexit\n\
715761
set -o pipefail\n\
762+
\n\
763+
GITHUB_TOKEN=$(bmat store get GITHUB_TOKEN)\n\
764+
\n\
716765
cat >$HOME/.netrc <<EOF\n\
717766
machine github.com\n\
718767
login x-access-token\n\
@@ -727,8 +776,6 @@ pub(crate) async fn run(
727776
.into(),
728777
});
729778

730-
buildenv.remove("GITHUB_TOKEN");
731-
732779
/*
733780
* By default, we assume that the target provides toolchains and other
734781
* development tools like git. While this makes sense for most jobs, in

0 commit comments

Comments
 (0)