fix(setup): refuse --dev rather than silently installing the release channel
`felis setup --dev` promised "install the dev channel (main HEAD)" and installed release: the flag only exported FELIS_CHANNEL, a variable nothing in the tree reads. deploy/bootstrap.sh reads FELIS_VERSION_BOOTSTRAP. Renaming the variable would have been a worse bug than the dead one, because it would look wired. setup runs bootstrap with FELIS_BOOTSTRAP_FROM_TUI=1, and on that arm every reader of FELIS_VERSION_BOOTSTRAP is unreachable: the channel case and its validation live in resolve_install_ref, which the TUI path skips outright, and use_release_binary is only consulted by the elif that `if bootstrap_from_tui` already short-circuited. setup re-images the host from the felis binary it is itself running; there is no channel to pick. So the flag refuses, exits 2 and names FELIS_VERSION_BOOTSTRAP=dev on the installer, which is the mechanism that does work. Refusing beats defaulting: the operator asked for dev, and release is the one answer they did not want. The refusal precedes the root check, or an unprivileged operator gets told about sudo instead of about the channel. channelName had no other caller and goes with it. Nothing else referenced --dev -- no doc, no script, no test -- so this removes a promise the tree only ever made to itself.
This commit is contained in:
2 files changed
+39
-16
No files matched your search
+13
-16
@@ -26,15 +26,6 @@ const hostBootstrapKubeconfigPath = "/etc/rancher/k3s/k3s.yaml"
|
|||||||
|
|
||||||
var errHostBootstrapCancelled = errors.New("host bootstrap cancelled")
|
var errHostBootstrapCancelled = errors.New("host bootstrap cancelled")
|
||||||
|
|
||||||
// channelName maps the --dev flag to the release channel deploy/bootstrap.sh
|
|
||||||
// understands. Release is the default so a bare `felis setup` is production.
|
|
||||||
func channelName(dev bool) string {
|
|
||||||
if dev {
|
|
||||||
return "dev"
|
|
||||||
}
|
|
||||||
return "release"
|
|
||||||
}
|
|
||||||
|
|
||||||
// cmdSetup is the normal first-run operator console. It is intentionally separate
|
// cmdSetup is the normal first-run operator console. It is intentionally separate
|
||||||
// from breakGlass: setup creates the initial Owner and optional web edge; breakGlass
|
// from breakGlass: setup creates the initial Owner and optional web edge; breakGlass
|
||||||
// is reserved for emergency local recovery/reset.
|
// is reserved for emergency local recovery/reset.
|
||||||
@@ -42,19 +33,25 @@ func cmdSetup(args []string, stdout, stderr io.Writer) int {
|
|||||||
fs := flag.NewFlagSet("setup", flag.ContinueOnError)
|
fs := flag.NewFlagSet("setup", flag.ContinueOnError)
|
||||||
fs.SetOutput(stderr)
|
fs.SetOutput(stderr)
|
||||||
cfgPath := fs.String("config", defaultSetupConfigPath, "path to felis.toml")
|
cfgPath := fs.String("config", defaultSetupConfigPath, "path to felis.toml")
|
||||||
dev := fs.Bool("dev", false, "install the dev channel (felis:dev, main HEAD) instead of the default release channel (felis:release, newest tag)")
|
dev := fs.Bool("dev", false, "rejected: the install channel is chosen by the bootstrap installer, not by setup")
|
||||||
if err := fs.Parse(args); err != nil {
|
if err := fs.Parse(args); err != nil {
|
||||||
if errors.Is(err, flag.ErrHelp) {
|
if errors.Is(err, flag.ErrHelp) {
|
||||||
return 0
|
return 0
|
||||||
}
|
}
|
||||||
return 2
|
return 2
|
||||||
}
|
}
|
||||||
// The channel governs which image tag/source ref the host bootstrap builds.
|
// setup cannot honour a channel, so it refuses rather than silently installing the
|
||||||
// runBootstrap forwards the whole environment, so exporting it here is enough
|
// other one. It used to export FELIS_CHANNEL here, which nothing has ever read --
|
||||||
// to reach deploy/bootstrap.sh without threading a parameter through the TUI.
|
// deploy/bootstrap.sh reads FELIS_VERSION_BOOTSTRAP -- so --dev was a silent no-op
|
||||||
if err := os.Setenv("FELIS_CHANNEL", channelName(*dev)); err != nil {
|
// that installed release. Renaming the variable would not fix it: on this path
|
||||||
fmt.Fprintf(stderr, "felis setup: %v\n", err)
|
// bootstrap takes the bootstrap_from_tui arm, which re-images the host from the
|
||||||
return 1
|
// binary setup is already running, and every reader of FELIS_VERSION_BOOTSTRAP
|
||||||
|
// (use_release_binary, resolve_install_ref) is unreachable from there. Choosing a
|
||||||
|
// channel means re-running the installer, which is what this points the operator at.
|
||||||
|
if *dev {
|
||||||
|
fmt.Fprintln(stderr, "felis setup: --dev is not supported here; setup re-images this host from the felis binary it is already running.")
|
||||||
|
fmt.Fprintln(stderr, "To install a different channel, re-run the bootstrap installer with FELIS_VERSION_BOOTSTRAP=dev (see CONTRIBUTING.md).")
|
||||||
|
return 2
|
||||||
}
|
}
|
||||||
configFlagSet := false
|
configFlagSet := false
|
||||||
fs.Visit(func(f *flag.Flag) {
|
fs.Visit(func(f *flag.Flag) {
|
||||||
|
|||||||
@@ -93,3 +93,29 @@ func TestHostBootstrapReadyRequiresMarkerAndArtifacts(t *testing.T) {
|
|||||||
t.Fatal("bootstrap should be ready when marker and host artifacts exist")
|
t.Fatal("bootstrap should be ready when marker and host artifacts exist")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --dev used to export FELIS_CHANNEL, which nothing reads, so `felis setup --dev`
|
||||||
|
// silently installed the RELEASE channel: the one outcome the operator did not ask
|
||||||
|
// for. Renaming the variable to the one bootstrap does read (FELIS_VERSION_BOOTSTRAP)
|
||||||
|
// would not have helped -- setup takes bootstrap's bootstrap_from_tui arm, where every
|
||||||
|
// reader of it is unreachable -- so the flag refuses instead of guessing. It has to
|
||||||
|
// refuse BEFORE the root check, or the message an unprivileged operator sees is about
|
||||||
|
// sudo rather than about the channel.
|
||||||
|
func TestSetupDevFlagRefusesInsteadOfSilentlyInstallingRelease(t *testing.T) {
|
||||||
|
var stdout, stderr strings.Builder
|
||||||
|
|
||||||
|
if code := cmdSetup([]string{"--dev"}, &stdout, &stderr); code != 2 {
|
||||||
|
t.Fatalf("want exit 2 for an unsupported channel flag, got %d (stderr: %s)", code, stderr.String())
|
||||||
|
}
|
||||||
|
msg := stderr.String()
|
||||||
|
if !strings.Contains(msg, "FELIS_VERSION_BOOTSTRAP=dev") {
|
||||||
|
t.Errorf("the refusal must name the mechanism that actually works:\n%s", msg)
|
||||||
|
}
|
||||||
|
if strings.Contains(msg, "must run as root") {
|
||||||
|
t.Errorf("the channel refusal must precede the root check:\n%s", msg)
|
||||||
|
}
|
||||||
|
// The dead variable is gone; setting it again would re-create a knob nothing reads.
|
||||||
|
if _, ok := os.LookupEnv("FELIS_CHANNEL"); ok {
|
||||||
|
t.Errorf("FELIS_CHANNEL has no reader anywhere and must not be exported")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user