diff --git a/.github/workflows/vermeer-ci.yml b/.github/workflows/vermeer-ci.yml index bc93ca4cc..11ba4a4c9 100644 --- a/.github/workflows/vermeer-ci.yml +++ b/.github/workflows/vermeer-ci.yml @@ -74,10 +74,11 @@ jobs: - name: Test deployment credentials and graph initialization failures run: | + go test -race ./apps/auth ./cmd/vermeer-token go test -race ./apps/common - go test -race ./apps/master/workers ./apps/master/services -run 'TestWorkerAvailability' + go test -race ./apps/master/workers ./apps/master/services go test -race ./apps/master/bl -run 'Test(WorkerAvailability|HugegraphLoadAuthorization)' - go test -race ./apps/worker -run 'Test(LoadReceivers|RecvVertex|ComputeDispatchCancellation)' + go test -race ./apps/worker go test -race ./apps/structure -run 'Test(VertexStore|GraphPropagates|MemoryVertex|HugegraphSchema)' - name: Build @@ -116,3 +117,6 @@ jobs: docker run --rm --entrypoint test vermeer:ci-check -f /go/bin/release-docs/LICENSE docker run --rm --entrypoint test vermeer:ci-check -f /go/bin/release-docs/NOTICE docker run --rm --entrypoint test vermeer:ci-check -d /go/bin/release-docs/licenses + + - name: Verify runtime credentials and native UI login + run: bash scripts/test-runtime-image.sh vermeer:ci-check diff --git a/vermeer/Dockerfile b/vermeer/Dockerfile index daa70b9f7..1236eb1af 100644 --- a/vermeer/Dockerfile +++ b/vermeer/Dockerfile @@ -34,11 +34,13 @@ ARG TARGETOS ARG TARGETARCH RUN --mount=type=cache,target=/go/pkg/mod \ --mount=type=cache,target=/root/.cache/go-build \ - GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/app + GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/app && \ + GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/vermeer-token ./cmd/vermeer-token FROM alpine EXPOSE 8080 -COPY --from=builder /go/bin/app /go/bin/app +COPY --from=builder --chmod=0755 /go/bin/app /go/bin/app +COPY --from=builder --chmod=0755 /go/bin/vermeer-token /go/bin/vermeer-token COPY --from=builder /src/config/ /go/bin/config/ COPY --from=builder /src/release-docs/ /go/bin/release-docs/ COPY --from=builder /usr/local/go/lib/time/zoneinfo.zip / diff --git a/vermeer/README.md b/vermeer/README.md index b1990d39f..a2abb2754 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -108,12 +108,15 @@ docker pull hugegraph/vermeer:latest ``` Create a dedicated config directory (e.g., `~/vermeer-config/`) with `master.ini` and `worker.ini` files (see [Configuration](#configuration) section). +Create its `token-factor` file using [Generating Auth Token](#generating-auth-token) before starting the authenticated Master. Run with Docker: ```bash # Master node (with Web UI enabled) -docker run -v ~/vermeer-config:/go/bin/config hugegraph/vermeer --env=master -auth token -auth_token_factor 1234 +docker run -v ~/vermeer-config:/go/bin/config \ + -e VERMEER_AUTH_TOKEN_FACTOR_FILE=/go/bin/config/token-factor \ + hugegraph/vermeer --env=master -auth token # Worker node docker run -v ~/vermeer-config:/go/bin/config hugegraph/vermeer --env=worker @@ -246,28 +249,92 @@ Deployment credentials require the trusted PD configuration described below. ### Generating Auth Token -When `auth=token` is enabled, you need to generate a token for login. Use the built-in token generator: +When `auth=token` is enabled, protect the signing factor in a file instead of placing it in process arguments. +Create the file before starting Master, and keep it across restarts: + +```bash +umask 077 +openssl rand -hex 32 > config/token-factor +``` + +Set `VERMEER_AUTH_TOKEN_FACTOR_FILE=/path/to/token-factor`, or set `auth_token_factor_file` in Master's INI file. +The environment file takes precedence over the INI file, and either file takes precedence over `auth_token_factor`. +The file must contain one valid UTF-8 line with at least 16 characters and cannot contain only whitespace. +Use a randomly generated value; the example above provides 32 random bytes encoded as 64 hexadecimal characters. +A terminal LF or CRLF is accepted; other spaces are preserved. An invalid configured file stops Master before it opens its listeners; +it never falls back to an inline or default factor. + +File-backed factors use **v2 HMAC-SHA256** tokens. Their digest carries a `v2:` version marker inside the existing Base58-encoded token; +the signed payload preserves the boundaries of user, space, nickname, client and timestamp fields. +Master in file mode accepts only v2 signatures and compares MACs in constant time. It never falls back to legacy SHA1 verification. +The existing `auth_token_factor` string configuration and source-only legacy token tool retain their historical protocol for older clients. +Those legacy tokens and v2 tokens cannot authenticate across modes. + +Images built from this revision include the native `/go/bin/vermeer-token` helper. Generate a protected login/service token file: ```bash # Run with Go -go run tools/token/token_generator.go -user admin -space default -factor 1234 -i +go run ./cmd/vermeer-token -user admin -space '$DEFAULT' \ + -factor-file config/token-factor -output-file config/admin-token -i -quiet # Or run in Docker (no local Go required) -docker run --rm -v $(pwd):/src -v $(pwd)/config:/src/config golang:1.23-alpine sh -c "cd /src && go run tools/token/token_generator.go -user admin -space default -factor 1234 -i" +docker run --rm --user "$(id -u):$(id -g)" -v "$PWD/config:/tokens" \ + --entrypoint /go/bin/vermeer-token \ + hugegraph/vermeer -user admin -space '$DEFAULT' \ + -factor-file /tokens/token-factor -output-file /tokens/admin-token -i -quiet ``` Parameters: - `-user` — Username (default: `foo`) - `-space` — Graph space name (default: `bar`) -- `-factor` — Must match `auth_token_factor` in `master.ini` +- `-factor-file` — Required. Read the same signing factor file used by Master; inline factors are not accepted. +- `-output-file` — Atomically write the Base58 token with mode `0600`, with no token or factor printed. +- `-quiet` — When no output file is set, print only the token for an intentional interactive retrieval. - `-client` — Client identifier (default: `hg`) - `-i` — Generate immortal token (never expires) +The runtime helper supports only the secure file-backed protocol and rejects `-factor`. +The source-only `tools/token/token_generator.go` utility retains its original `-factor` interface and diagnostic output for legacy +inline-configured Masters. It is not distributed in the runtime image and must not be used for file-backed deployments. + +Hubble accepts this plain token file through `vermeer.token_file`; it must not include a `Bearer` prefix or JSON. +Normal tokens expire after 24 hours. Long-lived service tokens must be protected like passwords. +Rotating the signing factor invalidates existing tokens; regenerate dependent service tokens and restart their clients together. +Token generation reuses Vermeer's native signing and encoding implementation and does not require a running Master or Kubernetes API access. +When upgrading an earlier file-based deployment to v2, retain its protected factor file but upgrade Master and the token helper together, +regenerate service token files, and restart dependent clients. Existing native UI cookies contain old signatures and require a new login token. +Hubble treats tokens as opaque text, so no Hubble API or token-file format change is required. + The generated token can be used in two ways: 1. **HTTP Header**: `Authorization: Bearer ` 2. **Login API**: `POST /api/v1/login` with body `{"token": ""}` (sets cookie) +### Native UI administrator password + +For a short administrator login, optionally configure Master with `VERMEER_ADMIN_PASSWORD_FILE=/path/to/admin-password`, +or `admin_password_file` in its INI file. The environment path takes precedence. This requires `auth=token` and a valid +file-backed v2 signing factor; legacy inline signing and `auth=none` reject the password configuration before listening. +An unconfigured password file preserves token-only login. + +Provide a protected `0600` password file, preferably a separately generated random 16-character alphanumeric credential. +Helm deployments generate and retain this credential separately from the internal signing factor and Hubble service token. +The file must be nonempty UTF-8 on one line, without CR/LF, and cannot contain only whitespace. All other bytes are preserved, +including leading or trailing spaces. A configured missing, unreadable or invalid file stops Master; it never disables authentication. + +Open `/` and paste this credential into **Admin password or token**. No user database or extra account is created: +successful password login issues an `admin` session in `$DEFAULT`, signed with v2 HMAC-SHA256 and valid for 24 hours. +Its cookie is HttpOnly and uses Secure for HTTPS, including requests forwarded with `X-Forwarded-Proto: https`. +The existing login request field remains `token`; it accepts either the short password or a valid opaque token. +Incorrect credentials return 401 without issuing a cookie. The short password is never accepted as an API Bearer token. +Hubble continues to use its independently generated long-lived service token without requiring user input. + +Password changes take effect after restarting Master. They change subsequent UI logins without rotating the signing factor +or Hubble token. Already issued UI sessions remain valid for their remaining 24-hour lifetime; rotate the signing factor +and regenerate dependent tokens when immediate session revocation is required. +With token authentication, Worker/Master metadata routes also require a valid session or Bearer token; +health probes, login and static login assets remain public. Explicit `auth=none` retains its existing behavior. + ### Worker Configuration (`worker.ini`) For a PD deployment using a shared secret, configure both Master and Worker with @@ -279,7 +346,7 @@ is sent as the internal `vermeer` credential and is never added to task paramete When no file is configured, the legacy credential remains available for older PD deployments; it does not authenticate to a modern PD with a different shared secret. -When either deployment credential file is configured, configure both Master and +When any PD or HugeGraph deployment credentials are configured, configure both Master and Worker with `pd_peers=["pd.internal:8686"]`, or set `VERMEER_PD_PEERS` to that JSON array (environment takes precedence). These deployment addresses replace task PD addresses for load and output discovery. Missing or invalid deployment peers @@ -316,8 +383,17 @@ direct adapter never forwards the user's login password. Worker Schema discovery and reads use these deployment credentials without adding them to persisted task parameters. A configured missing, unreadable, empty or invalid file fails the load; it never falls back to task credentials. Without a file, the existing -`load.hugegraph_username` / `load.hugegraph_password` behavior is retained. This -setting does not configure the separate HugeGraph result writer. +`load.hugegraph_username` / `load.hugegraph_password` behavior is retained unless +the following password-file form is configured. These settings do not configure the separate HugeGraph result writer. + +To reuse a Kubernetes Secret containing a raw Server password without converting it to JSON, set +`VERMEER_HUGEGRAPH_USERNAME` and `VERMEER_HUGEGRAPH_PASSWORD_FILE` together, or configure +`hugegraph_username` and `hugegraph_password_file` in Worker's INI file. Each environment value overrides its corresponding INI value. +The explicit JSON credentials file above takes precedence over this pair. If either member of the pair is configured, both are required; +a missing, unreadable or empty password file fails the load without using task credentials. +Password-file contents are read exactly, including quotes, backslashes, spaces and terminal newlines; mount the original Secret value +and avoid adding a newline with `echo`. These credentials are only used for Schema discovery/reads through the configured trusted PD peers +and do not enter persisted task parameters. ```ini [default] @@ -337,6 +413,10 @@ run_mode = worker worker_group = default ``` +For advanced connection troubleshooting, `grpc_dial_timeout` or `VERMEER_GRPC_DIAL_TIMEOUT` bounds Worker peer dialing. +It defaults to `5s` and must be a positive Go duration such as `500ms` or `10s`. An unreachable bootstrap peer fails +within this budget so the process supervisor or Kubernetes can retry; a healthy HTTP listener alone does not prove Worker availability. + ## Available Algorithms | Algorithm | Category | Description | @@ -365,7 +445,8 @@ worker_group = default Vermeer exposes a REST API on port `6688` (configurable in `master.ini`). -> **Note**: The Web UI (`/ui/*`) is only available when `auth=token` is configured. Set `auth_token_factor` in `master.ini` or pass `-auth token -auth_token_factor ` via command line. +> **Note**: The Web UI at `/` (with static assets under `/ui/*`) is only available when `auth=token` is configured. +> Configure the signing factor through `VERMEER_AUTH_TOKEN_FACTOR_FILE` for deployments, or `auth_token_factor` for local use. ### Key Endpoints @@ -378,7 +459,7 @@ Vermeer exposes a REST API on port `6688` (configurable in `master.ini`). | `/api/v1/tasks/{task_id}` | GET | Get task status and results | | `/api/v1/workers` | GET | List connected workers | | `/api/v1/workers/availability` | GET | Read worker readiness for the authenticated space | -| `/ui/` | GET | Web UI dashboard | +| `/` | GET | Web UI dashboard | Worker availability uses the normal space credential and returns only `{"available":true|false}` for that authenticated space's allocations or the @@ -609,7 +690,7 @@ vermeer/tools/protoc/linux64/protoc vermeer/apps/protos/*.proto --go-grpc_out=ve ## Monitoring -Access the Web UI dashboard at `http://master-ip:6688/ui/master.html` for: +Access the Web UI dashboard at `http://master-ip:6688/` for: > **Note**: The Web UI requires `auth=token` in `master.ini` (or `-auth token` via command line). Without token authentication, the UI routes are not registered. @@ -638,6 +719,18 @@ Access the Web UI dashboard at `http://master-ip:6688/ui/master.html` for: - Check network latency between master and workers - Profile algorithm with built-in metrics (access via API) +## Runtime image validation + +Validate a built image with Docker and host Python 3 (standard library only), without PD or Store: + +```bash +bash scripts/test-runtime-image.sh hugegraph/vermeer:your-build +``` + +The check covers non-root helper execution, private v2 token files, invalid-configuration startup failures, +short-password UI login, cookie/API authentication and embedded login assets. It binds Master only to local loopback, +uses temporary test credentials, applies bounded timeouts and removes its containers and files on exit. + ## Links - [Project Homepage](https://hugegraph.apache.org/docs/quickstart/hugegraph-computer/) @@ -652,4 +745,4 @@ See the main [Contributing Guide](../README.md#contributing) for how to contribu ## License -Vermeer is part of Apache HugeGraph-Computer, licensed under [Apache 2.0 License](https://github.com/apache/hugegraph-computer/blob/master/LICENSE). +Vermeer is part of Apache HugeGraph-Computer, licensed under [Apache 2.0 License](https://github.com/apache/hugegraph-computer/blob/master/LICENSE). \ No newline at end of file diff --git a/vermeer/apps/auth/admin_password.go b/vermeer/apps/auth/admin_password.go new file mode 100644 index 000000000..70c970b33 --- /dev/null +++ b/vermeer/apps/auth/admin_password.go @@ -0,0 +1,95 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 auth + +import ( + "crypto/subtle" + "encoding/json" + "errors" + "fmt" + "os" + "strings" + "unicode/utf8" + "vermeer/apps/common" + "vermeer/apps/structure" +) + +// AdminSessionMaxAge is the lifetime of a native UI password session in seconds. +const AdminSessionMaxAge = tokenMaxAge + +var adminPassword []byte + +func adminPasswordFile() string { + if file := os.Getenv("VERMEER_ADMIN_PASSWORD_FILE"); file != "" { + return file + } + file, _ := common.GetConfigDefault("admin_password_file", "").(string) + return file +} + +// ValidateAdminPasswordMode rejects a configured UI password when authentication is disabled. +func ValidateAdminPasswordMode(authType string) error { + if adminPasswordFile() != "" && authType != "token" { + return errors.New("admin password file requires file-backed v2 token authentication") + } + return nil +} + +func readAdminPasswordFile(file string) ([]byte, error) { + data, err := os.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("read admin password file: %w", err) + } + if len(data) == 0 || !utf8.Valid(data) || strings.ContainsAny(string(data), "\r\n") || + strings.TrimSpace(string(data)) == "" { + return nil, errors.New("admin password file must contain a nonempty single-line UTF-8 credential") + } + return data, nil +} + +// ExchangeAdminPassword authenticates the UI credential without exposing it as a Bearer token. +// The resulting session is an ordinary expiring v2 administrator token. +func ExchangeAdminPassword(credential string) (string, bool) { + if secureFactory == nil || len(adminPassword) == 0 || + subtle.ConstantTimeCompare([]byte(credential), adminPassword) != 1 { + return "", false + } + token := secureFactory.NewToken(structure.AdminName, structure.DefaultSpaceName, "native-ui") + secureFactory.Sign(token) + data, err := json.Marshal(token) + if err != nil { + return "", false + } + return ToBase58(string(data)), true +} + +func validSignature(token *Token) bool { + if secureFactory != nil { + return secureFactory.Verify(token) + } + return factory != nil && factory.Verify(token) +} + +// ValidLoginToken validates opaque token login before a session cookie is set. +func ValidLoginToken(token *Token) bool { + if token == nil || !validSignature(token) || tokenExpired(token) { + return false + } + _, err := structure.NewCredential(token.User, token.Space) + return err == nil +} diff --git a/vermeer/apps/auth/admin_password_test.go b/vermeer/apps/auth/admin_password_test.go new file mode 100644 index 000000000..ed90738e0 --- /dev/null +++ b/vermeer/apps/auth/admin_password_test.go @@ -0,0 +1,159 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 auth + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + "vermeer/apps/common" +) + +func isolateAdminPasswordConfig(t *testing.T) { + t.Helper() + previousFactory, previousSecure, previousPassword := factory, secureFactory, adminPassword + t.Cleanup(func() { factory, secureFactory, adminPassword = previousFactory, previousSecure, previousPassword }) + for _, key := range []string{"auth_token_factor", "auth_token_factor_file", "admin_password_file"} { + previous := common.GetConfigDefault(key, "") + common.SetConfig(key, "") + t.Cleanup(func() { common.SetConfig(key, previous) }) + } + for _, name := range []string{"VERMEER_AUTH_TOKEN_FACTOR_FILE", "VERMEER_ADMIN_PASSWORD_FILE"} { + t.Setenv(name, "") + } +} + +func writeAdminFixture(t *testing.T, name, data string) string { + t.Helper() + file := filepath.Join(t.TempDir(), name) + if err := os.WriteFile(file, []byte(data), 0600); err != nil { + t.Fatal(err) + } + return file +} + +func TestAdminPasswordFilePrecedenceAndRotation(t *testing.T) { + isolateAdminPasswordConfig(t) + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", writeAdminFixture(t, "factor", "synthetic-admin-ui-signing-factor")) + const configuredPassword = "ConfigPassword16" + const initialPassword = " First密碼\"\\Value " + configured := writeAdminFixture(t, "configured-password", configuredPassword) + override := writeAdminFixture(t, "environment-password", initialPassword) + common.SetConfig("admin_password_file", configured) + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", override) + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(configuredPassword); ok { + t.Fatal("environment password file must take precedence") + } + encoded, ok := ExchangeAdminPassword(initialPassword) + if !ok || encoded == initialPassword { + t.Fatal("password must exchange for a separate opaque session token") + } + session := ToTokenBase58(encoded) + now := time.Now().Unix() + if session == nil || session.User != "admin" || session.Space != "$DEFAULT" || + session.Time < now-1 || session.Time > now || !ValidLoginToken(session) { + t.Fatal("password login must issue a current v2 administrator session") + } + if !strings.HasPrefix(session.Digest, "v2:") || AdminSessionMaxAge != 24*60*60 { + t.Fatal("native UI sessions must use v2 signatures and the 24 hour token lifetime") + } + if ValidLoginToken(ToTokenBase58(initialPassword)) { + t.Fatal("short passwords must not authenticate as API tokens") + } + const rotatedPassword = "RotatedAdmin1234" + if err := os.WriteFile(override, []byte(rotatedPassword), 0600); err != nil { + t.Fatal(err) + } + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(initialPassword); ok { + t.Fatal("old password must stop working after a reload") + } + if _, ok := ExchangeAdminPassword(rotatedPassword); !ok { + t.Fatal("rotated password must authenticate") + } + if !ValidLoginToken(session) { + t.Fatal("password rotation alone must not rotate the token signing factor") + } + session.Time = time.Now().Unix() - 25*60*60 + secureFactory.Sign(session) + if ValidLoginToken(session) { + t.Fatal("expired UI sessions must be rejected") + } + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", "") + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(configuredPassword); !ok { + t.Fatal("INI password file must work without environment override") + } + common.SetConfig("admin_password_file", "") + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(configuredPassword); ok { + t.Fatal("unconfigured deployments must remain token-only") + } +} + +func TestAdminPasswordRequiresV2AndFailsClosed(t *testing.T) { + isolateAdminPasswordConfig(t) + passwordFile := writeAdminFixture(t, "password", "SyntheticAdmin16") + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", passwordFile) + common.SetConfig("auth_token_factor", "legacy-inline-factor") + if err := Init(); err == nil || factory != nil || secureFactory != nil { + t.Fatal("admin password configuration must reject legacy inline signing") + } + for _, mode := range []string{"none", "", "invalid"} { + if err := ValidateAdminPasswordMode(mode); err == nil { + t.Fatal("password configuration must reject disabled authentication") + } + } + factorFile := writeAdminFixture(t, "factor", "synthetic-admin-password-factor") + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factorFile) + for _, test := range []struct{ name, contents string }{ + {"missing", ""}, {"empty", ""}, {"whitespace", " \t"}, {"newline", "SyntheticPrivateValue\n"}, + {"carriage-return", "SyntheticPrivateValue\r"}, {"invalid-utf8", "SyntheticPrivateValue\xff"}, + } { + t.Run(test.name, func(t *testing.T) { + file := filepath.Join(t.TempDir(), "password") + if test.name != "missing" { + if err := os.WriteFile(file, []byte(test.contents), 0600); err != nil { + t.Fatal(err) + } + } + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", file) + err := Init() + if err == nil || factory != nil || secureFactory != nil || len(adminPassword) != 0 { + t.Fatal("bad password files must not leave any active authentication factory") + } + if strings.Contains(err.Error(), "SyntheticPrivateValue") { + t.Fatal("errors must not echo password contents") + } + if _, ok := ExchangeAdminPassword("SyntheticPrivateValue"); ok { + t.Fatal("invalid configuration must never authenticate") + } + }) + } +} diff --git a/vermeer/apps/auth/factor_file.go b/vermeer/apps/auth/factor_file.go new file mode 100644 index 000000000..4434ff03a --- /dev/null +++ b/vermeer/apps/auth/factor_file.go @@ -0,0 +1,51 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 auth + +import ( + "errors" + "fmt" + "os" + "strings" + "unicode/utf8" +) + +const minTokenSigningKeyCharacters = 16 + +// ReadTokenFactorFile is shared by the Master and the native token generator. +// A single terminal newline is accepted; no other whitespace is changed. +func ReadTokenFactorFile(file string) (string, error) { + data, err := os.ReadFile(file) + if err != nil { + return "", fmt.Errorf("read token factor file: %w", err) + } + factor := strings.TrimSuffix(strings.TrimSuffix(string(data), "\n"), "\r") + if err := validateTokenSigningKey(factor); err != nil { + return "", err + } + return factor, nil +} + +func validateTokenSigningKey(factor string) error { + if !utf8.ValidString(factor) || + utf8.RuneCountInString(factor) < minTokenSigningKeyCharacters || + strings.TrimSpace(factor) == "" || strings.ContainsAny(factor, "\r\n") { + return errors.New("token signing key must contain one UTF-8 line of at least 16 characters") + } + return nil +} diff --git a/vermeer/apps/auth/factor_file_test.go b/vermeer/apps/auth/factor_file_test.go new file mode 100644 index 000000000..d5d971ea3 --- /dev/null +++ b/vermeer/apps/auth/factor_file_test.go @@ -0,0 +1,102 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 auth + +import ( + "os" + "path/filepath" + "strings" + "testing" + "vermeer/apps/common" +) + +func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { + previousFactor := common.GetConfigDefault("auth_token_factor", "") + previousFile := common.GetConfigDefault("auth_token_factor_file", "") + previousFactory := factory + previousSecureFactory := secureFactory + t.Cleanup(func() { + common.SetConfig("auth_token_factor", previousFactor) + common.SetConfig("auth_token_factor_file", previousFile) + factory = previousFactory + secureFactory = previousSecureFactory + }) + common.SetConfig("auth_token_factor", "legacy-inline-factor") + configured := filepath.Join(t.TempDir(), "configured") + override := filepath.Join(t.TempDir(), "override") + if err := os.WriteFile(configured, []byte("configured-factor\r\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(override, []byte("environment-factor"), 0600); err != nil { + t.Fatal(err) + } + common.SetConfig("auth_token_factor_file", configured) + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", override) + if err := Init(); err != nil || factory != nil || secureFactory == nil || + string(secureFactory.signingKey) != "environment-factor" { + t.Fatal("environment factor file must override config and inline factor") + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", "") + if err := Init(); err != nil || factory != nil || secureFactory == nil || + string(secureFactory.signingKey) != "configured-factor" { + t.Fatal("configured factor file must override inline factor") + } + for _, test := range []struct{ name, data string }{ + {"missing", ""}, {"empty", ""}, {"invalid", "synthetic-secret\nsecond-line"}, + {"short-ascii", strings.Repeat("a", minTokenSigningKeyCharacters-1)}, + {"short-unicode", strings.Repeat("密", minTokenSigningKeyCharacters-1)}, + {"invalid-utf8", strings.Repeat("a", minTokenSigningKeyCharacters) + "\xff"}, + {"whitespace-only", strings.Repeat(" ", minTokenSigningKeyCharacters)}, + } { + t.Run(test.name, func(t *testing.T) { + file := filepath.Join(t.TempDir(), "factor") + if test.name != "missing" { + if err := os.WriteFile(file, []byte(test.data), 0600); err != nil { + t.Fatal(err) + } + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", file) + err := Init() + if err == nil || factory != nil || secureFactory != nil { + t.Fatal("invalid file must not leave an active fallback factory") + } + if strings.Contains(err.Error(), "synthetic-secret") { + t.Fatal("errors must not expose factor contents") + } + }) + } + common.SetConfig("auth_token_factor_file", "") + if err := Init(); err != nil || secureFactory != nil || factory == nil || factory.Factor != "legacy-inline-factor" { + t.Fatal("unconfigured deployments must preserve inline factors") + } +} + +func TestTokenFactorFileAcceptsMinimumLength(t *testing.T) { + factor := strings.Repeat("x", minTokenSigningKeyCharacters) + file := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(file, []byte(factor+"\n"), 0600); err != nil { + t.Fatal(err) + } + actual, err := ReadTokenFactorFile(file) + if err != nil { + t.Fatal(err) + } + if actual != factor { + t.Fatalf("unexpected factor after reading file: %q", actual) + } +} diff --git a/vermeer/apps/auth/filter.go b/vermeer/apps/auth/filter.go index d87a34eae..d1acd7267 100644 --- a/vermeer/apps/auth/filter.go +++ b/vermeer/apps/auth/filter.go @@ -19,7 +19,9 @@ package auth import ( "encoding/json" + "errors" "net/http" + "os" "strings" "time" "vermeer/apps/common" @@ -38,10 +40,48 @@ const ( ) var factory *TokenFactory - -func Init() { - factor := common.GetConfig("auth_token_factor").(string) +var secureFactory *SecureTokenFactory + +func Init() error { + factory = nil + secureFactory = nil + adminPassword = nil + passwordFile := adminPasswordFile() + file := os.Getenv("VERMEER_AUTH_TOKEN_FACTOR_FILE") + if file == "" { + file, _ = common.GetConfigDefault("auth_token_factor_file", "").(string) + } + if file != "" { + signingKey, err := ReadTokenFactorFile(file) + if err != nil { + factory = nil + return err + } + configuredFactory, err := NewSecureTokenFactory(signingKey) + if err != nil { + return err + } + var configuredPassword []byte + if passwordFile != "" { + configuredPassword, err = readAdminPasswordFile(passwordFile) + if err != nil { + return err + } + } + secureFactory = configuredFactory + adminPassword = configuredPassword + return nil + } + if passwordFile != "" { + return errors.New("admin password file requires file-backed v2 token authentication") + } + factor, inlineConfigured := common.GetConfig("auth_token_factor").(string) + if !inlineConfigured { + factory = nil + return errors.New("configure a token signing factor or factor file") + } factory = &TokenFactory{Factor: factor} + return nil } func NoneAuthFilter(ctx *gin.Context) { @@ -77,12 +117,12 @@ func TokenFilter(ctx *gin.Context) { goto LOGIN } - if !factory.Verify(token) { + if !validSignature(token) { text = "Invalid Token" goto LOGIN } - if time.Now().Unix()-token.Time > tokenMaxAge { + if tokenExpired(token) { text = "Expired Token" goto LOGIN } @@ -182,3 +222,7 @@ func DecodeBase58(str string) string { return res } + +func tokenExpired(token *Token) bool { + return time.Now().Unix()-token.Time > tokenMaxAge +} diff --git a/vermeer/apps/auth/secure_token.go b/vermeer/apps/auth/secure_token.go new file mode 100644 index 000000000..b73686293 --- /dev/null +++ b/vermeer/apps/auth/secure_token.go @@ -0,0 +1,79 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 auth + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "strings" +) + +const secureTokenPrefix = "v2:" + +// SecureTokenFactory keeps deployment signing keys out of the legacy token implementation. +// Its concrete type has no legacy Factor field or SHA1 signing path. +type SecureTokenFactory struct { + signingKey []byte +} + +// NewSecureTokenFactory uses versioned HMAC-SHA256 tokens without legacy fallback. +func NewSecureTokenFactory(signingKey string) (*SecureTokenFactory, error) { + if err := validateTokenSigningKey(signingKey); err != nil { + return nil, err + } + return &SecureTokenFactory{signingKey: []byte(signingKey)}, nil +} + +func (factory *SecureTokenFactory) NewToken(user, space, client string) *Token { + return newToken(user, space, client) +} + +func (factory *SecureTokenFactory) Sign(token *Token) { + if token == nil { + return + } + if len(factory.signingKey) == 0 { + token.Digest = "" + return + } + token.Digest = secureTokenPrefix + hex.EncodeToString(factory.signature(token)) +} + +func (factory *SecureTokenFactory) Verify(token *Token) bool { + if token == nil || len(factory.signingKey) == 0 { + return false + } + encoded, ok := strings.CutPrefix(token.Digest, secureTokenPrefix) + if !ok || len(encoded) != sha256.Size*2 { + return false + } + signature, err := hex.DecodeString(encoded) + return err == nil && hmac.Equal(signature, factory.signature(token)) +} + +func (factory *SecureTokenFactory) signature(token *Token) []byte { + // Fixed-field JSON preserves field boundaries and excludes the digest. + // Authenticate the nickname as well as every authorization field. + payload, _ := json.Marshal([5]interface{}{token.User, token.Space, token.Nick, token.Client, token.Time}) + mac := hmac.New(sha256.New, factory.signingKey) + _, _ = mac.Write([]byte("vermeer-token-v2\x00")) + _, _ = mac.Write(payload) + return mac.Sum(nil) +} diff --git a/vermeer/apps/auth/token.go b/vermeer/apps/auth/token.go index 725c46dff..42aba975c 100644 --- a/vermeer/apps/auth/token.go +++ b/vermeer/apps/auth/token.go @@ -18,6 +18,7 @@ under the License. package auth import ( + "crypto/hmac" "crypto/sha1" "encoding/hex" "io" @@ -69,6 +70,10 @@ func (factory *TokenFactory) getRandoms() *[4]string { // NewToken Create a Token by login-id, space, client(e.g. IP address). func (factory *TokenFactory) NewToken(user string, space string, client string) *Token { + return newToken(user, space, client) +} + +func newToken(user, space, client string) *Token { if user == "" { return nil } @@ -92,7 +97,7 @@ func (factory *TokenFactory) Verify(a *Token) bool { s := digest(factory.signSource(a)) - return a.Digest == s + return hmac.Equal([]byte(a.Digest), []byte(s)) } func digest(s string) string { diff --git a/vermeer/apps/auth/token_v2_test.go b/vermeer/apps/auth/token_v2_test.go new file mode 100644 index 000000000..a9da5f00a --- /dev/null +++ b/vermeer/apps/auth/token_v2_test.go @@ -0,0 +1,124 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 auth + +import ( + "math" + "strings" + "testing" +) + +func TestSecureTokenProtocolAndLegacyCompatibility(t *testing.T) { + const key = "legacy-compatibility-factor" + legacy := &TokenFactory{Factor: key} + secure, err := NewSecureTokenFactory(key) + if err != nil { + t.Fatal(err) + } + if secure.signingKey == nil { + t.Fatal("secure keys must be separate from legacy factor state") + } + token := &Token{User: "admin", Space: "$DEFAULT", Client: "compatibility", Time: math.MaxInt64} + legacy.Sign(token) + if token.Digest != "a89528d7bfec6d12addab5a403da64a148f03ce5" || !legacy.Verify(token) { + t.Fatal("legacy inline signing must preserve the historical protocol") + } + if secure.Verify(token) { + t.Fatal("file-backed factories must not accept legacy signatures") + } + secure.Sign(token) + // Independent HMAC-SHA256 fixture over the documented domain and JSON payload. + const expected = "v2:f567b23ada3378d0b216008b2076afd3577b2e69be49dab6efcaeb2618d338eb" + if token.Digest != expected || !secure.Verify(token) { + t.Fatal("unexpected v2 signing protocol") + } + if legacy.Verify(token) { + t.Fatal("legacy factories must not accept v2 signatures") + } + rotated, err := NewSecureTokenFactory("rotated-deployment-signing-key") + if err != nil { + t.Fatal(err) + } + if rotated.Verify(token) { + t.Fatal("rotating the signing key must invalidate old signatures") + } + if secure.Verify(nil) || legacy.Verify(nil) { + t.Fatal("nil tokens must be rejected") + } +} + +func TestSecureTokenRejectsTampering(t *testing.T) { + factory, err := NewSecureTokenFactory("synthetic-secure-signing-key") + if err != nil { + t.Fatal(err) + } + original := factory.NewToken("admin", "$DEFAULT", "hubble") + original.Nick = "display-name" + original.Time = math.MaxInt64 + factory.Sign(original) + mutations := map[string]func(*Token){ + "user": func(token *Token) { token.User = "another-user" }, + "space": func(token *Token) { token.Space = "another-space" }, + "nickname": func(token *Token) { token.Nick = "another-name" }, + "client": func(token *Token) { token.Client = "another-client" }, + "time": func(token *Token) { token.Time-- }, + "version": func(token *Token) { token.Digest = "v1:" + strings.TrimPrefix(token.Digest, "v2:") }, + "unversioned": func(token *Token) { token.Digest = strings.TrimPrefix(token.Digest, "v2:") }, + "truncated": func(token *Token) { token.Digest = token.Digest[:len(token.Digest)-1] }, + "invalid-hex": func(token *Token) { token.Digest = "v2:" + strings.Repeat("z", 64) }, + "signature": func(token *Token) { token.Digest = "v2:" + strings.Repeat("0", 64) }, + } + for name, mutate := range mutations { + t.Run(name, func(t *testing.T) { + token := *original + mutate(&token) + if factory.Verify(&token) { + t.Fatal("modified token must not authenticate") + } + }) + } + first := &Token{User: "a", Space: "bc", Client: "client", Time: 1} + second := &Token{User: "ab", Space: "c", Client: "client", Time: 1} + factory.Sign(first) + second.Digest = first.Digest + if factory.Verify(second) { + t.Fatal("signed fields must retain unambiguous boundaries") + } +} + +func TestSecureTokenFactoryRejectsInvalidKeys(t *testing.T) { + for _, key := range []string{"", "abc", "密碼", " ", "key\nsecond-line", "key\xff"} { + factory, err := NewSecureTokenFactory(key) + if err == nil || factory != nil { + t.Fatal("invalid secure keys must not create a legacy factory") + } + } +} + +func TestZeroValueSecureTokenFactoryFailsClosed(t *testing.T) { + var factory SecureTokenFactory + token := &Token{User: "admin", Space: "$DEFAULT", Client: "zero-value", Time: math.MaxInt64} + token.Digest = secureTokenPrefix + strings.Repeat("0", 64) + if factory.Verify(token) { + t.Fatal("zero-value factory must never authenticate a token") + } + factory.Sign(token) + if token.Digest != "" || factory.Verify(token) { + t.Fatal("zero-value signing must clear the signature and remain unusable") + } +} diff --git a/vermeer/apps/common/grpc_dial.go b/vermeer/apps/common/grpc_dial.go new file mode 100644 index 000000000..bd6ab4180 --- /dev/null +++ b/vermeer/apps/common/grpc_dial.go @@ -0,0 +1,43 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 common + +import ( + "context" + "fmt" + "os" + "time" + + "google.golang.org/grpc" +) + +// DialGRPCPeer bounds blocking bootstrap connections without changing stream lifetimes. +func DialGRPCPeer(ctx context.Context, target string, options ...grpc.DialOption) (*grpc.ClientConn, error) { + configured := os.Getenv("VERMEER_GRPC_DIAL_TIMEOUT") + if configured == "" { + configured = fmt.Sprint(GetConfigDefault("grpc_dial_timeout", "5s")) + } + timeout, err := time.ParseDuration(configured) + if err != nil || timeout <= 0 { + return nil, fmt.Errorf("grpc_dial_timeout / VERMEER_GRPC_DIAL_TIMEOUT must be a positive duration") + } + dialCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + options = append(options, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + return grpc.DialContext(dialCtx, target, options...) +} diff --git a/vermeer/apps/common/grpc_dial_test.go b/vermeer/apps/common/grpc_dial_test.go new file mode 100644 index 000000000..a12dc78ee --- /dev/null +++ b/vermeer/apps/common/grpc_dial_test.go @@ -0,0 +1,58 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 common + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +func TestGRPCBootstrapDialHasBoundedFailure(t *testing.T) { + previous := GetConfigDefault("grpc_dial_timeout", "5s") + t.Cleanup(func() { SetConfig("grpc_dial_timeout", previous) }) + SetConfig("grpc_dial_timeout", "1s") + t.Setenv("VERMEER_GRPC_DIAL_TIMEOUT", "40ms") + started := time.Now() + connection, err := DialGRPCPeer(context.Background(), "passthrough:///unreachable", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + <-ctx.Done() + return nil, ctx.Err() + })) + if connection != nil || !errors.Is(err, context.DeadlineExceeded) || time.Since(started) > time.Second { + t.Fatalf("bootstrap dial did not stop at its configured deadline: %v", err) + } +} + +func TestGRPCBootstrapDialRejectsInvalidTimeout(t *testing.T) { + for _, configured := range []string{"bad", "0s", "-1s"} { + t.Run(configured, func(t *testing.T) { + t.Setenv("VERMEER_GRPC_DIAL_TIMEOUT", configured) + connection, err := DialGRPCPeer(context.Background(), "unreachable") + if err == nil || connection != nil { + t.Fatal("invalid timeout must fail before dialing") + } + }) + } +} diff --git a/vermeer/apps/common/hugegraph_credentials.go b/vermeer/apps/common/hugegraph_credentials.go index 5f5633d2c..f6dcb2142 100644 --- a/vermeer/apps/common/hugegraph_credentials.go +++ b/vermeer/apps/common/hugegraph_credentials.go @@ -29,7 +29,23 @@ import ( func ResolveHugegraphCredentials(username, password string) (string, string, error) { file := deploymentConfig("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", "hugegraph_credentials_file") if file == "" { - return username, password, nil + deploymentUsername := deploymentConfig("VERMEER_HUGEGRAPH_USERNAME", "hugegraph_username") + passwordFile := deploymentConfig("VERMEER_HUGEGRAPH_PASSWORD_FILE", "hugegraph_password_file") + if deploymentUsername == "" && passwordFile == "" { + return username, password, nil + } + if deploymentUsername == "" || passwordFile == "" { + return "", "", errors.New("configure both HugeGraph username and password file") + } + data, err := os.ReadFile(passwordFile) + if err != nil { + return "", "", fmt.Errorf("read HugeGraph password file: %w", err) + } + if len(data) == 0 { + return "", "", errors.New("HugeGraph password file must not be empty") + } + // Password files are raw Secret values: preserve spaces, quotes and newlines. + return deploymentUsername, string(data), nil } data, err := os.ReadFile(file) if err != nil { diff --git a/vermeer/apps/common/hugegraph_credentials_test.go b/vermeer/apps/common/hugegraph_credentials_test.go index c5adae484..f6be84d78 100644 --- a/vermeer/apps/common/hugegraph_credentials_test.go +++ b/vermeer/apps/common/hugegraph_credentials_test.go @@ -95,3 +95,75 @@ func TestHugegraphCredentialsFileFailsClosed(t *testing.T) { }) } } + +func TestHugegraphPasswordFilePreservesSecretBytes(t *testing.T) { + isolatePDPeerConfig(t) + configured := filepath.Join(t.TempDir(), "configured-password") + override := filepath.Join(t.TempDir(), "environment-password") + const rawPassword = " quote\"backslash\\dollar$Unicode密碼\n" + if err := os.WriteFile(configured, []byte("config-password"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(override, []byte(rawPassword), 0600); err != nil { + t.Fatal(err) + } + SetConfig("hugegraph_username", "config-user") + SetConfig("hugegraph_password_file", configured) + t.Setenv("VERMEER_HUGEGRAPH_USERNAME", "environment-user") + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", override) + username, password, err := ResolveHugegraphCredentials("task-user", "task-password") + if err != nil || username != "environment-user" || password != rawPassword { + t.Fatal("raw Secret contents must survive file reading without escaping or trimming") + } + t.Setenv("VERMEER_HUGEGRAPH_USERNAME", "") + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", "") + username, password, err = ResolveHugegraphCredentials("task-user", "task-password") + if err != nil || username != "config-user" || password != "config-password" { + t.Fatal("config username/password file must override task credentials") + } + explicit := filepath.Join(t.TempDir(), "credentials.json") + if err := os.WriteFile(explicit, []byte(`{"username":"json-user","password":"json-password"}`), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", explicit) + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", filepath.Join(t.TempDir(), "missing")) + username, password, err = ResolveHugegraphCredentials("task-user", "task-password") + if err != nil || username != "json-user" || password != "json-password" { + t.Fatal("explicit JSON credentials must retain precedence over password-file credentials") + } +} + +func TestHugegraphPasswordFileFailsClosed(t *testing.T) { + for _, test := range []struct { + name, username, contents string + create, file bool + }{ + {name: "missing-user", contents: "synthetic-sensitive-password", create: true, file: true}, + {name: "missing-file-setting", username: "deployment-user"}, + {name: "missing-file", username: "deployment-user", file: true}, + {name: "empty-file", username: "deployment-user", create: true, file: true}, + } { + t.Run(test.name, func(t *testing.T) { + isolatePDPeerConfig(t) + file := filepath.Join(t.TempDir(), "password") + if test.create { + if err := os.WriteFile(file, []byte(test.contents), 0600); err != nil { + t.Fatal(err) + } + } + t.Setenv("VERMEER_HUGEGRAPH_USERNAME", test.username) + if test.file { + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", file) + } + username, password, err := ResolveHugegraphCredentials("task-user", "task-sensitive-password") + if err == nil || username != "" || password != "" { + t.Fatal("invalid deployment credentials must not fall back") + } + for _, secret := range []string{"synthetic-sensitive-password", "task-sensitive-password"} { + if strings.Contains(err.Error(), secret) { + t.Fatal("errors must not expose credentials") + } + } + }) + } +} diff --git a/vermeer/apps/common/pd_peers.go b/vermeer/apps/common/pd_peers.go index 3812c3f8b..162d86bfb 100644 --- a/vermeer/apps/common/pd_peers.go +++ b/vermeer/apps/common/pd_peers.go @@ -41,7 +41,9 @@ func resolvePDPeers(taskPeers []string) ([]string, error) { configured := deploymentConfig("VERMEER_PD_PEERS", "pd_peers") if configured == "" { if deploymentConfig("VERMEER_PD_PASSWORD_FILE", "pd_password_file") != "" || - deploymentConfig("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", "hugegraph_credentials_file") != "" { + deploymentConfig("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", "hugegraph_credentials_file") != "" || + deploymentConfig("VERMEER_HUGEGRAPH_USERNAME", "hugegraph_username") != "" || + deploymentConfig("VERMEER_HUGEGRAPH_PASSWORD_FILE", "hugegraph_password_file") != "" { return nil, errors.New("configure pd_peers when using deployment credentials") } return taskPeers, nil diff --git a/vermeer/apps/common/pd_peers_test.go b/vermeer/apps/common/pd_peers_test.go index a79e559c1..197fe95be 100644 --- a/vermeer/apps/common/pd_peers_test.go +++ b/vermeer/apps/common/pd_peers_test.go @@ -73,12 +73,14 @@ func TestPDPeersOverrideWarning(t *testing.T) { func isolatePDPeerConfig(t *testing.T) { t.Helper() - for _, key := range []string{"pd_peers", "pd_password_file", "hugegraph_credentials_file"} { + for _, key := range []string{"pd_peers", "pd_password_file", "hugegraph_credentials_file", + "hugegraph_username", "hugegraph_password_file"} { old := GetConfigDefault(key, "") SetConfig(key, "") t.Cleanup(func() { SetConfig(key, old) }) } - for _, key := range []string{"VERMEER_PD_PEERS", "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE"} { + for _, key := range []string{"VERMEER_PD_PEERS", "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE", + "VERMEER_HUGEGRAPH_USERNAME", "VERMEER_HUGEGRAPH_PASSWORD_FILE"} { t.Setenv(key, "") } } @@ -103,8 +105,9 @@ func TestPDPeersDeploymentPrecedence(t *testing.T) { } func TestPDPeersDeploymentCredentialsRequireConfig(t *testing.T) { - for _, key := range []string{"pd_password_file", "hugegraph_credentials_file", - "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE"} { + for _, key := range []string{"pd_password_file", "hugegraph_credentials_file", "hugegraph_username", "hugegraph_password_file", + "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE", + "VERMEER_HUGEGRAPH_USERNAME", "VERMEER_HUGEGRAPH_PASSWORD_FILE"} { t.Run(key, func(t *testing.T) { isolatePDPeerConfig(t) missing := filepath.Join(t.TempDir(), "not-created") diff --git a/vermeer/apps/master/bl/grpc_handlers.go b/vermeer/apps/master/bl/grpc_handlers.go index 4b5378c73..f116c8f02 100644 --- a/vermeer/apps/master/bl/grpc_handlers.go +++ b/vermeer/apps/master/bl/grpc_handlers.go @@ -24,6 +24,7 @@ import ( "strings" "sync" "time" + "vermeer/apps/common" "vermeer/apps/compute" "vermeer/apps/graphio" "vermeer/apps/master/schedules" @@ -52,6 +53,9 @@ func (h *ServerHandler) Init() { func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterReq) (*pb.HelloMasterResp, error) { h.locker.Lock() defer h.locker.Unlock() + if err := ctx.Err(); err != nil { + return nil, err + } p, _ := peer.FromContext(ctx) // check worker version, if not match, deny @@ -80,6 +84,10 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR workers := workerMgr.GetAllWorkers() var existWorkerName string for _, worker := range workers { + if workerMgr.IsAbandonedRegistration(worker) { + workerBl.KickOfflineSession(worker) + continue + } if worker.GrpcPeer == ip+":"+port { existWorkerName = worker.Name break @@ -87,9 +95,17 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR } if existWorkerName != "" { //如果worker已经存在，必须等待至grpc recv handler感知后，删除worker - for workerMgr.GetWorker(existWorkerName) != nil { + for existing := workerMgr.GetWorker(existWorkerName); existing != nil; existing = workerMgr.GetWorker(existWorkerName) { + if workerMgr.CanReplaceDisconnectedSession(existing) { + workerBl.KickOfflineSession(existing) + continue + } logrus.Infof("worker %v exist, wait one second", existWorkerName) - time.Sleep(1 * time.Second) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(time.Second): + } } } @@ -104,6 +120,16 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR logrus.Errorf("failed to add a WorkerClient to the WorkerManager, error: %s", err) return &pb.HelloMasterResp{}, err } + registered := false + defer func() { + if !registered { + workerMgr.RemoveWorkerSession(reqWorker) + if reqWorker.Connection != nil { + _ = reqWorker.Connection.Close() + } + } + }() + _, err = Scheduler.ChangeWorkerStatus(reqWorker.Name, schedules.WorkerOngoingStatusIdle) if err != nil { logrus.Errorf("failed to change worker status to idle, error: %s", err) @@ -117,27 +143,14 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR WorkerName: reqWorker.Name, } - for _, v := range workers { - if v.GrpcPeer == ip+":"+port { - logrus.Debugf("workerMgr has a duplicate address, ignore it") - continue - } - workerInfo := pb.WorkerInfo{ - Name: v.Name, - Id: v.Id, - GrpcPeer: v.GrpcPeer, - } - resp.Workers = append(resp.Workers, &workerInfo) - } - // connect to client workerClient := workerMgr.GetWorker(reqWorker.Name) dialOptions := grpc.WithDefaultCallOptions( grpc.MaxCallSendMsgSize(1*1024*1024*1024), grpc.MaxCallRecvMsgSize(1*1024*1024*1024)) - connection, err := grpc.Dial( - ip+":"+port, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + connection, err := common.DialGRPCPeer(ctx, + ip+":"+port, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { logrus.Errorf("connect worker error: %s", err) return &resp, err @@ -149,7 +162,10 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR resp.Spaces = append(resp.Spaces, space.Name()) } - initWorkerClient(workerClient) + initWorkerClient(ctx, workerClient) + if err := ctx.Err(); err != nil { + return nil, err + } graphs := graphMgr.GetGraphsByWorker(reqWorker.Name) taskCanRecovery := make([]*structure.TaskInfo, 0) @@ -195,13 +211,27 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR logrus.Infof("worker:%v recovery task:%v type:%v ", workerClient.Name, task.ID, task.Type) } }() + for _, v := range workerMgr.GetBootstrapWorkers() { + if v.GrpcPeer == ip+":"+port { + logrus.Debugf("workerMgr has a duplicate address, ignore it") + continue + } + workerInfo := pb.WorkerInfo{ + Name: v.Name, + Id: v.Id, + GrpcPeer: v.GrpcPeer, + } + resp.Workers = append(resp.Workers, &workerInfo) + } + + registered = true return &resp, nil } -func initWorkerClient(workerClient *workers.WorkerClient) { +func initWorkerClient(ctx context.Context, workerClient *workers.WorkerClient) { logrus.Infof("init worker: %s@%s", workerClient.Name, workerClient.GrpcPeer) // init memory limit - err := workers.SendMemLimit(workerClient, &workers.WorkerMemLimit{ + err := workers.SendMemLimitContext(ctx, workerClient, &workers.WorkerMemLimit{ MaxMem: threshold.GroupMaxMemory(workerClient.Group), MinFree: threshold.GroupMinFree(workerClient.Group), GcRatio: float32(threshold.GroupGcPct(workerClient.Group)) / 100, @@ -382,6 +412,7 @@ func (h *ServerHandler) UploadTPResult(ctx context.Context, in *pb.UploadTPResul type LoadGraphServer struct { streamServer pb.Master_LoadGraphTaskServer + worker *workers.WorkerClient sendMutex sync.Mutex } @@ -391,20 +422,16 @@ func (s *LoadGraphServer) SetServer(stream pb.Master_LoadGraphTaskServer) { func (s *LoadGraphServer) RecvHandler(workerName string) { logrus.Infof("load graph task recv handler setup: %s", workerName) + worker := s.worker for { req, err := s.streamServer.Recv() if err != nil { logrus.Errorf("load graph task recv: %s", err) + workerMgr.ClearTaskStream(worker, workers.LoadTaskStream, s.streamServer.Context()) time.Sleep(3 * time.Second) - if workerBl.KickOffline(workerName) { - break - } - // if !WorkerMgr.CheckWorkerAlive(workerName) { - // WorkerMgr.RemoveWorker(workerName) - // break - // } - continue + workerBl.KickOfflineSession(worker) + return } if req.State == graphio.LoadPartStatusPrepared { } else if req.State == graphio.LoadPartStatusDone { @@ -457,6 +484,7 @@ func (s *LoadGraphServer) AsyncLoad( type ComputeTaskServer struct { streamServer pb.Master_ComputeTaskServer + worker *workers.WorkerClient } func (s *ComputeTaskServer) SetServer(stream pb.Master_ComputeTaskServer) { @@ -465,20 +493,16 @@ func (s *ComputeTaskServer) SetServer(stream pb.Master_ComputeTaskServer) { func (s *ComputeTaskServer) RecvHandler(workerName string) { logrus.Infof("compute task recv handler setup: %s", workerName) + worker := s.worker for { req, err := s.streamServer.Recv() if err != nil { logrus.Errorf("compute task recv error: %s", err) + workerMgr.ClearTaskStream(worker, workers.ComputeTaskStream, s.streamServer.Context()) time.Sleep(3 * time.Second) - if workerBl.KickOffline(workerName) { - break - } - // if !WorkerMgr.CheckWorkerAlive(workerName) { - // WorkerMgr.RemoveWorker(workerName) - // break - // } - continue + workerBl.KickOfflineSession(worker) + return } _ = req } diff --git a/vermeer/apps/master/bl/server_manager.go b/vermeer/apps/master/bl/server_manager.go index e9856c51f..068891c2c 100644 --- a/vermeer/apps/master/bl/server_manager.go +++ b/vermeer/apps/master/bl/server_manager.go @@ -46,6 +46,7 @@ func (s *ServerManager) StartingLoadServer(workerName string, stream pb.Master_L s.locker.Lock() s.loadSevers[workerName] = loadServer worker := workerMgr.GetWorker(workerName) + loadServer.worker = worker workerMgr.SetTaskStream(worker, workers.LoadTaskStream, stream.Context()) s.locker.Unlock() defer func() { @@ -70,6 +71,7 @@ func (s *ServerManager) StartingComputeServer(workerName string, stream pb.Maste s.locker.Lock() s.computeSevers[workerName] = computeServer worker := workerMgr.GetWorker(workerName) + computeServer.worker = worker workerMgr.SetTaskStream(worker, workers.ComputeTaskStream, stream.Context()) s.locker.Unlock() defer func() { diff --git a/vermeer/apps/master/bl/worker_bl.go b/vermeer/apps/master/bl/worker_bl.go index 651e1e043..1d5fe7930 100644 --- a/vermeer/apps/master/bl/worker_bl.go +++ b/vermeer/apps/master/bl/worker_bl.go @@ -21,6 +21,7 @@ import ( "fmt" "sync" . "vermeer/apps/master/graphs" + "vermeer/apps/master/workers" "vermeer/apps/structure" "github.com/sirupsen/logrus" @@ -36,20 +37,28 @@ type WorkerBl struct { // KickOffline Checking the worker state and performing a forced termination if it's offline. // Return false if the worker is alive. func (wb *WorkerBl) KickOffline(workerName string) bool { - if !workerMgr.CheckWorkerAlive(workerName) { - - if err := wb.ReleaseWorker(workerName); err != nil { - logrus.Errorf("failed to perform ReleaseWorker with the name: %s as the worker is offline, caused by: %v", workerName, err) - } + return wb.KickOfflineSession(workerMgr.GetWorker(workerName)) +} - return true +func (wb *WorkerBl) KickOfflineSession(worker *workers.WorkerClient) bool { + if !workerMgr.ClaimOfflineSession(worker) { + return false } - - return false + if worker.Connection != nil { + _ = worker.Connection.Close() + } + if err := wb.releaseWorker(worker.Name, worker); err != nil { + logrus.Errorf("failed to release offline worker %s: %v", worker.Name, err) + } + return true } // ReleaseWorker Release the graph loaded by the worker, cancel the task running in the worker, and remove the worker from the WorkerManager. func (wb *WorkerBl) ReleaseWorker(workerName string) error { + return wb.releaseWorker(workerName, nil) +} + +func (wb *WorkerBl) releaseWorker(workerName string, session *workers.WorkerClient) error { graphs := graphMgr.GetGraphsByWorker(workerName) if len(graphs) == 0 { @@ -78,7 +87,11 @@ func (wb *WorkerBl) ReleaseWorker(workerName string) error { } } - workerMgr.RemoveWorker(workerName) + if session != nil { + workerMgr.RemoveWorkerSession(session) + } else { + workerMgr.RemoveWorker(workerName) + } return nil } diff --git a/vermeer/apps/master/master_main.go b/vermeer/apps/master/master_main.go index 91cd33180..dd5f0a5e9 100644 --- a/vermeer/apps/master/master_main.go +++ b/vermeer/apps/master/master_main.go @@ -40,6 +40,9 @@ func Main() { httpPeer := common.GetConfig("http_peer").(string) debugMode := common.GetConfig("debug_mode").(string) authType := common.GetConfigDefault("auth", "none").(string) + if err := auth.ValidateAdminPasswordMode(authType); err != nil { + logrus.Fatalf("token authentication initialization failed: %s", err) + } services.InfoMaster.IpAddr = httpPeer services.InfoMaster.DebugMod = debugMode services.InfoMaster.Version = version.Version @@ -50,7 +53,9 @@ func Main() { switch authType { case "token": sen.StaticFS("ui", asset.Assets) - auth.Init() + if err := auth.Init(); err != nil { + logrus.Fatalf("token authentication initialization failed: %s", err) + } services.SetRouters(sen, auth.TokenFilter) services.SetAdminRouters(sen, auth.TokenFilter, auth.AdminFilter) services.SetUI(sen) @@ -74,7 +79,9 @@ func Main() { } serverOptions := []grpc.ServerOption{ grpc.MaxRecvMsgSize(4 * 1024 * 1024 * 1024), - grpc.MaxSendMsgSize(4 * 1024 * 1024 * 1024)} + grpc.MaxSendMsgSize(4 * 1024 * 1024 * 1024), + grpc.StreamInterceptor(validateWorkerStream), + } // create grpc server grpcServer := grpc.NewServer(serverOptions...) diff --git a/vermeer/apps/master/services/http_login.go b/vermeer/apps/master/services/http_login.go index b8888328e..ecb001b12 100644 --- a/vermeer/apps/master/services/http_login.go +++ b/vermeer/apps/master/services/http_login.go @@ -18,7 +18,8 @@ under the License. package services import ( - "fmt" + "net/http" + "strings" "vermeer/apps/auth" "vermeer/apps/common" @@ -28,7 +29,7 @@ import ( //const cookieTokenExpire = 1 * time.Minute type LoginRequest struct { - Token *string + Token *string `json:"token"` } type LoginHandler struct { @@ -37,33 +38,33 @@ type LoginHandler struct { func (l *LoginHandler) POST(ctx *gin.Context) { req := LoginRequest{} - err := ctx.BindJSON(&req) - if isBad(err != nil, ctx, func() string { return fmt.Sprintf("request body not correct: %s", err) }) { + if err := ctx.ShouldBindJSON(&req); err != nil { + isBad(true, ctx, func() string { return "invalid login request" }) return } - - if req.Token != nil { - doTokenLogin(ctx, &req) - return - } - - if isBad(true, ctx, func() string { return fmt.Sprintf("request body not correct: %s", err) }) { + if req.Token == nil { + isBad(true, ctx, func() string { return "admin credential is required" }) return } + doTokenLogin(ctx, &req) } func doTokenLogin(ctx *gin.Context, req *LoginRequest) { + if session, okPassword := auth.ExchangeAdminPassword(*req.Token); okPassword { + setTokenCookie(ctx, session, auth.AdminSessionMaxAge) + ok(ctx, "login success") + return + } token := auth.ToTokenBase58(*req.Token) - if isBad(token == nil, ctx, func() string { return "invalid token string format" }) { + if !auth.ValidLoginToken(token) { + ctx.JSON(http.StatusUnauthorized, common.BaseResp{ErrCode: 1, Message: "invalid admin credential"}) return } - - setTokenCookie(ctx, *req.Token) - + setTokenCookie(ctx, *req.Token, 0) ok(ctx, "login success") } -func setTokenCookie(ctx *gin.Context, token string) { - // expires: session - ctx.SetCookie(auth.CookieKey, token, 0, "/", "", false, true) +func setTokenCookie(ctx *gin.Context, token string, maxAge int) { + secure := ctx.Request.TLS != nil || strings.EqualFold(ctx.GetHeader("X-Forwarded-Proto"), "https") + ctx.SetCookie(auth.CookieKey, token, maxAge, "/", "", secure, true) } diff --git a/vermeer/apps/master/services/http_login_password_test.go b/vermeer/apps/master/services/http_login_password_test.go new file mode 100644 index 000000000..5e0031eea --- /dev/null +++ b/vermeer/apps/master/services/http_login_password_test.go @@ -0,0 +1,261 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 services + +import ( + "bytes" + "encoding/json" + "math" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + "vermeer/apps/auth" + "vermeer/apps/common" + + "github.com/gin-gonic/gin" + "github.com/prometheus/client_golang/prometheus" + "github.com/sirupsen/logrus" +) + +const nativePasswordFixture = "AdminFixture1234" +const nativeSigningFixture = "synthetic-native-password-signing-factor" + +func configureNativePassword(t *testing.T) (string, string) { + t.Helper() + for _, key := range []string{"auth_token_factor", "auth_token_factor_file", "admin_password_file"} { + previous := common.GetConfigDefault(key, "") + common.SetConfig(key, "") + t.Cleanup(func() { common.SetConfig(key, previous) }) + } + directory := t.TempDir() + factor := filepath.Join(directory, "factor") + password := filepath.Join(directory, "password") + if err := os.WriteFile(factor, []byte(nativeSigningFixture), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(password, []byte(nativePasswordFixture), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factor) + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", password) + if err := auth.Init(); err != nil { + t.Fatal(err) + } + factory, err := auth.NewSecureTokenFactory(nativeSigningFixture) + if err != nil { + t.Fatal(err) + } + token := factory.NewToken("admin", "$DEFAULT", "hubble-service") + token.Time = math.MaxInt64 + factory.Sign(token) + encoded, err := json.Marshal(token) + if err != nil { + t.Fatal(err) + } + return password, auth.ToBase58(string(encoded)) +} + +func postNativeCredential(t *testing.T, handler http.Handler, + credential, forwardedProto string) *httptest.ResponseRecorder { + t.Helper() + body, err := json.Marshal(map[string]string{"token": credential}) + if err != nil { + t.Fatal(err) + } + request := httptest.NewRequest(http.MethodPost, "/api/v1/login", bytes.NewReader(body)) + request.Header.Set("Content-Type", "application/json") + if forwardedProto != "" { + request.Header.Set("X-Forwarded-Proto", forwardedProto) + } + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return response +} + +func TestNativeAdminPasswordCookieAndServiceToken(t *testing.T) { + passwordFile, serviceToken := configureNativePassword(t) + previousMode, previousOutput := gin.Mode(), logrus.StandardLogger().Out + var logs bytes.Buffer + logrus.SetOutput(&logs) + gin.SetMode(gin.TestMode) + t.Cleanup(func() { gin.SetMode(previousMode); logrus.SetOutput(previousOutput) }) + router := gin.New() + router.POST("/api/v1/login", (&LoginHandler{}).POST) + router.GET("/protected", auth.TokenFilter, auth.AdminFilter, func(ctx *gin.Context) { ctx.Status(http.StatusOK) }) + check := func(cookie *http.Cookie, bearer string, want int) { + t.Helper() + request := httptest.NewRequest(http.MethodGet, "/protected", nil) + if cookie != nil { + request.AddCookie(cookie) + } + if bearer != "" { + request.Header.Set(auth.HeaderKey, "Bearer "+bearer) + } + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != want { + t.Fatalf("protected status = %d, want %d", response.Code, want) + } + } + check(nil, "", http.StatusUnauthorized) + wrong := postNativeCredential(t, router, "IncorrectAdmin16", "") + if wrong.Code != http.StatusUnauthorized || len(wrong.Result().Cookies()) != 0 { + t.Fatal("incorrect UI password must not create a cookie") + } + response := postNativeCredential(t, router, nativePasswordFixture, "") + cookies := response.Result().Cookies() + if response.Code != http.StatusOK || len(cookies) != 1 || !cookies[0].HttpOnly || + cookies[0].MaxAge != auth.AdminSessionMaxAge || cookies[0].Secure { + t.Fatal("internal HTTP login must create its HttpOnly 24-hour session cookie") + } + cookie := cookies[0] + token := auth.ToTokenBase58(cookie.Value) + now := time.Now().Unix() + if token == nil || token.User != "admin" || token.Space != "$DEFAULT" || token.Time < now-1 || token.Time > now || + !strings.HasPrefix(token.Digest, "v2:") || cookie.Value == nativePasswordFixture { + t.Fatal("short password must exchange for a separate current v2 admin session") + } + check(cookie, "", http.StatusOK) + check(nil, nativePasswordFixture, http.StatusUnauthorized) + check(nil, serviceToken, http.StatusOK) + secureResponse := postNativeCredential(t, router, nativePasswordFixture, "https") + if secureResponse.Code != http.StatusOK || !secureResponse.Result().Cookies()[0].Secure { + t.Fatal("HTTPS browser sessions must carry the Secure cookie attribute") + } + serviceLogin := postNativeCredential(t, router, serviceToken, "") + if serviceLogin.Code != http.StatusOK || len(serviceLogin.Result().Cookies()) != 1 { + t.Fatal("long opaque service-token login must remain supported") + } + const rotated = "RotatedFixture16" + if err := os.WriteFile(passwordFile, []byte(rotated), 0600); err != nil { + t.Fatal(err) + } + if err := auth.Init(); err != nil { + t.Fatal(err) + } + if postNativeCredential(t, router, nativePasswordFixture, "").Code != http.StatusUnauthorized { + t.Fatal("old password must stop authenticating after password reload") + } + if postNativeCredential(t, router, rotated, "").Code != http.StatusOK { + t.Fatal("rotated password must authenticate") + } + check(nil, serviceToken, http.StatusOK) + check(cookie, "", http.StatusOK) + malformed := httptest.NewRequest(http.MethodPost, "/api/v1/login", strings.NewReader(`{"token":"`+rotated)) + malformed.Header.Set("Content-Type", "application/json") + malformedResponse := httptest.NewRecorder() + router.ServeHTTP(malformedResponse, malformed) + if malformedResponse.Code != http.StatusBadRequest { + t.Fatal("malformed login request must be rejected") + } + for _, observed := range []string{response.Body.String(), wrong.Body.String(), + malformedResponse.Body.String(), logs.String()} { + for _, sensitive := range []string{nativePasswordFixture, rotated, cookie.Value, serviceToken} { + if strings.Contains(observed, sensitive) { + t.Fatal("responses and logs must not expose credentials") + } + } + } +} + +func nativeRouteSentinel(t *testing.T, filters ...gin.HandlerFunc) *common.Sentinel { + t.Helper() + previousMetrics := common.PrometheusMetrics + previousRegisterer, previousGatherer := prometheus.DefaultRegisterer, prometheus.DefaultGatherer + previousMode := gin.Mode() + registry := prometheus.NewRegistry() + prometheus.DefaultRegisterer, prometheus.DefaultGatherer = registry, registry + t.Cleanup(func() { + common.PrometheusMetrics = previousMetrics + prometheus.DefaultRegisterer, prometheus.DefaultGatherer = previousRegisterer, previousGatherer + gin.SetMode(previousMode) + }) + sentinel := &common.Sentinel{} + sentinel.Init("127.0.0.1:0", "release") + SetRouters(sentinel, filters...) + return sentinel +} + +func TestNativeStatusRoutesRequireConfiguredAuthentication(t *testing.T) { + _, serviceToken := configureNativePassword(t) + factory, err := auth.NewSecureTokenFactory(nativeSigningFixture) + if err != nil { + t.Fatal(err) + } + reader := factory.NewToken("space-user", "own-space", "status-test") + factory.Sign(reader) + readerJSON, err := json.Marshal(reader) + if err != nil { + t.Fatal(err) + } + readerToken := auth.ToBase58(string(readerJSON)) + sentinel := nativeRouteSentinel(t, auth.TokenFilter) + login := postNativeCredential(t, sentinel, nativePasswordFixture, "") + if login.Code != http.StatusOK { + t.Fatal("native login route must remain reachable") + } + cookie := login.Result().Cookies()[0] + for _, path := range []string{"/workers", "/api/v1/workers", "/master", "/api/v1/master"} { + for _, test := range []struct { + name, bearer string + cookie *http.Cookie + want int + }{ + {name: "anonymous", want: http.StatusUnauthorized}, + {name: "raw-password", bearer: nativePasswordFixture, want: http.StatusUnauthorized}, + {name: "service-token", bearer: serviceToken, want: http.StatusOK}, + {name: "space-token", bearer: readerToken, want: http.StatusOK}, + {name: "password-session", cookie: cookie, want: http.StatusOK}, + } { + t.Run(path+"/"+test.name, func(t *testing.T) { + request := httptest.NewRequest(http.MethodGet, path, nil) + if test.bearer != "" { + request.Header.Set(auth.HeaderKey, "Bearer "+test.bearer) + } + if test.cookie != nil { + request.AddCookie(test.cookie) + } + response := httptest.NewRecorder() + sentinel.ServeHTTP(response, request) + if response.Code != test.want { + t.Fatalf("status = %d, want %d", response.Code, test.want) + } + }) + } + } + health := httptest.NewRecorder() + sentinel.ServeHTTP(health, httptest.NewRequest(http.MethodGet, "/api/v1/healthcheck", nil)) + if health.Code != http.StatusOK { + t.Fatal("health probes must remain public") + } +} + +func TestNativeStatusRoutesRetainExplicitNoneMode(t *testing.T) { + sentinel := nativeRouteSentinel(t, auth.NoneAuthFilter) + for _, path := range []string{"/api/v1/workers", "/api/v1/master"} { + response := httptest.NewRecorder() + sentinel.ServeHTTP(response, httptest.NewRequest(http.MethodGet, path, nil)) + if response.Code != http.StatusOK { + t.Fatal("explicit none mode must retain its historical route behavior") + } + } +} diff --git a/vermeer/apps/master/services/http_login_v2_test.go b/vermeer/apps/master/services/http_login_v2_test.go new file mode 100644 index 000000000..fa0211812 --- /dev/null +++ b/vermeer/apps/master/services/http_login_v2_test.go @@ -0,0 +1,130 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 services + +import ( + "bytes" + "encoding/json" + "math" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "vermeer/apps/auth" + "vermeer/apps/common" + + "github.com/gin-gonic/gin" +) + +func TestNativeLoginCookieUsesSecureDeploymentToken(t *testing.T) { + previousMode := gin.Mode() + previousFactor := common.GetConfig("auth_token_factor") + t.Cleanup(func() { gin.SetMode(previousMode); common.SetConfig("auth_token_factor", previousFactor) }) + gin.SetMode(gin.TestMode) + const key = "synthetic-native-ui-signing-key" + factorFile := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(factorFile, []byte(key), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factorFile) + common.SetConfig("auth_token_factor", key) + if err := auth.Init(); err != nil { + t.Fatal(err) + } + secure, err := auth.NewSecureTokenFactory(key) + if err != nil { + t.Fatal(err) + } + token := secure.NewToken("admin", "$DEFAULT", "native-ui") + token.Time = math.MaxInt64 + secure.Sign(token) + router := gin.New() + router.POST("/api/v1/login", (&LoginHandler{}).POST) + router.GET("/protected", auth.TokenFilter, auth.AdminFilter, func(ctx *gin.Context) { ctx.Status(http.StatusOK) }) + authenticate := func(token *auth.Token, want int) { + t.Helper() + encoded, err := json.Marshal(token) + if err != nil { + t.Fatal(err) + } + body, err := json.Marshal(map[string]string{"token": auth.ToBase58(string(encoded))}) + if err != nil { + t.Fatal(err) + } + login := httptest.NewRequest(http.MethodPost, "/api/v1/login", bytes.NewReader(body)) + login.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + router.ServeHTTP(response, login) + if want != http.StatusOK { + if response.Code != http.StatusUnauthorized || len(response.Result().Cookies()) != 0 { + t.Fatal("invalid token login must reject before issuing a cookie") + } + return + } + if response.Code != http.StatusOK { + t.Fatalf("native login status = %d", response.Code) + } + cookies := response.Result().Cookies() + if len(cookies) != 1 || !cookies[0].HttpOnly { + t.Fatal("native login must set its protected session cookie") + } + request := httptest.NewRequest(http.MethodGet, "/protected", nil) + request.AddCookie(cookies[0]) + checked := httptest.NewRecorder() + router.ServeHTTP(checked, request) + if checked.Code != want { + t.Fatalf("cookie authentication status = %d, want %d", checked.Code, want) + } + } + authenticate(token, http.StatusOK) + tampered := *token + tampered.Space = "another-space" + authenticate(&tampered, http.StatusUnauthorized) + legacy := &auth.TokenFactory{Factor: key} + legacy.Sign(token) + authenticate(token, http.StatusUnauthorized) +} + +func TestNativeLegacyTokenLoginRemainsSupported(t *testing.T) { + for _, key := range []string{"auth_token_factor", "auth_token_factor_file", "admin_password_file"} { + previous := common.GetConfigDefault(key, "") + common.SetConfig(key, "") + t.Cleanup(func() { common.SetConfig(key, previous) }) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", "") + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", "") + const factor = "synthetic-legacy-inline-factor" + common.SetConfig("auth_token_factor", factor) + if err := auth.Init(); err != nil { + t.Fatal(err) + } + factory := &auth.TokenFactory{Factor: factor} + token := factory.NewToken("admin", "$DEFAULT", "legacy-client") + factory.Sign(token) + encoded, err := json.Marshal(token) + if err != nil { + t.Fatal(err) + } + router := gin.New() + router.POST("/api/v1/login", (&LoginHandler{}).POST) + response := postNativeCredential(t, router, auth.ToBase58(string(encoded)), "") + if response.Code != http.StatusOK || len(response.Result().Cookies()) != 1 { + t.Fatal("valid historical inline-token login must remain supported") + } +} diff --git a/vermeer/apps/master/services/router.go b/vermeer/apps/master/services/router.go index 2c3887ec5..7474d1138 100644 --- a/vermeer/apps/master/services/router.go +++ b/vermeer/apps/master/services/router.go @@ -62,7 +62,7 @@ func SetRouters(sen *common.Sentinel, authFilters ...gin.HandlerFunc) { // /workers regVerAPI(sen, 1, "/workers", map[string]common.BaseHandler{ "": &WorkersHandler{}, - }) + }, authFilters...) regVerAPI(sen, 1, "/workers", map[string]common.BaseHandler{ "/availability": &WorkerAvailabilityHandler{}, }, authFilters...) @@ -70,7 +70,7 @@ func SetRouters(sen *common.Sentinel, authFilters ...gin.HandlerFunc) { // /master regVerAPI(sen, 1, "/master", map[string]common.BaseHandler{ "": &InfoMasterHandler{}, - }) + }, authFilters...) } diff --git a/vermeer/apps/master/worker_stream_metadata.go b/vermeer/apps/master/worker_stream_metadata.go new file mode 100644 index 000000000..984a176d7 --- /dev/null +++ b/vermeer/apps/master/worker_stream_metadata.go @@ -0,0 +1,69 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 master + +import ( + "context" + "strings" + "vermeer/apps/master/workers" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +var registeredWorkerStreamMethods = map[string]struct{}{ + "/master.Master/LoadGraphTask": {}, + "/master.Master/ComputeTask": {}, + "/master.Master/SuperStep": {}, +} + +func validateWorkerStreamMetadata(ctx context.Context, fullMethod string) error { + if _, required := registeredWorkerStreamMethods[fullMethod]; !required { + return nil + } + + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return status.Error(codes.InvalidArgument, + "worker_name metadata is required") + } + names := md.Get("worker_name") + if len(names) != 1 || names[0] == "" || strings.TrimSpace(names[0]) != names[0] { + return status.Error(codes.InvalidArgument, + "worker_name metadata must contain exactly one non-empty value") + } + if workers.WorkerManager.GetWorker(names[0]) == nil { + return status.Error(codes.FailedPrecondition, + "worker must register before opening task streams") + } + return nil +} + +func validateWorkerStream( + srv any, + stream grpc.ServerStream, + info *grpc.StreamServerInfo, + handler grpc.StreamHandler, +) error { + if err := validateWorkerStreamMetadata(stream.Context(), info.FullMethod); err != nil { + return err + } + return handler(srv, stream) +} diff --git a/vermeer/apps/master/worker_stream_metadata_test.go b/vermeer/apps/master/worker_stream_metadata_test.go new file mode 100644 index 000000000..35fd1c765 --- /dev/null +++ b/vermeer/apps/master/worker_stream_metadata_test.go @@ -0,0 +1,72 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 master + +import ( + "context" + "testing" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +func TestValidateWorkerStreamMetadataRejectsInvalidIdentity(t *testing.T) { + methods := []string{ + "/master.Master/LoadGraphTask", + "/master.Master/ComputeTask", + "/master.Master/SuperStep", + } + invalid := []struct { + name string + ctx context.Context + }{ + {"missing", context.Background()}, + {"empty", metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", ""))}, + {"padded", metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", " worker "))}, + {"duplicate", metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", "one", "worker_name", "two"))}, + } + + for _, method := range methods { + for _, test := range invalid { + t.Run(method+"/"+test.name, func(t *testing.T) { + if code := status.Code(validateWorkerStreamMetadata(test.ctx, method)); + code != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got %s", code) + } + }) + } + + unknown := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", "missing-worker-metadata-test")) + if code := status.Code(validateWorkerStreamMetadata(unknown, method)); + code != codes.FailedPrecondition { + t.Fatalf("%s: expected FailedPrecondition, got %s", method, code) + } + } +} + +func TestValidateWorkerStreamMetadataIgnoresOtherRPCs(t *testing.T) { + if err := validateWorkerStreamMetadata(context.Background(), + "/master.Master/FetchLoadPart"); err != nil { + t.Fatalf("unrelated RPC must not require stream metadata: %v", err) + } +} diff --git a/vermeer/apps/master/workers/bootstrap_workers_test.go b/vermeer/apps/master/workers/bootstrap_workers_test.go new file mode 100644 index 000000000..85cc23f5b --- /dev/null +++ b/vermeer/apps/master/workers/bootstrap_workers_test.go @@ -0,0 +1,169 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 workers + +import ( + "context" + "errors" + "net" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials/insecure" + "vermeer/apps/common" + "vermeer/apps/storage" +) + +func TestBootstrapExcludesEndedSessionsWithoutDroppingNewPeers(t *testing.T) { + connection := readyWorkerConnection(t) + fresh := &WorkerClient{Name: "fresh", Id: 1} + active := &WorkerClient{Name: "active", Id: 2, Connection: connection, taskStreams: readyTaskStreams()} + ended := &WorkerClient{Name: "ended", Id: 3, Connection: connection, taskStreams: readyTaskStreams()} + canceledCtx, cancel := context.WithCancel(context.Background()) + canceled := &WorkerClient{Name: "canceled", Id: 4, Connection: connection, taskStreams: readyTaskStreams()} + canceled.taskStreams[LoadTaskStream] = canceledCtx + manager := workerManager{workersByName: map[string]*WorkerClient{ + fresh.Name: fresh, active.Name: active, ended.Name: ended, canceled.Name: canceled, + }} + manager.ClearTaskStream(ended, LoadTaskStream, ended.taskStreams[LoadTaskStream]) + cancel() + peers := manager.GetBootstrapWorkers() + if len(peers) != 2 || peers[0] != fresh || peers[1] != active { + t.Fatal("bootstrap must preserve fresh/active peers and omit ended streams even with ready transport") + } + if manager.CheckWorkerAlive(ended.Name) { + t.Fatal("an ended session cannot remain alive") + } +} + +func TestOfflineCleanupCannotTouchReplacementSession(t *testing.T) { + previous := common.PrometheusMetrics.WorkerCnt + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_session_cleanup"}, nil) + t.Cleanup(func() { common.PrometheusMetrics.WorkerCnt = previous }) + connection := readyWorkerConnection(t) + old := &WorkerClient{Name: "reused", Connection: connection, taskStreams: readyTaskStreams()} + manager := workerManager{workersByName: map[string]*WorkerClient{old.Name: old}} + oldCtx := old.taskStreams[LoadTaskStream] + replacementCtx, cancel := context.WithCancel(context.Background()) + defer cancel() + manager.SetTaskStream(old, LoadTaskStream, replacementCtx) + manager.ClearTaskStream(old, LoadTaskStream, oldCtx) + if len(manager.GetBootstrapWorkers()) != 1 { + t.Fatal("old stream EOF must not terminate its replacement") + } + manager.ClearTaskStream(old, LoadTaskStream, replacementCtx) + var claimed atomic.Int32 + var wait sync.WaitGroup + for range 8 { + wait.Add(1) + go func() { + defer wait.Done() + if manager.ClaimOfflineSession(old) { + claimed.Add(1) + } + }() + } + wait.Wait() + if manager.CanReplaceDisconnectedSession(old) { + t.Fatal("an already claimed cleanup must wait rather than be reclaimed") + } + if claimed.Load() != 1 { + t.Fatal("only one EOF handler may clean up a session") + } + newer := &WorkerClient{Name: old.Name, Connection: connection, taskStreams: readyTaskStreams()} + manager.workersByName[old.Name] = newer + manager.RemoveWorkerSession(old) + manager.ClearTaskStream(old, LoadTaskStream, replacementCtx) + if manager.GetWorker(newer.Name) != newer || !manager.IsWorkerReady(newer) { + t.Fatal("late cleanup or EOF must not remove or terminate a new registration") + } +} + +func TestAbandonedRegistrationCanRegisterAgain(t *testing.T) { + previous := common.PrometheusMetrics.WorkerCnt + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_abandoned_registration"}, nil) + t.Cleanup(func() { common.PrometheusMetrics.WorkerCnt = previous }) + connection, err := grpc.NewClient("passthrough:///abandoned", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return nil, errors.New("worker gone") })) + if err != nil { + t.Fatal(err) + } + defer connection.Close() + connection.Connect() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + for connection.GetState() != connectivity.TransientFailure { + state := connection.GetState() + if !connection.WaitForStateChange(ctx, state) { + t.Fatal("expected a disconnected reverse transport") + } + } + store, err := storage.NewPebbleStore(storage.StoreOption{Path: t.TempDir()}) + if err != nil { + t.Fatal(err) + } + defer store.Close() + manager := workerManager{workersByName: map[string]*WorkerClient{}, idSeed: 1, store: store} + old := &WorkerClient{Name: "persisted-name", GrpcPeer: "127.0.0.1:6789", Connection: connection} + if _, err := manager.AddWorker(old); err != nil { + t.Fatal(err) + } + if !manager.IsAbandonedRegistration(old) || !manager.CanReplaceDisconnectedSession(old) { + t.Fatal("disconnected registration without a lifeline must be reclaimable") + } + fresh := &WorkerClient{Name: "new"} + manager.workersByName[fresh.Name] = fresh + if manager.IsAbandonedRegistration(fresh) || manager.CanReplaceDisconnectedSession(fresh) { + t.Fatal("an in-progress registration without a reverse connection is not abandoned") + } + active := &WorkerClient{Name: "active", Connection: connection, taskStreams: readyTaskStreams()} + manager.workersByName[active.Name] = active + if manager.IsAbandonedRegistration(active) { + t.Fatal("a peer with live task streams cannot be skipped merely for transient transport failure") + } + oldStream := active.taskStreams[LoadTaskStream] + replacementStream, done := context.WithCancel(context.Background()) + defer done() + manager.SetTaskStream(active, LoadTaskStream, replacementStream) + manager.ClearTaskStream(active, LoadTaskStream, oldStream) + if manager.ClaimOfflineSession(active) || manager.CanReplaceDisconnectedSession(active) { + t.Fatal("an obsolete stream EOF must not claim replacement live streams even during transient transport failure") + } + + if !manager.ClaimOfflineSession(old) { + t.Fatal("old abandoned registration must be claimed exactly once") + } + manager.RemoveWorkerSession(old) + replacement, err := manager.CreateWorker(old.GrpcPeer, "127.0.0.1", "test-version", "$") + if err != nil { + t.Fatal(err) + } + replacement.Connection = readyWorkerConnection(t) + if _, err := manager.AddWorker(replacement); err != nil { + t.Fatalf("same-address re-registration is blocked by a ghost: %v", err) + } + if replacement.Name != old.Name || manager.GetWorker(old.Name) != replacement { + t.Fatal("re-registration must preserve its stored identity with a new session") + } +} diff --git a/vermeer/apps/master/workers/worker_manager.go b/vermeer/apps/master/workers/worker_manager.go index 53773c3fc..52825a1b5 100644 --- a/vermeer/apps/master/workers/worker_manager.go +++ b/vermeer/apps/master/workers/worker_manager.go @@ -64,9 +64,11 @@ type WorkerClient struct { //LoadStream pb.Master_LoadGraphTaskServer `json:"-"` //ComputeStream pb.Master_ComputeTaskServer `json:"-"` //SuperStepStream pb.Master_SuperStepServer `json:"-"` - Connection *grpc.ClientConn `json:"-"` - Session pb.WorkerClient `json:"-"` - taskStreams [taskStreamCount]context.Context + Connection *grpc.ClientConn `json:"-"` + Session pb.WorkerClient `json:"-"` + taskStreams [taskStreamCount]context.Context + taskStreamsEnded [taskStreamCount]bool + offlineCleanup bool } func (wc *WorkerClient) Close() { @@ -233,11 +235,86 @@ func (wm *workerManager) GetAllWorkers() []*WorkerClient { return SortWorkersAsc(map2Workers(wm.workersByName)) } +// GetBootstrapWorkers retains newly registering peers, but excludes terminated sessions. +func (wm *workerManager) GetBootstrapWorkers() []*WorkerClient { + defer wm.Unlock(wm.Lock()) + peers := make([]*WorkerClient, 0, len(wm.workersByName)) + for _, worker := range wm.workersByName { + if !wm.sessionEndedLocked(worker) { + peers = append(peers, worker) + } + } + return SortWorkersAsc(peers) +} + +func (wm *workerManager) sessionEndedLocked(worker *WorkerClient) bool { + if worker.offlineCleanup || wm.abandonedRegistrationLocked(worker) || (worker.Connection != nil && worker.Connection.GetState() == connectivity.Shutdown) { + return true + } + for stream, ctx := range worker.taskStreams { + if worker.taskStreamsEnded[stream] || (ctx != nil && ctx.Err() != nil) { + return true + } + } + return false +} + +// CanReplaceDisconnectedSession requires a completed reverse dial and observed disconnection. +func (wm *workerManager) CanReplaceDisconnectedSession(worker *WorkerClient) bool { + defer wm.Unlock(wm.Lock()) + return worker != nil && wm.workersByName[worker.Name] == worker && !worker.offlineCleanup && worker.Connection != nil && + wm.sessionEndedLocked(worker) +} + +// A registration that never opened its lifeline cannot recover after its transport is lost. +func (wm *workerManager) IsAbandonedRegistration(worker *WorkerClient) bool { + defer wm.Unlock(wm.Lock()) + return worker != nil && wm.workersByName[worker.Name] == worker && wm.abandonedRegistrationLocked(worker) +} + +func (wm *workerManager) abandonedRegistrationLocked(worker *WorkerClient) bool { + if worker.Connection == nil { + return false + } + state := worker.Connection.GetState() + if state != connectivity.TransientFailure && state != connectivity.Shutdown { + return false + } + for stream, ctx := range worker.taskStreams { + if ctx != nil || worker.taskStreamsEnded[stream] { + return false + } + } + return true +} + +// ClaimOfflineSession gives only one old stream ownership of its session cleanup. +func (wm *workerManager) ClaimOfflineSession(worker *WorkerClient) bool { + defer wm.Unlock(wm.Lock()) + if worker == nil || wm.workersByName[worker.Name] != worker || worker.offlineCleanup { + return false + } + if !wm.sessionEndedLocked(worker) { + return false + } + worker.offlineCleanup = true + return true +} + +// RemoveWorkerSession cannot remove a new registration that reuses the same name. +func (wm *workerManager) RemoveWorkerSession(worker *WorkerClient) { + defer wm.Unlock(wm.Lock()) + if worker != nil && wm.workersByName[worker.Name] == worker { + delete(wm.workersByName, worker.Name) + common.PrometheusMetrics.WorkerCnt.WithLabelValues().Dec() + } +} + func (wm *workerManager) CheckWorkerAlive(name string) bool { defer wm.Unlock(wm.Lock()) worker, ok := wm.workersByName[name] - if !ok || worker.Connection == nil || worker.Connection.GetState() != connectivity.Ready { + if !ok || wm.sessionEndedLocked(worker) || worker.Connection == nil || worker.Connection.GetState() != connectivity.Ready { return false } @@ -255,6 +332,7 @@ func (wm *workerManager) SetTaskStream(worker *WorkerClient, stream TaskStream, defer wm.Unlock(wm.Lock()) if worker != nil && wm.workersByName[worker.Name] == worker { worker.taskStreams[stream] = ctx + worker.taskStreamsEnded[stream] = false } } @@ -262,6 +340,7 @@ func (wm *workerManager) ClearTaskStream(worker *WorkerClient, stream TaskStream defer wm.Unlock(wm.Lock()) if worker != nil && wm.workersByName[worker.Name] == worker && worker.taskStreams[stream] == ctx { worker.taskStreams[stream] = nil + worker.taskStreamsEnded[stream] = true } } @@ -271,7 +350,7 @@ func (wm *workerManager) IsWorkerReady(worker *WorkerClient) bool { } func (wm *workerManager) workerReadyLocked(worker *WorkerClient) bool { - if worker == nil || wm.workersByName[worker.Name] != worker || + if worker == nil || wm.workersByName[worker.Name] != worker || worker.offlineCleanup || worker.Connection == nil || worker.Connection.GetState() != connectivity.Ready { return false } diff --git a/vermeer/apps/master/workers/worker_util.go b/vermeer/apps/master/workers/worker_util.go index 6b0fa3ed9..eb507c2d4 100644 --- a/vermeer/apps/master/workers/worker_util.go +++ b/vermeer/apps/master/workers/worker_util.go @@ -82,7 +82,11 @@ func toMemoryLimitReq(limit *WorkerMemLimit) *pb.SetMemoryLimitReq { } -func doSendMemoryLimitReq(worker *WorkerClient, req *pb.SetMemoryLimitReq) (resp *pb.SetMemoryLimitResp, err error) { +func doSendMemoryLimitReq(worker *WorkerClient, req *pb.SetMemoryLimitReq) (*pb.SetMemoryLimitResp, error) { + return doSendMemoryLimitReqContext(context.Background(), worker, req) +} + +func doSendMemoryLimitReqContext(ctx context.Context, worker *WorkerClient, req *pb.SetMemoryLimitReq) (resp *pb.SetMemoryLimitResp, err error) { if worker == nil || req == nil { return nil, nil } @@ -98,7 +102,7 @@ func doSendMemoryLimitReq(worker *WorkerClient, req *pb.SetMemoryLimitReq) (resp } var response *pb.RuntimeActionResp - if response, err = worker.Session.RuntimeAction(context.Background(), request); err != nil { + if response, err = worker.Session.RuntimeAction(ctx, request); err != nil { return nil, err } diff --git a/vermeer/apps/master/workers/workers.go b/vermeer/apps/master/workers/workers.go index 963167040..dd2bcc438 100644 --- a/vermeer/apps/master/workers/workers.go +++ b/vermeer/apps/master/workers/workers.go @@ -18,6 +18,7 @@ under the License. package workers import ( + "context" "errors" "fmt" @@ -29,6 +30,10 @@ var WorkerManager = &workerManager{} var manager = WorkerManager func SendMemLimit(worker *WorkerClient, memLimit *WorkerMemLimit) error { + return SendMemLimitContext(context.Background(), worker, memLimit) +} + +func SendMemLimitContext(ctx context.Context, worker *WorkerClient, memLimit *WorkerMemLimit) error { if worker == nil { return errors.New("the argument `worker` cannot be nil") } @@ -36,7 +41,7 @@ func SendMemLimit(worker *WorkerClient, memLimit *WorkerMemLimit) error { return errors.New("the argument `memLimit` cannot be empty") } - if _, err := doSendMemoryLimitReq(worker, toMemoryLimitReq(memLimit)); err != nil { + if _, err := doSendMemoryLimitReqContext(ctx, worker, toMemoryLimitReq(memLimit)); err != nil { return fmt.Errorf("failed to send memory limit req to worker %s: %w", worker.Name, err) } logrus.Infof("sent memory limit '%s' via group '%s' to worker %s@%s", memLimit.toString(), worker.Group, worker.Name, worker.GrpcPeer) diff --git a/vermeer/apps/worker/grpc_handlers.go b/vermeer/apps/worker/grpc_handlers.go index 87d38b05d..9bb3bb5e4 100644 --- a/vermeer/apps/worker/grpc_handlers.go +++ b/vermeer/apps/worker/grpc_handlers.go @@ -67,36 +67,49 @@ func (ph *PeerHandler) SayHelloPeer(ctx context.Context, req *pb.HelloPeerReq) ( return &pb.HelloPeerResp{}, nil } +func incomingPeer(ctx context.Context) (string, *PeerClient, error) { + md, _ := metadata.FromIncomingContext(ctx) + names := md.Get("worker_name") + if len(names) != 1 || names[0] == "" { + return "", nil, status.Error(codes.InvalidArgument, "worker_name metadata is required") + } + peer := PeerMgr.GetPeer(names[0]) + if peer == nil { + return "", nil, status.Error(codes.FailedPrecondition, "peer must register before opening streams") + } + return names[0], peer, nil +} + func (ph *PeerHandler) Scatter(stream pb.Worker_ScatterServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).ScatterHandler.SetServer(stream) - PeerMgr.GetPeer(name).ScatterHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.ScatterHandler.receiveServer(name, stream) } func (ph *PeerHandler) LoadAction(stream pb.Worker_LoadActionServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).LoadActionHandler.SetServer(stream) - PeerMgr.GetPeer(name).LoadActionHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.LoadActionHandler.receiveServer(name, stream) } func (ph *PeerHandler) StepEnd(stream pb.Worker_StepEndServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).StepEndHandler.SetServer(stream) - PeerMgr.GetPeer(name).StepEndHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.StepEndHandler.receiveServer(name, stream) } func (ph *PeerHandler) SettingAction(stream pb.Worker_SettingActionServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).SettingActionHandler.SetServer(stream) - PeerMgr.GetPeer(name).SettingActionHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.SettingActionHandler.receiveServer(name, stream) } func (ph *PeerHandler) DeleteGraph(ctx context.Context, req *pb.DeleteGraphReq) (*pb.DeleteGraphResp, error) { @@ -404,36 +417,67 @@ const ( type LoadActionHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_LoadActionServer grpcClient pb.Worker_LoadActionClient locker sync.Mutex } -func (dh *LoadActionHandler) SetServer(stream pb.Worker_LoadActionServer) { - dh.mode = HandlerModeServer - dh.grpcServer = stream +func (dh *LoadActionHandler) SetServer(stream pb.Worker_LoadActionServer) uint64 { + return dh.setServer(stream, nil) } - -func (dh *LoadActionHandler) SetClient(client pb.Worker_LoadActionClient) { - dh.mode = HandlerModeClient - dh.grpcClient = client +func (dh *LoadActionHandler) setServer(stream pb.Worker_LoadActionServer, cancel context.CancelFunc) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerLoadStream, cancel) + dh.mode, dh.grpcServer, dh.grpcClient = HandlerModeServer, stream, nil + return dh.generation +} +func (dh *LoadActionHandler) SetClient(stream pb.Worker_LoadActionClient) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerLoadStream, nil) + dh.mode, dh.grpcClient, dh.grpcServer = HandlerModeClient, stream, nil + return dh.generation +} +func (dh *LoadActionHandler) IsClient() bool { + dh.locker.Lock() + defer dh.locker.Unlock() + return dh.mode == HandlerModeClient +} +func (dh *LoadActionHandler) receiveServer(name string, stream pb.Worker_LoadActionServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := dh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { dh.recv(name, dh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (dh *LoadActionHandler) RecvHandler(name string) { + dh.locker.Lock() + owner, generation, mode, server, client := dh.peer, dh.generation, dh.mode, dh.grpcServer, dh.grpcClient + dh.locker.Unlock() + dh.recv(name, owner, generation, mode, server, client) +} + +func (dh *LoadActionHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_LoadActionServer, client pb.Worker_LoadActionClient) { logrus.Infof("load action recv handler setup: %s", name) - dh.locker = sync.Mutex{} loadBl := LoadGraphBl{} - if dh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := dh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv load action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerLoadStream, generation) + return } //logrus.Infof("LoadActionHandler server recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.LoadAction_LoadVertex { @@ -467,17 +511,13 @@ func (dh *LoadActionHandler) RecvHandler(name string) { resp.Data) } } - } else if dh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := dh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv load action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerLoadStream, generation) + return } //logrus.Infof("LoadActionHandler client recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.LoadAction_LoadVertex { @@ -552,50 +592,77 @@ func (dh *LoadActionHandler) LoadAction(taskId int32, action pb.LoadAction, coun type ScatterHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_ScatterServer grpcClient pb.Worker_ScatterClient locker sync.Mutex } -func (sh *ScatterHandler) SetServer(stream pb.Worker_ScatterServer) { - sh.mode = HandlerModeServer - sh.grpcServer = stream +func (sh *ScatterHandler) SetServer(stream pb.Worker_ScatterServer) uint64 { + return sh.setServer(stream, nil) } - -func (sh *ScatterHandler) SetClient(client pb.Worker_ScatterClient) { - sh.mode = HandlerModeClient - sh.grpcClient = client +func (sh *ScatterHandler) setServer(stream pb.Worker_ScatterServer, cancel context.CancelFunc) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerScatterStream, cancel) + sh.mode, sh.grpcServer, sh.grpcClient = HandlerModeServer, stream, nil + return sh.generation +} +func (sh *ScatterHandler) SetClient(stream pb.Worker_ScatterClient) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerScatterStream, nil) + sh.mode, sh.grpcClient, sh.grpcServer = HandlerModeClient, stream, nil + return sh.generation +} +func (sh *ScatterHandler) IsClient() bool { + sh.locker.Lock() + defer sh.locker.Unlock() + return sh.mode == HandlerModeClient +} +func (sh *ScatterHandler) receiveServer(name string, stream pb.Worker_ScatterServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := sh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { sh.recv(name, sh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (sh *ScatterHandler) RecvHandler(name string) { + sh.locker.Lock() + owner, generation, mode, server, client := sh.peer, sh.generation, sh.mode, sh.grpcServer, sh.grpcClient + sh.locker.Unlock() + sh.recv(name, owner, generation, mode, server, client) +} + +func (sh *ScatterHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_ScatterServer, client pb.Worker_ScatterClient) { logrus.Infof("scatter recv handler setup: %s", name) - sh.locker = sync.Mutex{} - if sh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := sh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv scatter error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerScatterStream, generation) + return } cb := ComputeBl{} go cb.RecvScatter(resp.TaskId, resp.Data, resp.End, resp.SIdx) } - } else if sh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := sh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv scatter error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerScatterStream, generation) + return } cb := ComputeBl{} go cb.RecvScatter(resp.TaskId, resp.Data, resp.End, resp.SIdx) @@ -639,50 +706,77 @@ func (sh *ScatterHandler) SendScatter(taskId int32, step int32, count int32, end type StepEndHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_StepEndServer grpcClient pb.Worker_StepEndClient locker sync.Mutex } -func (sh *StepEndHandler) SetServer(stream pb.Worker_StepEndServer) { - sh.mode = HandlerModeServer - sh.grpcServer = stream +func (sh *StepEndHandler) SetServer(stream pb.Worker_StepEndServer) uint64 { + return sh.setServer(stream, nil) } - -func (sh *StepEndHandler) SetClient(client pb.Worker_StepEndClient) { - sh.mode = HandlerModeClient - sh.grpcClient = client +func (sh *StepEndHandler) setServer(stream pb.Worker_StepEndServer, cancel context.CancelFunc) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerStepStream, cancel) + sh.mode, sh.grpcServer, sh.grpcClient = HandlerModeServer, stream, nil + return sh.generation +} +func (sh *StepEndHandler) SetClient(stream pb.Worker_StepEndClient) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerStepStream, nil) + sh.mode, sh.grpcClient, sh.grpcServer = HandlerModeClient, stream, nil + return sh.generation +} +func (sh *StepEndHandler) IsClient() bool { + sh.locker.Lock() + defer sh.locker.Unlock() + return sh.mode == HandlerModeClient +} +func (sh *StepEndHandler) receiveServer(name string, stream pb.Worker_StepEndServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := sh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { sh.recv(name, sh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (sh *StepEndHandler) RecvHandler(name string) { + sh.locker.Lock() + owner, generation, mode, server, client := sh.peer, sh.generation, sh.mode, sh.grpcServer, sh.grpcClient + sh.locker.Unlock() + sh.recv(name, owner, generation, mode, server, client) +} + +func (sh *StepEndHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_StepEndServer, client pb.Worker_StepEndClient) { logrus.Infof("step end recv handler setup: %s", name) - sh.locker = sync.Mutex{} - if sh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := sh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv step end error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerStepStream, generation) + return } cb := ComputeBl{} go cb.StepEnd(resp.TaskId, resp.WorkerName) } - } else if sh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := sh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv step end error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerStepStream, generation) + return } cb := ComputeBl{} go cb.StepEnd(resp.TaskId, resp.WorkerName) @@ -716,36 +810,67 @@ func (sh *StepEndHandler) SendStepEnd(taskId int32) { type SettingActionHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_SettingActionServer grpcClient pb.Worker_SettingActionClient locker sync.Mutex } -func (dh *SettingActionHandler) SetServer(stream pb.Worker_SettingActionServer) { - dh.mode = HandlerModeServer - dh.grpcServer = stream +func (dh *SettingActionHandler) SetServer(stream pb.Worker_SettingActionServer) uint64 { + return dh.setServer(stream, nil) } - -func (dh *SettingActionHandler) SetClient(client pb.Worker_SettingActionClient) { - dh.mode = HandlerModeClient - dh.grpcClient = client +func (dh *SettingActionHandler) setServer(stream pb.Worker_SettingActionServer, cancel context.CancelFunc) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerSettingStream, cancel) + dh.mode, dh.grpcServer, dh.grpcClient = HandlerModeServer, stream, nil + return dh.generation +} +func (dh *SettingActionHandler) SetClient(stream pb.Worker_SettingActionClient) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerSettingStream, nil) + dh.mode, dh.grpcClient, dh.grpcServer = HandlerModeClient, stream, nil + return dh.generation +} +func (dh *SettingActionHandler) IsClient() bool { + dh.locker.Lock() + defer dh.locker.Unlock() + return dh.mode == HandlerModeClient +} +func (dh *SettingActionHandler) receiveServer(name string, stream pb.Worker_SettingActionServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := dh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { dh.recv(name, dh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (dh *SettingActionHandler) RecvHandler(name string) { + dh.locker.Lock() + owner, generation, mode, server, client := dh.peer, dh.generation, dh.mode, dh.grpcServer, dh.grpcClient + dh.locker.Unlock() + dh.recv(name, owner, generation, mode, server, client) +} + +func (dh *SettingActionHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_SettingActionServer, client pb.Worker_SettingActionClient) { logrus.Infof("load action recv handler setup: %s", name) - dh.locker = sync.Mutex{} settingBl := SettingBl{} - if dh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := dh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv setting action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerSettingStream, generation) + return } //logrus.Infof("LoadActionHandler server recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.SettingAction_SetOutEdges { @@ -764,17 +889,13 @@ func (dh *SettingActionHandler) RecvHandler(name string) { resp.Data) } } - } else if dh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := dh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv setting action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerSettingStream, generation) + return } //logrus.Infof("LoadActionHandler client recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.SettingAction_SetOutEdges { diff --git a/vermeer/apps/worker/peer_generation_test.go b/vermeer/apps/worker/peer_generation_test.go new file mode 100644 index 000000000..cd68623da --- /dev/null +++ b/vermeer/apps/worker/peer_generation_test.go @@ -0,0 +1,269 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 worker + +import ( + "context" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + "io" + "sync" + "testing" + "time" + "vermeer/apps/common" + pb "vermeer/apps/protos" +) + +type peerStreamFixture struct { + grpc.ServerStream + ctx context.Context + entered chan struct{} + release chan struct{} + enteredOnce sync.Once + releaseOnce sync.Once +} + +func (f *peerStreamFixture) Context() context.Context { return f.ctx } +func (f *peerStreamFixture) eof() error { + f.enteredOnce.Do(func() { close(f.entered) }) + <-f.release + return io.EOF +} +func (f *peerStreamFixture) finish() { f.releaseOnce.Do(func() { close(f.release) }) } +func newPeerStreamFixture(t *testing.T, ctx context.Context) *peerStreamFixture { + t.Helper() + f := &peerStreamFixture{ctx: ctx, entered: make(chan struct{}), release: make(chan struct{})} + t.Cleanup(f.finish) + return f +} +func peerTestMetrics(t *testing.T) { + t.Helper() + previous := common.PrometheusMetrics.WorkerCnt + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_peer_stream_generations"}, nil) + PeerMgr.Init() + t.Cleanup(func() { PeerMgr.Init(); common.PrometheusMetrics.WorkerCnt = previous }) +} +func peerCount(t *testing.T) float64 { + t.Helper() + metric := new(dto.Metric) + if err := common.PrometheusMetrics.WorkerCnt.WithLabelValues().Write(metric); err != nil { + t.Fatal(err) + } + return metric.GetGauge().GetValue() +} +func awaitPeerEvent(t *testing.T, ch <-chan struct{}) { + t.Helper() + select { + case <-ch: + case <-time.After(time.Second): + t.Fatal("peer stream did not reach expected lifecycle event") + } +} + +type testScatterServer struct{ *peerStreamFixture } + +func (f *testScatterServer) Recv() (*pb.ScatterReq, error) { return nil, f.eof() } +func (f *testScatterServer) Send(*pb.ScatterResp) error { return nil } + +type testLoadActionServer struct{ *peerStreamFixture } + +func (f *testLoadActionServer) Recv() (*pb.LoadActionReq, error) { return nil, f.eof() } +func (f *testLoadActionServer) Send(*pb.LoadActionResp) error { return nil } + +type testStepEndServer struct{ *peerStreamFixture } + +func (f *testStepEndServer) Recv() (*pb.StepEndReq, error) { return nil, f.eof() } +func (f *testStepEndServer) Send(*pb.StepEndResp) error { return nil } + +type testSettingActionServer struct{ *peerStreamFixture } + +func (f *testSettingActionServer) Recv() (*pb.SettingActionReq, error) { return nil, f.eof() } +func (f *testSettingActionServer) Send(*pb.SettingActionResp) error { return nil } + +type testScatterClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testScatterClient) Context() context.Context { return f.fixture.ctx } +func (f *testScatterClient) Recv() (*pb.ScatterResp, error) { return nil, f.fixture.eof() } +func (f *testScatterClient) Send(*pb.ScatterReq) error { return nil } + +type testLoadActionClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testLoadActionClient) Context() context.Context { return f.fixture.ctx } +func (f *testLoadActionClient) Recv() (*pb.LoadActionResp, error) { return nil, f.fixture.eof() } +func (f *testLoadActionClient) Send(*pb.LoadActionReq) error { return nil } + +type testStepEndClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testStepEndClient) Context() context.Context { return f.fixture.ctx } +func (f *testStepEndClient) Recv() (*pb.StepEndResp, error) { return nil, f.fixture.eof() } +func (f *testStepEndClient) Send(*pb.StepEndReq) error { return nil } + +type testSettingActionClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testSettingActionClient) Context() context.Context { return f.fixture.ctx } +func (f *testSettingActionClient) Recv() (*pb.SettingActionResp, error) { return nil, f.fixture.eof() } +func (f *testSettingActionClient) Send(*pb.SettingActionReq) error { return nil } + +type peerStreamTestCase struct { + name string + bind func(*PeerClient, *peerStreamFixture) func() + rpc func(*peerStreamFixture) error +} + +func peerStreamTestCases() []peerStreamTestCase { + return []peerStreamTestCase{ + {"Scatter", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.ScatterHandler.SetServer(&testScatterServer{f}) + return func() { peer.ScatterHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).Scatter(&testScatterServer{f}) }}, + {"LoadAction", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.LoadActionHandler.SetServer(&testLoadActionServer{f}) + return func() { peer.LoadActionHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).LoadAction(&testLoadActionServer{f}) }}, + {"StepEnd", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.StepEndHandler.SetServer(&testStepEndServer{f}) + return func() { peer.StepEndHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).StepEnd(&testStepEndServer{f}) }}, + {"SettingAction", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.SettingActionHandler.SetServer(&testSettingActionServer{f}) + return func() { peer.SettingActionHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).SettingAction(&testSettingActionServer{f}) }}, + } +} + +func peerClientTestCases() []peerStreamTestCase { + return []peerStreamTestCase{ + {"Scatter/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.ScatterHandler.SetClient(&testScatterClient{fixture: f}) + return func() { peer.ScatterHandler.RecvHandler(peer.Name) } + }, nil}, + {"LoadAction/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.LoadActionHandler.SetClient(&testLoadActionClient{fixture: f}) + return func() { peer.LoadActionHandler.RecvHandler(peer.Name) } + }, nil}, + {"StepEnd/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.StepEndHandler.SetClient(&testStepEndClient{fixture: f}) + return func() { peer.StepEndHandler.RecvHandler(peer.Name) } + }, nil}, + {"SettingAction/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.SettingActionHandler.SetClient(&testSettingActionClient{fixture: f}) + return func() { peer.SettingActionHandler.RecvHandler(peer.Name) } + }, nil}, + } +} + +func TestPeerOldEOFDoesNotRemoveNewRegistrationOrStream(t *testing.T) { + for _, test := range append(peerStreamTestCases(), peerClientTestCases()...) { + for _, replacePeer := range []bool{false, true} { + suffix := "rebound-stream" + if replacePeer { + suffix = "new-hello" + } + t.Run(test.name+"/"+suffix, func(t *testing.T) { + peerTestMetrics(t) + old := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + first := newPeerStreamFixture(t, context.Background()) + receive := test.bind(old, first) + done := make(chan struct{}) + go func() { receive(); close(done) }() + awaitPeerEvent(t, first.entered) + current := old + if replacePeer { + current = PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + } + second := newPeerStreamFixture(t, context.Background()) + receiveNew := test.bind(current, second) + first.finish() + awaitPeerEvent(t, done) + if PeerMgr.GetPeer("peer") != current || peerCount(t) != 1 { + t.Fatal("old EOF removed replacement registration/stream or inflated peer metric") + } + second.finish() + receiveNew() + if PeerMgr.GetPeer("peer") != nil || peerCount(t) != 0 { + t.Fatal("current EOF must retire exactly its own session") + } + }) + } + } +} + +func TestPeerRPCRequiresRegistrationMetadata(t *testing.T) { + for _, test := range peerStreamTestCases() { + t.Run(test.name, func(t *testing.T) { + peerTestMetrics(t) + missing := newPeerStreamFixture(t, context.Background()) + if status.Code(test.rpc(missing)) != codes.InvalidArgument { + t.Fatal("missing worker metadata must not panic") + } + unknown := newPeerStreamFixture(t, metadata.NewIncomingContext(context.Background(), metadata.Pairs("worker_name", "missing"))) + if status.Code(test.rpc(unknown)) != codes.FailedPrecondition { + t.Fatal("unknown worker must register before streaming") + } + }) + } +} + +func TestReplacingPeerCancelsOldServerRPC(t *testing.T) { + peerTestMetrics(t) + PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + fixture := newPeerStreamFixture(t, metadata.NewIncomingContext(context.Background(), metadata.Pairs("worker_name", "peer"))) + done := make(chan struct{}) + go func() { _ = new(PeerHandler).Scatter(&testScatterServer{fixture}); close(done) }() + awaitPeerEvent(t, fixture.entered) + replacement := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + awaitPeerEvent(t, done) + if PeerMgr.GetPeer("peer") != replacement { + t.Fatal("retiring RPC must preserve replacement peer") + } + fixture.finish() +} + +func TestPeerConnectionPublicationCannotReviveOldHello(t *testing.T) { + peerTestMetrics(t) + old := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + current := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + connection, err := grpc.NewClient("passthrough:///unused", grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatal(err) + } + if PeerMgr.SetPeerConnection(old, connection) || connection.GetState() != connectivity.Shutdown { + t.Fatal("late connection must be rejected and closed") + } + if PeerMgr.GetPeer("peer") != current || peerCount(t) != 1 { + t.Fatal("connection race must not change current registration") + } +} diff --git a/vermeer/apps/worker/peer_manager.go b/vermeer/apps/worker/peer_manager.go index 7daa4aff0..805cdd6ab 100644 --- a/vermeer/apps/worker/peer_manager.go +++ b/vermeer/apps/worker/peer_manager.go @@ -18,6 +18,7 @@ under the License. package worker import ( + "context" "sync" "vermeer/apps/common" pb "vermeer/apps/protos" @@ -30,6 +31,16 @@ import ( var PeerMgr PeerManager +type peerStreamKind uint8 + +const ( + peerScatterStream peerStreamKind = iota + peerLoadStream + peerStepStream + peerSettingStream + peerStreamCount +) + type PeerClient struct { Id int32 Name string @@ -40,6 +51,8 @@ type PeerClient struct { StepEndHandler StepEndHandler SettingActionHandler SettingActionHandler peerConn *grpc.ClientConn + streamGenerations [peerStreamCount]uint64 + streamCancels [peerStreamCount]context.CancelFunc peerSession pb.WorkerClient } @@ -48,34 +61,109 @@ type PeerManager struct { locker sync.Mutex } +func closePeerResources(connection *grpc.ClientConn, cancels [peerStreamCount]context.CancelFunc) { + for _, cancel := range cancels { + if cancel != nil { + cancel() + } + } + if connection != nil { + _ = connection.Close() + } +} + func (pm *PeerManager) Init() { - pm.locker = sync.Mutex{} + pm.locker.Lock() + previous := pm.peersByName pm.peersByName = make(map[string]*PeerClient) + pm.locker.Unlock() + for _, peer := range previous { + closePeerResources(peer.peerConn, peer.streamCancels) + } } -func (pm *PeerManager) AddPeer(name string, id int32, grpcPeer string) { +func (pm *PeerManager) AddPeer(name string, id int32, grpcPeer string) *PeerClient { pm.locker.Lock() - defer pm.locker.Unlock() - peer := PeerClient{ - Id: id, - Name: name, - GrpcPeer: grpcPeer, - Self: name == ServiceWorker.WorkerName, + previous := pm.peersByName[name] + peer := &PeerClient{Id: id, Name: name, GrpcPeer: grpcPeer, Self: name == ServiceWorker.WorkerName} + peer.ScatterHandler.peer = peer + peer.LoadActionHandler.peer = peer + peer.StepEndHandler.peer = peer + peer.SettingActionHandler.peer = peer + if pm.peersByName == nil { + pm.peersByName = make(map[string]*PeerClient) + } + pm.peersByName[name] = peer + if previous == nil { + common.PrometheusMetrics.WorkerCnt.WithLabelValues().Inc() + } + var connection *grpc.ClientConn + var cancels [peerStreamCount]context.CancelFunc + if previous != nil { + connection, cancels = previous.peerConn, previous.streamCancels } - pm.peersByName[name] = &peer - common.PrometheusMetrics.WorkerCnt.WithLabelValues().Inc() + pm.locker.Unlock() + closePeerResources(connection, cancels) + return peer +} + +func (pm *PeerManager) SetPeerConnection(peer *PeerClient, connection *grpc.ClientConn) bool { + pm.locker.Lock() + if peer == nil || pm.peersByName[peer.Name] != peer { + pm.locker.Unlock() + _ = connection.Close() + return false + } + peer.peerConn, peer.peerSession = connection, pb.NewWorkerClient(connection) + pm.locker.Unlock() + return true +} + +func (pm *PeerManager) bindPeerStream(peer *PeerClient, kind peerStreamKind, cancel context.CancelFunc) uint64 { + pm.locker.Lock() + if peer == nil || pm.peersByName[peer.Name] != peer { + pm.locker.Unlock() + return 0 + } + previous := peer.streamCancels[kind] + peer.streamGenerations[kind]++ + generation := peer.streamGenerations[kind] + peer.streamCancels[kind] = cancel + pm.locker.Unlock() + if previous != nil { + previous() + } + return generation +} + +// RemovePeerStream ignores both old Hello registrations and rebound stream generations. +func (pm *PeerManager) RemovePeerStream(peer *PeerClient, kind peerStreamKind, generation uint64) bool { + pm.locker.Lock() + if peer == nil || pm.peersByName[peer.Name] != peer || peer.streamGenerations[kind] != generation { + pm.locker.Unlock() + return false + } + delete(pm.peersByName, peer.Name) + connection, cancels := peer.peerConn, peer.streamCancels + common.PrometheusMetrics.WorkerCnt.WithLabelValues().Dec() + pm.locker.Unlock() + closePeerResources(connection, cancels) + logrus.Infof("removed peer session: %s", peer.Name) + return true } func (pm *PeerManager) RemovePeer(name string) { pm.locker.Lock() - defer pm.locker.Unlock() - _, ok := pm.peersByName[name] - if !ok { + peer := pm.peersByName[name] + if peer == nil { + pm.locker.Unlock() return } - logrus.Infof("removed peer:%v", name) delete(pm.peersByName, name) + connection, cancels := peer.peerConn, peer.streamCancels common.PrometheusMetrics.WorkerCnt.WithLabelValues().Dec() + pm.locker.Unlock() + closePeerResources(connection, cancels) } func (pm *PeerManager) GetPeer(name string) *PeerClient { diff --git a/vermeer/apps/worker/service.go b/vermeer/apps/worker/service.go index 0f0ef7408..16bf74faf 100644 --- a/vermeer/apps/worker/service.go +++ b/vermeer/apps/worker/service.go @@ -19,6 +19,7 @@ package worker import ( "context" + "fmt" "sync" "time" @@ -57,8 +58,19 @@ type Service struct { WorkerId int32 } -func (s *Service) Init() error { - var err error +func (s *Service) Init() (err error) { + defer func() { + if err != nil { + if s.masterConn != nil { + _ = s.masterConn.Close() + } + for _, peer := range PeerMgr.GetAllWorkers() { + if peer.peerConn != nil { + _ = peer.peerConn.Close() + } + } + } + }() //node, err := snowflake.NewNode(rand.Int63n(1023)) //if err != nil { // logrus.Errorf("new snowflake error: %s", err) @@ -77,8 +89,8 @@ func (s *Service) Init() error { grpc.MaxCallSendMsgSize(4*1024*1024*1024), grpc.MaxCallRecvMsgSize(4*1024*1024*1024)) - s.masterConn, err = grpc.Dial( - masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + s.masterConn, err = common.DialGRPCPeer(context.Background(), + masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { logrus.Errorf("connect master error: %s", err) return err @@ -97,68 +109,84 @@ func (s *Service) Init() error { logrus.Infof("Hello World, I am worker %s", s.WorkerName) PeerMgr.AddPeer(s.WorkerName, s.WorkerId, grpcPeer) md := metadata.New(map[string]string{"worker_name": s.WorkerName}) + // Keep registration tied to a live stream even if peer bootstrap fails. + // Compute/SuperStep stay unopened until the entire peer mesh is installed. + loadCtx := metadata.NewOutgoingContext(context.Background(), md) + s.LoadGraphHandler.grpcStream, err = s.MasterClient.LoadGraphTask(loadCtx) + if err != nil { + return fmt.Errorf("open registration lifeline: %w", err) + } for _, w := range resp.GetWorkers() { - PeerMgr.AddPeer(w.Name, w.Id, w.GrpcPeer) - peerClient := PeerMgr.GetPeer(w.Name) - peerClient.peerConn, err = grpc.Dial( - w.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + peerClient := PeerMgr.AddPeer(w.Name, w.Id, w.GrpcPeer) + peerConnection, err := common.DialGRPCPeer(context.Background(), + w.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { - logrus.Fatalf("connect to peer %s error: %s", w.GrpcPeer, err) - return err + return fmt.Errorf("connect to peer %s: %w", w.GrpcPeer, err) + } + if !PeerMgr.SetPeerConnection(peerClient, peerConnection) { + return fmt.Errorf("peer registration was replaced during bootstrap") } - peerClient.peerSession = pb.NewWorkerClient(peerClient.peerConn) ctx, cancel2 := context.WithTimeout(context.Background(), 5*time.Second) resp, err := peerClient.peerSession.SayHelloPeer( ctx, &pb.HelloPeerReq{SourceName: s.WorkerName, TargetName: w.Name, WorkerPeer: grpcPeer}) if err != nil { - logrus.Fatalf("say hello peer %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } // install scatter ctx = metadata.NewOutgoingContext(context.Background(), md) stream, err := peerClient.peerSession.Scatter(ctx) if err != nil { - logrus.Fatalf("create scatter stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) + } + if peerClient.ScatterHandler.SetClient(stream) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") } - peerClient.ScatterHandler.SetClient(stream) // install load action ctx = metadata.NewOutgoingContext(context.Background(), md) stream2, err := peerClient.peerSession.LoadAction(ctx) if err != nil { - logrus.Fatalf("create load action stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) + } + if peerClient.LoadActionHandler.SetClient(stream2) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") } - peerClient.LoadActionHandler.SetClient(stream2) // install step end ctx = metadata.NewOutgoingContext(context.Background(), md) stream3, err := peerClient.peerSession.StepEnd(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) + } + if peerClient.StepEndHandler.SetClient(stream3) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") } - peerClient.StepEndHandler.SetClient(stream3) // install setting action ctx = metadata.NewOutgoingContext(context.Background(), md) stream4, err := peerClient.peerSession.SettingAction(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) + } + if peerClient.SettingActionHandler.SetClient(stream4) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") } - peerClient.SettingActionHandler.SetClient(stream4) logrus.Infof("say hello peer %s, ok, %s", w.GrpcPeer, resp.GetStatus()) cancel2() } logrus.Infof("say hello master ok, worker id: %d", s.WorkerId) - ctx = metadata.NewOutgoingContext(context.Background(), md) - s.LoadGraphHandler.grpcStream, err = s.MasterClient.LoadGraphTask(ctx) - if err != nil { - logrus.Errorf("create load graph task stream error: %s", err) - return err - } - logrus.Infof("create load graph task stream ok") - ctx = metadata.NewOutgoingContext(context.Background(), md) s.ComputeTaskHandler.grpcStream, err = s.MasterClient.ComputeTask(ctx) if err != nil { @@ -205,16 +233,16 @@ func (s *Service) Run() { //与peer worker的连接 for _, w := range PeerMgr.GetAllWorkers() { - if w.ScatterHandler.mode == HandlerModeClient { + if w.ScatterHandler.IsClient() { go w.ScatterHandler.RecvHandler(w.Name) } - if w.LoadActionHandler.mode == HandlerModeClient { + if w.LoadActionHandler.IsClient() { go w.LoadActionHandler.RecvHandler(w.Name) } - if w.StepEndHandler.mode == HandlerModeClient { + if w.StepEndHandler.IsClient() { go w.StepEndHandler.RecvHandler(w.Name) } - if w.SettingActionHandler.mode == HandlerModeClient { + if w.SettingActionHandler.IsClient() { go w.SettingActionHandler.RecvHandler(w.Name) } } @@ -232,7 +260,9 @@ func (s *Service) CheckMasterAlive() bool { if s.masterConn == nil || s.masterConn.GetState() != connectivity.Ready { return false } - return true + return s.LoadGraphHandler != nil && s.LoadGraphHandler.grpcStream != nil && s.LoadGraphHandler.grpcStream.Context().Err() == nil && + s.ComputeTaskHandler != nil && s.ComputeTaskHandler.grpcStream != nil && s.ComputeTaskHandler.grpcStream.Context().Err() == nil && + s.SuperStepHandler != nil && s.SuperStepHandler.grpcStream != nil && s.SuperStepHandler.grpcStream.Context().Err() == nil } func (s *Service) ReconnectMaster() { @@ -242,6 +272,20 @@ func (s *Service) ReconnectMaster() { logrus.Infof("master is alive:%v", s.masterConn.GetState()) return } + bootstrapped := false + defer func() { + if !bootstrapped { + if s.masterConn != nil { + _ = s.masterConn.Close() + s.masterConn = nil + } + for _, peer := range PeerMgr.GetAllWorkers() { + if peer.peerConn != nil { + _ = peer.peerConn.Close() + } + } + } + }() logrus.Infof("try to reconnect master") //初始化 SpaceMgr.Init() @@ -262,8 +306,8 @@ func (s *Service) ReconnectMaster() { grpc.MaxCallRecvMsgSize(4*1024*1024*1024), ) var err error - s.masterConn, err = grpc.Dial( - masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + s.masterConn, err = common.DialGRPCPeer(context.Background(), + masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { logrus.Errorf("connect master error: %s", err) return @@ -275,7 +319,6 @@ func (s *Service) ReconnectMaster() { ctx, &pb.HelloMasterReq{WorkerPeer: grpcPeer, Version: version.Version}) if err != nil { logrus.Infof("reconnect say hello master error:%v", err) - s.masterConn = nil return } s.WorkerId = resp.GetWorkerId() @@ -283,85 +326,103 @@ func (s *Service) ReconnectMaster() { logrus.Infof("Reconnect, I am worker %s", s.WorkerName) PeerMgr.AddPeer(s.WorkerName, s.WorkerId, grpcPeer) md := metadata.New(map[string]string{"worker_name": s.WorkerName}) + loadCtx := metadata.NewOutgoingContext(context.Background(), md) + tempLoadGraphGrpcStream, err := s.MasterClient.LoadGraphTask(loadCtx) + if err != nil { + logrus.Errorf("reconnect registration lifeline: %v", err) + return + } + s.LoadGraphHandler.grpcStream = tempLoadGraphGrpcStream for _, workerInfo := range resp.GetWorkers() { //对已建立grpc连接的peer忽略。建立未知peer的grpc连接，并启动handler peer := PeerMgr.GetPeer(workerInfo.GetName()) if peer != nil { continue } - PeerMgr.AddPeer(workerInfo.Name, workerInfo.Id, workerInfo.GrpcPeer) - peerClient := PeerMgr.GetPeer(workerInfo.Name) - peerClient.peerConn, err = grpc.Dial( - workerInfo.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + peerClient := PeerMgr.AddPeer(workerInfo.Name, workerInfo.Id, workerInfo.GrpcPeer) + peerConnection, err := common.DialGRPCPeer(context.Background(), + workerInfo.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { - logrus.Fatalf("connect to peer %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("connect to peer %s error: %s", workerInfo.GrpcPeer, err) + return + } + if !PeerMgr.SetPeerConnection(peerClient, peerConnection) { + logrus.Error("peer registration was replaced during reconnect") return } - peerClient.peerSession = pb.NewWorkerClient(peerClient.peerConn) ctx, cancel2 := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel2() resp, err := peerClient.peerSession.SayHelloPeer( ctx, &pb.HelloPeerReq{SourceName: s.WorkerName, TargetName: workerInfo.Name, WorkerPeer: grpcPeer}) if err != nil { - logrus.Fatalf("say hello peer %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("say hello peer %s error: %s", workerInfo.GrpcPeer, err) + return } // install scatter ctx = metadata.NewOutgoingContext(context.Background(), md) stream, err := peerClient.peerSession.Scatter(ctx) if err != nil { - logrus.Fatalf("create scatter stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create scatter stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return + } + if peerClient.ScatterHandler.SetClient(stream) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return } - peerClient.ScatterHandler.SetClient(stream) // install load action ctx = metadata.NewOutgoingContext(context.Background(), md) stream2, err := peerClient.peerSession.LoadAction(ctx) if err != nil { - logrus.Fatalf("create load action stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create load action stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return + } + if peerClient.LoadActionHandler.SetClient(stream2) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return } - peerClient.LoadActionHandler.SetClient(stream2) // install step end ctx = metadata.NewOutgoingContext(context.Background(), md) stream3, err := peerClient.peerSession.StepEnd(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return + } + if peerClient.StepEndHandler.SetClient(stream3) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return } - peerClient.StepEndHandler.SetClient(stream3) // install setting action ctx = metadata.NewOutgoingContext(context.Background(), md) stream4, err := peerClient.peerSession.SettingAction(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return + } + if peerClient.SettingActionHandler.SetClient(stream4) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return } - peerClient.SettingActionHandler.SetClient(stream4) logrus.Infof("say hello peer %s, ok, %s", workerInfo.GrpcPeer, resp.GetStatus()) cancel2() - if peerClient.ScatterHandler.mode == HandlerModeClient { + if peerClient.ScatterHandler.IsClient() { go peerClient.ScatterHandler.RecvHandler(peerClient.Name) } - if peerClient.LoadActionHandler.mode == HandlerModeClient { + if peerClient.LoadActionHandler.IsClient() { go peerClient.LoadActionHandler.RecvHandler(peerClient.Name) } - if peerClient.StepEndHandler.mode == HandlerModeClient { + if peerClient.StepEndHandler.IsClient() { go peerClient.StepEndHandler.RecvHandler(peerClient.Name) } - if peerClient.SettingActionHandler.mode == HandlerModeClient { + if peerClient.SettingActionHandler.IsClient() { go peerClient.SettingActionHandler.RecvHandler(peerClient.Name) } } - ctx = metadata.NewOutgoingContext(context.Background(), md) - tempLoadGraphGrpcStream, err := s.MasterClient.LoadGraphTask(ctx) - if err != nil { - logrus.Errorf("reconnect create load graph task stream error: %s", err) - return - } - s.LoadGraphHandler.grpcStream = tempLoadGraphGrpcStream - logrus.Infof("reconnect create load graph task stream ok") - ctx = metadata.NewOutgoingContext(context.Background(), md) tempComputeGrpcStream, err := s.MasterClient.ComputeTask(ctx) if err != nil { @@ -380,4 +441,5 @@ func (s *Service) ReconnectMaster() { } s.SuperStepHandler.grpcStream = tempSuperStepGrpcStream logrus.Infof("reconnect create super step stream ok") + bootstrapped = true } diff --git a/vermeer/apps/worker/service_bootstrap_test.go b/vermeer/apps/worker/service_bootstrap_test.go new file mode 100644 index 000000000..022f17fa0 --- /dev/null +++ b/vermeer/apps/worker/service_bootstrap_test.go @@ -0,0 +1,102 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 worker + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" + "vermeer/apps/common" + pb "vermeer/apps/protos" +) + +type unavailableBootstrapMaster struct { + pb.UnimplementedMasterServer + peer string + lifelineClosed chan struct{} +} + +func (m *unavailableBootstrapMaster) SayHelloMaster(context.Context, *pb.HelloMasterReq) (*pb.HelloMasterResp, error) { + return &pb.HelloMasterResp{WorkerId: 1, WorkerName: "joining", Workers: []*pb.WorkerInfo{{Name: "stale", Id: 2, GrpcPeer: m.peer}}}, nil +} +func (m *unavailableBootstrapMaster) LoadGraphTask(stream pb.Master_LoadGraphTaskServer) error { + <-stream.Context().Done() + close(m.lifelineClosed) + return nil +} + +func TestWorkerBootstrapFailureReturnsAndClosesRegistrationLifeline(t *testing.T) { + previousMaster := common.GetConfig("master_peer") + previousPeer := common.GetConfig("grpc_peer") + previousMetrics := common.PrometheusMetrics.WorkerCnt + t.Cleanup(func() { + common.SetConfig("master_peer", previousMaster) + common.SetConfig("grpc_peer", previousPeer) + common.PrometheusMetrics.WorkerCnt = previousMetrics + }) + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_bootstrap_workers"}, nil) + t.Setenv("VERMEER_GRPC_DIAL_TIMEOUT", "400ms") + blackhole, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer blackhole.Close() + go func() { + conn, err := blackhole.Accept() + if err == nil { + defer conn.Close() + buffer := make([]byte, 4096) + for { + if _, err = conn.Read(buffer); err != nil { + return + } + } + } + }() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + master := &unavailableBootstrapMaster{peer: blackhole.Addr().String(), lifelineClosed: make(chan struct{})} + server := grpc.NewServer() + pb.RegisterMasterServer(server, master) + go server.Serve(listener) + defer server.Stop() + common.SetConfig("master_peer", listener.Addr().String()) + common.SetConfig("grpc_peer", "127.0.0.1:6789") + service := new(Service) + started := time.Now() + err = service.Init() + if !errors.Is(err, context.DeadlineExceeded) || time.Since(started) > 3*time.Second { + t.Fatalf("stale peer must fail bounded Init: %v", err) + } + if service.masterConn.GetState() != connectivity.Shutdown { + t.Fatal("failed bootstrap must close master transport") + } + select { + case <-master.lifelineClosed: + case <-time.After(time.Second): + t.Fatal("failed bootstrap must close its registration lifeline") + } +} diff --git a/vermeer/cmd/vermeer-token/main.go b/vermeer/cmd/vermeer-token/main.go new file mode 100644 index 000000000..5357d9766 --- /dev/null +++ b/vermeer/cmd/vermeer-token/main.go @@ -0,0 +1,112 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 ( + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "math" + "os" + "path/filepath" + "vermeer/apps/auth" +) + +func main() { + if err := run(os.Args[1:], os.Stdout, os.Stderr); err != nil { + if errors.Is(err, flag.ErrHelp) { + return + } + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} + +func run(args []string, stdout, stderr io.Writer) error { + flags := flag.NewFlagSet("vermeer-token", flag.ContinueOnError) + flags.SetOutput(stderr) + user := flags.String("user", "foo", "user name") + space := flags.String("space", "bar", "space name") + factorFile := flags.String("factor-file", "", "read the token factor from this file") + outputFile := flags.String("output-file", "", "write only the token to this file with mode 0600") + quiet := flags.Bool("quiet", false, "print only the token when -output-file is not set") + client := flags.String("client", "hg", "client identifier") + immortal := flags.Bool("i", false, "immortal token") + if err := flags.Parse(args); err != nil { + return err + } + if flags.NArg() != 0 { + return errors.New("unexpected positional arguments") + } + if *user == "" { + return errors.New("user must not be empty") + } + if *factorFile == "" { + return errors.New("-factor-file is required for deployment tokens") + } + signingKey, err := auth.ReadTokenFactorFile(*factorFile) + if err != nil { + return err + } + secureFactory, err := auth.NewSecureTokenFactory(signingKey) + if err != nil { + return err + } + tokenObj := secureFactory.NewToken(*user, *space, *client) + if *immortal { + tokenObj.Time = math.MaxInt64 + } + secureFactory.Sign(tokenObj) + data, err := json.Marshal(tokenObj) + if err != nil { + return errors.New("encode token") + } + token := auth.ToBase58(string(data)) + if *outputFile != "" { + return writeTokenFile(*outputFile, token) + } + if *quiet { + _, err = fmt.Fprintln(stdout, token) + return err + } + _, err = fmt.Fprintf(stdout, " user: %s\n space: %s\n client: %s\nimmortal: %t\n token: %s\n", + *user, *space, *client, *immortal, token) + return err +} + +func writeTokenFile(path, token string) error { + file, err := os.CreateTemp(filepath.Dir(path), ".vermeer-token-*") + if err != nil { + return fmt.Errorf("create token output: %w", err) + } + defer os.Remove(file.Name()) + if _, err = file.WriteString(token + "\n"); err != nil { + _ = file.Close() + return fmt.Errorf("write token output: %w", err) + } + if err = file.Close(); err != nil { + return fmt.Errorf("close token output: %w", err) + } + // Rename publishes a complete 0600 file and never follows a destination symlink. + if err = os.Rename(file.Name(), path); err != nil { + return fmt.Errorf("publish token output: %w", err) + } + return nil +} diff --git a/vermeer/cmd/vermeer-token/main_test.go b/vermeer/cmd/vermeer-token/main_test.go new file mode 100644 index 000000000..65acdc95e --- /dev/null +++ b/vermeer/cmd/vermeer-token/main_test.go @@ -0,0 +1,210 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You 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 ( + "bytes" + "encoding/json" + "math" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + "vermeer/apps/auth" + "vermeer/apps/common" + "vermeer/apps/structure" + + "github.com/gin-gonic/gin" +) + +func TestDeploymentTokenNativeAuthenticationAndRotation(t *testing.T) { + gin.SetMode(gin.TestMode) + factorFile := filepath.Join(t.TempDir(), "factor") + output := filepath.Join(t.TempDir(), "token") + const firstFactor = "synthetic-部署-factor\\with-quotes\"" + if err := os.WriteFile(factorFile, []byte(firstFactor+"\n"), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factorFile) + args := []string{"-factor-file", factorFile, "-output-file", output, "-quiet", + "-user", "admin", "-space", "$DEFAULT", "-i"} + var stdout, stderr bytes.Buffer + if err := run(args, &stdout, &stderr); err != nil { + t.Fatal(err) + } + if stdout.Len() != 0 || stderr.Len() != 0 { + t.Fatal("file output must not log credentials") + } + data, err := os.ReadFile(output) + if err != nil { + t.Fatal(err) + } + info, err := os.Stat(output) + if err != nil || info.Mode().Perm() != 0600 { + t.Fatal("token file must have mode 0600") + } + token := strings.TrimSpace(string(data)) + parsed := auth.ToTokenBase58(token) + factory, err := auth.NewSecureTokenFactory(firstFactor) + if err != nil { + t.Fatal(err) + } + if parsed == nil || parsed.Time != math.MaxInt64 || + !strings.HasPrefix(parsed.Digest, "v2:") || !factory.Verify(parsed) { + t.Fatal("helper token must use the native factory and immortal timestamp") + } + old := common.GetConfigDefault("auth_token_factor", "") + t.Cleanup(func() { common.SetConfig("auth_token_factor", old) }) + common.SetConfig("auth_token_factor", "different-config-factor") + if err := auth.Init(); err != nil { + t.Fatal(err) + } + router := gin.New() + router.GET("/protected", auth.TokenFilter, auth.AdminFilter, func(ctx *gin.Context) { + credential := ctx.MustGet(structure.CredentialKey).(*structure.Credential) + if credential.Space() != "$DEFAULT" { + t.Fatal("token service space changed") + } + ctx.Status(http.StatusOK) + }) + check := func(encoded string, want int) { + t.Helper() + request := httptest.NewRequest(http.MethodGet, "/protected", nil) + request.Header.Set("Authorization", "Bearer "+encoded) + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != want { + t.Fatalf("native filter status = %d, want %d", response.Code, want) + } + } + check(token, http.StatusOK) + legacyFactory := auth.TokenFactory{Factor: firstFactor} + legacyToken := *parsed + legacyFactory.Sign(&legacyToken) + legacyData, err := json.Marshal(legacyToken) + if err != nil { + t.Fatal(err) + } + check(auth.ToBase58(string(legacyData)), http.StatusUnauthorized) + parsed.Time = time.Now().Unix() - 25*60*60 + factory.Sign(parsed) + expired, err := json.Marshal(parsed) + if err != nil { + t.Fatal(err) + } + check(auth.ToBase58(string(expired)), http.StatusUnauthorized) + if err := os.WriteFile(factorFile, []byte("rotated-deployment-factor"), 0600); err != nil { + t.Fatal(err) + } + if err := auth.Init(); err != nil { + t.Fatal(err) + } + check(token, http.StatusUnauthorized) + if err := run(args, &stdout, &stderr); err != nil { + t.Fatal(err) + } + rotated, err := os.ReadFile(output) + if err != nil { + t.Fatal(err) + } + check(strings.TrimSpace(string(rotated)), http.StatusOK) +} + +func TestTokenOutputAndInputFailures(t *testing.T) { + var stdout, stderr bytes.Buffer + const secret = "synthetic-sensitive-factor" + factorFile := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(factorFile, []byte(secret), 0600); err != nil { + t.Fatal(err) + } + if err := run([]string{"-factor-file", factorFile, "-quiet", "-i"}, &stdout, &stderr); err != nil { + t.Fatal(err) + } + if auth.ToTokenBase58(strings.TrimSpace(stdout.String())) == nil || stderr.Len() != 0 { + t.Fatal("quiet output must contain only the native token") + } + stdout.Reset() + if err := run([]string{"-factor-file", factorFile}, &stdout, &stderr); err != nil { + t.Fatal(err) + } + if strings.Contains(stdout.String(), secret) || strings.Contains(stdout.String(), "json:") { + t.Fatal("interactive metadata must not print the factor or signed JSON") + } + for _, content := range []string{"", "\n", "secret\nsecond-line", "secret\rsecond-line"} { + file := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(file, []byte(content), 0600); err != nil { + t.Fatal(err) + } + stdout.Reset() + stderr.Reset() + if err := run([]string{"-factor-file", file}, &stdout, &stderr); err == nil { + t.Fatal("invalid factor files must fail") + } + if stdout.Len() != 0 || stderr.Len() != 0 { + t.Fatal("invalid factors must not be printed") + } + } + for _, args := range [][]string{ + {}, + {"-factor-file", filepath.Join(t.TempDir(), "missing")}, + {"-factor", secret}, + {"-factor", secret, "-factor-file", factorFile}, + {"-user", "", "-factor-file", factorFile}, + {"-factor-file", factorFile, "-output-file", filepath.Join(t.TempDir(), "missing", "token")}, + } { + stdout.Reset() + stderr.Reset() + if err := run(args, &stdout, &stderr); err == nil { + t.Fatal("invalid arguments must fail") + } + if stdout.Len() != 0 { + t.Fatal("failed generation must not expose token contents") + } + } +} + +func TestTokenFileReplacesSymlinkWithoutReadingTarget(t *testing.T) { + directory := t.TempDir() + target := filepath.Join(directory, "target") + output := filepath.Join(directory, "token") + if err := os.WriteFile(target, []byte("keep-existing-file"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, output); err != nil { + t.Fatal(err) + } + factorFile := filepath.Join(directory, "factor") + if err := os.WriteFile(factorFile, []byte("synthetic-output-file-factor"), 0600); err != nil { + t.Fatal(err) + } + var stdout, stderr bytes.Buffer + if err := run([]string{"-factor-file", factorFile, "-output-file", output, "-i"}, &stdout, &stderr); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(target) + if err != nil || string(data) != "keep-existing-file" { + t.Fatal("token output must not follow a symlink") + } + info, err := os.Lstat(output) + if err != nil || !info.Mode().IsRegular() || info.Mode().Perm() != 0600 { + t.Fatal("published token must be a private regular file") + } +} diff --git a/vermeer/scripts/test-runtime-image.sh b/vermeer/scripts/test-runtime-image.sh new file mode 100755 index 000000000..523dbdee4 --- /dev/null +++ b/vermeer/scripts/test-runtime-image.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. +# +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +exec python3 "${SCRIPT_DIR}/test_runtime_image.py" "$@" diff --git a/vermeer/scripts/test_runtime_image.py b/vermeer/scripts/test_runtime_image.py new file mode 100755 index 000000000..48a179a99 --- /dev/null +++ b/vermeer/scripts/test_runtime_image.py @@ -0,0 +1,268 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. +# +"""Check the real runtime image without PD/Store or externally exposed ports.""" + +import argparse +import hashlib +import hmac +import http.cookiejar +import http.cookies +import json +import os +from pathlib import Path +import secrets +import string +import subprocess +import sys +import tempfile +import time +import urllib.error +import urllib.request +import uuid + + +class SmokeFailure(Exception): + """A diagnostic that never includes credential values or raw service output.""" + + +def require(condition, message): + if not condition: + raise SmokeFailure(message) + + +def docker(arguments, timeout=30, success=True): + try: + result = subprocess.run( + ["docker", *arguments], capture_output=True, timeout=timeout, check=False + ) + except subprocess.TimeoutExpired as error: + raise SmokeFailure("Docker operation exceeded its bounded timeout") from error + if success and result.returncode != 0: + raise SmokeFailure("Docker operation failed; inspect the candidate image locally") + return result + + +def write_private(path, contents): + descriptor = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(descriptor, "wb") as stream: + stream.write(contents) + + +def decode_token(encoded, factor): + alphabet = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" + number = 0 + try: + for character in encoded: + number = number * 58 + alphabet.index(character) + raw = number.to_bytes((number.bit_length() + 7) // 8, "big") + token = json.loads(raw) + payload = [token["u"], token["s"], token["n"], token["c"], token["t"]] + source = b"vermeer-token-v2\x00" + json.dumps( + payload, ensure_ascii=False, separators=(",", ":") + ).encode() + expected = "v2:" + hmac.new(factor.encode(), source, hashlib.sha256).hexdigest() + require(hmac.compare_digest(token["d"], expected), "runtime token has an invalid v2 signature") + return token + except (ValueError, KeyError, TypeError) as error: + raise SmokeFailure("runtime token is not a valid opaque v2 credential") from error + + +def opener(cookie_jar=None): + # The only HTTP destination is the Docker port bound to local loopback. + handlers = [urllib.request.ProxyHandler({})] + if cookie_jar is not None: + handlers.append(urllib.request.HTTPCookieProcessor(cookie_jar)) + return urllib.request.build_opener(*handlers) + + +def request(client, base, path, body=None, bearer=None): + headers = {} + if body is not None: + headers["Content-Type"] = "application/json" + body = json.dumps(body).encode() + if bearer is not None: + headers["Authorization"] = "Bearer " + bearer + try: + response = client.open(urllib.request.Request(base + path, data=body, headers=headers), timeout=3) + except urllib.error.HTTPError as error: + response = error + with response: + return response.status, response.headers, response.read(2 * 1024 * 1024) + + +def check_image(image): + containers = [] + prefix = "vermeer-image-smoke-" + uuid.uuid4().hex[:12] + + def run(arguments, *, name, timeout=30, success=True): + container_name = prefix + "-" + name + containers.append(container_name) + try: + return docker(["run", "--name", container_name, "--rm", *arguments], timeout, success) + except SmokeFailure as error: + raise SmokeFailure("runtime image " + name + " check failed: " + str(error)) from error + + try: + if docker(["image", "inspect", image], success=False).returncode != 0: + docker(["pull", image], timeout=180) + modes = run( + ["--entrypoint", "stat", image, "-c", "%a", "/go/bin/app", "/go/bin/vermeer-token"], + name="modes", + ).stdout.splitlines() + require(modes == [b"755", b"755"], "runtime app and helper must both be executable mode 0755") + nonroot = run( + [ + "--user", "100:101", "--read-only", "--tmpfs", "/tmp:rw,nosuid,nodev,mode=1777", + "--entrypoint", "/bin/sh", image, "-ec", + """umask 077 +work=$(mktemp -d /tmp/vermeer-token.XXXXXX) +trap 'rm -rf "$work"' EXIT +printf '%s' synthetic-nonroot-runtime-factor > "$work/factor" +/go/bin/vermeer-token -factor-file "$work/factor" -output-file "$work/token" \ + -user admin -space '$DEFAULT' -client runtime-smoke -i -quiet > "$work/out" 2> "$work/err" +test ! -s "$work/out" +test ! -s "$work/err" +test "$(stat -c %a "$work/token")" = 600 +cat "$work/token" +""", + ], + name="nonroot", + ) + nonroot_token = decode_token(nonroot.stdout.decode().strip(), "synthetic-nonroot-runtime-factor") + require(nonroot_token["t"] == 2**63 - 1, "helper must support long-lived v2 service tokens") + rejected = run( + ["--entrypoint", "/go/bin/vermeer-token", image, "-factor", "synthetic-legacy-factor", "-quiet"], + name="legacy-flag", success=False, + ) + require(rejected.returncode != 0 and not rejected.stdout and + b"flag provided but not defined: -factor" in rejected.stderr, + "runtime helper must reject legacy inline factors") + missing = run( + ["--entrypoint", "/go/bin/vermeer-token", image, "-factor-file", "/missing-factor", "-quiet"], + name="missing-factor", success=False, + ) + require(missing.returncode != 0 and not missing.stdout and b"read token factor file" in missing.stderr, + "helper must fail closed for a missing factor file") + + with tempfile.TemporaryDirectory(prefix="vermeer-runtime-smoke-") as directory: + private = Path(directory) + factor = secrets.token_hex(32) + password = "".join(secrets.choice(string.ascii_letters + string.digits) for _ in range(16)) + write_private(private / "factor", factor.encode()) + write_private(private / "password", password.encode()) + write_private(private / "empty-password", b"") + mount = ["-v", f"{private}:/probe:ro"] + for label, environment, arguments in [ + ("master-factor", ["VERMEER_AUTH_TOKEN_FACTOR_FILE=/missing-factor"], ["-auth=token"]), + ("master-password", ["VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/factor", + "VERMEER_ADMIN_PASSWORD_FILE=/missing-password"], ["-auth=token"]), + ("master-empty", ["VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/factor", + "VERMEER_ADMIN_PASSWORD_FILE=/probe/empty-password"], ["-auth=token"]), + ("master-none", ["VERMEER_ADMIN_PASSWORD_FILE=/probe/password"], ["-auth=none"]), + ]: + environment_args = [item for value in environment for item in ("-e", value)] + failed = run([*mount, *environment_args, image, "-env=master", *arguments], + name=label, timeout=15, success=False) + observed = failed.stdout + failed.stderr + require(failed.returncode != 0 and b"token authentication initialization failed" in observed, + "Master must reject invalid deployment credentials before listening: " + label) + require(factor.encode() not in observed and password.encode() not in observed, + "startup errors must not expose credentials") + service = run( + [*mount, "--entrypoint", "/go/bin/vermeer-token", image, + "-factor-file", "/probe/factor", "-user", "admin", "-space", "$DEFAULT", + "-client", "runtime-smoke", "-i", "-quiet"], name="service-token", + ).stdout.decode().strip() + decode_token(service, factor) + master_name = prefix + "-master" + containers.append(master_name) + docker([ + "run", "--rm", "-d", "--name", master_name, + "-p", "127.0.0.1::6688", *mount, + "-e", "VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/factor", + "-e", "VERMEER_ADMIN_PASSWORD_FILE=/probe/password", + image, "-env=master", "-auth=token", + ]) + address = docker(["port", master_name, "6688/tcp"]).stdout.decode().strip() + require(address.startswith("127.0.0.1:"), "smoke Master must bind only local loopback") + base = "http://" + address + anonymous = opener() + deadline = time.monotonic() + 45 + while True: + try: + if request(anonymous, base, "/api/v1/healthcheck")[0] == 200: + break + except (OSError, urllib.error.URLError): + pass + require(time.monotonic() < deadline, "smoke Master did not become healthy within 45 seconds") + time.sleep(0.2) + status, _, html = request(anonymous, base, "/") + require(status == 200 and b"Admin password or token" in html and b'id="admin_token"' in html, + "production image must embed the short-credential login UI") + for path in ("/api/v1/master", "/api/v1/workers"): + require(request(anonymous, base, path)[0] == 401, "native metadata must reject anonymous requests") + require(request(anonymous, base, path, bearer=password)[0] == 401, + "short UI password must not act as an API Bearer token") + require(request(anonymous, base, path, bearer=service)[0] == 200, + "long-lived v2 service token must retain API access") + wrong = "x" * 16 if password != "x" * 16 else "y" * 16 + bad_status, bad_headers, bad_body = request(anonymous, base, "/api/v1/login", {"token": wrong}) + require(bad_status == 401 and not bad_headers.get("Set-Cookie"), + "incorrect administrator password must not issue a session") + session_client = opener(http.cookiejar.CookieJar()) + status, headers, body = request(session_client, base, "/api/v1/login", {"token": password}) + cookie = http.cookies.SimpleCookie() + cookie.load(headers.get("Set-Cookie", "")) + require(status == 200 and "vat" in cookie and cookie["vat"]["httponly"] and + cookie["vat"]["max-age"] == "86400", "password login must issue an HttpOnly 24-hour cookie") + cookie_value = cookie["vat"].value + session = decode_token(cookie_value, factor) + require(session["u"] == "admin" and session["s"] == "$DEFAULT" and + abs(time.time() - session["t"]) < 10, "UI session must carry the current admin identity") + for path in ("/api/v1/master", "/api/v1/workers"): + require(request(session_client, base, path)[0] == 200, "native UI session must authorize metadata") + require(request(anonymous, base, "/api/v1/login", {"token": service})[0] == 200, + "long opaque token login must remain supported") + observed_logs = docker(["logs", master_name]) + logs = observed_logs.stdout + observed_logs.stderr + for sensitive in (factor, password, service, cookie_value): + require(sensitive.encode() not in logs + body + bad_body, + "service output must not expose credentials or raw cookies") + finally: + if containers: + docker(["rm", "-f", *containers], timeout=15, success=False) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("image", help="locally built image tag or published image URL/digest") + arguments = parser.parse_args() + require(arguments.image and not arguments.image.startswith("-"), "an image reference is required") + check_image(arguments.image) + print("runtime image smoke passed: executable helpers, v2 tokens, fail-closed startup and authenticated UI") + + +if __name__ == "__main__": + try: + main() + except SmokeFailure as error: + print("runtime image smoke failed: " + str(error), file=sys.stderr) + sys.exit(1) + except Exception as error: + print("runtime image smoke failed with " + type(error).__name__, file=sys.stderr) + sys.exit(1) diff --git a/vermeer/ui/ui/lib/functions.js b/vermeer/ui/ui/lib/functions.js index faf736189..20206119b 100644 --- a/vermeer/ui/ui/lib/functions.js +++ b/vermeer/ui/ui/lib/functions.js @@ -20,7 +20,7 @@ login: function () { const token = $('#admin_token').val(); if (!token) { - showDelModal('empty token'); + showDelModal('Enter an admin password or token.'); return; } @@ -29,6 +29,10 @@ function (data) { console.log(data.message); location.reload(true); + }, + function (err) { + const message = err.responseJSON && err.responseJSON.message; + showDelModal(message || 'Login failed.'); } ); }, @@ -141,22 +145,22 @@ } }, error: function (err) { - if (err.status === 401) { - showDelModal('Login First!'); + if (error) { + if (caller) { + error.apply(caller, [err]); + } else { + error(err); + } return; - } else { - console.log('ajax request failed:', err); } - if (!error) { - let data = JSON.parse(err.responseText); - showDelModal(data.message); + if (err.status === 401) { + showDelModal('Login First!'); return; - } - if (caller) { - error.apply(caller, [err]); } else { - error(err); + console.log('ajax request failed:', err.status); } + let data = JSON.parse(err.responseText); + showDelModal(data.message); } }); } diff --git a/vermeer/ui/ui/master.html b/vermeer/ui/ui/master.html index 27cb10605..49dd3abc8 100644 --- a/vermeer/ui/ui/master.html +++ b/vermeer/ui/ui/master.html @@ -62,8 +62,9 @@
- - + +