Skip to content

Commit 9fc8b3c

Browse files
Bmad epic 6 (#321)
* Create Epic 6 story files and update sprint status Add story files for all four Epic 6 stories (Identity & Audit integration tests) and transition epic to in-progress with all stories at ready-for-dev status. Co-authored-by: Cursor <cursoragent@cursor.com> * Add integration tests for Group and GroupAlias types (Story 6.1) Covers create, update, and delete with full Vault state verification for both Group (identity groups) and GroupAlias (asymmetric API with PrepareInternalValues accessor lookup). Includes exact policy list assertions and mount_accessor cross-verification against sys/auth. Co-authored-by: Cursor <cursoragent@cursor.com> * Add integration tests for Identity OIDC types (Story 6.2) Integration tests covering IdentityOIDCScope, IdentityOIDCAssignment, IdentityOIDCClient, and IdentityOIDCProvider full lifecycle (create, update, delete) with Vault state verification. Includes 4 decoder methods, 4 controller registrations, and review-driven assertions for redirect_uris, assignments, and allowed_client_ids. Co-authored-by: Cursor <cursoragent@cursor.com> * Add integration tests for Identity Token types (Story 6.3) Co-authored-by: Cursor <cursoragent@cursor.com> * Add integration tests for Audit and AuditRequestHeader types (Story 6.4) Co-authored-by: Cursor <cursoragent@cursor.com> * Add Epic 6 retrospective and Story 7.0 test performance refactoring task Epic 6 retro completed — short epic with clean code reviews. Story 7.0 added as warm-up task for Epic 7 to refactor RandomSecret integration tests to share fixtures via BeforeAll/AfterAll, eliminating ~100-150s of redundant setup/teardown across 4 Context blocks. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 7ce3e42 commit 9fc8b3c

17 files changed

Lines changed: 4133 additions & 7 deletions

_bmad-output/implementation-artifacts/6-1-integration-tests-for-group-and-groupalias-types.md

Lines changed: 688 additions & 0 deletions
Large diffs are not rendered by default.

_bmad-output/implementation-artifacts/6-2-integration-tests-for-identity-oidc-types.md

Lines changed: 630 additions & 0 deletions
Large diffs are not rendered by default.

_bmad-output/implementation-artifacts/6-3-integration-tests-for-identity-token-types.md

Lines changed: 595 additions & 0 deletions
Large diffs are not rendered by default.

_bmad-output/implementation-artifacts/6-4-integration-tests-for-audit-types.md

Lines changed: 544 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# Story 7.0: Refactor RandomSecret Test to Use Shared Fixtures
2+
3+
Status: backlog
4+
5+
## Story
6+
7+
As an operator developer,
8+
I want the RandomSecret integration tests to share prerequisite resources across Context blocks using Ginkgo BeforeAll/AfterAll,
9+
So that redundant setup/teardown is eliminated and the integration test suite runs significantly faster.
10+
11+
## Background / Motivation
12+
13+
Performance profiling of the integration test suite (Epic 6 retrospective, 2026-05-02) identified RandomSecret and VaultSecret tests as the biggest time outliers — consuming ~31% of total test time despite not integrating with any external tools (no PostgreSQL, RabbitMQ, LDAP, etc.).
14+
15+
Root cause analysis revealed that the slowness is **structural**: each of the 4 `Context` blocks in `randomsecret_controller_test.go` independently creates and tears down the same 6 prerequisite resources (PasswordPolicy, 2 Policies, 2 KubernetesAuthEngineRoles, SecretEngineMount), each requiring its own `Eventually` polling loop (2-second interval, typically 2-6 seconds per poll). This results in ~12 redundant `Eventually` round-trips × 3 extra copies = ~100-150 seconds of wasted time.
16+
17+
### Current test structure (problematic)
18+
19+
```
20+
Describe("Random Secret controller for v2 secrets")
21+
Context("retain policy") → creates 6-8 prereqs, tests, tears down all
22+
Context("multi-key") → creates 6 prereqs (same), tests, tears down all
23+
Context("multi-key-recreate") → creates 6 prereqs (same), tests, tears down all
24+
Context("refreshPeriod") → creates 6 prereqs (same), tests, tears down all
25+
```
26+
27+
### Target test structure
28+
29+
```
30+
Describe("Random Secret controller for v2 secrets")
31+
BeforeAll → creates shared prereqs once
32+
AfterAll → tears down shared prereqs once
33+
Context("retain policy") → creates only test-specific resources
34+
Context("multi-key") → creates only test-specific resources
35+
Context("multi-key-recreate") → creates only test-specific resources
36+
Context("refreshPeriod") → creates only test-specific resources
37+
```
38+
39+
## Scope
40+
41+
| File | Change | Impact |
42+
|------|--------|--------|
43+
| `controllers/randomsecret_controller_test.go` (1717 lines) | Major refactor — lift shared prereqs to BeforeAll/AfterAll | Primary target |
44+
| `controllers/vaultsecret_controller_test.go` (679 lines) | No change — single Context, nothing to deduplicate | Out of scope |
45+
| `controllers/vaultsecret_controller_v2_test.go` (491 lines) | No change — single Context, nothing to deduplicate | Out of scope |
46+
47+
Cross-file fixture sharing (between RandomSecret and VaultSecret v2 tests) is explicitly out of scope — it would require deeper structural changes for marginal gain.
48+
49+
## Acceptance Criteria
50+
51+
1. **Given** the refactored `randomsecret_controller_test.go` **When** the full integration test suite runs **Then** all existing RandomSecret tests pass with identical assertions and behavior.
52+
53+
2. **Given** shared prerequisites (PasswordPolicy, Policies, KubernetesAuthEngineRoles, SecretEngineMount) **When** the Describe block starts **Then** they are created exactly once in BeforeAll and torn down exactly once in AfterAll.
54+
55+
3. **Given** Context 1 (retain) needs extra resources (secret-reader policy + role for VaultSecret reading) **When** that Context runs **Then** the extra resources are created/destroyed within that Context's BeforeAll/AfterAll or inline, not in the shared set.
56+
57+
4. **Given** all 4 Context blocks **When** they execute sequentially **Then** there is no cross-Context state leakage — each Context's test-specific resources (RandomSecret, VaultSecret) are fully self-contained.
58+
59+
5. **Given** the refactored suite **When** compared to the pre-refactor run **Then** the RandomSecret test group duration decreases by at least 40%.
60+
61+
## Tasks / Subtasks
62+
63+
- [ ] Task 1: Identify shared prerequisites
64+
- [ ] 1.1: Catalog which YAML fixtures are identical across all 4 Context blocks
65+
- [ ] 1.2: Identify Context-1-only extras (secret-reader policy + KubernetesAuthEngineRole)
66+
67+
- [ ] Task 2: Refactor to BeforeAll/AfterAll
68+
- [ ] 2.1: Add `BeforeAll` at `Describe` level — create shared PasswordPolicy, Policies, KubernetesAuthEngineRoles, SecretEngineMount with their `Eventually` blocks
69+
- [ ] 2.2: Add `AfterAll` at `Describe` level — delete shared resources with their `Eventually` blocks
70+
- [ ] 2.3: Remove duplicated setup/teardown from each of the 4 `It` blocks
71+
- [ ] 2.4: For Context 1, keep the extra secret-reader resources scoped to that Context
72+
73+
- [ ] Task 3: Verify isolation
74+
- [ ] 3.1: Ensure each Context's test-specific resources (RandomSecret instances, VaultSecret instances) use unique names and don't collide
75+
- [ ] 3.2: Verify the SecretEngineMount and its KV path remain available across all Contexts
76+
77+
- [ ] Task 4: End-to-end verification
78+
- [ ] 4.1: Run `make integration` and verify all tests pass (RandomSecret + all others)
79+
- [ ] 4.2: Compare before/after timing from Ginkgo JSON report to confirm improvement
80+
81+
## Dev Notes
82+
83+
### Ginkgo BeforeAll/AfterAll semantics
84+
85+
Ginkgo v2 `BeforeAll` runs once before the first spec in a container (Describe/Context). If it fails, all specs in that container are skipped. This is the desired behavior — if shared infrastructure can't be created, there's no point running any of the tests.
86+
87+
Variables created in `BeforeAll` must be declared at the container scope (before the `BeforeAll` block) so they're accessible in the `It` blocks.
88+
89+
### The refreshPeriod test's ~20-second wait is inherent
90+
91+
The `refreshPeriod` Context (Context 4) uses `11-randomsecret-refresh-v2.yaml` with `refreshPeriod: 15s`, plus a 5-second `Consistently` check. This ~20-second wait is testing real timer behavior and cannot be optimized away. The savings from this refactoring come from eliminating the ~4× duplicated infrastructure setup, not from changing test logic.
92+
93+
### Shared fixture catalog
94+
95+
| Resource | YAML path | Used by |
96+
|----------|-----------|---------|
97+
| PasswordPolicy | `test/randomsecret/v2/00-passwordpolicy-simple-password-policy-v2.yaml` | All 4 Contexts |
98+
| Policy (kv-engine-admin) | `test/randomsecret/v2/01-policy-kv-engine-admin-v2.yaml` | All 4 Contexts |
99+
| Policy (secret-writer) | `test/randomsecret/v2/04-policy-secret-writer-v2.yaml` | All 4 Contexts |
100+
| Policy (secret-reader) | `test/vaultsecret/v2/00-policy-secret-reader-v2.yaml` | Context 1 only |
101+
| KubernetesAuthEngineRole (kv-engine-admin) | `test/randomsecret/v2/02-kubernetesauthenginerole-kv-engine-admin-v2.yaml` | All 4 Contexts |
102+
| KubernetesAuthEngineRole (secret-writer) | `test/randomsecret/v2/05-kubernetesauthenginerole-secret-writer-v2.yaml` | All 4 Contexts |
103+
| KubernetesAuthEngineRole (secret-reader) | `test/vaultsecret/v2/00-kubernetesauthenginerole-secret-reader-v2.yaml` | Context 1 only |
104+
| SecretEngineMount | `test/randomsecret/v2/03-secretenginemount-kv-v2.yaml` | All 4 Contexts |
105+
106+
### Estimated time savings
107+
108+
- Current: ~4 × (6 prereq creates + 6 prereq deletes) × ~3-4s each = ~150-200s in prereq overhead
109+
- After: 1 × (6 prereq creates + 6 prereq deletes) × ~3-4s each = ~40-50s in prereq overhead
110+
- **Net savings: ~100-150 seconds** (~40-60% of total RandomSecret test time)
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
# Epic 6 Retrospective — Identity & Audit Integration Tests
2+
3+
**Date:** 2026-05-02
4+
**Facilitator:** Bob (Scrum Master)
5+
**Participants:** Raffa (Project Lead), Alice (Product Owner), Charlie (Senior Dev), Dana (QA Engineer), Amelia (Developer Agent)
6+
7+
---
8+
9+
## Epic Summary
10+
11+
| Metric | Value |
12+
|--------|-------|
13+
| Epic | 6: Identity & Audit Integration Tests |
14+
| Stories | 4 of 4 completed (100%) |
15+
| Duration | ~3 days (April 30 – May 2, 2026) |
16+
| Scope | Integration tests for 11 types across 4 families |
17+
| Types tested | Group, GroupAlias, IdentityOIDCScope/Assignment/Client/Provider, IdentityTokenConfig/Key/Role, Audit, AuditRequestHeader |
18+
| Infrastructure added | None (all Tier 1) |
19+
| Debug failures | 1 total (Story 6.4 — AuditRequestHeader delete returns HTTP 400 not nil) |
20+
| Code review findings | 4 total (2 in Story 6.2, 2 in Story 6.4) — all patches |
21+
| Production code fixes | 0 |
22+
| Regressions | 0 |
23+
| Coverage delta | 46.0% → 53.7% (+7.7 pp) |
24+
| Integration test specs | 63 → 83+ (20+ new specs) |
25+
26+
### AI Models Used
27+
28+
| Story | Model |
29+
|-------|-------|
30+
| 6.1 — Group/GroupAlias | Claude Opus 4.6 (Cursor) |
31+
| 6.2 — IdentityOIDC (4 types) | Claude Opus 4.6 (Cursor) |
32+
| 6.3 — IdentityToken (3 types) | Claude Opus 4 |
33+
| 6.4 — Audit/AuditRequestHeader | Claude Opus 4.6 |
34+
35+
---
36+
37+
## Epic 5 Retrospective Follow-Through
38+
39+
| Action Item | Status |
40+
|-------------|--------|
41+
| ObservedGeneration baseline assertion guidance | ✅ Applied — All 4 stories recorded baseline before update, asserted strictly greater after. Zero code review intervention needed. |
42+
| Document write-only Vault endpoints when discovered | N/A — No write-only endpoints encountered in Epic 6 |
43+
| PKI `CreateOrUpdateConfig` dual bug (carried from Epic 2) | ❌ Still in Epic 7 backlog — confirmed non-blocking |
44+
| AC#4 extra-field handling → Story 7-4 (carried from Epic 1) | ⏳ Still in backlog — scope continues to grow |
45+
| Story 4.2 `omitempty` workaround revert | ⏳ Epic 7.5 backlog |
46+
| Continue Opus-class models | ✅ All stories used Opus 4/4.6 |
47+
| Continue code review process | ✅ Reviews ran on all 4 stories, caught 4 findings |
48+
| Three-tier infrastructure classification | ✅ All 11 types correctly classified as Tier 1 |
49+
| Story ordering by complexity/dependency | ✅ Followed Epic 5 retro's suggested ordering exactly: 6.1 → 6.2 → 6.3 → 6.4 |
50+
51+
Completed 5/9, N/A 1/9, in-progress 2/9, not addressed 1/9.
52+
53+
---
54+
55+
## Successes
56+
57+
1. **Cleanest epic to date.** Only 1 debug failure and 4 review findings across 4 stories covering 11 types. This is the lowest issue rate of any epic.
58+
59+
2. **Largest single-epic coverage gain.** 7.7 percentage points (46.0% → 53.7%) — exceeding Epic 5's 4.0 pp gain. Combined with Epic 5, the two epics gained nearly 12 points.
60+
61+
3. **Zero infrastructure setup.** All 11 types were Tier 1 (Vault internal APIs). No Helm charts, no service deployments, no external dependency debugging. This is the primary reason for the fast execution.
62+
63+
4. **Story spec maturity payoff.** Detailed dev notes — Vault API response shapes, checked type assertion reminders, dependency chain ordering, delete verification patterns — gave dev agents nearly everything they needed. This directly caused the drop from 8 review findings (Epic 5) to 4 (Epic 6).
64+
65+
5. **ObservedGeneration guidance fully embedded.** The Epic 5 retro action item worked perfectly — all 4 stories used baseline assertions without code review intervention. This action item can be retired.
66+
67+
6. **Third reconciler variant tested.** Story 6.4's Audit type uses `VaultAuditResource` — the third and final reconciler variant. All three variants (VaultResource, VaultEngineResource, VaultAuditResource) now have integration test coverage.
68+
69+
7. **GroupAlias PrepareInternalValues worked first try.** The most complex type in Epic 6 (accessor lookup + group canonical_id resolution + conditional alias creation) had zero debug failures.
70+
71+
---
72+
73+
## Challenges
74+
75+
1. **AuditRequestHeader delete returns HTTP 400 (Story 6.4).** Vault returns HTTP 400 (not nil or 404) for non-existent request headers. This was the only debug failure in the epic — a genuine Vault API inconsistency rather than a test design issue. Resolved by accepting the expected `vault.ResponseError` status codes in the delete verification.
76+
77+
2. **Missing assertions in Story 6.2 review.** Client create test didn't assert `redirect_uris` or `assignments`; Provider create test didn't assert `allowed_client_ids`. Minor completeness gaps caught in review and patched.
78+
79+
---
80+
81+
## Key Insights
82+
83+
1. **Preparation quality compounds.** Epic 6's smooth execution is a direct result of improvements made in Epics 4 and 5 retros: tier classification, story ordering, ObservedGeneration guidance, detailed dev notes. Each retro's improvements reduce the next epic's friction.
84+
85+
2. **Zero-infrastructure epics execute dramatically faster.** Epic 6 (0 infrastructure, 11 types, 3 days) vs Epic 5 (1 new infrastructure, 6 types, 2 days) vs Epic 4 (3 new infrastructure, 6 types, 1 day but compressed). Infrastructure setup is the dominant variable in execution speed.
86+
87+
3. **Review finding rate is a quality indicator for story specs, not dev agents.** The drop from 8 to 4 findings correlates with story spec detail level, not model choice. Both Opus 4 and 4.6 produced clean results when given sufficient context.
88+
89+
---
90+
91+
## Action Items
92+
93+
### Process Improvements
94+
95+
1. **Continue detailed dev notes in story specs**
96+
- Owner: Bob (Scrum Master)
97+
- Description: Maintain Vault API response shapes, delete verification patterns, and dependency chains in story specs. This is the primary driver behind review finding reduction.
98+
- Success criteria: Epic 7 story specs maintain or exceed Epic 6 detail level
99+
100+
### Technical Debt (Carried)
101+
102+
2. **PKI `CreateOrUpdateConfig` dual bug** (CARRIED from Epic 2)
103+
- Owner: Epic 7
104+
- Priority: Medium — confirmed non-blocking through Epic 6
105+
106+
3. **AC#4 extra-field handling → Story 7-4** (CARRIED from Epic 1)
107+
- Owner: Epic 7, Story 7.4
108+
- Priority: High — primary target of Epic 7
109+
110+
4. **Story 4.2 `omitempty` workaround revert**
111+
- Owner: Epic 7.5, Story 7.5.1
112+
- Status: Properly tracked
113+
114+
### Dismissed
115+
116+
5. ~~ObservedGeneration baseline assertion tracking~~ — Fully embedded in practice. All 4 Epic 6 stories applied it without intervention. No longer needs explicit tracking.
117+
118+
### Team Agreements
119+
120+
- Continue using Opus-class models — validated across 19 consecutive stories (Epics 2–6)
121+
- Continue the code review process — trend is positive (8 → 4 findings)
122+
- Three-tier infrastructure classification is standard practice
123+
- Story ordering by complexity/dependency remains effective
124+
125+
---
126+
127+
## Epic 7 Preparation
128+
129+
### Dependencies on Epic 6
130+
131+
None. Epic 7 shifts domain from "add integration test coverage" to "hardening" (webhooks, error paths, credential resolution, extra-field audit).
132+
133+
### Infrastructure Requirements
134+
135+
None for Stories 7.1–7.3. Story 7.4 may need mock Vault API responses for Tier 3 types (cloud providers) that cannot be tested against a live service.
136+
137+
### Suggested Story Ordering
138+
139+
| Order | Story | Scope | Complexity |
140+
|-------|-------|-------|------------|
141+
| 1 | 7.1 | Webhook `ValidateUpdate` immutable path tests | Low — unit tests, table-driven |
142+
| 2 | 7.2 | `PrepareInternalValues` unit tests (15 types) | Medium — mock-based, wide scope |
143+
| 3 | 7.3 | Error path integration tests | Medium — invalid auth, unreachable Vault |
144+
| 4 | 7.4 | Audit Vault API responses + harden `IsEquivalentToDesiredState` (46 types) | High — 3-phase, large scope |
145+
| 5 | 7.5 | Drift detection integration tests | Medium — depends on 7.4 |
146+
147+
### New Patterns to Watch
148+
149+
- **Unit tests (7.1, 7.2):** First stories focused on pure unit tests rather than integration tests
150+
- **Error path testing (7.3):** Testing failure conditions rather than happy paths
151+
- **Multi-phase audit (7.4):** Audit → Fix → Test across 46 types — largest single story scope to date
152+
- **Story 7.4 scope risk:** May need to be broken into sub-stories during story creation
153+
154+
### Readiness Assessment
155+
156+
- Testing & Quality: All 83+ specs passing, coverage at 53.7%
157+
- Technical Health: Codebase stable, zero regressions across 6 epics
158+
- Infrastructure: None needed for initial stories
159+
- Unresolved Blockers: None
160+
161+
### Verdict
162+
163+
**Ready to proceed with Epic 7.** No prep work needed. Story ordering: 7.1 → 7.2 → 7.3 → 7.4 → 7.5. Monitor Story 7.4 scope during story creation.
164+
165+
---
166+
167+
## Team Performance
168+
169+
Epic 6 delivered 4 stories covering integration tests for 11 identity and audit types (Group, GroupAlias, IdentityOIDCScope/Assignment/Client/Provider, IdentityTokenConfig/Key/Role, Audit, AuditRequestHeader) in ~3 days — the cleanest epic to date with only 1 debug failure and 4 review findings. All types were Tier 1 (no infrastructure), enabling the fastest per-type execution rate. Coverage grew 7.7 percentage points (46.0% → 53.7%), the largest single-epic gain. All three reconciler variants (VaultResource, VaultEngineResource, VaultAuditResource) now have integration test coverage. The team is well-positioned for Epic 7's shift to hardening work.

_bmad-output/implementation-artifacts/sprint-status.yaml

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,22 @@
3535
# - Dev moves story to 'review', then runs code-review (fresh context, different LLM recommended)
3636

3737
generated: 2026-04-11
38-
last_updated: 2026-04-29
38+
last_updated: 2026-05-02
39+
# Epic 6 done + retrospective completed 2026-05-02
40+
# Story 6.4 done 2026-05-02
41+
# Story 6.4 review 2026-05-02
42+
# Story 6.4 in-progress 2026-05-02
43+
# Story 6.3 done 2026-05-02
44+
# Story 6.3 review 2026-05-02
45+
# Story 6.2 done 2026-05-01
46+
# Story 6.2 review 2026-05-01
47+
# Story 6.1 done 2026-05-01
48+
# Story 6.1 review 2026-05-01
49+
# Story 6.4 ready-for-dev 2026-05-01
50+
# Story 6.3 ready-for-dev 2026-04-30
51+
# Story 6.2 ready-for-dev 2026-04-30
52+
# Story 6.1 ready-for-dev 2026-04-30
53+
# Epic 6 in-progress 2026-04-30
3954
# Epic 5 done + retrospective completed 2026-04-29
4055
# Story 5.3 done 2026-04-29
4156
# Story 5.3 review 2026-04-29
@@ -138,15 +153,16 @@ development_status:
138153
epic-5-retrospective: done
139154

140155
# Epic 6: Identity & Audit Integration Tests
141-
epic-6: backlog
142-
6-1-integration-tests-for-group-and-groupalias-types: backlog
143-
6-2-integration-tests-for-identity-oidc-types: backlog
144-
6-3-integration-tests-for-identity-token-types: backlog
145-
6-4-integration-tests-for-audit-types: backlog
146-
epic-6-retrospective: optional
156+
epic-6: done
157+
6-1-integration-tests-for-group-and-groupalias-types: done
158+
6-2-integration-tests-for-identity-oidc-types: done
159+
6-3-integration-tests-for-identity-token-types: done
160+
6-4-integration-tests-for-audit-types: done
161+
epic-6-retrospective: done
147162

148163
# Epic 7: Hardening — Webhooks, Error Paths & Credential Resolution
149164
epic-7: backlog
165+
7-0-refactor-randomsecret-test-shared-fixtures: backlog
150166
7-1-webhook-validation-tests-for-immutable-spec-path-rule: backlog
151167
7-2-unit-tests-for-prepareinternalvalues-credential-resolution: backlog
152168
7-3-error-path-integration-tests: backlog

0 commit comments

Comments
 (0)