Skip to content

Commit 48ba879

Browse files
Complete Epic 4 retrospective and close Epic 4
- Fix ConditionsAware compile-time checks in policy_types.go and rabbitmqsecretenginerole_types.go (copy-paste bugs referencing wrong types) - Add non-deletable type delete verification rule to project-context.md - Add Epic 7.5 (CRD Field Annotation Refactor, 7 stories) to sprint status - Mark epic-4 as done and epic-4-retrospective as done - Save retrospective document with action items and Epic 5 preparation Made-with: Cursor
1 parent d48b026 commit 48ba879

5 files changed

Lines changed: 195 additions & 4 deletions

File tree

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
# Epic 4 Retrospective — Auth Engine Integration Tests
2+
3+
**Date:** 2026-04-23
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 | 4: Auth Engine Integration Tests |
14+
| Stories | 3 of 3 completed (100%) |
15+
| Duration | ~1 day (April 23, 2026) |
16+
| Scope | Integration tests for 6 auth engine types across 3 auth methods |
17+
| Debug failures | 7 total (2 + 2 + 3) — all environmental or external service quirks, zero operator code bugs |
18+
| Code review findings | 4 patches across 3 stories (0 + 3 + 1) |
19+
| New infrastructure | OpenLDAP wired into `make integration` (`deploy-ldap`), Keycloak 26.2 deployed to Kind (`deploy-keycloak`) |
20+
| Production code fixes | 1 (`ldapauthengineconfig_types.go``omitempty` on TLS version fields, later identified as incorrect workaround) |
21+
| New test patterns | Auth engine dependency chain (mount → config → role/group), non-deletable config persistence verification |
22+
| Technical debt documented | 1 new (Story 4.2 `omitempty` workaround violates CRD rules — tracked in Epic 7.5 Story 7.5.1) |
23+
| Regressions | 0 |
24+
| Coverage delta | 38.8% → 42.0% |
25+
26+
### AI Models Used
27+
28+
| Story | Model |
29+
|-------|-------|
30+
| 4.1 — KubernetesAuthEngineConfig/Role | Claude Opus 4.6 |
31+
| 4.2 — LDAPAuthEngineConfig/Group | Claude Opus 4.6 |
32+
| 4.3 — JWTOIDCAuthEngineConfig/Role | Claude Opus 4 |
33+
34+
---
35+
36+
## Epic 3 Retrospective Follow-Through
37+
38+
| Action Item | Status |
39+
|-------------|--------|
40+
| Fix `CreateOrUpdateConfig` dual bug in PKI engine (carried from Epic 2) | ❌ Not addressed — downgraded from "must fix before Epic 5" to "fix in Epic 7" after confirming no Epic 5 types use VaultPKIEngineResource |
41+
| AC#4 extra-field handling → Story 7-4 | ⏳ In backlog — 6 more auth engine types confirmed with this pattern in Epic 4 |
42+
| Policy `ConditionsAware` copy-paste bug | ✅ Fixed during this retrospective (also found a second bug in `rabbitmqsecretenginerole_types.go`) |
43+
| Add checked type assertion rule to project-context.md | ✅ Applied — all 3 stories used checked type assertions consistently |
44+
| Continue using Opus 4.6 exclusively | ⚠️ Mostly — Stories 4.1/4.2 used Opus 4.6, Story 4.3 used Claude Opus 4 |
45+
| Story specs must flag infrastructure scope | ✅ Applied — all 3 stories explicitly classified infrastructure tier |
46+
47+
Completed 2/6, in progress 1/6, not addressed 3/6. Key correction: PKI bug was incorrectly flagged as blocking Epic 5 in previous retros. The `VaultPKIEngineResource` reconciler is used exclusively by `PKISecretEngineConfig` — none of Epic 5's types (Database, RabbitMQ, GitHub, Azure, Quay, Kubernetes) use it.
48+
49+
---
50+
51+
## Successes
52+
53+
1. **Real infrastructure over mocks.** All 3 stories deployed real services in Kind — Kubernetes API (already available), OpenLDAP (manifests wired into `make integration`), and Keycloak 26.2 (full OIDC provider with realm import). Vault actually validated these connections end-to-end.
54+
55+
2. **Infrastructure template compounding.** The `deploy-*` Makefile target pattern (create namespace → apply manifests → wait for ready) was established in Story 4.2 and reused identically in Story 4.3. This template is directly reusable for RabbitMQ in Epic 5.
56+
57+
3. **Story ordering by infrastructure complexity worked well.** 4.1 (no new infra) → 4.2 (LDAP — medium) → 4.3 (Keycloak — high). Each story built on patterns from the previous one.
58+
59+
4. **Zero operator code bugs in debug failures.** All 7 debug failures were environmental (port conflicts, stale CRs, Keycloak filename conventions, health port, kubeconfig context) or Vault API surprises (`tls_min_version` empty string rejection, `policies` returned as `[]interface{}`). The operator code itself was solid.
60+
61+
5. **Coverage grew 3.2 percentage points** (38.8% → 42.0%) — the largest single-epic coverage gain since Epic 2.
62+
63+
---
64+
65+
## Challenges
66+
67+
1. **Non-deletable config persistence verification missed twice.** Both Stories 4.2 and 4.3 were caught in code review for the same gap: when deleting a `IsDeletable=false` type, the test verified K8s deletion but didn't prove the Vault config persisted. Now codified as a project-context rule.
68+
69+
2. **Story 4.2 `omitempty` workaround violated CRD rules.** Vault 1.19.0 rejected empty `tls_min_version`/`tls_max_version` strings. The fix (adding `omitempty` to JSON tags) was a workaround that violates Rule 2 of the CRD Field Default & Validation Rules — non-zero defaults (`tls12`) should NOT have `omitempty`. The correct fix should be in the API interaction layer. Revert tracked in Epic 7.5 Story 7.5.1.
70+
71+
3. **Keycloak external service quirks caused most debug time.** Story 4.3 had 3 debug failures, all Keycloak-specific: realm import filename convention (`test-realm-realm.json` not `test-realm.json`), health endpoint on management port 9000 not app port 8080, and wrong kubeconfig context. These are one-time learning costs that won't recur.
72+
73+
---
74+
75+
## Key Insights
76+
77+
1. **Real infrastructure beats mocks for integration test confidence.** When Vault writes OIDC config and actually fetches Keycloak's discovery endpoint, or LDAP config and actually binds to OpenLDAP, that's proof the operator works — not just that it generates correct JSON.
78+
79+
2. **Never change CRD annotations as a workaround for Vault API issues.** When Vault rejects a field value, fix the operator's API interaction (`toMap()`, `IsEquivalentToDesiredState`, or webhook `Default()`), not the CRD schema annotations. CRD annotations define the Kubernetes API contract; Vault compatibility is a separate concern.
80+
81+
3. **PKI bug urgency was overstated for two retros.** The "must fix before Epic 5" claim from Epic 2 was never re-validated against Epic 5's actual scope. Lesson: always check whether a carried action item is truly blocking before escalating its priority.
82+
83+
---
84+
85+
## Action Items
86+
87+
### Process Improvements
88+
89+
1. **Non-deletable config persistence verification rule added to project-context.md**
90+
- Status: ✅ Done during retrospective
91+
- Description: When `IsDeletable=false`, delete tests must verify Vault config persists after CR deletion
92+
93+
2. **CRD annotation workaround rule** (to be added)
94+
- Owner: Bob (Scrum Master)
95+
- Description: Never change CRD annotations (`omitempty`, `kubebuilder:default`) as a workaround for Vault API compatibility. Fix the API interaction layer instead.
96+
- Success criteria: Rule added to project-context.md Critical Don't-Miss Rules section
97+
98+
### Technical Debt
99+
100+
3. **Fix `CreateOrUpdateConfig` dual bug in PKI engine** (CARRIED from Epic 2 — priority downgraded)
101+
- Owner: Dev Agent (Epic 7)
102+
- Priority: Medium — confirmed NOT blocking Epic 5
103+
- Description: Two bugs in `vautlpkiengineobject.go:105-118`: (a) write path hardcoded to CRL, (b) equivalence check uses wrong payload
104+
105+
4. **AC#4 extra-field handling → Story 7-4** (CARRIED from Epic 1)
106+
- Owner: Story 7-4 (Epic 7)
107+
- Priority: Medium — scope growing, all auth engine types confirmed with this pattern
108+
- Status: Properly deferred
109+
110+
5. **Revert Story 4.2 `omitempty` workaround on `TLSMinVersion`/`TLSMaxVersion`**
111+
- Owner: Epic 7.5, Story 7.5.1 (Task 1.2)
112+
- Priority: Low — functional workaround, proper fix tracked
113+
- Status: Already planned
114+
115+
### Items Resolved During Retrospective
116+
117+
6. **Policy and RabbitMQSecretEngineRole `ConditionsAware` copy-paste bugs — FIXED**
118+
- `policy_types.go:37`: `&PKISecretEngineRole{}``&Policy{}`
119+
- `rabbitmqsecretenginerole_types.go:145`: `&RabbitMQSecretEngineConfig{}``&RabbitMQSecretEngineRole{}`
120+
121+
7. **Epic 7.5 (CRD Field Annotation Refactor) added to sprint-status.yaml**
122+
- 7 stories, positioned before Epic D1
123+
124+
### Team Agreements
125+
126+
- Continue using Opus-class models for integration test stories — validated across 12 consecutive stories (Epics 2–4)
127+
- Continue the code review process — catches real issues consistently
128+
- Infrastructure "install in Kind" template is now standard — reuse for RabbitMQ in Epic 5
129+
- Story ordering by ascending infrastructure complexity (proven in Epics 3 and 4)
130+
131+
---
132+
133+
## Epic 5 Preparation
134+
135+
### Dependencies on Epic 4
136+
137+
- VaultResource integration test pattern (established Epics 2–4) — direct reuse
138+
- `deploy-*` Makefile target template — reuse for RabbitMQ
139+
- Auth engine dependency chain pattern (mount → config → role) — analogous for secret engines
140+
- Non-deletable config persistence rule — ready for application
141+
142+
### Infrastructure Requirements by Story
143+
144+
| Story | External Dependency | Classification | Infrastructure Scope |
145+
|-------|-------------------|----------------|---------------------|
146+
| 5.1 — DatabaseSecretEngineConfig/Role | PostgreSQL | Already deployed (`deploy-postgresql`) | Low — no new infra |
147+
| 5.2 — RabbitMQ secret engine types | RabbitMQ | Install in Kind | Medium — new `deploy-rabbitmq` target |
148+
| 5.3 — Remaining secret engine types | Mixed | See below | Mixed |
149+
150+
### Story 5.3 Type Classification
151+
152+
| Type | Dependency | Classification | Rationale |
153+
|------|-----------|---------------|-----------|
154+
| KubernetesSecretEngineConfig/Role | Kubernetes API | Tier 1: Already available | Kind cluster IS the Kubernetes instance |
155+
| GitHubSecretEngineConfig/Role | GitHub App + custom Vault plugin | Tier 3: Skip | Cloud service, custom plugin already registered but no GitHub App |
156+
| AzureSecretEngineConfig/Role | Azure cloud | Tier 3: Skip | Cloud provider |
157+
| QuaySecretEngineConfig/Role/StaticRole | Quay registry + custom Vault plugin | Tier 3: Skip | Heavy deployment (PostgreSQL + Redis + Quay + custom Vault plugin not installed) |
158+
159+
### Readiness Assessment
160+
161+
- Testing & Quality: All tests passing, coverage at 42.0%
162+
- Technical Health: PKI bug confirmed non-blocking, copy-paste bugs fixed
163+
- Infrastructure: PostgreSQL already deployed, RabbitMQ needs new target
164+
- Unresolved Blockers: None
165+
166+
### Verdict
167+
168+
**Ready to proceed with Epic 5.** Story ordering: 5.1 (PostgreSQL — already there) → 5.2 (RabbitMQ — medium infra) → 5.3 (Kubernetes secret engine from Kind + skip cloud/Quay types).
169+
170+
---
171+
172+
## Team Performance
173+
174+
Epic 4 delivered 3 stories covering integration tests for 6 auth engine types (KubernetesAuthEngineConfig, KubernetesAuthEngineRole, LDAPAuthEngineConfig, LDAPAuthEngineGroup, JWTOIDCAuthEngineConfig, JWTOIDCAuthEngineRole) with real infrastructure deployments (OpenLDAP, Keycloak), zero operator code bugs, zero regressions, and 3.2 percentage points of coverage growth. The retrospective fixed 2 copy-paste bugs, added a testing rule to project-context.md, tracked Epic 7.5 in sprint status, corrected the PKI bug priority, and identified the Story 4.2 CRD annotation workaround as a mistake to revert. The team is well-positioned for Epic 5.

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
generated: 2026-04-11
3838
last_updated: 2026-04-23
39+
# Epic 4 retrospective completed 2026-04-23
3940
# Story 4.3 done 2026-04-23
4041
# Story 4.3 review 2026-04-23
4142
# Story 4.3 in-progress 2026-04-23
@@ -111,11 +112,11 @@ development_status:
111112
epic-3-retrospective: done
112113

113114
# Epic 4: Auth Engine Integration Tests
114-
epic-4: in-progress
115+
epic-4: done
115116
4-1-integration-tests-for-kubernetesauthengineconfig-and-kubernetesauthenginerole: done
116117
4-2-integration-tests-for-ldapauthengineconfig-and-ldapauthenginegroup: done
117118
4-3-integration-tests-for-jwtoidcauthengineconfig-and-jwtoidcauthenginerole: done
118-
epic-4-retrospective: optional
119+
epic-4-retrospective: done
119120

120121
# Epic 5: Secret Engine Integration Tests
121122
epic-5: backlog
@@ -141,6 +142,17 @@ development_status:
141142
7-5-drift-detection-integration-tests: backlog
142143
epic-7-retrospective: optional
143144

145+
# Epic 7.5: CRD Field Annotation Refactor
146+
epic-7-5: backlog
147+
7-5-1-ldap-auth-engine-types-annotation-refactor: backlog
148+
7-5-2-jwtoidc-auth-engine-types-annotation-refactor: backlog
149+
7-5-3-kubernetes-auth-and-secret-engine-types-annotation-refactor: backlog
150+
7-5-4-azure-and-gcp-auth-secret-engine-types-annotation-refactor: backlog
151+
7-5-5-pki-secret-engine-types-annotation-refactor: backlog
152+
7-5-6-identity-and-remaining-types-annotation-refactor: backlog
153+
7-5-7-validation-marker-sweep-enum-pattern-minlength-maxlength: backlog
154+
epic-7-5-retrospective: optional
155+
144156
# --- Phase 1.5: Documentation Improvement ---
145157

146158
# Epic D1: Documentation Standards & Missing CertAuth Engine Docs

_bmad-output/project-context.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,11 @@ Vault typically acts as an intermediary between vault-config-operator and a secu
140140

141141
This rule applies to all current and future integration tests. When adding a new type, classify its external dependency into one of these three categories and document the decision in the story file.
142142

143+
#### Non-Deletable Type Delete Verification
144+
- When a type has `IsDeletable() == false` (e.g., auth engine configs like KubernetesAuthEngineConfig, LDAPAuthEngineConfig, JWTOIDCAuthEngineConfig), the delete test **must** verify that the Vault configuration **persists** after the CR is deleted from Kubernetes — not just that K8s deletion succeeds.
145+
- After deleting the CR and confirming K8s `NotFound`, read the Vault path and assert the config data is still present (e.g., verify a key field like `url` or `kubernetes_host` still has the expected value).
146+
- This proves the `IsDeletable=false` contract: CR deletion removes the Kubernetes object but intentionally leaves the Vault state intact until the parent mount is deleted.
147+
143148
#### Integration Test Pattern
144149
- Uses Ginkgo v2 `Describe`/`Context`/`It` BDD blocks with dot-imported `gomega` matchers.
145150
- Test fixtures are YAML files in `test/` directory, loaded via `controllertestutils.decoder.Get<TypeName>Instance("../test/<path>.yaml")`.

api/v1alpha1/policy_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
3535

3636
var _ vaultutils.VaultObject = &Policy{}
37-
var _ vaultutils.ConditionsAware = &PKISecretEngineRole{}
37+
var _ vaultutils.ConditionsAware = &Policy{}
3838

3939
func (d *Policy) GetVaultConnection() *vaultutils.VaultConnection {
4040
return d.Spec.Connection

api/v1alpha1/rabbitmqsecretenginerole_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ type RabbitMQSecretEngineRoleList struct {
142142
Items []RabbitMQSecretEngineRole `json:"items"`
143143
}
144144

145-
var _ vaultutils.ConditionsAware = &RabbitMQSecretEngineConfig{}
145+
var _ vaultutils.ConditionsAware = &RabbitMQSecretEngineRole{}
146146

147147
func (d *RabbitMQSecretEngineRole) GetVaultConnection() *vaultutils.VaultConnection {
148148
return d.Spec.Connection

0 commit comments

Comments
 (0)