starters: generate and publish local + aws Nebi workspaces - #594
Conversation
edb692d to
1b2694c
Compare
Add a Nebi/Pixi starter workspace for the local (kind) provider under starters/local/, plus a starters CI workflow. The workspace ships a placeholder config.yaml (project_name: CHANGEME), a pixi.toml pinning the local toolchain, a lock file, and a README walking the import -> edit -> validate -> deploy flow. Local drives kind via an embedded Go library, so unlike the cloud starters it pins no OpenTofu; the default env has no required deps and nic runs from PATH until the prefix.dev github-releases channel is live (#579). The starters workflow validates on every PR (unedited config must be rejected, a filled copy must pass) and publishes the bundle to quay.io only on workflow_dispatch or push to main, so PRs never push. Registry namespace defaults to nebari_environments pending confirmation on #560. An .gitignore negation keeps starters/**/config.yaml visible despite the config*.yaml ignore rule. Part of #560
Starters are published as OCI bundles to a registry, so the rendered workspaces do not belong in the tree. Replace the committed starters/local/ files with a generator and templates: examples/ stays the single source of truth for config content, and there is nothing to drift. scripts/gen-starters.sh renders examples/<provider>-config.yaml into a workspace by replacing the identity-bearing values with the CHANGEME sentinel and filling the pixi.toml and README templates. Line edits keep the examples' inline comments intact. Scope is local and aws for now. The local provider drives kind through a Go library so it needs no OpenTofu; aws pins it, which is the point of a pinned toolchain. CI generates the starters, asserts each is rejected while unedited and passes once filled, and on dispatch publishes both to quay and re-imports them to confirm the placeholders survive the round trip. Part of #560.
The validate job asserted that nic rejects an unedited starter, which is only true once the CHANGEME check (#561) is on main, so the job sat permanently red. Derive that expectation instead: the filled config is always required to validate, and since the only difference between the two is the placeholders, a passing validate on the unedited one means the feature is absent rather than the starter being wrong. Report that and carry on, and start enforcing automatically once #561 merges. Also fixes a silent generator bug. #439 moved the GitOps URL from git_repository.url to repository.existing.url, so the substitution rule stopped matching and the aws starter shipped a real repository URL. The only guard was 'some CHANGEME exists', which the other three fields satisfied. Every field must now match or generation fails loudly. The placeholder fields are declared once and both the substitution and the fill-for-CI script are derived from them, so CI no longer keeps its own copy of the field list to drift out of sync. Part of #560.
The starter READMEs have no version-varying content, so generating them bought nothing: two committed files replace a template, three of the five substitution tokens, an awk render, and two case statements. The generator copies them verbatim. This also removes the awk gsub hazard where an ampersand in the prose would silently rewrite itself, and the stale line telling readers to edit a template that no longer exists. Three bugs found while reviewing: The NIC_VERSION fallback message was unreachable. Under set -e the failed command substitution aborts the script with exit 128 before the :? guard runs, so a clone without tags failed silently. sed -i without a backup suffix is GNU-only, so every run failed for contributors on macOS. Write to a temporary file and move it into place. repository.existing.path embedded the project name (clusters/my-nebari-aws) with no placeholder rule, so the aws starter shipped CHANGEME for project_name next to a hardcoded one. It is now a placeholder like the rest. Adding a provider to PROVIDERS without a placeholder_fields arm used to produce a starter with every real value intact; generation now fails when a provider declares no fields. Part of #560.
Publishing ran on every main push and tagged with git describe, which resolves to the PREVIOUS tag, so any post-release edit to an example silently republished an already-released bundle with different bytes. A version tag has to keep meaning the same thing, so publishing is now driven by the tag event and takes its version from GITHUB_REF_NAME. The publish job holds registry credentials and had none of the protections every other credentialed job here uses. It now runs under an environment gate, declares a concurrency group that is never cancelled mid-push so a tag cannot end up with one starter published and the other not, and takes its secrets at step level rather than leaving them in scope for every step. The pixi installer is pinned to a commit and an exact version, matching the k3d pin in the deployment tests, and nebi is pinned rather than floating. The credentials check that skipped publishing when QUAY_TOKEN was absent made a rotated or renamed secret look like a passing build; the job now fails if the environment does not supply them. Dropped the conditional that reported PENDING when nic could not yet reject placeholders. It described a temporary state as a permanent branch, and the check it stood in for is the whole point of the job. This is red until the CHANGEME rejection lands and green immediately after. Added the two checks that were missing: no unsubstituted template tokens reach the registry, and every generated pixi.toml parses before publish. The round trip now also re-validates the imported config rather than only grepping it. Part of #560.
ed9305f to
b868442
Compare
…ssertions Addresses the review findings on this branch. Publishing is now tag-only. workflow_dispatch could target any ref: from a branch it published starter-*:vmain and pinned a version read from the PREVIOUS tag, and from an existing tag it overwrote a released bundle - the rewrite the trigger comment says must never happen. Deployment-branch rules live in repo settings and cannot be reviewed from the workflow file, so the guard belongs in the `if`. A manual re-publish is a re-run of the tag's run. Wires up the real credentials: QUAY_OCI_STARTERS_USERNAME is a variable, so it reads from vars, and QUAY_OCI_STARTERS_TOKEN from secrets. The OpenTofu floor was wrong in a way that defeats its own purpose. `>=1.11,<2` admits 1.11.0-1.11.2, which pkg/tofu.MinVersion (1.11.3) rejects - and rejection is a silent fallback to downloading tofu, not an error, so a workspace that re-solves against the lower bound phones home despite pinning its toolchain. Floor raised to 1.11.3. The anti-drift guard only proved a prefix matched SOMETHING. It now requires exactly one match: zero means a key was renamed and a real value would ship untouched, more than one means a same-named key appeared at the same indent and blanking both would bury a real value under a plausible-looking placeholder. Failures accumulate and report together, per the report-and-block convention - a restructure of examples/ usually moves more than one key. Placeholder edits now keep the trailing comment on the line they replace, so `path: CHANGEME # Optional subdirectory within the repository` survives. The comments were being stripped from precisely the five lines the reader has to edit, while the header comment claimed they survived. Replaces the .fill-for-ci.sed mechanism with assertions that are not derived from the generator's own field list. Filling the placeholders back in and checking the result validates mostly re-proved pkg/nic/examples_test.go plus "sed round-trips", and both halves came from the same list, so a prefix that matched the wrong line passed. CI now asserts the error comes from the placeholder gate (`placeholder value`, which only appears after a successful unmarshal) and names exactly the expected field paths, which nic derives from the parsed config. Note that grepping the message for CHANGEME alone cannot carry this: go-yaml quotes the offending source line, so a parse error on a CHANGEME line mentions it too. Also: `make starters`, README.local.md no longer documents OpenTofu for the one provider that has none, README.aws.md lists the fifth placeholder field it was missing, ::error:: annotations instead of bare FAIL echoes, an explicit channel for the nebi install in the credentialed job (with a floor-plus-ceiling so patch releases resolve), and a note that dist/ is also GoReleaser's output dir.
marcelovilla
left a comment
There was a problem hiding this comment.
Thanks for this PR @viniciusdc!
I left some inline comments and I'm leaving a general one here:
I think we could benefit from a small go cmd—similar to what you implemented in cmd/docgen and the placeholder validation—to render the starter config files and the companion pixi.toml file. Using a bash script for this feels fragile. For example, the yaml keys are hardcoded, including their indentation:
placeholder_fields() {
case "$1" in
local)
printf '%s\n' \
'project_name: '
;;
aws)
printf '%s\n' \
'project_name: ' \
'domain: ' \
' email: ' \
' url: ' \
' path: '
;;
esac
}While the CI assertions you wrote would catch this, it would only do so in a CI environment. Having this as a small go cmd would make it straightforward to add unit tests to catch this before.
Initially I thought one argument against doing this in go would be one of your comments on the bash script:
# Applied as line edits rather than a YAML round trip, so the inline comments
# that make the examples useful survive into the starter - including the comment
# on a replaced line, which is preserved and reattached (those five lines are
# exactly the ones whose hint the reader needs most).
However, I tested it against examples/aws-config.yaml using goccy/go-yaml, and both comments and blank lines are preserved. when resolving each field as a path (using yaml.PathString), reading the token line and column, and then injecting a placeholder there.
Not necessarily a blocker and could be a follow-up issue if there's any urgency to get this merged soon. Just wanted to raise it here because I think it makes for a more robust approach.
| name = "nebari-__PROVIDER__" | ||
| channels = ["conda-forge", "https://prefix.dev/github-releases"] | ||
| platforms = ["linux-64", "linux-aarch64", "osx-arm64", "osx-64"] | ||
| version = "0.1.0" |
There was a problem hiding this comment.
Should this version be static? I'm not super familiar with Pixi workspaces so not sure if having the NIC release here would buy us anything. I know it's already specified below in nebari-infrastructure-core and that nebi diff will still render other differences in the TOML file.
There was a problem hiding this comment.
Static is deliberate, and it matches the snippet in #560 (which also has version = "0.1.0"). This is the workspace version - the starter as a thing - not the tool it pins; nic version lives in [dependencies] below and the bundle tag carries it too. Setting it to the nic release would give us three copies of the same string with no consumer reading this one. Leaving as is unless you feel strongly.
…nale nic outputs landed (#606) and belongs beside the other entry points in the workspace's task list. The comment on [feature.tools] named only linux-aarch64 and described kubernetes-client as lagging there. Checked against conda-forge with pixi lock: there are no kubernetes-client candidates for linux-aarch64 OR osx-64, so the narrower platform list is right but was under-explaining itself - it drops two platforms and the comment accounted for one. linux-64 and osx-arm64 both solve (k9s 0.51.0, kubernetes-client 1.34.3).
|
Thanks @marcelovilla - agreed on the Go cmd, and thanks for actually testing the comment-preservation bit rather than taking the script comment at face value. That was the one argument I had for staying in shell and you removed it. Two things that make it more clear-cut than it looks:
And it deletes machinery rather than porting it. Three things in the current script only exist because it is doing text surgery: the hardcoded prefixes-with-indentation you quoted, the "each prefix must match exactly one line" guard (with a path, it either resolves or it does not - there is no wrong-line case to defend against), and the sed dance that reattaches a trailing comment to a replaced line. All three go away. On sequencing: I would rather do it as its own PR than grow this one, and it is not urgent for merge since publishing is blocked on #579 anyway. But I do want it in before the first real tag - nothing is published yet, so there is no bundle compatibility to preserve, and that is the cheapest this change will ever be. It also folds into the ADR that is still owed here, which can then just record that generation is Go and consistent with docgen instead of having to justify a shell exception. Starting on it now. The four inline comments are answered, and two of them are fixed in aefc233. |
|
Opened #615 with the Go generator, stacked on this branch so the diff there is just the swap. Output is byte-identical - I diffed both generators' full trees before deleting the script - so this is a fragility change, not a behaviour one. The three things that go away are the ones that only existed to do text surgery: the exactly-one-match guard, the sed that reattached a trailing comment, and the hardcoded indentation. Comments and blank lines still survive, via the token line/column approach you described. One thing worth reporting back: block scalars need rejecting explicitly. Their token is the |
marcelovilla
left a comment
There was a problem hiding this comment.
Thanks for addressing all the comments @viniciusdc! Approving now
Part of #560.
What
Adds the machinery to build and publish Nebi starter workspaces, scoped to the
localandawsproviders.Starters are distributed as OCI bundles, so the rendered workspaces are deliberately not committed here. Only their source lives in the tree:
scripts/gen-starters.shrendersexamples/<provider>-config.yamlinto a workspace, replacing the identity-bearing values with theCHANGEMEsentinel and filling inpixi.toml. It uses line edits rather than a YAML round trip, so the examples' inline comments survive intact.starters/templates/holds thepixi.tomltemplate and the per-provider READMEs, which are copied verbatim.dist/, which is gitignored.examples/stays the single source of truth for config content, so there is no second copy to drift.Per-provider differences
localplaceholders onlyproject_name: kind runs everything locally, the certificate is self-signed and the GitOps repo is created automatically. It gets noopentofudependency, since that provider drives kind through a Go library.awsplaceholdersproject_name,domain, the ACME email and the GitOps repository URL and path, and pinsopentofuat>=1.11.3,<2. Pinning it is the point of a pinned toolchain: without itnicfalls back to downloading an unpinned OpenTofu at deploy time. The floor has to clearpkg/tofu.MinVersion(1.11.3) - below thatnicrejects the PATH binary and downloads one anyway, silently, which defeats the pin. Everything else (region, availability zones, instance types, Longhorn, EFS) stays a working default.CI
validate-startersgenerates both starters and checks that no template token went unsubstituted, that everypixi.tomlparses, and that an unedited starter is rejected by the placeholder gate naming exactly the expected fields (see Validation below).publish-startersis tag-driven, and deliberately not dispatchable. A publishedv<x.y.z>bundle has to keep meaning the same bytes; amainpush resolves to the previous tag, and aworkflow_dispatchcould target any ref - from a branch it would publishstarter-*:vmain, from an existing tag it would overwrite a released bundle. Deployment-branch rules live in repo settings and can't be reviewed from the workflow file, so the guard is theif. A manual re-publish is a re-run of the tag's own run. The job runs under anenvironmentgate with a concurrency group that is never cancelled mid-push, resolvespixi lock, publishes each workspace withnebi publish --local, then re-imports them and re-validates to confirm the placeholders survived the round trip.Starters publish under the existing
nebariproject with astarters/subdir, mirroring how the charts live atquay.io/nebari/charts.Scope
Only
localandaws. The other providers keep theirexamples/entries and can be added to the generator's provider list when we want them; the generator fails loudly if a provider is added without declaring its placeholder fields.The
$schemamodeline (#562) is not included; it needs the generated schema published first, and that work is itself still in flight (#507 was closed unmerged, superseded by #600).Not covered here, and each a natural follow-up rather than a gap this PR hides: the end-to-end
nebi import->pixi install->nebi run deploykind test in CI, and the docs-site page for the import-edit-deploy flow. Both are #560 DoD items. #560 also still reads "one per provider" across all six - the local+aws cut wants ratifying on the issue rather than living implicitly in this PR.nebi >= 0.10is required to publish or import these, since asset-layer bundling is what carriesconfig.yamland the README alongsidepixi.tomlandpixi.lock. Older versions silently ship only the two lock files.This does not conflict with ADR-0005's "docgen is the authoritative config-reference generator". That rule scopes to reference material read from the Go config structs; this reads
examples/*.yamlas text and never touches Go types, so no second struct-reader is introduced.Dependencies
The
CHANGEMErejection has landed (#583, closing #561), sovalidate-startersis green: both starters are rejected unedited, the rejection comes from the placeholder gate, and it names exactly the expected field paths.Publishing still needs #579.
pixi lockcannot resolvenebari-infrastructure-corefrom the prefix.dev channel until the package is published there, so publish is blocked rather than merely degraded - and because the publish job is tag-only it has never executed. The firstv*tag is where that path gets exercised for real.Validation
The starter is asserted through
nicrather than through the generator's own field list, which is what keeps the check honest:placeholder value, emitted only after a successful unmarshal - so this doubles as proof the line-edit surgery left loadable YAML)nicderives from the parsed configThat second assertion is deliberately a hand-written list in CI. An earlier revision filled the placeholders back in using a sed script the generator emitted from the same list it used to insert them; that could not catch a prefix matching the wrong line, and the "filled config validates" half mostly re-proved
pkg/nic/examples_test.go.The generator requires each declared prefix to match exactly one line - zero means a key was renamed and a real value would ship untouched, more than one means a same-named key appeared at the same indent and blanking both would bury a real value under a plausible-looking placeholder. Failures accumulate and report together.
Placeholder edits keep the trailing comment on the line they replace, so
path: CHANGEME # Optional subdirectory within the repositoryreaches the reader with its hint intact.[tool.nebi.bundle] includeis a strict allowlist (config.yaml,README.mdalongside the always-bundledpixi.toml/pixi.lock), which is a stronger answer than theexcludelist #560 asked for: a filled config, a kubeconfig or tfstate a user creates later cannot enter a republished bundle by default.Test plan
./scripts/gen-starters.sh dist/startersrenders both providers; the aws config keeps its inline comments__TOKEN__in the rendered output; bothpixi.tomlfiles parseproject_namefor local;certificate.acme.email,domain,project_name,repository.existing.path,repository.existing.urlfor aws)path:key intoexamples/aws-config.yamlmakes the generator fail withmatched 2 lines ... want exactly 1and exit 1make startersrenders both providers; the awspath:placeholder keeps its inline commentactionlintclean;./scripts/check-action-pins.shpasses