From 886d09e0cbddd8a0cec4e520cfeaad7695c7ee45 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 16 Oct 2026 19:10:55 +0200 Subject: [PATCH 1/2] feat(config): ruleset-only ct.dynamicGroup for groups ct no longer owns ct.dynamicGroup({ group, dynamic }) manages only the dynamic status and ruleset of an existing group, found in ct's state or the OpenTofu id map. It writes no state entry and has no delete path. Refused at load alongside ct.group for the same key or when declared twice. Also: ct state rm treats id-map groups as not state-only, ct refresh --group and ct adopt group --with-dynamic accept id-map groups (adopt emits a ct.dynamicGroup snippet), ct coverage labels them owned elsewhere, and a bare string group scope that only the id map knows fails with a hint to use { group }. --- docs/handbuch/blueprints.md | 2 +- docs/handbuch/dynamic-groups.md | 35 ++- docs/handbuch/group-member-fields.md | 2 +- docs/handbuch/permissions.md | 8 +- src/application/operations/adopt-group.ts | 194 +++++++----- src/application/operations/apply.ts | 4 +- src/application/operations/coverage.ts | 14 +- src/application/operations/refresh.ts | 28 +- src/application/operations/state.ts | 33 +- src/config/context.ts | 75 +++++ src/coverage/report.ts | 38 ++- src/engine/build.ts | 48 ++- src/engine/execute.ts | 21 +- src/engine/synthetic.ts | 2 +- src/engine/types.ts | 8 + src/permissions/plan.ts | 4 +- src/permissions/scope.ts | 3 +- src/resolve/resolver.ts | 22 ++ tests/dynamic-group.test.ts | 351 ++++++++++++++++++++++ 19 files changed, 789 insertions(+), 103 deletions(-) create mode 100644 tests/dynamic-group.test.ts diff --git a/docs/handbuch/blueprints.md b/docs/handbuch/blueprints.md index 34b6d22..d2922e8 100644 --- a/docs/handbuch/blueprints.md +++ b/docs/handbuch/blueprints.md @@ -4,7 +4,7 @@ sources: - src/config/context.ts - src/engine/graph.ts - src/engine/hierarchy.ts -sources_hash: 0bf15d98760f937b +sources_hash: f0eb68d512a54ebb reviewed: 2026-08-28 --- diff --git a/docs/handbuch/dynamic-groups.md b/docs/handbuch/dynamic-groups.md index 915e3a5..7058337 100644 --- a/docs/handbuch/dynamic-groups.md +++ b/docs/handbuch/dynamic-groups.md @@ -6,7 +6,7 @@ sources: - src/engine/dynamic.ts - src/engine/synthetic.ts - src/application/operations/adopt-group.ts -sources_hash: 5d0be1f4c92a8efb +sources_hash: 11cdc86261ab9d0c reviewed: 2026-08-28 --- @@ -512,6 +512,39 @@ This is pinned by a live-gated test in `tests/dynamic.integration.test.ts` it fails); it is skipped by default and requires an explicit opt-in against a dev instance. +## Ruleset-only declaration: `ct.dynamicGroup` (#202) + +When another tool owns the group itself (typically OpenTofu's `churchtools_group`), the group +leaves ct's config **and** state, but its ruleset can still be managed from here: + +```ts +ct.dynamicGroup({ group: "example_autogroup", dynamic: true }); +ct.dynamicGroup({ + group: "example_paused", + dynamic: { status: "manual", ruleset: { ref: "./rulesets/example_paused.json" } }, +}); +``` + +`dynamic` takes exactly the three forms of `ct.group({ dynamic })`. The group is found by key: +ct's state first, then the OpenTofu id map (`.ct/ids..json`). A key found in neither, or a +group that no longer exists in ChurchTools, is a plan-time error, never a create. + +What it deliberately does **not** do: + +- It writes **no** `ct-state` entry and does not appear in `ct state list`. +- It has **no** delete path. Dropping the declaration means "stop managing the ruleset" (the same + as dropping `dynamic` from a `ct.group`); it never demotes the group to `none`, never deletes the + ruleset, never touches the group row. +- It manages no other group field, no `parents` and no member fields. + +Plan items render as an update (or no-op) on the group with only the `dynamic` field changing, and +the safety rails above (fetch-failed degradation, unportable-id and `stereotype` warnings, +status-only flips skipping the ruleset PUT) apply unchanged. A key declared with both `ct.group` and +`ct.dynamicGroup`, or with `ct.dynamicGroup` twice, is refused at config load: one owner per ruleset. + +`ct refresh --group ` and `ct adopt group --with-dynamic` also accept an id-map group; the +adopt output is then a `ct.dynamicGroup` snippet and no state entry is written. + ## Managed guard: undeclared groups stay invisible Dynamic-group state is only ever fetched for a group that is **both**: (a) diff --git a/docs/handbuch/group-member-fields.md b/docs/handbuch/group-member-fields.md index bb4673d..011cab7 100644 --- a/docs/handbuch/group-member-fields.md +++ b/docs/handbuch/group-member-fields.md @@ -1,5 +1,5 @@ --- -sources_hash: 6809989e79b1fa43 +sources_hash: 75e94f40f8a2f6d9 title: Group member fields sources: - src/engine/member-fields.ts diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index 2bf177d..f9a41f0 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -7,7 +7,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: 066465b31f159b1a +sources_hash: 7974aecfe2a96172 reviewed: 2026-08-28 --- @@ -543,7 +543,7 @@ the rest of the DSL uses — so both spellings are interchangeable. | `scopeField` | Reference form | Resolved against | | -------------------- | ----------------------------------------- | ----------------------------------------------------------------- | -| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | +| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups; `{ group }` also the id map | | `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | | `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | | `cdb_bereich` | `{ department: "" }` | managed Bereiche, then `GET /departments` (#108) | @@ -596,6 +596,10 @@ Three things are hard errors at **plan** time, never a guessed `dataId`: group", so on e.g. a `cdb_station` right it would either fail confusingly or — worse — match an unrelated group that happens to carry that key. +A **bare string** group scope resolves from ct's state (or a group declared in the same config) +only. Once a group leaves ct's state because OpenTofu owns it, the bare string fails at plan time +with a hint to write `{ group: "" }`, which resolves through the id map (#202). + ### Comment viewers: from catalog-only to declarable (#151) `cdb_comment_viewer` (Kommentare-Viewer) had a reference form since #102 but no diff --git a/src/application/operations/adopt-group.ts b/src/application/operations/adopt-group.ts index f7ccdd7..a5400be 100644 --- a/src/application/operations/adopt-group.ts +++ b/src/application/operations/adopt-group.ts @@ -27,6 +27,8 @@ import { RESOURCES, configSnippet, fromInformation, slug } from "../../resources import { ReverseResolver, type RoleCatalogEntry } from "../../resolve/reverse.js"; import type { RefKind } from "../../resolve/refs.js"; import { formatPortablizeWarnings, portablizeRuleset, scanUnportablized } from "../../config/query-refs.js"; +import { CATALOG_DIR } from "../../permissions/catalog-store.js"; +import { loadIdMap } from "../../resolve/idMap.js"; import { chooseAdoptKey, loadState, saveState, upsert, type State } from "../../state/state.js"; import type { CtWarning, OperationResult, ProjectRequest } from "../contracts.js"; import { InMemoryMutationLock } from "../prepared-operation-store.js"; @@ -357,6 +359,14 @@ export async function runAdoptGroups( const project = await resolveProject(opts); return mutationLock.runExclusive(project.statePath, async () => { const state = await loadState(project.statePath, project.host); + // Groups OpenTofu owns (#202): present in the id map by id, absent from ct's state. Adopting one + // would write a state entry that duplicates the map, so only its ruleset is captured, as a + // `ct.dynamicGroup` snippet. + const idMap = await loadIdMap(project.host, join(project.cwd, CATALOG_DIR)); + const mappedGroupKeyById = new Map(); + for (const entry of idMap?.entries ?? []) { + if (entry.type === "group") mappedGroupKeyById.set(entry.id, entry.key); + } const { client } = await authedSession(); let resolvedIds: number[]; @@ -409,7 +419,117 @@ export async function runAdoptGroups( roleCatalog = await reverse.roleGroupTypeCatalog(); } + /** Capture a group's dynamic ruleset to `rulesets/.json`; the `dynamic` value for the snippet, or undefined. */ + const captureRulesetFile = async ( + id: number, + key: string, + ): Promise<{ status: DynamicStatus; ruleset: { ref: string } } | undefined> => { + const captured = await captureDynamic(id, client); + if (captured) { + const relPath = `rulesets/${key}.json`; + let rulesetToWrite = captured.normalizedRuleset; + if (opts.portableRulesets !== false) { + // Managed group ids come from state (no catalog for `group`), including any group this + // same run already adopted; the master-data kinds come from the catalog maps above. + const groupMap = new Map(mappedGroupKeyById); + // Managed role definitions, likewise from STATE rather than the live catalog (#125): + // state gives a per-host id under a shared logical key, which is what makes a + // duplicate role name fixable by adopting the role instead of renaming master data. + // (`ReverseResolver.idToKeyByKind("role-def")` keys by slug(name) off the catalog and + // is exactly the ambiguous mapping this replaces.) + const roleDefMap = new Map(); + for (const r of Object.values(state.resources)) { + if (r.type === "group") groupMap.set(r.id, r.key); + if (r.type === "group-role") roleDefMap.set(r.id, r.key); + } + const { ruleset, warnings: portableWarnings } = portablizeRuleset(captured.normalizedRuleset, { + idToKeyByKind: { ...portableCatalogMaps, group: groupMap, "role-def": roleDefMap }, + roleCatalog, + groupTypeIdToKey, + }); + rulesetToWrite = ruleset; + // Report every dimension left numeric, with its reason (#101). The old output said only + // "left N unmanaged id(s) numeric", which named neither the dimension nor the fix — so a + // capture that silently froze prod's ids into a cross-host file looked like a clean run. + if (portableWarnings.length > 0) { + const lines = formatPortablizeWarnings(portableWarnings); + if (opts.strictRulesets) { + throw new Error( + `--strict-rulesets: ${relPath} would contain ${portableWarnings.length} unportablized ` + + `(host-specific) id(s), so nothing was written:\n` + + lines.map((l) => ` ${l}`).join("\n"), + ); + } + addWarning({ + code: "RULESET_NOT_PORTABLE", + message: + `${relPath} keeps ${portableWarnings.length} host-specific id(s) — NOT portable to another host:\n` + + lines.map((line) => ` ${line}`).join("\n"), + }); + } + } else { + // Verbatim capture (--no-portable-rulesets): every entity id in the file is this host's. + // Say so once per ruleset rather than let the opt-out quietly imply the ids are fine. + const left = scanUnportablized(captured.normalizedRuleset); + if (left.length > 0) { + if (opts.strictRulesets) { + throw new Error( + `--strict-rulesets with --no-portable-rulesets: ${relPath} would contain ` + + `${left.length} host-specific id(s) and nothing would rewrite them.`, + ); + } + addWarning({ + code: "RULESET_NOT_PORTABLE", + message: + `${relPath} captured verbatim (--no-portable-rulesets): ${left.length} host-specific ` + + `id(s) kept as-is — NOT portable to another host.`, + }); + } + } + if (!opts.dryRun) { + await mkdir(join(project.cwd, "rulesets"), { recursive: true }); + await writeFile(join(project.cwd, relPath), `${JSON.stringify(rulesetToWrite, null, 2)}\n`, "utf8"); + } + return { status: captured.status, ruleset: { ref: `./${relPath}` } }; + } + return undefined; + }; + /** `ct adopt group` on a group the id map owns: capture only the ruleset, write no state entry (#202). */ + const adoptMappedGroup = async (id: number, key: string): Promise => { + if (!opts.withDynamic) { + addWarning({ + code: "ADOPT_SKIPPED_ID_MAP", + message: + `group #${id} is "${key}" in the OpenTofu id map, which owns it — nothing adopted. ` + + `Pass --with-dynamic to capture its ruleset as a ct.dynamicGroup declaration.`, + }); + return null; + } + const dynamic = await captureRulesetFile(id, key); + if (!dynamic) { + addWarning({ + code: "ADOPT_SKIPPED_ID_MAP", + message: `group #${id} ("${key}", id map) is not a dynamic group — nothing to capture.`, + }); + return null; + } + return [ + `ct.dynamicGroup({`, + ` group: ${JSON.stringify(key)},`, + ` dynamic: { status: ${JSON.stringify(dynamic.status)}, ruleset: { ref: ${JSON.stringify(dynamic.ruleset.ref)} } },`, + `});`, + ].join("\n"); + }; + for (const id of resolvedIds) { + const mappedKey = !Object.values(state.resources).some((r) => r.type === "group" && r.id === id) + ? mappedGroupKeyById.get(id) + : undefined; + if (mappedKey !== undefined) { + const snippet = await adoptMappedGroup(id, mappedKey); + if (snippet !== null) results.push({ id, key: mappedKey, fields: {}, snippet }); + continue; + } const resource = await client.get>(GROUP_SPEC.itemPath(id)); // An already-managed group keeps its adopted key unless --rekey says otherwise (#123). This // is the mode that made the bug bite: the documented ruleset-refresh workflow passes a LIST @@ -447,78 +567,8 @@ export async function runAdoptGroups( snippetFields.memberFields = memberFields.declarations; } if (opts.withDynamic) { - const captured = await captureDynamic(id, client); - if (captured) { - const relPath = `rulesets/${key}.json`; - let rulesetToWrite = captured.normalizedRuleset; - if (opts.portableRulesets !== false) { - // Managed group ids come from state (no catalog for `group`), including any group this - // same run already adopted; the master-data kinds come from the catalog maps above. - const groupMap = new Map(); - // Managed role definitions, likewise from STATE rather than the live catalog (#125): - // state gives a per-host id under a shared logical key, which is what makes a - // duplicate role name fixable by adopting the role instead of renaming master data. - // (`ReverseResolver.idToKeyByKind("role-def")` keys by slug(name) off the catalog and - // is exactly the ambiguous mapping this replaces.) - const roleDefMap = new Map(); - for (const r of Object.values(state.resources)) { - if (r.type === "group") groupMap.set(r.id, r.key); - if (r.type === "group-role") roleDefMap.set(r.id, r.key); - } - const { ruleset, warnings: portableWarnings } = portablizeRuleset(captured.normalizedRuleset, { - idToKeyByKind: { ...portableCatalogMaps, group: groupMap, "role-def": roleDefMap }, - roleCatalog, - groupTypeIdToKey, - }); - rulesetToWrite = ruleset; - // Report every dimension left numeric, with its reason (#101). The old output said only - // "left N unmanaged id(s) numeric", which named neither the dimension nor the fix — so a - // capture that silently froze prod's ids into a cross-host file looked like a clean run. - if (portableWarnings.length > 0) { - const lines = formatPortablizeWarnings(portableWarnings); - if (opts.strictRulesets) { - throw new Error( - `--strict-rulesets: ${relPath} would contain ${portableWarnings.length} unportablized ` + - `(host-specific) id(s), so nothing was written:\n` + - lines.map((l) => ` ${l}`).join("\n"), - ); - } - addWarning({ - code: "RULESET_NOT_PORTABLE", - message: - `${relPath} keeps ${portableWarnings.length} host-specific id(s) — NOT portable to another host:\n` + - lines.map((line) => ` ${line}`).join("\n"), - }); - } - } else { - // Verbatim capture (--no-portable-rulesets): every entity id in the file is this host's. - // Say so once per ruleset rather than let the opt-out quietly imply the ids are fine. - const left = scanUnportablized(captured.normalizedRuleset); - if (left.length > 0) { - if (opts.strictRulesets) { - throw new Error( - `--strict-rulesets with --no-portable-rulesets: ${relPath} would contain ` + - `${left.length} host-specific id(s) and nothing would rewrite them.`, - ); - } - addWarning({ - code: "RULESET_NOT_PORTABLE", - message: - `${relPath} captured verbatim (--no-portable-rulesets): ${left.length} host-specific ` + - `id(s) kept as-is — NOT portable to another host.`, - }); - } - } - if (!opts.dryRun) { - await mkdir(join(project.cwd, "rulesets"), { recursive: true }); - await writeFile( - join(project.cwd, relPath), - `${JSON.stringify(rulesetToWrite, null, 2)}\n`, - "utf8", - ); - } - snippetFields.dynamic = { status: captured.status, ruleset: { ref: `./${relPath}` } }; - } + const captured = await captureRulesetFile(id, key); + if (captured) snippetFields.dynamic = captured; } const snippet = configSnippet("group", key, snippetFields, { todos }); diff --git a/src/application/operations/apply.ts b/src/application/operations/apply.ts index 6b2be7b..41325a7 100644 --- a/src/application/operations/apply.ts +++ b/src/application/operations/apply.ts @@ -275,14 +275,14 @@ export async function executePreparedApply( }); for (const key of resources.created) { const item = plan.items.find((candidate) => candidate.key === key); - const id = context.state.resources[key]?.id; + const id = context.state.resources[key]?.id ?? item?.id ?? undefined; if (item && id !== undefined) { observer.emit({ type: "resource-created", resourceType: item.type, key, id }); } } for (const key of resources.updated) { const item = plan.items.find((candidate) => candidate.key === key); - const id = context.state.resources[key]?.id; + const id = context.state.resources[key]?.id ?? item?.id ?? undefined; if (item && id !== undefined) { observer.emit({ type: "resource-updated", resourceType: item.type, key, id }); } diff --git a/src/application/operations/coverage.ts b/src/application/operations/coverage.ts index cbfd798..0205293 100644 --- a/src/application/operations/coverage.ts +++ b/src/application/operations/coverage.ts @@ -5,6 +5,7 @@ import { authedSession, type AuthedSession } from "../../api/session.js"; import { buildCoverageReport, decodeGroupsWithRoles, type CoverageReport } from "../../coverage/report.js"; import { CATALOG_DIR, loadHostCatalog } from "../../permissions/catalog-store.js"; import { fetchPermissionRows, type PermissionReader } from "../../permissions/fetch.js"; +import { loadIdMap, type IdMap } from "../../resolve/idMap.js"; import { slug } from "../../resources/registry.js"; import { loadState, type State } from "../../state/state.js"; import type { OperationResult, ProjectRequest } from "../contracts.js"; @@ -31,6 +32,7 @@ export interface CoverageOperationDependencies { resolveProject?: typeof resolveProject; loadHostCatalog?: typeof loadHostCatalog; loadState?: typeof loadState; + loadIdMap?: typeof loadIdMap; authedSession?: () => Promise; collectCoverage?: typeof collectCoverage; observer?: OperationObserver; @@ -41,6 +43,7 @@ export async function collectCoverage( client: PermissionReader & Pick, host: string, state: State, + idMap: Pick | null = null, ): Promise { const [groupRows, groupTypeRows, dynamicRows, roleDefRows] = await Promise.all([ client.getAll>(GROUPS_WITH_ROLES), @@ -68,6 +71,9 @@ export async function collectCoverage( groupTypeNames, dynamicGroupIds: parseDynamicGroupIds(dynamicRows.data), groupRolePermissions: Array.isArray(groupRolePermissions) ? groupRolePermissions : [], + idMapGroupKeys: new Map( + (idMap?.entries ?? []).filter((e) => e.type === "group").map((e) => [e.id, e.key] as const), + ), }); } @@ -85,9 +91,15 @@ export async function runCoverage( project.host, join(project.cwd, CATALOG_DIR), ); + const idMap = await (dependencies.loadIdMap ?? loadIdMap)(project.host, join(project.cwd, CATALOG_DIR)); const { client } = await (dependencies.authedSession ?? authedSession)(); observer.emit({ type: "phase-started", phase: "build-coverage" }); - const complete = await (dependencies.collectCoverage ?? collectCoverage)(client, project.host, state); + const complete = await (dependencies.collectCoverage ?? collectCoverage)( + client, + project.host, + state, + idMap, + ); let roleInstances = complete.roleInstances; if (request.type) { diff --git a/src/application/operations/refresh.ts b/src/application/operations/refresh.ts index b586715..883612c 100644 --- a/src/application/operations/refresh.ts +++ b/src/application/operations/refresh.ts @@ -3,6 +3,9 @@ import { CtApiError } from "../../api/ctClient.js"; import { fetchDynamicGroupIds } from "../../api/dynamicGroups.js"; import { authedSession, type AuthedSession } from "../../api/session.js"; import { assertNotPeople } from "../../engine/guard.js"; +import { join } from "node:path"; +import { CATALOG_DIR } from "../../permissions/catalog-store.js"; +import { loadIdMap, type IdMap } from "../../resolve/idMap.js"; import { loadState, type ManagedResource, type State } from "../../state/state.js"; import type { CtWarning, OperationResult, ProjectRequest } from "../contracts.js"; import { noopObserver, type OperationObserver } from "../ports.js"; @@ -38,6 +41,7 @@ export interface RefreshOperationDependencies { project?: ProjectResolutionDependencies; resolveProject?: typeof resolveProject; loadState?: typeof loadState; + loadIdMap?: typeof loadIdMap; authedSession?: () => Promise; selectTargets?: typeof selectRefreshTargets; observer?: OperationObserver; @@ -48,14 +52,22 @@ export async function selectRefreshTargets( client: Pick, state: State, groupKey: string | undefined, + idMap: Pick | null = null, ): Promise { const dynamicIds = await fetchDynamicGroupIds(client); if (groupKey !== undefined) { - const managed = state.resources[groupKey]; + let managed = state.resources[groupKey]; if (!managed || managed.type !== "group") { - throw new Error( - `--group "${groupKey}" is not a managed group in this state file. Adopt or declare it first.`, - ); + // A group OpenTofu owns is in the id map, not in ct's state (#202); its ruleset is still ours. + const mapped = idMap?.ids.get(`group\u0000${groupKey}`); + if (mapped === undefined) { + throw new Error( + `--group "${groupKey}" is not a managed group in this state file or the OpenTofu id map. ` + + `Adopt or declare it first.`, + ); + } + const at = new Date(0).toISOString(); + managed = { type: "group", id: mapped, key: groupKey, fields: {}, adoptedAt: at, updatedAt: at }; } if (!dynamicIds.has(managed.id)) { throw new Error( @@ -86,9 +98,15 @@ export async function runRefresh( observer.emit({ type: "phase-started", phase: "resolve-project" }); const project = await (dependencies.resolveProject ?? resolveProject)(request, dependencies.project); const state = await (dependencies.loadState ?? loadState)(project.statePath, project.host); + const idMap = await (dependencies.loadIdMap ?? loadIdMap)(project.host, join(project.cwd, CATALOG_DIR)); const { client } = await (dependencies.authedSession ?? authedSession)(); observer.emit({ type: "phase-started", phase: "select-refresh-targets" }); - const targets = await (dependencies.selectTargets ?? selectRefreshTargets)(client, state, request.group); + const targets = await (dependencies.selectTargets ?? selectRefreshTargets)( + client, + state, + request.group, + idMap, + ); const outcomes: RefreshOutcome[] = []; // The caution has to reach the operator BEFORE membership is recomputed. Returning it in diff --git a/src/application/operations/state.ts b/src/application/operations/state.ts index ac73431..ad34b0b 100644 --- a/src/application/operations/state.ts +++ b/src/application/operations/state.ts @@ -1,5 +1,8 @@ import { loadConfig } from "../../config/load.js"; import { resourceType } from "../../resources/registry.js"; +import { join } from "node:path"; +import { CATALOG_DIR } from "../../permissions/catalog-store.js"; +import { loadIdMap, type IdMap } from "../../resolve/idMap.js"; import { collectRefs, isRef, type Ref } from "../../resolve/refs.js"; import { refKindResolvesLive } from "../../resolve/resolver.js"; import { loadState, saveState, type ManagedResource } from "../../state/state.js"; @@ -14,6 +17,7 @@ export interface StateOperationDependencies { loadState?: typeof loadState; saveState?: typeof saveState; loadConfig?: typeof loadConfig; + loadIdMap?: typeof loadIdMap; lock?: MutationLock; } @@ -84,25 +88,37 @@ function declarationId(type: string, key: string): string { async function configUsage( configPath: string, dependencies: StateOperationDependencies, + idMap: Pick | null, ): Promise { const { resources, permissions } = await (dependencies.loadConfig ?? loadConfig)(configPath); - const declared = new Set(resources.map((resource) => declarationId(resource.type, resource.key))); + // A ruleset-only `ct.dynamicGroup` (#202) never creates its group, so it is not a declaration the + // guard has to protect — removing the state entry cannot make the next plan propose a create. + const declared = new Set( + resources + .filter((resource) => !resource.rulesetOnly) + .map((resource) => declarationId(resource.type, resource.key)), + ); const referenceCounts = new Map(); const stateOnlyRefs = new Set(); - const countRef = (key: string, resolvesLive: boolean): void => { + // A group key the OpenTofu id map already resolves is NOT state-only (#202): the resolver falls + // back to the map, so dropping the state entry leaves the reference resolvable. + const mappedGroup = (key: string): boolean => idMap?.ids.has(`group\u0000${key}`) ?? false; + const countRef = (key: string, resolvesLive: boolean, isGroup = false): void => { referenceCounts.set(key, (referenceCounts.get(key) ?? 0) + 1); - if (!resolvesLive) stateOnlyRefs.add(key); + if (!resolvesLive && !(isGroup && mappedGroup(key))) stateOnlyRefs.add(key); }; const addRef = (ref: Ref): void => { // The compound kinds address a permission DOMAIN, and the key they carry here is the GROUP or // GROUP TYPE the domain hangs off — so each is classified by the kind of that target, not by the // compound kind itself. A group_role domain needs its group in managed state; a group_type_role // domain resolves its type from `/group/grouptypes` like any other group-type ref. - if (ref.kind === "group-role") countRef(ref.group, refKindResolvesLive("group")); - else if (ref.kind === "group-member-field") countRef(ref.group, refKindResolvesLive("group")); + if (ref.kind === "group-role") countRef(ref.group, refKindResolvesLive("group"), true); + else if (ref.kind === "group-member-field") countRef(ref.group, refKindResolvesLive("group"), true); else if (ref.kind === "group-type-role") countRef(ref.groupType, refKindResolvesLive("group-type")); - else countRef(ref.key, refKindResolvesLive(ref.kind)); + else countRef(ref.key, refKindResolvesLive(ref.kind), ref.kind === "group"); }; + // The group a `ct.dynamicGroup` names resolves from state or the id map and nothing else. + for (const resource of resources) if (resource.rulesetOnly) countRef(resource.key, false, true); // Refs reachable from the resources themselves (`campus: "mainz"` on a group) as well as from the // permission set — the old check only walked the latter, so the count it reported was partial too. for (const ref of collectRefs(resources)) addRef(ref); @@ -151,9 +167,14 @@ export async function removeStateEntry( const warnings: CtWarning[] = []; if (!request.force) { try { + const idMap = await (dependencies.loadIdMap ?? loadIdMap)( + project.host, + join(project.cwd, CATALOG_DIR), + ); const { declared, referenceCounts, stateOnlyRefs } = await configUsage( project.configPath, dependencies, + idMap, ); // Declaredness is checked FIRST because it is the more specific verdict: it is matched on // type AND key, while the state-only reference check below can only match on key. A key that diff --git a/src/config/context.ts b/src/config/context.ts index 4d6e7fe..359cb82 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -106,6 +106,17 @@ export interface MemberFieldInput { [prop: string]: unknown; } +/** + * A ruleset-only dynamic-group declaration (#202): names an existing group by key and manages + * nothing but its dynamic status and ruleset. See {@link ConfigContext.dynamicGroup}. + */ +export interface DynamicGroupInput { + /** Logical key of the group; resolved from ct's state first, then the OpenTofu id map. */ + group: string; + /** The same three forms `ct.group({ dynamic })` takes. `true` means `./rulesets/.json`. */ + dynamic: DynamicInput; +} + export interface ResourceInput { key: string; /** Ordering hint: apply this resource after `parent`. A dependency edge only — NOT managed hierarchy. */ @@ -390,6 +401,21 @@ export interface ConfigContext { * someone had already created the Bereich by hand. */ department(input: ResourceInput): void; + /** + * Ruleset-only declaration (#202) for a group ct does not own (typically one OpenTofu manages as + * `churchtools_group`). It names the group by key and manages nothing but its dynamic status and + * ruleset: + * + * ```ts + * ct.dynamicGroup({ group: "example_autogroup", dynamic: true }); + * ``` + * + * The id resolves from ct's state first, then the OpenTofu id map; a key found in neither is a + * plan-time error, never a create. It writes no ct-state entry and has no delete path: dropping the + * declaration just stops managing the ruleset (it never demotes the group or deletes the ruleset). + * A key may not also be declared with `ct.group`, nor twice with `ct.dynamicGroup`. + */ + dynamicGroup(input: DynamicGroupInput): void; /** Master-data group role (`/group/roles`). Named `roleDefinition` to avoid colliding * with the `groupRole` permission function below. */ roleDefinition(input: ResourceInput): void; @@ -783,6 +809,12 @@ export function createContext(): { try { const resource = toDesired(type, input, location); if (seen.has(resource.key)) { + if (resources.some((r) => r.key === resource.key && r.rulesetOnly)) { + throw new Error( + `${type} "${resource.key}": the key is also declared with ct.dynamicGroup. One owner per ` + + `ruleset — put "dynamic" on this declaration, or drop it and keep only ct.dynamicGroup.`, + ); + } throw new Error(`Duplicate logical key "${resource.key}" in config.`); } seen.add(resource.key); @@ -791,6 +823,48 @@ export function createContext(): { throw relocate(err, location); } }; + const defineDynamicGroup = (input: DynamicGroupInput): void => { + const location = captureCallSite(); + try { + const group = input?.group; + if (typeof group !== "string" || group.length === 0) { + throw new Error(`dynamicGroup declaration is missing a string "group" (the group's key).`); + } + if (input.dynamic === undefined) { + throw new Error( + `dynamicGroup "${group}": "dynamic" is required (true, a ".json" string, or { status, ruleset }).`, + ); + } + const extra = Object.keys(input).filter((k) => k !== "group" && k !== "dynamic"); + if (extra.length > 0) { + throw new Error( + `dynamicGroup "${group}": unsupported field(s) ${extra.map((k) => `"${k}"`).join(", ")}. ` + + `ct.dynamicGroup manages only the ruleset; everything else about the group belongs to its owner.`, + ); + } + const owner = resources.find((r) => r.key === group); + if (owner) { + throw new Error( + owner.rulesetOnly + ? `dynamicGroup "${group}" is declared twice. A group's ruleset has exactly one declaration.` + : `dynamicGroup "${group}": the key is also declared as ${owner.type} "${group}". One owner per ` + + `ruleset — put "dynamic" on that declaration, or drop it and keep only ct.dynamicGroup.`, + ); + } + if (seen.has(group)) throw new Error(`Duplicate logical key "${group}" in config.`); + seen.add(group); + resources.push({ + type: "group", + key: group, + fields: {}, + dynamic: desugarDynamic("group", group, input.dynamic), + dependsOn: [], + rulesetOnly: true, + }); + } catch (err) { + throw relocate(err, location); + } + }; const definePermissionInner = (domainType: DomainType, input: PermissionInput): void => { if (typeof input.key !== "string" || !input.key) throw new Error(`${domainType} declaration missing a string "key".`); @@ -846,6 +920,7 @@ export function createContext(): { const ct: ConfigContext = { campus: define("campus"), group: define("group"), + dynamicGroup: defineDynamicGroup, groupType: define("group-type"), ageGroup: define("age-group"), targetGroup: define("target-group"), diff --git a/src/coverage/report.ts b/src/coverage/report.ts index f57f38d..adce94c 100644 --- a/src/coverage/report.ts +++ b/src/coverage/report.ts @@ -61,6 +61,8 @@ export interface DeclarabilityVerdict { export interface RoleInstanceCoverage extends RoleInstance { managedGroupKey: string | null; + /** The key the OpenTofu id map knows this group by (#202): owned elsewhere, so neither managed nor "unmanaged". */ + ownedElsewhereKey?: string; verdict: DeclarabilityVerdict; } @@ -72,11 +74,20 @@ export interface TypeCoverage { dynamic: number; /** Unmanaged groups that nevertheless carry authored grants — the real "you are missing this" number. */ unmanagedWithGrants: number; + /** Groups the OpenTofu id map owns (#202); left out of `unmanagedWithGrants`. Present only when non-zero. */ + ownedElsewhere?: number; } export interface CoverageReport { host: string; - groups: { total: number; managed: number; dynamic: number; managedDynamic: number }; + groups: { + total: number; + managed: number; + dynamic: number; + managedDynamic: number; + /** Groups in the OpenTofu id map that ct's state does not hold (#202). Present only when non-zero. */ + ownedElsewhere?: number; + }; grants: { authored: number; roleInstances: number; @@ -98,6 +109,8 @@ export interface CoverageInput { dynamicGroupIds: Set; /** Every row of `/permissions/group_role`, unfiltered (inherited rows are dropped here). */ groupRolePermissions: RawPermission[]; + /** Group id → key for groups the OpenTofu id map owns (#202). Optional: no map, no such groups. */ + idMapGroupKeys?: ReadonlyMap; } /** @@ -165,6 +178,12 @@ export function buildCoverageReport(input: CoverageInput): CoverageReport { if (r.type === "group") managedGroupKeyById.set(r.id, r.key); } + // A group the id map owns and ct's state does not (#202) is "owned elsewhere", not "unmanaged". + const ownedElsewhereKeyById = new Map(); + for (const [id, key] of input.idMapGroupKeys ?? []) { + if (!managedGroupKeyById.has(id)) ownedElsewhereKeyById.set(id, key); + } + const rowsByDomainId = new Map(); for (const row of groupRolePermissions) { const list = rowsByDomainId.get(row.domainId); @@ -183,6 +202,9 @@ export function buildCoverageReport(input: CoverageInput): CoverageReport { roleInstances.push({ ...role, managedGroupKey: managedGroupKeyById.get(group.id) ?? null, + ...(ownedElsewhereKeyById.has(group.id) + ? { ownedElsewhereKey: ownedElsewhereKeyById.get(group.id)! } + : {}), verdict, }); } @@ -207,7 +229,8 @@ export function buildCoverageReport(input: CoverageInput): CoverageReport { const managed = managedGroupKeyById.has(group.id); if (managed) t.managed += 1; if (dynamicGroupIds.has(group.id)) t.dynamic += 1; - if (!managed && groupsWithGrants.has(group.id)) t.unmanagedWithGrants += 1; + if (!managed && ownedElsewhereKeyById.has(group.id)) t.ownedElsewhere = (t.ownedElsewhere ?? 0) + 1; + else if (!managed && groupsWithGrants.has(group.id)) t.unmanagedWithGrants += 1; } const blockingDimensions = new Set(); @@ -220,6 +243,9 @@ export function buildCoverageReport(input: CoverageInput): CoverageReport { managed: groups.filter((g) => managedGroupKeyById.has(g.id)).length, dynamic: groups.filter((g) => dynamicGroupIds.has(g.id)).length, managedDynamic: groups.filter((g) => dynamicGroupIds.has(g.id) && managedGroupKeyById.has(g.id)).length, + ...(groups.some((g) => ownedElsewhereKeyById.has(g.id)) + ? { ownedElsewhere: groups.filter((g) => ownedElsewhereKeyById.has(g.id)).length } + : {}), }, grants: { authored: roleInstances.reduce((n, r) => n + r.verdict.grantCount, 0), @@ -285,7 +311,7 @@ export function renderCoverage(report: CoverageReport): string { const lines: string[] = []; const g = report.groups; lines.push( - `${g.total} groups · ${g.managed} managed · ${report.grants.authored} authored grants over ` + + `${g.total} groups · ${g.managed} managed${g.ownedElsewhere ? ` · ${g.ownedElsewhere} owned elsewhere (id map)` : ""} · ${report.grants.authored} authored grants over ` + `${report.grants.roleInstances} role instances`, ); lines.push(""); @@ -317,7 +343,11 @@ export function renderCoverage(report: CoverageReport): string { export function renderRoleInstances(instances: readonly RoleInstanceCoverage[]): string { return instances .map((r) => { - const managed = r.managedGroupKey ? `managed as "${r.managedGroupKey}"` : "unmanaged"; + const managed = r.managedGroupKey + ? `managed as "${r.managedGroupKey}"` + : r.ownedElsewhereKey + ? `owned elsewhere (id map) as "${r.ownedElsewhereKey}"` + : "unmanaged"; const verdict = r.verdict.declarable ? "declarable" : `blocked by ${[...r.verdict.blockedBy, ...r.verdict.unknownAuthIds.map((a) => `authId ${a}`)].join(", ")}`; diff --git a/src/engine/build.ts b/src/engine/build.ts index 2a05a6c..cde11eb 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -121,16 +121,56 @@ export async function buildPlan( opts: BuildOptions = {}, ): Promise { // Keyed by logical key (globally unique), not CT id (unique only within a type — the Mainz campus is id 0). + const resolver = opts.resolver ?? new Resolver({ client, state, desired }); + + // Ruleset-only declarations (`ct.dynamicGroup`, #202) name groups ct may not own. Give each a + // VIRTUAL state entry (id from state or the id map) so the ordinary fetch / fold / diff machinery + // sees it, without ever touching the real state: nothing here is saved, and a virtual entry has no + // delete path because the key is desired. + const rulesetOnlyKeys = new Set(); + const effective: State = { ...state, resources: { ...state.resources } }; + for (const d of desired) { + if (!d.rulesetOnly) continue; + rulesetOnlyKeys.add(d.key); + // Always virtual, even for a group in ct's state: an empty snapshot keeps the group's own fields + // out of drift reporting, and the real entry (if any) is left exactly as it is. + const at = new Date(0).toISOString(); + effective.resources[d.key] = { + type: "group", + id: resolver.groupIdForRuleset(d.key), + key: d.key, + fields: {}, + adoptedAt: at, + updatedAt: at, + }; + } + const { actual, unresolved, fetchFailed, fetchErrors, warnings: fetchWarnings = [], - } = await fetchActual(client, Object.values(state.resources)); + } = await fetchActual(client, Object.values(effective.resources)); + + // A ruleset-only target that is gone from ChurchTools is an error, never a recreate. + for (const key of rulesetOnlyKeys) { + if (!actual.has(key) && !fetchFailed.has(key) && !unresolved.has(key)) { + throw new Error( + `dynamicGroup "${key}": group #${effective.resources[key]!.id} does not exist in ChurchTools. ` + + `ct.dynamicGroup never creates a group — fix the id map / state or drop the declaration.`, + ); + } + } // Synthetic sub-resource fields (parents, dynamic, …) fold into the diff on both sides. - const folded = await foldSynthetic({ client, state, desired, actual, configDir: opts.configDir }); + const folded = await foldSynthetic({ + client, + state: effective, + desired, + actual, + configDir: opts.configDir, + }); fetchErrors.push(...folded.errors); // A synthetic field whose ACTUAL side could not be read makes its resource undiffable, exactly // like a failed top-level GET. Marking it fetch-failed is what stops a transient 429 on @@ -142,7 +182,6 @@ export async function buildPlan( // Resolution pass (#20): rewrite Ref-valued fields (and the dynamic ruleset, walked deeply) to // numbers / pending markers AFTER folding, BEFORE computePlan — so the diff stays number↔number. // Unknown/ambiguous refs THROW here (a config error, not a degrade-and-continue fetch error). - const resolver = opts.resolver ?? new Resolver({ client, state, desired }); const resolved = await Promise.all( folded.desired.map(async (d) => { const fields = await resolver.resolveValue(d.fields, `${d.type} "${d.key}"`); @@ -162,7 +201,8 @@ export async function buildPlan( return targets.length === 0 ? d : { ...d, dependsOn: [...d.dependsOn, ...targets] }; }); - const plan = computePlan(ordered, state, actual, { unresolved, fetchFailed }); + const plan = computePlan(ordered, effective, actual, { unresolved, fetchFailed }); + for (const item of plan.items) if (rulesetOnlyKeys.has(item.key)) item.rulesetOnly = true; // A `groupTypeId` change is a MIGRATION, not a field update (#171): CT refuses the field on the // ordinary update and wants `POST /groups/{id}/grouptype` with a role mapping. Resolved here, while diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 91b6895..1119668 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -158,7 +158,12 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise; + /** + * Set by `ct.dynamicGroup` (#202): this declaration manages ONLY the group's dynamic status and + * ruleset, on a group ct does not own (its id comes from ct's state or the OpenTofu id map). It + * never creates, never deletes, and never writes a state entry. Only valid on a group. + */ + rulesetOnly?: boolean; } export type PlanAction = "create" | "update" | "delete" | "no-op"; @@ -130,6 +136,8 @@ export interface PlanItem { * catalog), so the plan renders the same mapping the apply will send. */ groupTypeMigration?: GroupTypeMigration; + /** Carried from {@link DesiredResource.rulesetOnly} (#202): apply writes the ruleset and nothing to ct's state. */ + rulesetOnly?: boolean; } /** Pick a stable human-facing label without coupling renderers to resource-specific branches. */ diff --git a/src/permissions/plan.ts b/src/permissions/plan.ts index e2b005c..f5e91c6 100644 --- a/src/permissions/plan.ts +++ b/src/permissions/plan.ts @@ -334,7 +334,9 @@ export async function buildPermissionPlan( // scopeField — in one async pass here, so the per-grant `resolveScope` below stays synchronous. const scopeRefs = await resolveScopeRefs(permissions, refResolver, state); // Keys declared as groups in the config — valid scope targets even before they are created. - const declaredGroupKeys = new Set(desired.filter((r) => r.type === "group").map((r) => r.key)); + const declaredGroupKeys = new Set( + desired.filter((r) => r.type === "group" && !r.rulesetOnly).map((r) => r.key), + ); // one bulk fetch per distinct domainType — but only for CONCRETE domains. A pending domain (#69) // is a group type created this run: it has no live grants, so nothing to fetch (and on a fresh // instance the fetch would be a pure waste, or a spurious fetchError). diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index d81d9ba..e5b5c01 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -386,7 +386,8 @@ export function resolveScope( pending.push({ key, id: null, type: "group" }); } else { throw new Error( - `Scope key "${key}" does not resolve to a managed group. Declare/adopt it, use a group already under management, or pass a raw numeric dataId if this right's scope is not a group (see the catalog's scopeField).`, + `Scope key "${key}" does not resolve to a managed group. Declare/adopt it, use a group already under management, or pass a raw numeric dataId if this right's scope is not a group (see the catalog's scopeField). ` + + `If "${key}" is a group OpenTofu owns (it is in the id map, not in ct's state), a bare string cannot reach it — write { group: "${key}" } instead.`, ); } } diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts index 326ea42..4c3accb 100644 --- a/src/resolve/resolver.ts +++ b/src/resolve/resolver.ts @@ -252,6 +252,9 @@ export class Resolver { this.host = deps.host ?? deps.state.host; this.idMap = deps.idMap ?? null; for (const d of deps.desired) { + // A ruleset-only declaration (#202) names a group someone else owns; it creates nothing, so it + // must not turn that key into a same-run "pending" target — it resolves from state / id map. + if (d.rulesetOnly) continue; let set = this.declaredByType.get(d.type); if (!set) { set = new Set(); @@ -507,6 +510,25 @@ export class Resolver { return resolved; } + /** + * The id of the group a `ct.dynamicGroup` (#202) names: ct's state first, then the OpenTofu id + * map. Never a pending ref and never a create — a key found in neither is a plan-time error. + */ + groupIdForRuleset(key: string): number { + const managed = this.state.resources[key]; + if (managed && managed.type === "group") return managed.id; + const mapped = this.idMap?.ids.get(`group\u0000${key}`); + if (mapped !== undefined) return mapped; + const map = this.idMap + ? `, and ${this.idMap.path} has no group "${key}" either. If OpenTofu owns it now, refresh the ` + + `map (\`tofu state pull | ct ids sync --tofu-state -\`); otherwise adopt it or fix the key.` + : `. Adopt it, or add it to the OpenTofu id map (.ct/ids..json) if OpenTofu owns it.`; + throw new Error( + `dynamicGroup "${key}" on ${this.host}: no group with this key is in ct's state${map} ` + + `ct.dynamicGroup never creates a group.`, + ); + } + /** * Resolve the group half of a group_role ref to a managed group id (state ∪ declared ∪ id map). A * group declared in this run resolves to a {@link PendingRef} when the call site can finish it diff --git a/tests/dynamic-group.test.ts b/tests/dynamic-group.test.ts new file mode 100644 index 0000000..a73d281 --- /dev/null +++ b/tests/dynamic-group.test.ts @@ -0,0 +1,351 @@ +import { describe, expect, it, vi } from "vitest"; +import { evaluateConfig } from "../src/config/context.js"; +import { buildPlan } from "../src/engine/build.js"; +import { executePlan } from "../src/engine/execute.js"; +import { renderPlan } from "../src/engine/render.js"; +import { CtApiError } from "../src/api/ctClient.js"; +import { emptyState, type State } from "../src/state/state.js"; +import { buildCoverageReport, renderCoverage, renderRoleInstances } from "../src/coverage/report.js"; +import { selectRefreshTargets } from "../src/application/operations/refresh.js"; +import { removeStateEntry } from "../src/application/operations/state.js"; +import { resolveScope } from "../src/permissions/scope.js"; + +const HOST = "https://example.church.tools"; +const RULESET = { description: "demo", query: {}, process: {} }; +const idMap = { path: ".ct/ids.example.json", ids: new Map([["group\u0000example_auto", 7]]) }; + +function fakeClient(opts: { ruleset?: unknown; status?: string; groupMissing?: boolean } = {}) { + const calls: string[] = []; + return { + calls, + get: async (path: string): Promise => { + calls.push(`GET ${path}`); + if (path === "/groups/7") { + if (opts.groupMissing) throw new CtApiError("nf", 404, null); + return { id: 7, name: "Example", information: { groupTypeId: 1, groupStatusId: 1 } } as T; + } + if (path === "/dynamicgroups/7/ruleset") + return { ...(opts.ruleset as object), dynamicGroupUpdateStarted: "t" } as T; + if (path === "/dynamicgroups/7/status") return { dynamicGroupStatus: opts.status ?? "active" } as T; + throw new CtApiError(`not found: ${path}`, 404, null); + }, + }; +} + +async function desiredFor(dynamic: unknown) { + const { resources } = await evaluateConfig((ct) => { + ct.dynamicGroup({ group: "example_auto", dynamic: dynamic as never }); + }); + return resources; +} + +describe("ct.dynamicGroup declaration", () => { + it("desugars to a ruleset-only group resource with no fields", async () => { + const [r] = await desiredFor(true); + expect(r).toMatchObject({ + type: "group", + key: "example_auto", + fields: {}, + rulesetOnly: true, + dynamic: { status: "active", ruleset: { ref: "./rulesets/example_auto.json" } }, + }); + }); + + it("takes the explicit and path forms like ct.group", async () => { + expect((await desiredFor("./r/x.json"))[0]?.dynamic).toEqual({ + status: "active", + ruleset: { ref: "./r/x.json" }, + }); + expect((await desiredFor({ status: "manual", ruleset: RULESET }))[0]?.dynamic?.status).toBe("manual"); + }); + + it("refuses a key also declared with ct.group, in either order", async () => { + await expect( + evaluateConfig((ct) => { + ct.group({ key: "example_auto", name: "Example" }); + ct.dynamicGroup({ group: "example_auto", dynamic: true }); + }), + ).rejects.toThrow(/also declared as group/); + await expect( + evaluateConfig((ct) => { + ct.dynamicGroup({ group: "example_auto", dynamic: true }); + ct.group({ key: "example_auto", name: "Example" }); + }), + ).rejects.toThrow(/also declared with ct.dynamicGroup/); + }); + + it("refuses two declarations for one group, a missing dynamic and stray fields", async () => { + await expect( + evaluateConfig((ct) => { + ct.dynamicGroup({ group: "g", dynamic: true }); + ct.dynamicGroup({ group: "g", dynamic: true }); + }), + ).rejects.toThrow(/declared twice/); + await expect(evaluateConfig((ct) => ct.dynamicGroup({ group: "g" } as never))).rejects.toThrow( + /"dynamic" is required/, + ); + await expect( + evaluateConfig((ct) => ct.dynamicGroup({ group: "g", dynamic: true, name: "x" } as never)), + ).rejects.toThrow(/unsupported field/); + }); +}); + +describe("ct.dynamicGroup plan", () => { + const inline = { status: "active", ruleset: RULESET }; + + it("plans one dynamic update when the ruleset changed, and nothing else", async () => { + const { Resolver } = await import("../src/resolve/resolver.js"); + const state = emptyState(HOST); + const desired = await desiredFor(inline); + const resolver = new Resolver({ client: { get: async () => [] as never }, state, desired, idMap }); + const { plan } = await buildPlan( + fakeClient({ ruleset: { ...RULESET, description: "old" } }), + state, + desired, + { + resolver, + }, + ); + expect(plan.items).toHaveLength(1); + expect(plan.items[0]).toMatchObject({ key: "example_auto", id: 7, action: "update", rulesetOnly: true }); + expect(plan.items[0]!.changes.map((c) => c.field)).toEqual(["dynamic"]); + const text = renderPlan(plan); + expect(text).toContain("group.example_auto"); + expect(text).not.toMatch(/\+ |- group/); + }); + + it("resolves from the id map for real and no-ops (acceptance: id map, not state)", async () => { + const { Resolver } = await import("../src/resolve/resolver.js"); + const state = emptyState(HOST); + const desired = await desiredFor(inline); + const resolver = new Resolver({ client: { get: async () => [] as never }, state, desired, idMap }); + const { plan } = await buildPlan(fakeClient({ ruleset: RULESET }), state, desired, { resolver }); + expect(plan.items).toHaveLength(1); + expect(plan.items[0]).toMatchObject({ id: 7, action: "no-op" }); + expect(renderPlan(plan)).toContain("No changes"); + }); + + it("plans a no-op and writes nothing when the declaration is dropped", async () => { + const state = emptyState(HOST); + const client = fakeClient({ ruleset: RULESET }); + const { plan } = await buildPlan(client, state, []); + expect(plan.items).toEqual([]); + expect(client.calls).toEqual([]); + expect(renderPlan(plan)).toContain("No changes"); + }); + + it("fails at plan time for a key in neither state nor the id map, never a create", async () => { + const { Resolver } = await import("../src/resolve/resolver.js"); + const state = emptyState(HOST); + const desired = await desiredFor(inline); + const resolver = new Resolver({ + client: { get: async () => [] as never }, + state, + desired, + idMap: { path: ".ct/ids.example.json", ids: new Map() }, + }); + await expect(buildPlan(fakeClient({}), state, desired, { resolver })).rejects.toThrow( + /no group with this key.*never creates a group/s, + ); + }); + + it("fails (not a recreate) when the group is gone from ChurchTools", async () => { + const { Resolver } = await import("../src/resolve/resolver.js"); + const state = emptyState(HOST); + const desired = await desiredFor(inline); + const resolver = new Resolver({ client: { get: async () => [] as never }, state, desired, idMap }); + await expect(buildPlan(fakeClient({ groupMissing: true }), state, desired, { resolver })).rejects.toThrow( + /does not exist in ChurchTools/, + ); + }); + + it("degrades an unreadable ruleset to fetch-failed (#126)", async () => { + const { Resolver } = await import("../src/resolve/resolver.js"); + const state = emptyState(HOST); + const desired = await desiredFor(inline); + const resolver = new Resolver({ client: { get: async () => [] as never }, state, desired, idMap }); + const client = { + get: async (path: string): Promise => { + if (path === "/groups/7") return { id: 7, name: "Example", information: {} } as T; + throw new CtApiError("busy", 429, null); + }, + }; + const { plan, fetchErrors } = await buildPlan(client, state, desired, { resolver }); + expect(plan.items[0]).toMatchObject({ action: "no-op", note: "fetch-failed" }); + expect(fetchErrors.length).toBe(1); + }); + + it("prefers the state id when the group is in ct's state, and never reports drift on its fields", async () => { + const { Resolver } = await import("../src/resolve/resolver.js"); + const state = emptyState(HOST); + state.resources.example_auto = { + type: "group", + id: 7, + key: "example_auto", + fields: { name: "Old name" }, + adoptedAt: "t", + updatedAt: "t", + }; + const desired = await desiredFor(inline); + const resolver = new Resolver({ client: { get: async () => [] as never }, state, desired }); + const { plan } = await buildPlan(fakeClient({ ruleset: RULESET }), state, desired, { resolver }); + expect(plan.items[0]).toMatchObject({ action: "no-op" }); + expect(plan.items[0]!.drift).toBeUndefined(); + }); +}); + +describe("ct.dynamicGroup apply", () => { + it("writes the ruleset and status and leaves ct's state untouched", async () => { + const state: State = emptyState(HOST); + const save = vi.fn(async () => {}); + const calls: string[] = []; + const client = { + request: async (method: string, path: string): Promise => { + calls.push(`${method} ${path}`); + return {} as T; + }, + }; + const result = await executePlan( + { + items: [ + { + type: "group", + key: "example_auto", + id: 7, + action: "update", + rulesetOnly: true, + actual: { name: "Example" }, + changes: [ + { + field: "dynamic", + from: { status: "active", ruleset: { description: "old", query: {}, process: {} } }, + to: { status: "active", ruleset: RULESET }, + }, + ], + }, + ], + }, + { client, state, statePath: "s.json", save }, + ); + expect(result.updated).toEqual(["example_auto"]); + expect(calls).toEqual(["PUT /dynamicgroups/7/ruleset", "PUT /dynamicgroups/7/status"]); + expect(Object.keys(state.resources)).toEqual([]); + expect(save).not.toHaveBeenCalled(); + }); +}); + +describe("bare-string group scopes", () => { + it("fail with a hint to use { group } when the group is only in the id map", () => { + expect(() => resolveScope(["example_auto"], emptyState(HOST), new Set(), {})).toThrow( + /write \{ group: "example_auto" \} instead/, + ); + }); +}); + +describe("ct state rm guard", () => { + const entry = { + type: "group", + id: 7, + key: "example_auto", + fields: {}, + adoptedAt: "t", + updatedAt: "t", + }; + const project = { + cwd: "/project", + configPath: "/project/ct.config.ts", + statePath: "/project/state.json", + environmentsPath: "/project/ct.envs.json", + configDisplayPath: "ct.config.ts", + stateDisplayPath: "state.json", + environment: "dev", + protected: false, + host: HOST, + }; + const deps = async (withMap: boolean, dynamicGroup: boolean) => { + const state = emptyState(HOST); + state.resources.example_auto = { ...entry }; + const loaded = await evaluateConfig((ct) => { + if (dynamicGroup) ct.dynamicGroup({ group: "example_auto", dynamic: true }); + ct.groupRole({ key: "r", group: "example_auto", role: "Leader", grants: [] } as never); + }); + return { + state, + saveState: vi.fn(async () => {}), + resolveProject: vi.fn(async () => project), + loadState: vi.fn(async () => state), + loadConfig: vi.fn(async () => ({ ...loaded, configDir: "/project" })), + loadIdMap: vi.fn(async () => + withMap ? { ...idMap, meta: null, entries: [], keysByLabel: new Map() } : null, + ), + }; + }; + + it("refuses removal of a state-only group a grant still names", async () => { + const d = await deps(false, false); + await expect(removeStateEntry({ type: "group", key: "example_auto" }, d)).rejects.toThrow( + /only ct's state can resolve/, + ); + }); + + it("allows removal when the id map already resolves the group", async () => { + const d = await deps(true, true); + const result = await removeStateEntry({ type: "group", key: "example_auto" }, d); + expect(result.value.removed).toBe(true); + expect(d.state.resources.example_auto).toBeUndefined(); + }); + + it("does not treat a ruleset-only declaration as a declaration that would recreate the group", async () => { + const d = await deps(true, true); + await expect(removeStateEntry({ type: "group", key: "example_auto" }, d)).resolves.toBeDefined(); + }); + + it("still refuses when a ct.dynamicGroup names a group only ct's state can resolve", async () => { + const d = await deps(false, true); + await expect(removeStateEntry({ type: "group", key: "example_auto" }, d)).rejects.toThrow( + /only ct's state can resolve/, + ); + }); +}); + +describe("ct refresh --group with the id map", () => { + const client = { getAll: async () => ({ data: [{ id: 7 }] }) } as never; + + it("selects an id-map group that is not in state", async () => { + const targets = await selectRefreshTargets(client, emptyState(HOST), "example_auto", idMap); + expect(targets.map((t) => [t.key, t.id])).toEqual([["example_auto", 7]]); + }); + + it("still rejects a key in neither", async () => { + await expect(selectRefreshTargets(client, emptyState(HOST), "nope", idMap)).rejects.toThrow( + /state file or the OpenTofu id map/, + ); + }); +}); + +describe("ct coverage with the id map", () => { + it("labels id-map groups as owned elsewhere, not unmanaged", () => { + const report = buildCoverageReport({ + host: HOST, + state: emptyState(HOST), + groups: [ + { + id: 7, + name: "Example", + groupTypeId: 1, + roles: [{ domainId: 70, groupId: 7, groupName: "Example", groupTypeId: 1, roleName: "Leader" }], + }, + ], + groupTypeNames: new Map([[1, "Type"]]), + dynamicGroupIds: new Set([7]), + groupRolePermissions: [{ authId: 1, domainId: 70, domainType: "group_role", dataId: [] } as never], + idMapGroupKeys: new Map([[7, "example_auto"]]), + }); + expect(report.groups.ownedElsewhere).toBe(1); + expect(report.byType[0]).toMatchObject({ unmanagedWithGrants: 0, ownedElsewhere: 1 }); + expect(renderCoverage(report)).toContain("1 owned elsewhere (id map)"); + for (const r of report.roleInstances) { + expect(renderRoleInstances([r])).toContain('owned elsewhere (id map) as "example_auto"'); + } + }); +}); From 2ade7f1257973af5f80139d3c991bc8d62c200d6 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Fri, 16 Oct 2026 19:40:52 +0200 Subject: [PATCH 2/2] fix(config): harden ct.dynamicGroup review findings - state rm: read the id map softly so an unreadable map makes the guard stricter instead of skipping it - plan: refuse a ct.dynamicGroup whose key ct's state holds as another type - adopt: no empty block or state write when every id-map group is skipped; say no state was written; warn that --key/--with-member-fields do not apply - coverage: show the owned-elsewhere count per type - share virtualGroupEntry between plan and refresh; apply's event id fallback only for ruleset-only items --- docs/handbuch/dynamic-groups.md | 6 ++- docs/handbuch/group-member-fields.md | 6 ++- src/application/operations/adopt-group.ts | 31 ++++++++++-- src/application/operations/apply.ts | 5 +- src/application/operations/refresh.ts | 5 +- src/application/operations/state.ts | 17 +++++-- src/commands/adopt-group.ts | 6 +++ src/coverage/report.ts | 16 +++--- src/engine/build.ts | 21 ++++---- src/state/state.ts | 10 ++++ tests/adopt-group-command.test.ts | 59 +++++++++++++++++++++++ tests/dynamic-group.test.ts | 28 +++++++++++ 12 files changed, 178 insertions(+), 32 deletions(-) diff --git a/docs/handbuch/dynamic-groups.md b/docs/handbuch/dynamic-groups.md index 7058337..1286318 100644 --- a/docs/handbuch/dynamic-groups.md +++ b/docs/handbuch/dynamic-groups.md @@ -6,7 +6,7 @@ sources: - src/engine/dynamic.ts - src/engine/synthetic.ts - src/application/operations/adopt-group.ts -sources_hash: 11cdc86261ab9d0c +sources_hash: 93139d34ad478e84 reviewed: 2026-08-28 --- @@ -543,7 +543,9 @@ status-only flips skipping the ruleset PUT) apply unchanged. A key declared with `ct.dynamicGroup`, or with `ct.dynamicGroup` twice, is refused at config load: one owner per ruleset. `ct refresh --group ` and `ct adopt group --with-dynamic` also accept an id-map group; the -adopt output is then a `ct.dynamicGroup` snippet and no state entry is written. +adopt output is then a `ct.dynamicGroup` snippet and no state entry is written. The id map's key +wins over `--key`, and `--with-member-fields` does not apply (the group's owner declares those); +adopt warns about either. Without `--with-dynamic`, an id-map group is skipped with a warning. ## Managed guard: undeclared groups stay invisible diff --git a/docs/handbuch/group-member-fields.md b/docs/handbuch/group-member-fields.md index 011cab7..99b41c6 100644 --- a/docs/handbuch/group-member-fields.md +++ b/docs/handbuch/group-member-fields.md @@ -1,5 +1,5 @@ --- -sources_hash: 75e94f40f8a2f6d9 +sources_hash: 50621f5e4d9e5064 title: Group member fields sources: - src/engine/member-fields.ts @@ -145,6 +145,10 @@ Rows the group's member form shows but that are not group-scoped (person master data, group-type defaults) are not emitted: only `/memberfields/group` rows can be created or updated. +A group the OpenTofu id map owns (see `ct.dynamicGroup` in +[Dynamic groups](dynamic-groups.md)) is not adopted with its member fields: its +owner declares them, so adopt ignores the flag for that group and warns. + !!! note "Why opt-in, and not the default — and why that is temporary" Whether an owned child resource is adopted automatically is a project-wide contract, not a per-feature choice. That contract now exists diff --git a/src/application/operations/adopt-group.ts b/src/application/operations/adopt-group.ts index a5400be..cf27c7f 100644 --- a/src/application/operations/adopt-group.ts +++ b/src/application/operations/adopt-group.ts @@ -496,6 +496,17 @@ export async function runAdoptGroups( }; /** `ct adopt group` on a group the id map owns: capture only the ruleset, write no state entry (#202). */ const adoptMappedGroup = async (id: number, key: string): Promise => { + // The map's key and OpenTofu's ownership win: say so rather than drop the options silently. + const ignored = [ + ...(opts.key && opts.key !== key ? [`--key (the id map names it "${key}")`] : []), + ...(opts.withMemberFields ? ["--with-member-fields (the group's owner declares those)"] : []), + ]; + if (ignored.length > 0) { + addWarning({ + code: "ADOPT_ID_MAP_OPTION_IGNORED", + message: `group #${id} ("${key}", id map): ignored ${ignored.join(" and ")}.`, + }); + } if (!opts.withDynamic) { addWarning({ code: "ADOPT_SKIPPED_ID_MAP", @@ -521,10 +532,13 @@ export async function runAdoptGroups( ].join("\n"); }; + const stateGroupIds = new Set( + Object.values(state.resources) + .filter((r) => r.type === "group") + .map((r) => r.id), + ); for (const id of resolvedIds) { - const mappedKey = !Object.values(state.resources).some((r) => r.type === "group" && r.id === id) - ? mappedGroupKeyById.get(id) - : undefined; + const mappedKey = stateGroupIds.has(id) ? undefined : mappedGroupKeyById.get(id); if (mappedKey !== undefined) { const snippet = await adoptMappedGroup(id, mappedKey); if (snippet !== null) results.push({ id, key: mappedKey, fields: {}, snippet }); @@ -600,7 +614,16 @@ export async function runAdoptGroups( }; } - await saveState(project.statePath, state); + // Only id-map groups were selected and every one was skipped: nothing to save or paste. + if (results.length === 0) { + return { + operation: "adopt", + project, + warnings, + value: { groups: [], configBlock: null, noMatches: false, dryRun: false }, + }; + } + if (reports.length > 0) await saveState(project.statePath, state); if (reports.some((report) => report.action === "updated")) { addWarning({ code: "ADOPT_ALREADY_MANAGED", diff --git a/src/application/operations/apply.ts b/src/application/operations/apply.ts index 41325a7..b998544 100644 --- a/src/application/operations/apply.ts +++ b/src/application/operations/apply.ts @@ -275,14 +275,15 @@ export async function executePreparedApply( }); for (const key of resources.created) { const item = plan.items.find((candidate) => candidate.key === key); - const id = context.state.resources[key]?.id ?? item?.id ?? undefined; + const id = context.state.resources[key]?.id; if (item && id !== undefined) { observer.emit({ type: "resource-created", resourceType: item.type, key, id }); } } for (const key of resources.updated) { const item = plan.items.find((candidate) => candidate.key === key); - const id = context.state.resources[key]?.id ?? item?.id ?? undefined; + // A ruleset-only item (#202) has no state entry by design; its id is on the plan item. + const id = item?.rulesetOnly ? (item.id ?? undefined) : context.state.resources[key]?.id; if (item && id !== undefined) { observer.emit({ type: "resource-updated", resourceType: item.type, key, id }); } diff --git a/src/application/operations/refresh.ts b/src/application/operations/refresh.ts index 883612c..26dfbf6 100644 --- a/src/application/operations/refresh.ts +++ b/src/application/operations/refresh.ts @@ -6,7 +6,7 @@ import { assertNotPeople } from "../../engine/guard.js"; import { join } from "node:path"; import { CATALOG_DIR } from "../../permissions/catalog-store.js"; import { loadIdMap, type IdMap } from "../../resolve/idMap.js"; -import { loadState, type ManagedResource, type State } from "../../state/state.js"; +import { loadState, virtualGroupEntry, type ManagedResource, type State } from "../../state/state.js"; import type { CtWarning, OperationResult, ProjectRequest } from "../contracts.js"; import { noopObserver, type OperationObserver } from "../ports.js"; import { resolveProject, type ProjectResolutionDependencies } from "../project.js"; @@ -66,8 +66,7 @@ export async function selectRefreshTargets( `Adopt or declare it first.`, ); } - const at = new Date(0).toISOString(); - managed = { type: "group", id: mapped, key: groupKey, fields: {}, adoptedAt: at, updatedAt: at }; + managed = virtualGroupEntry(groupKey, mapped); } if (!dynamicIds.has(managed.id)) { throw new Error( diff --git a/src/application/operations/state.ts b/src/application/operations/state.ts index ad34b0b..4f335c8 100644 --- a/src/application/operations/state.ts +++ b/src/application/operations/state.ts @@ -166,11 +166,20 @@ export async function removeStateEntry( const warnings: CtWarning[] = []; if (!request.force) { + // Read the id map SOFTLY and outside the catch below: a malformed or foreign-host map must make + // the guard stricter (no map → mapped groups count as state-only), never skip it. + let idMap: Awaited> = null; + try { + idMap = await (dependencies.loadIdMap ?? loadIdMap)(project.host, join(project.cwd, CATALOG_DIR)); + } catch (caught) { + warnings.push({ + code: "ID_MAP_UNREADABLE", + message: + `Could not read the OpenTofu id map (${caught instanceof Error ? caught.message : String(caught)}) — ` + + `checking references as if no group were in it.`, + }); + } try { - const idMap = await (dependencies.loadIdMap ?? loadIdMap)( - project.host, - join(project.cwd, CATALOG_DIR), - ); const { declared, referenceCounts, stateOnlyRefs } = await configUsage( project.configPath, dependencies, diff --git a/src/commands/adopt-group.ts b/src/commands/adopt-group.ts index 404ff64..516f515 100644 --- a/src/commands/adopt-group.ts +++ b/src/commands/adopt-group.ts @@ -78,10 +78,16 @@ export function adoptGroupCommand(): Command { return; } for (const group of result.value.groups) { + // action null: an id-map group (#202) — only its ruleset was captured, no state was written. + if (group.action === null) { + success(`Captured the ruleset of group #${group.id} ("${group.key}", id map) — no state written`); + continue; + } success( `${group.action === "created" ? "Adopted" : "Updated"} group #${group.id} as "${group.key}" → ${result.project.stateDisplayPath}`, ); } + if (result.value.configBlock === null) return; info(result.value.groups.length === 1 ? "Config entry:" : "Config entries (paste into your config):"); process.stdout.write(`${result.value.configBlock}\n`); }); diff --git a/src/coverage/report.ts b/src/coverage/report.ts index adce94c..764152a 100644 --- a/src/coverage/report.ts +++ b/src/coverage/report.ts @@ -229,10 +229,12 @@ export function buildCoverageReport(input: CoverageInput): CoverageReport { const managed = managedGroupKeyById.has(group.id); if (managed) t.managed += 1; if (dynamicGroupIds.has(group.id)) t.dynamic += 1; - if (!managed && ownedElsewhereKeyById.has(group.id)) t.ownedElsewhere = (t.ownedElsewhere ?? 0) + 1; + // ownedElsewhereKeyById already excludes managed ids, so the two branches are disjoint. + if (ownedElsewhereKeyById.has(group.id)) t.ownedElsewhere = (t.ownedElsewhere ?? 0) + 1; else if (!managed && groupsWithGrants.has(group.id)) t.unmanagedWithGrants += 1; } + const ownedElsewhere = groups.filter((g) => ownedElsewhereKeyById.has(g.id)).length; const blockingDimensions = new Set(); for (const r of roleInstances) for (const d of r.verdict.blockedBy) blockingDimensions.add(d); @@ -243,9 +245,7 @@ export function buildCoverageReport(input: CoverageInput): CoverageReport { managed: groups.filter((g) => managedGroupKeyById.has(g.id)).length, dynamic: groups.filter((g) => dynamicGroupIds.has(g.id)).length, managedDynamic: groups.filter((g) => dynamicGroupIds.has(g.id) && managedGroupKeyById.has(g.id)).length, - ...(groups.some((g) => ownedElsewhereKeyById.has(g.id)) - ? { ownedElsewhere: groups.filter((g) => ownedElsewhereKeyById.has(g.id)).length } - : {}), + ...(ownedElsewhere > 0 ? { ownedElsewhere } : {}), }, grants: { authored: roleInstances.reduce((n, r) => n + r.verdict.grantCount, 0), @@ -316,14 +316,18 @@ export function renderCoverage(report: CoverageReport): string { ); lines.push(""); const nameWidth = Math.max(12, ...report.byType.map((t) => t.name.length)); + // The "elsewhere" column appears only when the id map owns any group (#202), so a host without + // OpenTofu keeps the table it always had. + const elsewhere = g.ownedElsewhere !== undefined; lines.push( `${"by type".padEnd(nameWidth)} ${"total".padStart(5)} ${"managed".padStart(7)} ` + - `${"dynamic".padStart(7)} unmanaged+grants`, + `${"dynamic".padStart(7)} unmanaged+grants${elsewhere ? " elsewhere" : ""}`, ); for (const t of report.byType) { lines.push( ` ${t.name.padEnd(nameWidth - 2)} ${String(t.total).padStart(5)} ${String(t.managed).padStart(7)} ` + - `${String(t.dynamic).padStart(7)} ${String(t.unmanagedWithGrants).padStart(16)}`, + `${String(t.dynamic).padStart(7)} ${String(t.unmanagedWithGrants).padStart(16)}` + + (elsewhere ? ` ${String(t.ownedElsewhere ?? 0).padStart(9)}` : ""), ); } lines.push(""); diff --git a/src/engine/build.ts b/src/engine/build.ts index cde11eb..d0b1c70 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -7,7 +7,7 @@ */ import type { CtClient } from "../api/ctClient.js"; import { CtApiError } from "../api/ctClient.js"; -import type { ManagedResource, State } from "../state/state.js"; +import { virtualGroupEntry, type ManagedResource, type State } from "../state/state.js"; import type { DesiredResource, Plan } from "./types.js"; import { RESOURCES, type CtWriteClient } from "../resources/registry.js"; import { computePlan } from "./plan.js"; @@ -132,17 +132,18 @@ export async function buildPlan( for (const d of desired) { if (!d.rulesetOnly) continue; rulesetOnlyKeys.add(d.key); + // Logical keys are unique across types, so a non-group entry under this key would be hidden + // from the plan (no fetch, no stale report) by the overlay below. Refuse rather than shadow it. + const held = state.resources[d.key]; + if (held && held.type !== "group") { + throw new Error( + `dynamicGroup "${d.key}": ct's state holds "${d.key}" as a ${held.type} (#${held.id}). Logical keys ` + + `are unique across types — rename one of them.`, + ); + } // Always virtual, even for a group in ct's state: an empty snapshot keeps the group's own fields // out of drift reporting, and the real entry (if any) is left exactly as it is. - const at = new Date(0).toISOString(); - effective.resources[d.key] = { - type: "group", - id: resolver.groupIdForRuleset(d.key), - key: d.key, - fields: {}, - adoptedAt: at, - updatedAt: at, - }; + effective.resources[d.key] = virtualGroupEntry(d.key, resolver.groupIdForRuleset(d.key)); } const { diff --git a/src/state/state.ts b/src/state/state.ts index 5311b83..0702082 100644 --- a/src/state/state.ts +++ b/src/state/state.ts @@ -69,6 +69,16 @@ export function emptyState(host: string): State { return { version: 1, host, resources: {} }; } +/** + * An in-memory stand-in for a group ct reaches but does not own (#202, a `ct.dynamicGroup` or an + * id-map group): enough for the fetch / fold / refresh machinery, never saved, and with an empty + * snapshot so the owner's fields never surface as drift. + */ +export function virtualGroupEntry(key: string, id: number): ManagedResource { + const at = new Date(0).toISOString(); + return { type: "group", id, key, fields: {}, adoptedAt: at, updatedAt: at }; +} + /** * Load the state file at `path`, validating its shape and asserting it belongs * to `host`. A missing file yields an empty state for `host`. `host` is the diff --git a/tests/adopt-group-command.test.ts b/tests/adopt-group-command.test.ts index a901840..498f0fb 100644 --- a/tests/adopt-group-command.test.ts +++ b/tests/adopt-group-command.test.ts @@ -745,3 +745,62 @@ describe("ct adopt group --with-member-fields (#135)", () => { expect(state.resources.area_a!.memberFields).toBeUndefined(); }); }); + +describe("ct adopt group — a group the OpenTofu id map owns (#202)", () => { + async function mapGroups(entries: Array<{ key: string; id: number }>): Promise { + const { writeIdMap } = await import("../src/resolve/idMap.js"); + await writeIdMap( + HOST, + entries.map((e) => ({ type: "group", ...e })), + join(workDir, ".ct"), + ); + } + + async function captureStdout(fn: () => Promise): Promise { + const writes: string[] = []; + const spy = vi.spyOn(process.stdout, "write").mockImplementation((s) => { + writes.push(String(s)); + return true; + }); + try { + await fn(); + } finally { + spy.mockRestore(); + } + return writes.join(""); + } + + it("--with-dynamic emits a ct.dynamicGroup snippet and writes no state entry", async () => { + await mapGroups([{ key: "tofu_auto", id: 30 }]); + const out = await captureStdout(() => run(["group", "30", "--with-dynamic", "--state", statePath])); + expect(out).toContain("ct.dynamicGroup({"); + expect(out).toContain('group: "tofu_auto"'); + expect(out).not.toContain("→ "); + await expect(readFile(statePath, "utf8")).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it("prints no empty block and saves nothing when every selected group is skipped", async () => { + await mapGroups([ + { key: "tofu_auto", id: 30 }, + { key: "tofu_static", id: 31 }, + ]); + const out = await captureStdout(() => run(["group", "30", "31", "--state", statePath])); + expect(out).not.toContain("// group"); + expect(out).not.toContain("null"); + await expect(readFile(statePath, "utf8")).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it("warns that --key and --with-member-fields do not apply to it", async () => { + await mapGroups([{ key: "tofu_auto", id: 30 }]); + const { runAdoptGroups } = await import("../src/application/operations/adopt-group.js"); + const result = await runAdoptGroups({ + ids: ["30"], + key: "other", + statePath, + withDynamic: true, + withMemberFields: true, + }); + expect(result.warnings.map((w) => w.code)).toContain("ADOPT_ID_MAP_OPTION_IGNORED"); + expect(result.value.groups).toEqual([expect.objectContaining({ key: "tofu_auto", action: null })]); + }); +}); diff --git a/tests/dynamic-group.test.ts b/tests/dynamic-group.test.ts index a73d281..f93907c 100644 --- a/tests/dynamic-group.test.ts +++ b/tests/dynamic-group.test.ts @@ -149,6 +149,24 @@ describe("ct.dynamicGroup plan", () => { ); }); + it("refuses a key that ct's state holds as another resource type", async () => { + const { Resolver } = await import("../src/resolve/resolver.js"); + const state = emptyState(HOST); + state.resources.example_auto = { + type: "campus", + id: 3, + key: "example_auto", + fields: {}, + adoptedAt: "t", + updatedAt: "t", + }; + const desired = await desiredFor(inline); + const resolver = new Resolver({ client: { get: async () => [] as never }, state, desired, idMap }); + await expect(buildPlan(fakeClient({ ruleset: RULESET }), state, desired, { resolver })).rejects.toThrow( + /state holds "example_auto" as a campus/, + ); + }); + it("fails (not a recreate) when the group is gone from ChurchTools", async () => { const { Resolver } = await import("../src/resolve/resolver.js"); const state = emptyState(HOST); @@ -300,6 +318,15 @@ describe("ct state rm guard", () => { await expect(removeStateEntry({ type: "group", key: "example_auto" }, d)).resolves.toBeDefined(); }); + it("fails closed, not open, when the id map is unreadable", async () => { + const d = await deps(true, true); + d.loadIdMap.mockRejectedValueOnce(new Error("Id map was generated for another host")); + await expect(removeStateEntry({ type: "group", key: "example_auto" }, d)).rejects.toThrow( + /only ct's state can resolve/, + ); + expect(d.state.resources.example_auto).toBeDefined(); + }); + it("still refuses when a ct.dynamicGroup names a group only ct's state can resolve", async () => { const d = await deps(false, true); await expect(removeStateEntry({ type: "group", key: "example_auto" }, d)).rejects.toThrow( @@ -344,6 +371,7 @@ describe("ct coverage with the id map", () => { expect(report.groups.ownedElsewhere).toBe(1); expect(report.byType[0]).toMatchObject({ unmanagedWithGrants: 0, ownedElsewhere: 1 }); expect(renderCoverage(report)).toContain("1 owned elsewhere (id map)"); + expect(renderCoverage(report)).toMatch(/unmanaged\+grants\s+elsewhere/); for (const r of report.roleInstances) { expect(renderRoleInstances([r])).toContain('owned elsewhere (id map) as "example_auto"'); }