Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/wiki/Plugin-Architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ The same verbs enable and disable a builtin family: `nself remove <builtin>` cre
- **Builtin source**: `builtinFamily{slug, build}` values listed in `cmd/commands/builtin_families.go` (the composition root; the admin family, ADR 0022). A builtin family is mounted unless `<pluginDir>/<slug>/.disabled` exists (D19), and its commands are real cobra commands that keep root's pre-run hooks. `admin` is the only builtin family today (P7-CANON-19). Its help group is "Account & Meta:" in v1.4 and "Plugin Commands:" in v1.5 (`NSELF_V15=1`); a family already mounted is not mounted twice.
- **Mount** (`cmd/commands/plugin_mount.go`): per Spec a cobra subtree is added before the invocation decorator — root `Use: <command>`, one nested node per subcommand segment, every node `DisableFlagParsing: true`, annotations `nself.plugin=<slug>`, `nself.mount.source=installed|builtin`, `nself.side_effect`, `nself.output`, `nself.json`, `nself.args`, `nself.flags`, and (additive, D21) `nself.confirm` (compact JSON, absent when null) and `nself.surface`. GroupID `plugins` ("Plugin Commands:", added to root when at least one mount exists). Installed RunE execs the discovered `Spec.BinaryPath` after a fresh containment check. The unknown-command proxy uses the same check. Both execute the real path directly, preserving `$0` and leaving no temporary links or inherited file descriptor. The directory owner can still replace the file between check and exec; that owner already controls plugin binaries. Argv remains verbatim after `stripRootPersistentFlags` (which keeps `--json`), with no shell, inherited stdout/stderr and the plugin's exit status passed through.
- **Pre-run isolation (D20)**: the old proxy ran the plugin before cobra, so root `PersistentPreRunE` (OTel init, deprecation warning, command log, `--version`, source guard, monorepo chdir and notice, licence migration) never applied. Each installed mount root therefore sets its own no-op `PersistentPreRunE` and `PersistentPostRunE` (cobra runs the nearest), in both modes; the invocation decorator also skips installed bodies, so they take no project operation lock or lock token. A mounted plugin sees the same cwd, stdout, stderr and exit code as before. Builtin families keep root's pre-run.
- **An invocation that starts with a flag keeps today's behaviour**: when `os.Args[1]` is a flag (for example `nself --no-deprecation-warnings claw run x`), the old intercept never proxied — cobra answered with its unknown-command error. Mounting is skipped for those invocations, so their output and exit code stay byte-identical to the base binary in both modes. The mount itself is additive new surface for everything else, ungated in v1.4 per ADR 0021.
- **An invocation that starts with a flag**: mounting is skipped when `os.Args[1]` is a flag (for example `nself --no-deprecation-warnings claw run x`); ordinary plugin commands retain the base binary's behaviour. In v1.5 mode, breakout relocation still runs an installed, enabled plugin through its validated manifest binary, including with leading root flags. `NSELF_NO_AUTO_ADD=1` prevents installing a missing plugin but permits running one already installed. The mount itself is additive new surface for everything else, ungated in v1.4 per ADR 0021.
- **Collisions**: a `command` equal to a core verb, any cobra-native or builtin-family command or alias in the current tree (help, completion, man, version included), a break-out owned by another plugin, or another Spec's command (both lose) is not mounted; each collision prints exactly one stderr warning per process naming both owners (E405, unless `--quiet`/`--no-deprecation-warnings`) and is a `doctor` finding (the `plugin-mount` check lists every Problem). Warnings and doctor rows escape control characters in names and messages. A command that equals a core command scheduled as a break-out to the same slug (the generated breakouts table) is skipped silently in v1.4 mode: core wins, and core already proxies.
- **Registry**: an annotated node gets `canon: plugin` and the additive field `plugin: <slug>`; installed-source attributes come from the annotations with the defaults above, builtin-source attributes from the family's canon fragment entry (which must say `canon: plugin`). The completeness check does not require a canon fragment for installed mounts, and validation still rejects `canon: plugin` on non-annotated (cobra-native) entries. `counts.plugin` is additive; `counts.top_level` and the command inventory exclude `canon: plugin` commands.
- **Hermetic generation**: generators never call the installed mount — the committed registry and inventory never depend on the machine's plugin directory. Builtin families are always mounted, so the committed registry will contain `admin` and no installed plugin.
Expand Down
9 changes: 5 additions & 4 deletions .github/wiki/cmd-deploy.md
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,11 @@ file. v1.4 mode keeps the push described above, byte for byte. One flow runs per
`docker-compose.yml`). A stub `ssl/certificates/<dir>/*.pem` tree mirrors the target's names, so
generated cert paths are the target's. `nself build` runs there with the target env cascade. Your
own project tree is not written to; the stub files are never shipped. The copy leaves out
`nginx/` files that carry the build marker (`# Generated by nself build`): they are stale
`nginx/` files that carry the build marker (`# Generated by nself build`) unless allowlisted: they are stale
generated output and the build writes what it owns. Only the nginx files this build generated,
plus the unmarked files you name in `NSELF_DEPLOY_NGINX_INCLUDE`, are shipped. Every other file
of your `nginx/` tree is still copied (the build may read it) but not shipped, and the deploy
plus the files you name in `NSELF_DEPLOY_NGINX_INCLUDE`, are shipped. Allowlisted marked
sites survive the build's stale-site prune; a freshly regenerated site takes precedence.
Every other file of your `nginx/` tree is still copied (the build may read it) but not shipped, and the deploy
output lists it before anything is sent: `[not shipped] nginx/<path> (not generated by this
build; add it to NSELF_DEPLOY_NGINX_INCLUDE to ship it)`. A marked file of yours that the build
did not write again is listed as `[not shipped] nginx/<path> (stale generated output this build
Expand Down Expand Up @@ -954,7 +955,7 @@ a separate protocol.
| `NSELF_DEPLOY_HEALTH_TIMEOUT` | `30` | Seconds to wait for green container health |
| `NSELF_BLUE_PORT_OFFSET` | `0` | Port offset for blue containers |
| `NSELF_GREEN_PORT_OFFSET` | `100` | Port offset for green containers |
| `NSELF_DEPLOY_NGINX_INCLUDE` | — | v1.5 remote deploy: comma-separated globs relative to `nginx/` (`sites/hand.conf,includes/*.conf`; `path.Match` per path segment) naming unmarked nginx files to ship besides the ones the build generated. Read from the target's env cascade. Absolute entries, `..` and anything under `conf.d` are refused. |
| `NSELF_DEPLOY_NGINX_INCLUDE` | — | v1.5 remote deploy: comma-separated globs relative to `nginx/` (`sites/hand.conf,includes/*.conf`; `path.Match` per path segment) naming nginx files to ship besides the ones the build generated. Marked sites are preserved across the build's prune unless regenerated. Read from the target's env cascade. Absolute entries, `..` and anything under `conf.d` are refused. |
| `NSELF_DEPLOY_ENV` | `production` | Deploy target environment set by `nself deploy` after resolving `--env` / positional argument. Values: `local`, `staging`, `production`. Exposed for subprocesses and plugins. |

A remote environment needs a host; with none the deploy is refused (`E483`). Multi-region orchestration is available via the dedicated `nself region` command (`list`, `add`, `status`, `promote`).
Expand Down
7 changes: 7 additions & 0 deletions cmd/commands/deploy_scratch.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,16 @@ func runScratchBuild(ctx context.Context, workdir, env string, snap deploy.Targe
if err := writeFrontStubSSL(scratch, frontedBy, snap.FrontSSL); err != nil {
return "", fmt.Errorf("deploy: writing the stub ssl tree: %w", err)
}
restoreSites, err := preserveScratchNginxSites(scratch, policy)
if err != nil {
return "", fmt.Errorf("deploy: preserving allowlisted nginx sites: %w", err)
}
if _, err := deployScratchBuildFn(ctx, scratch, true, nil); err != nil {
return "", fmt.Errorf("deploy: scratch build failed, nothing was sent: %w", err)
}
if err := restoreSites(); err != nil {
return "", fmt.Errorf("deploy: restoring allowlisted nginx sites: %w", err)
}
return scratch, nil
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/commands/deploy_scratch_nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ import (
)

// deployNginxIncludeEnv is the operator allowlist: comma-separated globs
// relative to nginx/ for unmarked files that should ship.
// relative to nginx/ for files that should ship, including marked sites that
// the scratch build's stale-site prune would otherwise remove.
const deployNginxIncludeEnv = "NSELF_DEPLOY_NGINX_INCLUDE"

// scratchLeavesGenerated reports whether the scratch copy must leave out the
Expand Down
79 changes: 79 additions & 0 deletions cmd/commands/deploy_scratch_nginx_preserve.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package commands

import (
"os"
"path/filepath"

"github.com/nself-org/cli/internal/build"
"github.com/nself-org/cli/internal/deploy"
"github.com/nself-org/cli/internal/nginxtopo"
)

type preservedNginxSite struct {
path string
data []byte
mode os.FileMode
}

// Purpose: preserve allowlisted marked sites across the build's stale-site prune.
// Inputs: scratch project root and resolved nginx allowlist policy.
// Outputs: a restore function holding snapshots, or a filesystem error.
// Constraints: snapshot only regular scratch files; freshly generated sites win.
// SPORT: D-1147, NSELF_DEPLOY_NGINX_INCLUDE preservation during remote deploy.
func preserveScratchNginxSites(scratch string, policy *deploy.NginxPolicy) (func() error, error) {
var saved []preservedNginxSite
nginxDir, err := nginxtopo.ServedNginxDir(scratch, "") // the scratch project's own tree, as scratchCopyProject lays it
if err != nil {
return nil, err
}
dir := filepath.Join(nginxDir, "sites")
entries, err := os.ReadDir(dir)
if err != nil && !os.IsNotExist(err) {
return nil, err
}
for _, entry := range entries {
if !entry.Type().IsRegular() || !policy.Allows("sites/"+entry.Name()) {
continue
}
p := filepath.Join(dir, entry.Name())
data, err := os.ReadFile(p)
if err != nil {
return nil, err
}
if !build.IsGeneratedNginxConf(data) {
continue
}
info, err := entry.Info()
if err != nil {
return nil, err
}
saved = append(saved, preservedNginxSite{p, data, info.Mode().Perm()})
}
return func() error { return restoreScratchNginxSites(saved) }, nil
}

// Purpose: restore allowlisted sites removed by the scratch build.
// Inputs: pre-build site snapshots, including contents and permissions.
// Outputs: restored missing files, or an error that stops staging.
// Constraints: exclusive creation preserves existing build outputs and symlinks.
// SPORT: D-1147, NSELF_DEPLOY_NGINX_INCLUDE preservation during remote deploy.
func restoreScratchNginxSites(saved []preservedNginxSite) error {
for _, s := range saved {
f, err := os.OpenFile(s.path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, s.mode)
if os.IsExist(err) {
continue
}
if err != nil {
return err
}
_, err = f.Write(s.data)
closeErr := f.Close()
if err != nil {
return err
}
if closeErr != nil {
return closeErr
}
}
return nil
}
33 changes: 33 additions & 0 deletions cmd/commands/deploy_scratch_nginx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,39 @@ func TestScratchAllowlistedMarkedFileIsCopiedAndShipped(t *testing.T) {
}
}

func TestScratchAllowlistedMarkedSiteSurvivesBuildPrune(t *testing.T) {
proj := nginxProject(t, "NSELF_DEPLOY_NGINX_INCLUDE=sites/stale.conf,sites/api.conf\n")
wf(t, filepath.Join(proj, "nginx", "sites", "api.conf"), testBuildMarker+"# old API\n")
wf(t, filepath.Join(proj, "nginx", "sites", "other.conf"), testBuildMarker+"# not allowlisted\n")
ran := 0
stub := scratchBuildStub(t, &ran, nil)
useScratchBuild(t, func(ctx context.Context, root string, remote bool, steps []deployStep) ([]deployStep, error) {
// Reproduce the real build's marked-site prune before it regenerates api.conf.
if err := os.Remove(filepath.Join(root, "nginx", "sites", "stale.conf")); err != nil {
t.Fatal(err)
}
return stub(ctx, root, remote, steps)
})
scratch, err := runScratchBuild(context.Background(), proj, "staging", deploy.TargetSnapshot{})
if err != nil {
t.Fatal(err)
}
p := v15Params{workdir: proj, target: "staging", srv: controlplane.Server{Name: "s1", RemotePath: "/opt/nself"}}
set, err := stageScratch(p, scratch)
if err != nil {
t.Fatal(err)
}
if !contains(set.Files, "nginx/sites/stale.conf") || contains(set.Files, "nginx/sites/other.conf") {
t.Errorf("staged sites = %v", set.Files)
}
for name, want := range map[string]string{"stale.conf": testBuildMarker + "# stale generated output\n", "api.conf": testBuildMarker + "server { listen 80; }\n"} {
got, err := os.ReadFile(filepath.Join(scratch, "nginx", "sites", name))
if err != nil || string(got) != want {
t.Errorf("%s = %q, %v; want %q", name, got, err, want)
}
}
}

func TestScratchReportsSymlinksUnderNginx(t *testing.T) {
proj := nginxProject(t, "NSELF_DEPLOY_NGINX_INCLUDE=sites/link.conf\n")
if err := os.Symlink("hand.conf", filepath.Join(proj, "nginx", "sites", "link.conf")); err != nil {
Expand Down
9 changes: 9 additions & 0 deletions cmd/commands/relocate.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"github.com/nself-org/cli/internal/errs"
"github.com/nself-org/cli/internal/lock"
"github.com/nself-org/cli/internal/plugin"
"github.com/nself-org/cli/internal/plugin/mount"
)

// envNoAutoAdd turns the break-out auto-add off: the command fails with E404.
Expand Down Expand Up @@ -78,6 +79,14 @@ func relocateBreakout(cmdName string, args []string) (handled bool, err error) {
if !ok || mountedBy(walkNames(RootCmd, row.To[:1]), row.Plugin) {
return false, nil
}
// Leading root flags skip mounting, but a validated installed plugin can
// still run without auto-add (including when NSELF_NO_AUTO_ADD=1).
specs, _ := mount.Discover(resolvePluginDir(), canonTable.Verbs)
for _, spec := range specs {
if spec.Slug == row.Plugin && spec.Command == row.To[0] {
return true, plugin.ProxyBinaryAt(spec.BinaryPath, resolvePluginDir(), row.Plugin, pluginArgs)
}
}
old := cmdName
if hasPrefix(append([]string{cmdName}, args...), row.From) {
old = strings.Join(row.From, " ")
Expand Down
64 changes: 64 additions & 0 deletions cmd/commands/relocate_installed_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package commands

import (
"errors"
"os"
"path/filepath"
"strings"
"testing"
)

// TestRelocateInstalledWithLeadingFlags checks that skipped mounting does not
// turn an installed breakout into an automatic install or an E404 refusal.
func TestRelocateInstalledWithLeadingFlags(t *testing.T) {
manifest, err := os.ReadFile(filepath.Join("testdata", "mount", "demo", "plugin.json"))
if err != nil {
t.Fatal(err)
}
manifest = []byte(strings.ReplaceAll(string(manifest), "demo", "ci"))
manifest = []byte(strings.ReplaceAll(string(manifest), "nself-ci", "nself-ci-installed"))
for _, noAutoAdd := range []string{"", "1"} {
for _, command := range []string{"ci", "runner"} {
t.Run(command+"/no-auto-add="+noAutoAdd, func(t *testing.T) {
proof, calls, warn := relocateSetup(t, "", func(string, string) (string, error) {
return "", errors.New("installed plugin must not be installed again")
})
t.Setenv(envNoAutoAdd, noAutoAdd)
dir := t.TempDir()
t.Setenv("NSELF_PLUGIN_DIR", dir)
for _, sub := range []string{"ci", "bin"} {
if err := os.MkdirAll(filepath.Join(dir, sub), 0o755); err != nil {
t.Fatal(err)
}
}
if err := os.WriteFile(filepath.Join(dir, "ci", "plugin.json"), manifest, 0o644); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, "bin", "nself-ci-installed"), []byte(stubScript), 0o755); err != nil {
t.Fatal(err)
}
previousArgs := os.Args
os.Args = []string{"nself", "--json", command, "verify", "x"}
t.Cleanup(func() { os.Args = previousArgs })
mountInstalledPluginsFromDisk(RootCmd)
if mountedBy(walkNames(RootCmd, []string{"ci"}), "ci") {
t.Fatal("leading root flags must skip mounting for this regression")
}
handled, err := relocateInvocation(os.Args[1:])
if !handled || err != nil {
t.Fatalf("handled=%v err=%v", handled, err)
}
want := "verify x --json"
if command == "runner" {
want = "nodes " + want
}
if got := readProof(t, proof)["argv"]; got != want {
t.Fatalf("argv=%q, want %q", got, want)
}
if len(*calls) != 0 || warn.Len() != 0 {
t.Fatalf("installed plugin was added again: calls=%v warning=%q", *calls, warn.String())
}
})
}
}
}
Loading