Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flowey: use typestate pattern for gh variables #643

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f4f97df
try and get something working
justus-camp-microsoft Jan 9, 2025
82db3d9
fmt
justus-camp-microsoft Jan 9, 2025
ff116bb
test printing pr event
justus-camp-microsoft Jan 10, 2025
46a3016
plumb through is_object
justus-camp-microsoft Jan 10, 2025
65fed09
Merge branch 'main' into ghcontext_typestate
justus-camp-microsoft Jan 10, 2025
dcfa810
try toJSON instead
justus-camp-microsoft Jan 10, 2025
478329b
try echoing instead?
justus-camp-microsoft Jan 10, 2025
cbdd984
try another strategy
justus-camp-microsoft Jan 10, 2025
db8a9c9
add parentheses
justus-camp-microsoft Jan 10, 2025
c7f63cf
let's try jq
justus-camp-microsoft Jan 10, 2025
e360854
better typestate impl (still needs do_read de-dupe)
justus-camp-microsoft Jan 11, 2025
ebec5e1
plumb is_object through on write path
justus-camp-microsoft Jan 11, 2025
d55fc8a
add from_json
justus-camp-microsoft Jan 13, 2025
f778042
clippy
justus-camp-microsoft Jan 13, 2025
b71c81b
see what happens when the event doesn't exist
justus-camp-microsoft Jan 13, 2025
bfa2abd
remove unused events
justus-camp-microsoft Jan 13, 2025
ff200aa
remove from_json and add back test event
justus-camp-microsoft Jan 13, 2025
1d8e3ff
move event to the right place
justus-camp-microsoft Jan 13, 2025
06bade7
try another way of passing is_object through
justus-camp-microsoft Jan 14, 2025
667fbb5
add comment about not adding further behavior to GhContextVar
justus-camp-microsoft Jan 15, 2025
86f2354
Merge branch 'main' into ghcontext_typestate
justus-camp-microsoft Jan 15, 2025
3aa18b6
move to separate module
justus-camp-microsoft Jan 15, 2025
d88b355
shared helper
justus-camp-microsoft Jan 15, 2025
a5b3562
clippy
justus-camp-microsoft Jan 15, 2025
1cd416e
restructure modules, remove test event
justus-camp-microsoft Jan 16, 2025
c20f33a
move global vars to typestate without removing GhContextVar
justus-camp-microsoft Jan 16, 2025
1395a24
remove ghcontextvar
justus-camp-microsoft Jan 17, 2025
7abe491
submodule structure
justus-camp-microsoft Jan 17, 2025
3f45028
re-introduce custom secrets flow
justus-camp-microsoft Jan 17, 2025
ac30808
Merge branch 'main' into ghcontext_typestate
justus-camp-microsoft Jan 17, 2025
9f7afd1
comments
Jan 24, 2025
440251b
docs
Jan 24, 2025
a293393
doc tests
Jan 24, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 111 additions & 45 deletions .github/workflows/openvmm-ci.yaml

Large diffs are not rendered by default.

156 changes: 111 additions & 45 deletions .github/workflows/openvmm-pr.yaml

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions flowey/flowey_cli/src/cli/debug/interrogate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

use crate::cli::FlowBackendCli;
use flowey_core::github_context::GhVarState;
use flowey_core::node::steps::rust::RustRuntimeServices;
use flowey_core::node::user_facing::ClaimedGhParam;
use flowey_core::node::user_facing::GhPermission;
Expand Down Expand Up @@ -168,10 +169,10 @@ impl flowey_core::node::NodeCtxBackend for InterrogateCtx {
_uses: &str,
_with: BTreeMap<String, ClaimedGhParam>,
_condvar: Option<String>,
_outputs: BTreeMap<String, Vec<(String, bool)>>,
_outputs: BTreeMap<String, Vec<(String, bool, bool)>>,
_permissions: BTreeMap<GhPermission, GhPermissionValue>,
_gh_to_rust: Vec<(String, String, bool)>,
_rust_to_gh: Vec<(String, String, bool)>,
_gh_to_rust: Vec<GhVarState>,
_rust_to_gh: Vec<GhVarState>,
) {
println!("[step][yaml] # {}", label);
self.idx_tracker += 1;
Expand Down
7 changes: 4 additions & 3 deletions flowey/flowey_cli/src/cli/exec_snippet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use crate::cli::FlowBackendCli;
use anyhow::Context;
use flowey_core::github_context::GhVarState;
use flowey_core::node::steps::rust::RustRuntimeServices;
use flowey_core::node::user_facing::ClaimedGhParam;
use flowey_core::node::user_facing::GhPermission;
Expand Down Expand Up @@ -310,10 +311,10 @@ impl flowey_core::node::NodeCtxBackend for ExecSnippetCtx<'_, '_> {
_uses: &str,
_with: BTreeMap<String, ClaimedGhParam>,
_condvar: Option<String>,
_outputs: BTreeMap<String, Vec<(String, bool)>>,
_outputs: BTreeMap<String, Vec<(String, bool, bool)>>,
_permissions: BTreeMap<GhPermission, GhPermissionValue>,
_gh_to_rust: Vec<(String, String, bool)>,
_rust_to_gh: Vec<(String, String, bool)>,
_gh_to_rust: Vec<GhVarState>,
_rust_to_gh: Vec<GhVarState>,
) {
self.idx_tracker += 1;
}
Expand Down
25 changes: 18 additions & 7 deletions flowey/flowey_cli/src/flow_resolver/stage1_dag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

use anyhow::Context;
use flowey_core::github_context::GhVarState;
use flowey_core::node::read_var_internals;
use flowey_core::node::steps::rust::RustRuntimeServices;
use flowey_core::node::user_facing::ClaimedGhParam;
Expand Down Expand Up @@ -599,8 +600,8 @@ pub(crate) enum Step {
>,
},
GitHubYaml {
gh_to_rust: Vec<(String, String, bool)>,
rust_to_gh: Vec<(String, String, bool)>,
gh_to_rust: Vec<GhVarState>,
rust_to_gh: Vec<GhVarState>,
label: String,
step_id: String,
uses: String,
Expand Down Expand Up @@ -779,10 +780,10 @@ impl flowey_core::node::NodeCtxBackend for EmitFlowCtx<'_> {
uses: &str,
with: BTreeMap<String, ClaimedGhParam>,
condvar: Option<String>,
outputs: BTreeMap<String, Vec<(String, bool)>>,
outputs: BTreeMap<String, Vec<(String, bool, bool)>>,
permissions: BTreeMap<GhPermission, GhPermissionValue>,
mut gh_to_rust: Vec<(String, String, bool)>,
mut rust_to_gh: Vec<(String, String, bool)>,
mut gh_to_rust: Vec<GhVarState>,
mut rust_to_gh: Vec<GhVarState>,
) {
let mut fresh_yaml_var = || {
*self.yaml_var_ordinal += 1;
Expand All @@ -806,7 +807,12 @@ impl flowey_core::node::NodeCtxBackend for EmitFlowCtx<'_> {
let (backing_var, is_secret) = read_var_internals(&v);
let backing_var = backing_var.unwrap();
let new_gh_var_name = fresh_yaml_var();
rust_to_gh.push((backing_var.clone(), new_gh_var_name.clone(), is_secret));
rust_to_gh.push(GhVarState {
backing_var: backing_var.clone(),
raw_name: new_gh_var_name.clone(),
is_object: false,
is_secret,
});
(k, format!("${{{{ env.{} }}}}", new_gh_var_name))
}
})
Expand All @@ -815,7 +821,12 @@ impl flowey_core::node::NodeCtxBackend for EmitFlowCtx<'_> {
for (name, output_vars) in outputs {
for output in output_vars {
let gh_context_var_name = format!("steps.{step_id}.outputs.{name}");
gh_to_rust.push((gh_context_var_name, output.0, output.1));
gh_to_rust.push(GhVarState {
backing_var: output.0,
raw_name: gh_context_var_name,
is_secret: output.1,
is_object: output.2,
});
}
}

Expand Down
37 changes: 29 additions & 8 deletions flowey/flowey_cli/src/pipeline_resolver/github_yaml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,20 +836,28 @@ fn resolve_flow_as_github_yaml_steps(
output_steps.push(map.into());
}

for (rust_var, gh_var, is_secret) in rust_to_gh {
for gh_var_state in rust_to_gh {
let mut cmd = String::new();

// flowey considers all GitHub vars to be typed as raw strings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stale comment - not actually true with the introduction of is_object

let set_gh_env_var =
var_db_cmd(&rust_var, is_secret, None, true, Some(gh_var.clone()));
let set_gh_env_var = var_db_cmd(
&gh_var_state.backing_var,
gh_var_state.is_secret,
None,
!gh_var_state.is_object,
Some(gh_var_state.raw_name.clone()),
);
writeln!(cmd, r#"{set_gh_env_var}"#)?;

let mut map = serde_yaml::Mapping::new();
map.insert("run".into(), serde_yaml::Value::String(cmd));
map.insert("shell".into(), "bash".into());
map.insert(
"name".into(),
serde_yaml::Value::String(format!("🌼 Write to '{gh_var}'")),
serde_yaml::Value::String(format!(
"🌼 Write to '{}'",
gh_var_state.raw_name
)),
);

if condvar.is_some() {
Expand Down Expand Up @@ -878,17 +886,30 @@ fn resolve_flow_as_github_yaml_steps(
output_steps.push(step);
}

for (gh_var, rust_var, is_secret) in gh_to_rust {
for gh_var_state in gh_to_rust {
// flowey considers all GitHub vars to be typed as raw strings
let write_rust_var = var_db_cmd(&rust_var, is_secret, Some("{0}"), true, None);
let cmd = format!(r#"${{{{ {gh_var} }}}}"#);
let write_rust_var = var_db_cmd(
&gh_var_state.backing_var,
gh_var_state.is_secret,
Some("{0}"),
!gh_var_state.is_object,
None,
);
let cmd = if gh_var_state.is_object {
format!(r#"${{{{ toJSON({}) }}}}"#, gh_var_state.raw_name)
} else {
format!(r#"${{{{ {} }}}}"#, gh_var_state.raw_name)
};

let mut map = serde_yaml::Mapping::new();
map.insert("run".into(), serde_yaml::Value::String(cmd));
map.insert("shell".into(), write_rust_var.into());
map.insert(
"name".into(),
serde_yaml::Value::String(format!("🌼 Read from '{gh_var}'")),
serde_yaml::Value::String(format!(
"🌼 Read from '{}'",
gh_var_state.raw_name
)),
);
if condvar.is_some() {
map.insert("if".into(), "${{ fromJSON(env.FLOWEY_CONDITION) }}".into());
Expand Down
87 changes: 87 additions & 0 deletions flowey/flowey_core/src/github_context.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

//! Core types and traits used to read GitHub context variables.

use crate::node::{user_facing::GhContextVar, ClaimVar, NodeCtx, ReadVar, StepCtx};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each import on its own line please.

the guide has some suggestions on how to configure rust-analyzer to do this for you

use serde::{de::DeserializeOwned, Deserialize, Serialize};
use std::collections::BTreeMap;

#[derive(Serialize, Deserialize)]
pub struct Head {
#[serde(rename = "ref")]
pub head_ref: String,
}

#[derive(Serialize, Deserialize)]
pub struct GhContextVarReaderEventPullRequest {
pub head: Head,
}

pub enum Root {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you still want to stuff these ZSTs into their own mod state inline module, just to avoid polluting this namespace with generic-sounding things like Root and Event

pub enum Event {}

#[derive(Clone)]
pub struct GhVarState {
pub raw_name: String,
pub backing_var: String,
pub is_secret: bool,
pub is_object: bool,
}

pub struct GhContextVarReader<'a, S> {
pub ctx: NodeCtx<'a>,
pub _state: std::marker::PhantomData<S>,
}

impl<S> GhContextVarReader<'_, S> {
fn read_var<T: Serialize + DeserializeOwned>(
&self,
var_name: String,
is_secret: bool,
is_object: bool,
) -> ReadVar<T> {
let (var, write_var) = self.ctx.new_maybe_secret_var(is_secret, "");
let write_var = write_var.claim(&mut StepCtx {
backend: self.ctx.backend.clone(),
});
let var_state = GhVarState {
raw_name: var_name.clone(),
backing_var: write_var.backing_var,
is_secret: write_var.is_secret,
is_object,
};
let gh_to_rust = vec![var_state];

self.ctx.backend.borrow_mut().on_emit_gh_step(
&format!("🌼 read {}", var_name),
"",
BTreeMap::new(),
None,
BTreeMap::new(),
BTreeMap::new(),
gh_to_rust,
Vec::new(),
);
var
}
}

impl<'a> GhContextVarReader<'a, Root> {
pub fn global(&self, gh_var: GhContextVar) -> ReadVar<String> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that you have this read_var helper, I really think that it's not that hard to swap the current constants with standalone helpers - especially since you've already needed to touch those files as part of this global method refactoring.

the only thing you'll need to tweak is the user-defined secret var handling, but even that should be pretty straightforward, if you simply swap the current type that flow uses (i.e: the GhContextVar type we want to go away), with a far more limited newtype around a String called GhUserSecretVar, which is what gets created at the Pipeline level, and plumbed down to a new pub fn secret() method that hangs off this GhContextVarReader.

self.read_var(gh_var.as_raw_var_name(), gh_var.is_secret(), false)
}

pub fn event(self) -> GhContextVarReader<'a, Event> {
GhContextVarReader {
ctx: self.ctx,
_state: std::marker::PhantomData,
}
}
}

impl GhContextVarReader<'_, Event> {
pub fn pull_request(self) -> ReadVar<Option<GhContextVarReaderEventPullRequest>> {
self.read_var("github.event.pull_request".to_string(), false, true)
}
}
1 change: 1 addition & 0 deletions flowey/flowey_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//! this crate!** The crate you should be using is called `flowey`, which only
//! exports user-facing types / traits.

pub mod github_context;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this mod should be a mod under mod node, not a new top-level mod.

this should also help with some of the visibility issues you might've run into

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call, this fixed pretty much all the visibility stuff I was having issues with

pub mod node;
pub mod patch;
pub mod pipeline;
Loading
Loading