diff --git a/cmd/apiserver/bind.go b/cmd/apiserver/bind.go new file mode 100644 index 00000000..05c651a5 --- /dev/null +++ b/cmd/apiserver/bind.go @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "errors" + "fmt" + "net" +) + +// errEmptyPlainBindAddress refuses an empty --rest-bind-address. Unlike the +// TLS flag, where empty turns the listener off, an empty address here would +// reach net.Listen and bind every interface on a random port. +var errEmptyPlainBindAddress = errors.New( + "--rest-bind-address is empty, which would bind every interface on a random port; " + + "give a loopback address such as " + defaultPlainBindAddress) + +// defaultPlainBindAddress is where the plain-HTTP listener binds unless a +// flag says otherwise. That listener serves the whole API without a client +// certificate, so it stays on loopback: `kubectl port-forward` reaches it +// through the pod's own network namespace, and no other pod can. +const defaultPlainBindAddress = "127.0.0.1:3370" + +// checkPlainBindAddress refuses an empty or unparsable address for the +// plain listener, and reports whether a valid one is off loopback, where it +// serves the whole API without a certificate to every pod that can reach +// it. A deployment applied from an older manifest still passes `:3370`. +func checkPlainBindAddress(addr string) (bool, error) { + if addr == "" { + return false, errEmptyPlainBindAddress + } + + host, _, err := net.SplitHostPort(addr) + if err != nil { + return false, fmt.Errorf("--rest-bind-address %q: %w", addr, err) + } + + if host == "localhost" { + return false, nil + } + + ip := net.ParseIP(host) + + return ip == nil || !ip.IsLoopback(), nil +} diff --git a/cmd/apiserver/bind_flag_test.go b/cmd/apiserver/bind_flag_test.go new file mode 100644 index 00000000..4627657e --- /dev/null +++ b/cmd/apiserver/bind_flag_test.go @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" +) + +// The constant being loopback proves nothing if the flag is registered with +// a literal instead: `":3370"` back at the flag call binds every interface +// again. So the registration itself has to name defaultPlainBindAddress. +func TestPlainListenerFlagUsesTheLoopbackDefault(t *testing.T) { + t.Parallel() + + file, err := parser.ParseFile(token.NewFileSet(), "main.go", nil, 0) + if err != nil { + t.Fatalf("parse main.go: %v", err) + } + + found := false + + ast.Inspect(file, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || len(call.Args) < 3 { + return true + } + + name, ok := call.Args[1].(*ast.BasicLit) + if !ok || name.Value != `"rest-bind-address"` { + return true + } + + found = true + + if ident, ok := call.Args[2].(*ast.Ident); !ok || ident.Name != "defaultPlainBindAddress" { + t.Errorf("rest-bind-address is registered with a default other than defaultPlainBindAddress") + } + + return true + }) + + if !found { + t.Fatal("no rest-bind-address flag registration found in main.go") + } +} diff --git a/cmd/apiserver/bind_test.go b/cmd/apiserver/bind_test.go new file mode 100644 index 00000000..0b9a89ff --- /dev/null +++ b/cmd/apiserver/bind_test.go @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "net" + "os" + "regexp" + "testing" +) + +// The plain listener has no authentication. A default that binds every +// interface hands the full API to any pod that can reach the pod IP. +func TestPlainListenerDefaultsToLoopback(t *testing.T) { + t.Parallel() + + assertLoopback(t, "default", defaultPlainBindAddress) +} + +// The manifest passes the flag explicitly, so it overrides the default and +// needs the same guard. +func TestStandManifestKeepsPlainListenerOnLoopback(t *testing.T) { + t.Parallel() + + raw, err := os.ReadFile("../../stand/blockstor-apiserver-deploy.yaml") + if err != nil { + t.Fatalf("read manifest: %v", err) + } + + matches := regexp.MustCompile(`--rest-bind-address=(\S+)`).FindAllSubmatch(raw, -1) + if len(matches) == 0 { + t.Fatal("the manifest no longer sets --rest-bind-address; check what binds the plain listener") + } + + for _, m := range matches { + assertLoopback(t, "manifest", string(m[1])) + } +} + +func assertLoopback(t *testing.T, where, addr string) { + t.Helper() + + host, _, err := net.SplitHostPort(addr) + if err != nil { + t.Fatalf("%s: %q is not host:port: %v", where, addr, err) + } + + ip := net.ParseIP(host) + if ip == nil || !ip.IsLoopback() { + t.Fatalf("%s: plain listener binds %q, want a loopback address", where, addr) + } +} + +// An empty address is refused, since net.Listen would bind every interface +// on a random port, and an address off loopback is flagged as exposed. +func TestCheckPlainBindAddress(t *testing.T) { + t.Parallel() + + for addr, want := range map[string]struct{ exposed, fails bool }{ + "": {fails: true}, + "3370": {fails: true}, + "127.0.0.1:3370": {}, + "[::1]:3370": {}, + "localhost:3370": {}, + ":3370": {exposed: true}, + "0.0.0.0:3370": {exposed: true}, + "10.0.0.5:3370": {exposed: true}, + } { + exposed, err := checkPlainBindAddress(addr) + if (err != nil) != want.fails || exposed != want.exposed { + t.Errorf("checkPlainBindAddress(%q) = %v, %v; want exposed=%v fails=%v", addr, exposed, err, want.exposed, want.fails) + } + } +} diff --git a/cmd/apiserver/main.go b/cmd/apiserver/main.go index 7ac2028d..99d876a2 100644 --- a/cmd/apiserver/main.go +++ b/cmd/apiserver/main.go @@ -86,8 +86,9 @@ type apiserverFlags struct { func parseFlags() *apiserverFlags { flags := &apiserverFlags{} - flag.StringVar(&flags.restAddr, "rest-bind-address", ":3370", - "Plain-HTTP debug bind address. NOT exposed by the in-cluster Service — reachable only via `kubectl port-forward` for local debugging.") + flag.StringVar(&flags.restAddr, "rest-bind-address", defaultPlainBindAddress, + "Plain-HTTP debug bind address. It serves the full API without authentication, so the default is loopback: "+ + "`kubectl port-forward` still reaches it, other pods do not.") flag.StringVar(&flags.tlsAddr, "tls-bind-address", ":3371", "Mutual-TLS bind address (upstream LINSTOR HTTPS port). The in-cluster Service exposes ONLY this port. Requires --tls-cert-file/--tls-key-file/--tls-client-ca-file; empty disables the TLS listener.") flag.StringVar(&flags.tlsCertFile, "tls-cert-file", "", @@ -192,6 +193,17 @@ func registerProbesAndREST(mgr manager.Manager, st store.Store, flags *apiserver return errors.Wrap(err, "add readyz check") } + exposed, err := checkPlainBindAddress(flags.restAddr) + if err != nil { + return err + } + + if exposed { + ctrl.Log.WithName("setup").Info("WARNING: the plain-HTTP listener serves the whole API without a "+ + "client certificate and is bound off loopback; any pod that can reach this address can use it", + "address", flags.restAddr) + } + err = mgr.Add(&rest.Server{ Addr: flags.restAddr, TLS: tlsOptions(flags), diff --git a/cmd/controller/bind.go b/cmd/controller/bind.go new file mode 100644 index 00000000..05c651a5 --- /dev/null +++ b/cmd/controller/bind.go @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "errors" + "fmt" + "net" +) + +// errEmptyPlainBindAddress refuses an empty --rest-bind-address. Unlike the +// TLS flag, where empty turns the listener off, an empty address here would +// reach net.Listen and bind every interface on a random port. +var errEmptyPlainBindAddress = errors.New( + "--rest-bind-address is empty, which would bind every interface on a random port; " + + "give a loopback address such as " + defaultPlainBindAddress) + +// defaultPlainBindAddress is where the plain-HTTP listener binds unless a +// flag says otherwise. That listener serves the whole API without a client +// certificate, so it stays on loopback: `kubectl port-forward` reaches it +// through the pod's own network namespace, and no other pod can. +const defaultPlainBindAddress = "127.0.0.1:3370" + +// checkPlainBindAddress refuses an empty or unparsable address for the +// plain listener, and reports whether a valid one is off loopback, where it +// serves the whole API without a certificate to every pod that can reach +// it. A deployment applied from an older manifest still passes `:3370`. +func checkPlainBindAddress(addr string) (bool, error) { + if addr == "" { + return false, errEmptyPlainBindAddress + } + + host, _, err := net.SplitHostPort(addr) + if err != nil { + return false, fmt.Errorf("--rest-bind-address %q: %w", addr, err) + } + + if host == "localhost" { + return false, nil + } + + ip := net.ParseIP(host) + + return ip == nil || !ip.IsLoopback(), nil +} diff --git a/cmd/controller/bind_flag_test.go b/cmd/controller/bind_flag_test.go new file mode 100644 index 00000000..4627657e --- /dev/null +++ b/cmd/controller/bind_flag_test.go @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" +) + +// The constant being loopback proves nothing if the flag is registered with +// a literal instead: `":3370"` back at the flag call binds every interface +// again. So the registration itself has to name defaultPlainBindAddress. +func TestPlainListenerFlagUsesTheLoopbackDefault(t *testing.T) { + t.Parallel() + + file, err := parser.ParseFile(token.NewFileSet(), "main.go", nil, 0) + if err != nil { + t.Fatalf("parse main.go: %v", err) + } + + found := false + + ast.Inspect(file, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || len(call.Args) < 3 { + return true + } + + name, ok := call.Args[1].(*ast.BasicLit) + if !ok || name.Value != `"rest-bind-address"` { + return true + } + + found = true + + if ident, ok := call.Args[2].(*ast.Ident); !ok || ident.Name != "defaultPlainBindAddress" { + t.Errorf("rest-bind-address is registered with a default other than defaultPlainBindAddress") + } + + return true + }) + + if !found { + t.Fatal("no rest-bind-address flag registration found in main.go") + } +} diff --git a/cmd/controller/bind_test.go b/cmd/controller/bind_test.go new file mode 100644 index 00000000..a5909f56 --- /dev/null +++ b/cmd/controller/bind_test.go @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "net" + "testing" +) + +// With --enable-rest-api the controller serves the plain API too, with no +// authentication, so its default has to stay on loopback as well. +func TestPlainListenerDefaultsToLoopback(t *testing.T) { + t.Parallel() + + host, _, err := net.SplitHostPort(defaultPlainBindAddress) + if err != nil { + t.Fatalf("%q is not host:port: %v", defaultPlainBindAddress, err) + } + + if ip := net.ParseIP(host); ip == nil || !ip.IsLoopback() { + t.Fatalf("plain listener defaults to %q, want a loopback address", defaultPlainBindAddress) + } +} + +// An empty address is refused, since net.Listen would bind every interface +// on a random port, and an address off loopback is flagged as exposed. +func TestCheckPlainBindAddress(t *testing.T) { + t.Parallel() + + for addr, want := range map[string]struct{ exposed, fails bool }{ + "": {fails: true}, + "3370": {fails: true}, + "127.0.0.1:3370": {}, + "[::1]:3370": {}, + "localhost:3370": {}, + ":3370": {exposed: true}, + "0.0.0.0:3370": {exposed: true}, + "10.0.0.5:3370": {exposed: true}, + } { + exposed, err := checkPlainBindAddress(addr) + if (err != nil) != want.fails || exposed != want.exposed { + t.Errorf("checkPlainBindAddress(%q) = %v, %v; want exposed=%v fails=%v", addr, exposed, err, want.exposed, want.fails) + } + } +} diff --git a/cmd/controller/main.go b/cmd/controller/main.go index aa8124e4..408bd484 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -72,8 +72,9 @@ func main() { var tlsOpts []func(*tls.Config) flag.BoolVar(&enableRestAPI, "enable-rest-api", false, "Mount the LINSTOR-compatible REST API alongside the reconcilers. Default is OFF — the apiserver runs as a separate Deployment (cmd/apiserver) since Phase 11.x. Set to true only for the legacy single-binary deployment.") - flag.StringVar(&restAddr, "rest-bind-address", ":3370", - "The address the LINSTOR-compatible REST API binds to (upstream LINSTOR plain-text port is 3370). Ignored when --enable-rest-api=false.") + flag.StringVar(&restAddr, "rest-bind-address", defaultPlainBindAddress, + "The address the plain-HTTP LINSTOR-compatible REST API binds to. It has no authentication, so the default is loopback, "+ + "reachable through `kubectl port-forward` only. Ignored when --enable-rest-api=false.") flag.StringVar(&controllerNamespace, "controller-namespace", "", "Namespace where the controller's own Secrets/ConfigMaps live (default: $POD_NAMESPACE, then 'blockstor-system').") flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ @@ -307,6 +308,18 @@ func main() { } if enableRestAPI { + exposed, err := checkPlainBindAddress(restAddr) + if err != nil { + setupLog.Error(err, "Invalid plain-HTTP listener address") + os.Exit(1) + } + + if exposed { + setupLog.Info("WARNING: the plain-HTTP listener serves the whole API without a client "+ + "certificate and is bound off loopback; any pod that can reach this address can use it", + "address", restAddr) + } + ready.ArmREST() if err := mgr.Add(&rest.Server{ diff --git a/docs/layer-stack.md b/docs/layer-stack.md index d1fc9ce6..602331e0 100644 --- a/docs/layer-stack.md +++ b/docs/layer-stack.md @@ -37,7 +37,9 @@ the dispatcher falls through to the satellite's default-DRBD path. ### Per-RD via REST ```bash -curl -XPOST http://controller:3370/v1/resource-definitions \ +# the plain port listens on the pod's loopback; reach it through a port-forward: +# kubectl -n blockstor-system port-forward deploy/blockstor-apiserver 3370:3370 +curl -XPOST http://localhost:3370/v1/resource-definitions \ -H 'Content-Type: application/json' \ -d '{"resource_definition":{"name":"pvc-1","layer_stack":["LUKS","STORAGE"]}}' ``` diff --git a/docs/usage.md b/docs/usage.md index 37ab083a..7d4d7b7b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -64,7 +64,7 @@ kubectl -n blockstor-system rollout status daemonset/blockstor-satellite ## Point the `linstor` client at the apiserver -Install the client (`apt install linstor-client`, or `pip install python-linstor`). The apiserver Service exposes a mutual-TLS listener on port **3371**; a plain-HTTP debug port **3370** is bound pod-local only. The simplest way to reach it from a workstation is a port-forward to the plain-HTTP port: +Install the client (`apt install linstor-client`, or `pip install python-linstor`). The apiserver Service exposes a mutual-TLS listener on port **3371**; a plain-HTTP debug port **3370** is bound to the pod's loopback, with no authentication, so only a port-forward reaches it. The simplest way to reach it from a workstation is a port-forward to the plain-HTTP port: ```sh kubectl -n blockstor-system port-forward deploy/blockstor-apiserver 3370:3370 diff --git a/pkg/rest/server.go b/pkg/rest/server.go index ead74011..1e01c075 100644 --- a/pkg/rest/server.go +++ b/pkg/rest/server.go @@ -74,7 +74,7 @@ const maxRequestBodyBytes int64 = 1 << 20 // reach the apiserver directly. Both may be nil/empty in tests that // only exercise the legacy KV-backed path. type Server struct { - Addr string // e.g. ":3370" — plain-HTTP debug port (NOT exposed by the in-cluster Service; reachable only via kubectl port-forward) + Addr string // e.g. "127.0.0.1:3370" — plain-HTTP debug port with no authentication; keep it on loopback so only kubectl port-forward reaches it Store store.Store Client client.Client // controller-runtime client for native-object endpoints Namespace string // namespace where blockstor's own Secrets/ConfigMaps live diff --git a/stand/blockstor-apiserver-deploy.yaml b/stand/blockstor-apiserver-deploy.yaml index ba08e266..f21dc51a 100644 --- a/stand/blockstor-apiserver-deploy.yaml +++ b/stand/blockstor-apiserver-deploy.yaml @@ -109,10 +109,10 @@ spec: image: __REGISTRY__/blockstor-apiserver:dev imagePullPolicy: Always args: - # Plain-HTTP debug listener. NOT exposed by the Service — - # reachable only via `kubectl port-forward` to this pod for - # local debugging / the `linstor` CLI during development. - - --rest-bind-address=:3370 + # Plain-HTTP debug listener with no authentication. Bound to + # loopback, so only `kubectl port-forward` to this pod reaches + # it (local debugging, the `linstor` CLI); other pods cannot. + - --rest-bind-address=127.0.0.1:3370 # Mutual-TLS listener. The ONLY port the Service exposes. # Requires + verifies client certs against the mounted CA; # hot-reloads cert/key/CA on cert-manager rotation without a @@ -129,8 +129,6 @@ spec: ports: # rest-tls is the in-cluster endpoint (Service-exposed). - {name: rest-tls, containerPort: 3371} - # rest-debug stays pod-local (port-forward only). - - {name: rest-debug, containerPort: 3370} - {name: health, containerPort: 8081} volumeMounts: # cert-manager-issued serving material (tls.crt/tls.key/ diff --git a/tests/e2e/tiebreaker-r-d-cleanup.sh b/tests/e2e/tiebreaker-r-d-cleanup.sh index aa817c2d..e650a287 100755 --- a/tests/e2e/tiebreaker-r-d-cleanup.sh +++ b/tests/e2e/tiebreaker-r-d-cleanup.sh @@ -64,7 +64,7 @@ N3=$WORKER_3 # so 60s is comfortable on the QEMU stand. CONVERGE=${CONVERGE:-60} -# blockstor's REST/apiserver is exposed inside the cluster on :3370 +# blockstor's plain REST port :3370 listens on the pod's loopback only # (LinstorPlain). Use port-forward + a host-side `linstor` CLI so the # scenario can drive the same REST endpoints the user reaches by # `kubectl exec`-ing into the satellite — no satellite-pod coupling.