forked from lei.chen/myxce
test(apiserver): add integration tests for P0+P1 backend features
Covers ~30 integration tests across 8 sections from the apiserver-backend-integration-test-plan: - §1: Cluster config push (store population, merge semantics, zero-preserve) - §2: Host control (OnHostUpdated merge, validateHost, preCheck, batch ops) - §3: Resource value push (all 7 value types, nil/unknown discriminant, overwrite) - §4: Heartbeat keep-alive (interval update, zero-preserve) - §5: Cross-feature (config+heartbeat, config+host, worker+config field preservation) - §6: RBAC (cluster_admin bypass, algorithm confusion, expired tokens, hostgroup bypass) - §7: Store thread safety (concurrent config pushes, host control+config, resource values) - §8: Input safety (hostname length, empty lists, malformed payloads) All tests use CGO_ENABLED=0 with //go:build !cgo build tag. §8.1 hostname-length skipped pending P1-2b implementation.
This commit is contained in:
parent
0a3de8b040
commit
86a65d640a
8 changed files with 1805 additions and 24 deletions
459
src/docs/apiserver-backend-integration-test-plan.md
Normal file
459
src/docs/apiserver-backend-integration-test-plan.md
Normal file
|
|
@ -0,0 +1,459 @@
|
|||
# Integration Test Plan — P0 + P1 Features
|
||||
|
||||
## Purpose
|
||||
|
||||
Verify end-to-end correctness of every feature delivered by tasks P0-1 through P1-3,
|
||||
exercising the full stack: handler → store → handler merge → store readback.
|
||||
These tests **do not** require a live C controller or CGO — they use the pure-Go stores
|
||||
and `DefaultHandler` directly, which is what unit tests already do. The integration layer
|
||||
adds: **multi-step sequences**, **cross-feature interactions**, and **edge-case coverage**
|
||||
that unit tests leave out.
|
||||
|
||||
---
|
||||
|
||||
## Test Environment
|
||||
|
||||
- All tests run with `CGO_ENABLED=0` (pure Go)
|
||||
- Each test calls `store.InitStores()` to reset all global state
|
||||
- `utils.Log = zap.NewNop()` to suppress log output
|
||||
- **§1–§5, §7, §8**: call handler/store methods directly — no HTTP server required
|
||||
- **§6 (RBAC)**: requires a test HTTP server via `httptest.NewRecorder` with a full Gin router; JWT tokens must be real HS256-signed tokens using the configured signing secret (typically `"xce-api-key"` in test config); `ctx.Set("user", "admin")` does **not** satisfy `GetUsernameFromToken()` which reads and parses the `Authorization` header
|
||||
- **JWT IP binding:** `isValidToken` checks `claims.IP == ctx.ClientIP()`; in `httptest`, `ClientIP()` reads from `req.RemoteAddr` — set `req.RemoteAddr = "127.0.0.1:0"` on every test request and embed `"ip": "127.0.0.1"` in the JWT claims; a mismatch returns 401 for the wrong reason, silently invalidating the test
|
||||
- **Auth config:** `isValidToken` dereferences `utils.AuthenticationYamlConfig`, a package-level pointer that is `nil` until `LoadConfig()` is called — before any JWT test set `utils.AuthenticationYamlConfig = &utils.Authentication{Token: utils.TokenConfig{SecretKey: "xce-api-key"}}` directly; do not call `LoadConfig()` (reads a file that won't exist in CI)
|
||||
|
||||
---
|
||||
|
||||
## 1. P0-1: Cluster Config Push
|
||||
|
||||
### 1.1 Config push populates all stores in a single call
|
||||
|
||||
**Steps:**
|
||||
1. Call `dispatchConfigPush(handler, queues, resources, hostConfigs, cluster)` with non-nil data for all four.
|
||||
2. Verify `store.Queues`, `store.Resources`, `store.Hosts`, `store.Clusters` all contain the pushed data.
|
||||
|
||||
**Asserts:** A single config push updates all four store types in sequence (queues → resources → hosts → cluster); the four updates are NOT atomic — a concurrent reader may see a partial state.
|
||||
|
||||
### 1.2 Second config push merges (does not replace)
|
||||
|
||||
**Steps:**
|
||||
1. Seed `store.Hosts` with worker-sourced data (HostStatus, CPUFactor, NumCPUs, etc.)
|
||||
2. Call `h.OnHostConfigUpdated()` with config-sourced fields (HostModel, HostType, MaxSlot)
|
||||
3. Call `h.OnHostConfigUpdated()` a second time with `HostModel: "modelB"` (distinct from step 2's "modelA")
|
||||
4. Verify worker-sourced fields from step 1 are still preserved
|
||||
5. Verify `HostModel == "modelB"` (config-sourced field reflects the second push, not the first)
|
||||
|
||||
**Asserts:** Partial merge is idempotent and correctly preserves the other source's fields across multiple pushes.
|
||||
|
||||
### 1.3 Cluster update with zero HeartbeatInterval preserves existing value
|
||||
|
||||
**Steps:**
|
||||
1. `store.Clusters.Put("c1", ClusterInfo{HeartbeatInterval: 45})`
|
||||
2. Call `h.OnClusterUpdated(&ClusterInfo{ClusterName: "c1", HeartbeatInterval: 0, Vcpus: 8})`
|
||||
3. Verify `HeartbeatInterval` is still 45 (zero is ignored per `if incoming.HeartbeatInterval > 0` guard)
|
||||
|
||||
**Asserts:** Cluster push with `HeartbeatInterval=0` does not zero out the stored interval.
|
||||
|
||||
### 1.4 Config push with nil cluster argument is a no-op
|
||||
|
||||
**Steps:**
|
||||
1. Call `dispatchConfigPush(handler, nil, nil, nil, nil)`
|
||||
2. Verify `store.Clusters.Count() == 0`
|
||||
|
||||
**Covers:** `dispatchConfigPush` skips the cluster update when the cluster argument is nil. (The empty-name guard lives in the CGO layer `cClusterConfToClusterInfo` and is not reachable in `CGO_ENABLED=0` tests; a `ClusterInfo{ClusterName:""}` passed directly would be stored under an empty key.)
|
||||
|
||||
### 1.5 Config push with nil queues/resources/hosts is safe
|
||||
|
||||
**Steps:**
|
||||
1. Call `dispatchConfigPush(handler, nil, nil, nil, nil)`
|
||||
2. Verify no panics, no store entries.
|
||||
|
||||
**Covers:** Nil-safety of the dispatch loop.
|
||||
|
||||
---
|
||||
|
||||
## 2. P0-2: Host Control via Backend
|
||||
|
||||
### 2.1 Valid control action (open/close) routes through to backend
|
||||
|
||||
**Steps:**
|
||||
1. `store.Hosts.Put("node01", HostInfo{HostName: "node01"})`
|
||||
2. Call `preCheckHostControlRequest(&HostControlRequest{ControlAction: "open", Hostname: []string{"node01"}})`
|
||||
3. Assert `allFlag == false`, `err == nil`
|
||||
|
||||
**Asserts:** Valid actions pass pre-check.
|
||||
|
||||
### 2.2 Invalid action is rejected
|
||||
|
||||
**Steps:**
|
||||
1. Call `preCheckHostControlRequest(&HostControlRequest{ControlAction: "reboot", Hostname: []string{"node01"}})`
|
||||
2. Assert error is returned.
|
||||
|
||||
**Asserts:** Only "open" and "close" are accepted.
|
||||
|
||||
### 2.3 Comment length limit (512 chars)
|
||||
|
||||
**Steps:**
|
||||
1. Call `preCheckHostControlRequest` with `Comment` of 513 chars
|
||||
2. Assert error
|
||||
3. Call with 512 chars — assert no error
|
||||
|
||||
**Already covered in unit tests.** Integration adds: verify the error message is human-readable.
|
||||
|
||||
### 2.4 Hostname validation checks store existence
|
||||
|
||||
**Steps:**
|
||||
1. `store.Hosts.Put("node01", HostInfo{HostName: "node01"})`
|
||||
2. Call `validateHost(ctx, action, []string{"node01", "nonexistent"})`
|
||||
3. Assert `node01` in success list, `nonexistent` in failed map
|
||||
|
||||
**Note:** `validateHost` calls `GetUsernameFromToken()` internally, requiring the same JWT setup as §6 — set `utils.AuthenticationYamlConfig`, set `req.RemoteAddr = "127.0.0.1:0"`, embed `"ip": "127.0.0.1"` in the JWT claims, and pass the token via `Authorization` header. `ctx.Set("user", "admin")` is **not** sufficient. See §6 Test Environment notes for the full setup checklist. Unlike §2.1–2.3, this test is grouped with the §6 JWT-required tests in `controllers/integration_test.go`.
|
||||
|
||||
### 2.5 RBAC: host:control requires cluster_admin role
|
||||
|
||||
**Steps:**
|
||||
1. Load RBAC config with `cluster_admin` having `host:control`
|
||||
2. Use `httptest.NewRecorder` + Gin router with a spy handler that sets a response body
|
||||
3. Call with a user that is NOT cluster_admin — assert 403 response
|
||||
4. Call with cluster_admin user — assert 2xx and spy handler body present
|
||||
|
||||
**Note:** `RequirePermission` short-circuits for `cluster_admin` via early `return` (without calling `ctx.Next()`). Asserting `ctx.IsAborted() == false` gives a **false pass** — the middleware doesn't abort, but the handler is also never called. Use an HTTP recorder to verify the handler response actually appeared.
|
||||
|
||||
**Covers:** Route middleware integration.
|
||||
|
||||
### 2.6 RBAC: host:view accessible to basic_user
|
||||
|
||||
**Steps:**
|
||||
1. Load RBAC config with `basic_user` having `host:view`
|
||||
2. Call `RequirePermission("host:view")` with basic_user
|
||||
3. Assert request proceeds
|
||||
|
||||
### 2.7 Concurrent control requests do not race
|
||||
|
||||
**Already covered:** `TestPreCheckHostControlRequest_NoRace` (50 goroutines).
|
||||
Integration adds: test that concurrent `ControlHost` calls to the same host don't corrupt store state.
|
||||
|
||||
### 2.8 Batch response construction
|
||||
|
||||
**Steps:**
|
||||
1. Build success and failed slices directly (do NOT call `ControlHost` — it calls the CGO stub which returns `errNoCGO` under `CGO_ENABLED=0`):
|
||||
- `succeededItems := []map[string]interface{}{{"node": "node01"}, {"node": "node02"}}`
|
||||
- `failedItems := []map[string]interface{}{{"node": "bad", "reason": "not found"}}`
|
||||
2. Call `BuildBatchOperationResponse("close", "host", succeededItems, failedItems)`
|
||||
3. Verify returned status is `"partially_accepted"`, success list has 2 entries, failed map has 1
|
||||
|
||||
**Covers:** Batch response construction logic; the CGO backend call is out of scope for pure-Go integration tests.
|
||||
|
||||
---
|
||||
|
||||
## 3. P1-1: Resource Value Push
|
||||
|
||||
### 3.1 All 7 value types convert correctly (pure-Go path)
|
||||
|
||||
**Steps:**
|
||||
1. For each value type (i32, i64, u32, u64, fval, sval, bval):
|
||||
- Create `rawResvalData` with that type
|
||||
- Call `rawResvalToDistribution(r)`
|
||||
- Assert `ValueType`, `NumericValue`/`StringValue`/`BooleanValue` match expected
|
||||
|
||||
**Already covered in unit tests.** Integration adds: round-trip through `OnResourceLocationUpdated`:
|
||||
2. Call `h.OnResourceLocationUpdated(rd)` for each type
|
||||
3. Read back from `store.ResourceLocations` using composite key `name:host`
|
||||
4. Assert stored values match
|
||||
|
||||
### 3.2 Nil val produces nil return (no store update)
|
||||
|
||||
**Steps:**
|
||||
1. Create `rawResvalData{valNil: true, ...}`
|
||||
2. Assert `rawResvalToDistribution` returns nil
|
||||
3. Assert `store.ResourceLocations.Count() == 0`
|
||||
|
||||
### 3.3 Unknown discriminant produces nil return with warn log
|
||||
|
||||
**Steps:**
|
||||
1. Create `rawResvalData{valWhich: 99, ...}`
|
||||
2. Assert `rawResvalToDistribution` returns nil
|
||||
3. Assert `store.ResourceLocations.Count() == 0`
|
||||
|
||||
### 3.4 Multiple resource values in a single push
|
||||
|
||||
**Steps:**
|
||||
1. Call `h.OnResourceLocationUpdated(rd1)` then `h.OnResourceLocationUpdated(rd2)` with different `name:host` keys
|
||||
2. Assert `store.ResourceLocations.Count() == 2`
|
||||
3. Assert each key returns the correct value
|
||||
|
||||
### 3.5 Resource value update overwrites previous value
|
||||
|
||||
**Steps:**
|
||||
1. `h.OnResourceLocationUpdated(rd)` with `NumericValue: 100.0`
|
||||
2. `h.OnResourceLocationUpdated(rd)` with same `name:host` but `NumericValue: 200.0`
|
||||
3. Assert stored value is 200.0 (last-write-wins)
|
||||
|
||||
**Covers:** Store `Put` is a full replacement, not merge.
|
||||
|
||||
### 3.6 Composite key format is `res_name:location`
|
||||
|
||||
**Steps:**
|
||||
1. Push a resource with `ResourceName: "mem"` and `Location: "node01"`
|
||||
2. Assert `store.ResourceLocations.Has("mem:node01") == true`
|
||||
3. Assert `store.ResourceLocations.Has("mem") == false`
|
||||
|
||||
---
|
||||
|
||||
## 4. P1-3: Heartbeat Keep-Alive
|
||||
|
||||
> **Isolation requirement:** `heartbeatIntervalSecs` is a package-level `var int32` in the `backend` package shared across all tests in that package. Tests in this section MUST NOT call `t.Parallel()`. Run the full heartbeat test group sequentially to avoid cross-test interference via `SetHeartbeatInterval`.
|
||||
|
||||
### 4.1 Heartbeat interval updates on cluster config push
|
||||
|
||||
**Steps:**
|
||||
1. `atomic.StoreInt32(&heartbeatIntervalSecs, 30)` (default)
|
||||
2. Call `h.OnClusterUpdated(&ClusterInfo{ClusterName: "c1", HeartbeatInterval: 45})`
|
||||
3. Assert `atomic.LoadInt32(&heartbeatIntervalSecs) == 45`
|
||||
|
||||
**Covers:** The P1-3 integration point — cluster config push triggers heartbeat interval change.
|
||||
|
||||
### 4.2 Cluster push with HeartbeatInterval=0 preserves current interval
|
||||
|
||||
**Steps:**
|
||||
1. `atomic.StoreInt32(&heartbeatIntervalSecs, 45)`
|
||||
2. Call `h.OnClusterUpdated(&ClusterInfo{ClusterName: "c1", HeartbeatInterval: 0})`
|
||||
3. Assert `atomic.LoadInt32(&heartbeatIntervalSecs) == 45` (unchanged)
|
||||
|
||||
**Covers:** The `> 0` guard in `OnClusterUpdated` and `SetHeartbeatInterval`.
|
||||
|
||||
---
|
||||
|
||||
## 5. Cross-Feature Integration
|
||||
|
||||
### 5.1 Config push updates heartbeat interval AND stores cluster data
|
||||
|
||||
**Steps:**
|
||||
1. Default `heartbeatIntervalSecs = 30`
|
||||
2. Call `dispatchConfigPush(h, nil, nil, nil, &ClusterInfo{ClusterName: "c1", HeartbeatInterval: 60, Vcpus: 32})`
|
||||
3. Assert `store.Clusters.Get("c1")` has `HeartbeatInterval=60, Vcpus=32`
|
||||
4. Assert `atomic.LoadInt32(&heartbeatIntervalSecs) == 60`
|
||||
|
||||
**Covers:** P0-1 + P1-3 interaction in a single dispatch.
|
||||
|
||||
### 5.2 Config push + host control on same host
|
||||
|
||||
**Steps:**
|
||||
1. `dispatchConfigPush(h, nil, nil, []HostInfo{{HostName: "node01", HostModel: "x86_64"}}, nil)`
|
||||
2. Assert `store.Hosts.Get("node01")` has `HostModel="x86_64"`
|
||||
3. Assert `store.Hosts.Has("node01") == true` — verifies config-push data is visible in the host store
|
||||
4. Call `preCheckHostControlRequest(&HostControlRequest{ControlAction: "close", Hostname: []string{"node01"}})` — assert `allFlag == false`, `err == nil` (validates action format; store lookup is handled by `validateHost` which requires JWT — see §2.4 note)
|
||||
|
||||
**Covers:** Data created by config push is written to the host store and is visible to subsequent operations.
|
||||
|
||||
### 5.3 Resource push after host config push — same host referenced
|
||||
|
||||
**Steps:**
|
||||
1. `h.OnHostConfigUpdated(&HostInfo{HostName: "node01", HostModel: "x86_64"})`
|
||||
2. `h.OnResourceLocationUpdated(&ResourceDistributionInfo{ResourceName: "mem", Location: "node01", NumericValue: 65536.0, ValueType: NUMERIC, IsDynamic: true})`
|
||||
3. Assert `store.Hosts.Get("node01")` has `HostModel="x86_64"` (preserved)
|
||||
4. Assert `store.ResourceLocations.Get("mem:node01")` has `NumericValue=65536.0`
|
||||
5. Assert `store.Hosts.Get("node01").HostModel` still "x86_64" (host config was not overwritten)
|
||||
|
||||
**Covers:** Two independent push paths don't interfere with each other's store data.
|
||||
|
||||
### 5.4 Worker push after config push preserves config fields
|
||||
|
||||
**Steps:**
|
||||
1. `h.OnHostConfigUpdated(&HostInfo{HostName: "node01", HostModel: "x86_64", MaxSlot: 8})`
|
||||
2. `h.OnHostUpdated(&HostInfo{HostName: "node01", HostStatus: 3, CPUFactor: 2.5, NumCPUs: 16})`
|
||||
3. Assert `got.HostModel == "x86_64" && got.MaxSlot == 8` (config preserved)
|
||||
4. Assert `got.HostStatus == 3 && got.CPUFactor == 2.5` (worker updated)
|
||||
|
||||
**Covers:** Two-push merge correctness — already tested in unit tests but verified here at integration scope.
|
||||
|
||||
### 5.5 Multiple sequential config pushes (idempotent)
|
||||
|
||||
**Steps:**
|
||||
1. Push config with `ClusterInfo{Vcpus: 16, HeartbeatInterval: 30}`
|
||||
2. Push config with `ClusterInfo{Vcpus: 32, HeartbeatInterval: 60}`
|
||||
3. Assert store has `Vcpus=32, HeartbeatInterval=60` (last-write-wins for config fields)
|
||||
4. Assert `heartbeatIntervalSecs == 60`
|
||||
|
||||
**Covers:** Rapid config push updates don't leave stale data.
|
||||
|
||||
---
|
||||
|
||||
## 6. RBAC Integration (End-to-End)
|
||||
|
||||
> **RBAC isolation:** Call `resetRBACState()` (not `store.InitStores()`) at the start of each RBAC test. `resetRBACState()` zeros `rbacConfig`, preventing `AddPermissionsForRole` from appending duplicate permissions across test reloads. `store.InitStores()` does not reset RBAC state.
|
||||
|
||||
### 6.1 Full middleware chain: JWT → RBAC → handler
|
||||
|
||||
**Steps:**
|
||||
1. Set up Gin router with real middleware (`RequirePermission("host:control")`)
|
||||
2. Create a valid JWT token for a cluster_admin user
|
||||
3. POST `/xce/v1/hosts/control` with `action=close, host_name=["node01"]`
|
||||
4. Assert 202 Accepted (or mocked backend response)
|
||||
|
||||
5. Create a valid JWT token for a basic_user
|
||||
6. POST `/xce/v1/hosts/control` — assert 403 Forbidden
|
||||
|
||||
7. POST `/xce/v1/hosts/select` with basic_user — assert passes (host:view)
|
||||
|
||||
**Note:** This requires spinning up a test HTTP server via `httptest.NewRecorder`. Do not mock at the middleware level — the purpose of §6 is to exercise the real middleware chain end-to-end.
|
||||
|
||||
### 6.2 RBAC: cluster_admin bypasses permission check
|
||||
|
||||
**Steps:**
|
||||
1. Set up Gin router with `RequirePermission("host:control")`
|
||||
2. Sign a JWT for "admin1" with role "cluster_admin" and issue `POST /xce/v1/hosts/control`
|
||||
3. Assert response status is **not 403** — verifies the middleware did not block the request
|
||||
|
||||
**Note:** `RequirePermission` returns early for `cluster_admin` without calling `ctx.Next()`, so downstream handlers are **never reached**. Do not assert a spy handler body or `ctx.IsAborted() == false` — both give false results. To verify full handler execution for a privileged user, write a separate test with a non-cluster_admin user that has explicit `host:control` permission (see §6.3).
|
||||
|
||||
### 6.3 RBAC: basic_user cannot control hosts but can view
|
||||
|
||||
**Steps:**
|
||||
1. Assign user "user1" role "basic_user"
|
||||
2. Call `RequirePermission("host:control")` — assert 403
|
||||
3. Call `RequirePermission("host:view")` — assert passes
|
||||
|
||||
### 6.4 JWT algorithm confusion is rejected
|
||||
|
||||
**Steps:**
|
||||
1. Configure test server with HS256 signing secret `"xce-api-key"`
|
||||
2. Craft a token header claiming `"alg": "none"` or a mismatched algorithm (RS256)
|
||||
3. POST to `/xce/v1/hosts/control` with that token (use valid IP binding so IP mismatch is not the cause of rejection)
|
||||
4. Assert 401 Unauthorized
|
||||
5. Assert response body contains a token/signature error string (e.g., `"token"`, `"invalid"`, or `"signature"`) — not `"ip"` or `"expired"` — to confirm the rejection is due to algorithm/signature failure, not an unrelated check
|
||||
|
||||
**Covers:** Prevents algorithm confusion attacks where an attacker downgrades signing to `none`.
|
||||
|
||||
### 6.5 Expired JWT is rejected
|
||||
|
||||
**Steps:**
|
||||
1. Sign a JWT with `exp = time.Now().Unix() - 3600` (expired 1 hour ago)
|
||||
2. POST to any protected endpoint
|
||||
3. Assert 401 Unauthorized
|
||||
|
||||
**Covers:** Token replay after expiry — ensures expired tokens cannot be reused.
|
||||
|
||||
### 6.6 Known gap: hostgroup domain restriction bypass
|
||||
|
||||
> **Documents a known limitation, not a working feature.** `validateHost` at `hosts.go:505-507` contains a `// TODO` that silently skips hostgroup domain enforcement. This test makes the gap explicit so it is visible in code review and prevents accidental regression if the TODO is removed.
|
||||
|
||||
**Steps:**
|
||||
1. Provision a user with `host:control` scoped to hostgroup "gpu-nodes"
|
||||
2. Issue `POST /xce/v1/hosts/control` targeting a host NOT in "gpu-nodes"
|
||||
3. **Assert request is accepted** (not 403) — documents that hostgroup scoping is not yet enforced
|
||||
|
||||
**Note:** When P1-2b implements hostgroup enforcement, flip this assertion to assert 403 and remove the `// TODO` marker.
|
||||
|
||||
### 6.7 Post-logout token reuse
|
||||
|
||||
> **Design gap:** JWTs are stateless and the current server does not maintain a JTI blacklist. A token valid at logout time remains valid until expiry. This test documents the current behavior as an explicit design decision.
|
||||
|
||||
**Steps:**
|
||||
1. Sign a valid JWT with a 1-hour expiry
|
||||
2. Call `POST /xce/v1/users/logout` (or equivalent) — record any logout response
|
||||
3. Immediately reuse the same token on `GET /xce/v1/hosts` — assert **200 OK** (not 401)
|
||||
|
||||
**Note:** If a JTI blacklist is added in future, flip this assertion to expect 401 and add the blacklist provisioning step.
|
||||
|
||||
---
|
||||
|
||||
## 7. Store Thread Safety
|
||||
|
||||
> **CI requirement:** All §7 tests must run with `-race`: `go test -race ./store/...`. A test that passes without `-race` provides no guarantee of absence of data races — the Go race detector instruments memory accesses at runtime and is the authoritative check.
|
||||
|
||||
### 7.1 Concurrent config pushes don't corrupt data
|
||||
|
||||
**Steps:**
|
||||
1. Start 10 goroutines, each calling `h.OnHostConfigUpdated(&HostInfo{HostName: "node01", ...})` with slightly different HostModel values
|
||||
2. Wait for all goroutines to complete
|
||||
3. Assert `store.Hosts.Get("node01")` returns a valid HostInfo (no zero-value corruption)
|
||||
4. Assert `HostName` field is "node01"
|
||||
|
||||
**Race detection note:** A simple value-validity assertion cannot prove absence of a data race — concurrent writes all produce plausible values. Run this test with `go test -race ./store/...` to detect RMW races via the Go race detector. The assertion above verifies structural integrity; `-race` detects the underlying race condition.
|
||||
|
||||
### 7.2 Concurrent host control + config push
|
||||
|
||||
**Steps:**
|
||||
1. Goroutine A: call `OnHostConfigUpdated` for "node01" in a loop
|
||||
2. Goroutine B: call `validateHost` checking "node01" existence in a loop
|
||||
3. Assert no panics and no store corruption after 100 iterations
|
||||
|
||||
### 7.3 Concurrent resource value pushes for same key
|
||||
|
||||
**Steps:**
|
||||
1. Start 10 goroutines, each calling `OnResourceLocationUpdated` for `"mem:node01"` with different NumericValue
|
||||
2. Assert final stored value is one of the 10 values (no corruption)
|
||||
3. Assert key exists exactly once
|
||||
|
||||
---
|
||||
|
||||
## 8. Input Safety
|
||||
|
||||
### 8.1 Hostname length limits
|
||||
|
||||
> **Skip until P1-2b is implemented.** No hostname length validation exists in `preCheckHostControlRequest` yet — this test will pass vacuously. Add `//go:build p1_2b` build tag or comment out pending that task.
|
||||
|
||||
**Steps:**
|
||||
1. Call `preCheckHostControlRequest` with hostname exceeding the P1-2b limit (>1024 chars)
|
||||
2. Assert `err != nil` with message indicating length exceeded
|
||||
|
||||
### 8.2 Empty hostname list
|
||||
|
||||
**Steps:**
|
||||
1. Call `preCheckHostControlRequest` with `Hostname: []`
|
||||
2. Assert `err != nil` — inspect the actual error message from `preCheckHostControlRequest` and assert it contains the expected string (e.g., "no hostnames provided" or similar)
|
||||
|
||||
### 8.3 Select expression length (current limit: 4094)
|
||||
|
||||
**Steps:**
|
||||
1. Call handler with `Select` string > 4094 chars
|
||||
2. Assert error
|
||||
3. Call with 4094 chars — assert accepted
|
||||
|
||||
### 8.4 Special characters in hostnames
|
||||
|
||||
**Steps:**
|
||||
1. Call with hostname containing `<script>alert(1)</script>`
|
||||
2. Assert the hostname is stored as a string without interpretation (XSS safety at API layer)
|
||||
|
||||
---
|
||||
|
||||
## Test File Organization
|
||||
|
||||
| File | Tests |
|
||||
|------|-------|
|
||||
| `backend/integration_test.go` | §1 (config push), §3 (resource push), §4 (heartbeat), §5 (cross-feature) |
|
||||
| `controllers/integration_test.go` | §2.1–2.3, §2.5–2.8 (host control — no JWT required), §2.4 (host validation — **requires full JWT setup; see §6 prerequisites**), §6 (RBAC middleware), §8 (input safety) |
|
||||
| `store/concurrency_test.go` | §7 (thread safety) |
|
||||
|
||||
All test files use `//go:build !cgo` or `CGO_ENABLED=0` compatible patterns.
|
||||
|
||||
**Package declarations:**
|
||||
- `backend/integration_test.go` must declare `package backend` (not `package backend_test`) to access unexported symbols: `heartbeatIntervalSecs`, `rawResvalToDistribution`, `dispatchConfigPush`
|
||||
- `controllers/integration_test.go` may use `package controllers_test` for black-box tests, but any test accessing unexported helpers must use `package controllers`
|
||||
|
||||
**JWT signing secret in tests:** Integration tests in `controllers/integration_test.go` that create real JWTs must use a test-only signing secret (e.g., `"xce-api-key"` from the test config file). Never use the production signing secret. Document the expected test secret in a comment at the top of the test file so it is clearly identifiable during security audits.
|
||||
|
||||
---
|
||||
|
||||
## Priority Classification
|
||||
|
||||
| Priority | Tests | Rationale |
|
||||
|----------|-------|-----------|
|
||||
| **P0 — Must have** | 1.1, 1.2, 1.3, 2.1, 2.2, 2.5, 3.1, 3.4, 4.1, 5.1 | Core functionality and merge correctness |
|
||||
| **P1 — Should have** | 1.4, 1.5, 2.4, 2.6, 2.8, 3.2, 3.3, 3.5, 3.6, 4.2, 5.2, 5.3, 6.1, 6.2, 6.3, 6.4, 6.5, 6.6, 6.7 | Edge cases, RBAC, JWT security, and known gap documentation |
|
||||
| **P2 — Nice to have** | 2.3, 2.7, 5.4, 5.5, 7.1, 7.2, 7.3, 8.1–8.4 | Robustness and stress |
|
||||
|
||||
---
|
||||
|
||||
## Existing Test Coverage
|
||||
|
||||
The following unit tests already cover much of the ground above and should NOT be duplicated:
|
||||
|
||||
| Existing Test File | What It Covers | Integration Gap |
|
||||
|--------------------|----------------|-----------------|
|
||||
| `backend/handler_test.go` | Single-push merge for hosts, clusters, dispatch | Multi-push sequences, cross-feature |
|
||||
| `backend/convert_test.go` | All 7 value types, nil val, unknown discriminant | Store round-trip after conversion |
|
||||
| `backend/heartbeat_test.go` | SetHeartbeatInterval, runHeartbeat call/cancel | Interval change triggered by OnClusterUpdated |
|
||||
| `controllers/host_control_test.go` | preCheck validation, concurrent no-race | Full handler flow with store interaction |
|
||||
| `controllers/rbac_check_test.go` | host:control/host:view permissions | Full middleware chain with JWT |
|
||||
|
|
@ -98,15 +98,8 @@
|
|||
|
||||
## P1-2: Host Resource Requirement Select
|
||||
|
||||
- [ ] **#11 P1-2a: C select expression parser (PEG + eval)**
|
||||
- `include/resreq.h` — Replace empty `xce_resreq_select_` with expression tree types, add enums, add extern declarations
|
||||
- `lib/xce_select.peg` — New PEG grammar for select boolean expressions (right-recursive)
|
||||
- `lib/resreq.c` — Implement `parse_select()`, `eval_select()`, `free_select()`; update `kill_primary_resreq()`
|
||||
- `lib/meson.build` — Add `xce_select.peg` to explicit source list
|
||||
|
||||
- [>] **#12 P1-2b: Go EvalSelectForHost + FilterHostByResReqSelect** (blocked by #11)
|
||||
- `backend/clib.go` — Add `EvalSelectForHost()` using `cgo.Handle` for callback context, add `#include "resreq.h"`, extern declarations, `xce_select_destroy` helper
|
||||
- `controllers/hosts.go` — Implement `FilterHostByResReqSelect`, update `preCheckHostRequest()` limit from 4094→1024 bytes
|
||||
> Moved to [`src/docs/p1-2-host-select.md`](p1-2-host-select.md) for deeper investigation.
|
||||
> Open questions: PEG tooling availability, `eval_select` input contract, CGO error propagation.
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -121,8 +114,9 @@
|
|||
|
||||
## Final
|
||||
|
||||
- [>] **#14 Build and test verification** (blocked by all others)
|
||||
- `make test` in `src/frontend/` passes
|
||||
- `ninja -C build` for C components passes
|
||||
- RBAC verification
|
||||
- Input safety verification (1024-byte limit, 256-host limit)
|
||||
- [x] **#14 Build and test verification**
|
||||
- `make test` in `src/frontend/` passes — all backend, controllers, store, utils, xcecli tests green
|
||||
- `ninja -C build` for C components passes — required meson reconfigure + CGO val_helpers.h fix
|
||||
- RBAC verification — `host:control` (cluster_admin) and `host:view` (all authenticated) confirmed in routes + rbac.yaml
|
||||
- Input safety — comment limit 512 chars; select expression limit 4094 chars (1024 reduction deferred to P1-2b)
|
||||
- **CGO fix applied:** extracted `static` val helpers from `clib.go` preamble into `val_helpers.h` shared header; `convert.go` now includes it; added explicit `float64()` cast for `C.double` return
|
||||
Loading…
Add table
Add a link
Reference in a new issue