Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ test: manifests generate fmt vet envtest ## Run tests.

# note: envtest requires docker, podman will not work
.PHONY: integration
integration: kind-setup deploy-vault deploy-ingress deploy-postgresql deploy-ldap deploy-keycloak vault manifests generate fmt vet envtest ## Run tests.
integration: kind-setup deploy-vault deploy-ingress deploy-postgresql deploy-rabbitmq deploy-ldap deploy-keycloak vault manifests generate fmt vet envtest ## Run tests.
export VAULT_TOKEN=$$($(KUBECTL) get secret vault-init -n vault -o jsonpath='{.data.root_token}' | base64 -d) ;\
export VAULT_ADDR="http://localhost:$(VAULT_HOST_PORT)" ;\
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out --tags=integration -timeout 30m
Expand Down Expand Up @@ -180,6 +180,12 @@ deploy-postgresql: kubectl helm
$(KUBECTL) wait --for=condition=ready pod -l app.kubernetes.io/instance=postgresql \
-n test-vault-config-operator --timeout=$(KUBECTL_WAIT_TIMEOUT)

.PHONY: deploy-rabbitmq
deploy-rabbitmq: kubectl
$(KUBECTL) create namespace test-vault-config-operator --dry-run=client -o yaml | $(KUBECTL) apply -f -
$(KUBECTL) apply -f ./integration/rabbitmq -n test-vault-config-operator
$(KUBECTL) wait --for=condition=ready -n test-vault-config-operator pod -l app=rabbitmq --timeout=$(KUBECTL_WAIT_TIMEOUT)

.PHONY: deploy-ldap
deploy-ldap: kubectl
$(KUBECTL) create namespace ldap --dry-run=client -o yaml | $(KUBECTL) apply -f -
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

171 changes: 171 additions & 0 deletions _bmad-output/implementation-artifacts/epic-5-retro-2026-04-29.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
# Epic 5 Retrospective — Secret Engine Integration Tests

**Date:** 2026-04-29
**Facilitator:** Bob (Scrum Master)
**Participants:** Raffa (Project Lead), Alice (Product Owner), Charlie (Senior Dev), Dana (QA Engineer), Amelia (Developer Agent)

---

## Epic Summary

| Metric | Value |
|--------|-------|
| Epic | 5: Secret Engine Integration Tests |
| Stories | 3 of 3 completed (100%) |
| Duration | ~2 days (April 28-29, 2026) |
| Scope | Integration tests for 6 secret engine types across 3 engine families |
| Types tested | DatabaseSecretEngineConfig/Role, RabbitMQSecretEngineConfig/Role, KubernetesSecretEngineConfig/Role |
| Types correctly skipped (Tier 3) | GitHubSecretEngineConfig/Role, AzureSecretEngineConfig/Role, QuaySecretEngineConfig/Role/StaticRole |
| Debug failures | 5 total (2 + 2 + 1) — mix of external service quirks and static analysis catches |
| Code review findings | 8 total (2 + 2 + 4) |
| New infrastructure | RabbitMQ deployed to Kind via plain manifests (pivoted from Helm due to Bitnami paywall) |
| Production code fixes | 1 (Story 5.3 fixed pre-existing Story 5.2 vhosts type assertion bug) |
| Regressions | 0 |
| Coverage delta | 42.0% → 46.0% (+4.0 pp) |

### AI Models Used

| Story | Model |
|-------|-------|
| 5.1 — DatabaseSecretEngineConfig/Role | Claude Opus 4 |
| 5.2 — RabbitMQSecretEngineConfig/Role | Cursor (Opus 4.6) |
| 5.3 — KubernetesSecretEngineConfig/Role | Claude Opus 4.6 |

---

## Epic 4 Retrospective Follow-Through

| Action Item | Status |
|-------------|--------|
| Non-deletable config persistence rule (project-context.md) | ✅ Applied — Story 5.2 used this for RabbitMQSecretEngineConfig (IsDeletable=false) |
| CRD annotation workaround rule to project-context.md | 🚫 Dismissed — Epic 7.5 covers systematic CRD annotation refactor, making standalone rule redundant |
| PKI `CreateOrUpdateConfig` dual bug (carried from Epic 2) | ❌ Still in backlog (Epic 7) — confirmed non-blocking for Epics 5 and 6 |
| AC#4 extra-field handling → Story 7-4 | ⏳ Still in backlog — scope continues to grow with each new type |
| Story 4.2 `omitempty` workaround revert | ⏳ In Epic 7.5 backlog (Story 7.5.1, Task 1.2) |
| Continue Opus-class models | ✅ All 3 stories used Opus-class (Opus 4, Opus 4.6) |
| Continue code review process | ✅ All 3 stories reviewed, 8 findings total including cross-story bug detection |
| Infrastructure "install in Kind" template reuse | ✅ RabbitMQ deployed using the pattern (adapted from Helm to plain manifests) |
| Story ordering by ascending infrastructure complexity | ✅ Applied: 5.1 (no infra) → 5.2 (new RabbitMQ) → 5.3 (no infra) |

Completed 4/9, in progress 2/9, not addressed 1/9, dismissed 1/9, deferred 1/9.

---

## Successes

1. **Epic was well-sized.** Prep work from Epic 4's retro (tier classification, infrastructure assessment) eliminated ambiguity and made execution smooth. This was the fastest epic — 3 stories in ~2 days.

2. **Bitnami paywall pivot was seamless.** When Bitnami's RabbitMQ Helm chart image hit a paywall, the dev agent switched to plain K8s manifests using the official `rabbitmq:3-management` image within the same session. This followed the same manifest-based pattern used for OpenLDAP and Keycloak in Epic 4.

3. **Largest single-epic coverage gain.** 4.0 percentage points (42.0% → 46.0%), covering 6 secret engine types with full CRUD lifecycle tests.

4. **Story 5.3 tier classification was exemplary.** The table classifying each type (Kubernetes = Tier 1, GitHub/Azure/Quay = Tier 3) with clear rationale was the best application of the three-tier rule yet.

5. **Cross-story bug detection.** Story 5.3 caught and fixed a pre-existing Story 5.2 bug (RabbitMQ `vhosts` type assertion). The code review process and story sequencing enabled this.

6. **Zero regressions.** All 63 integration test specs pass. Every story verified coexistence with all prior tests.

---

## Challenges

1. **Bitnami Helm paywall (Story 5.2).** The Bitnami RabbitMQ Helm chart image (`docker.io/bitnami/rabbitmq:4.1.3-debian-12-r1`) was behind a paywall since August 2025. This was an unanticipated external dependency failure. Resolved by switching to plain manifests with the official image.

2. **Vault RabbitMQ `/config/connection` is write-only (Story 5.2).** GET returns HTTP 405. The story spec assumed this endpoint was readable. Resolved by using the lease endpoint (`/config/lease`) for persistence verification and relying on `ReconcileSuccessful=True` with `verifyConnection: true` for connection config validation.

3. **ObservedGeneration assertion quality (Stories 5.1, 5.3).** Dev agents don't naturally produce a baseline check before asserting ObservedGeneration increased after an update. They check "greater than zero" instead of "greater than the pre-update value." Caught in code review twice.

---

## Key Insights

1. **Infrastructure prep in the previous retro pays compounding dividends.** Epic 5's smooth execution was directly attributable to Epic 4's readiness assessment: Story 5.1 was "Low — no new infra" because PostgreSQL was already deployed, Story 5.2 had a clear template to follow, and Story 5.3 had the tier table pre-built.

2. **The three-tier infrastructure classification is one of the most valuable project rules.** It was applied perfectly in Story 5.3 and prevented wasted effort on GitHub/Azure/Quay types that cannot be meaningfully integration-tested.

3. **Write-only Vault endpoints exist and should be documented when discovered.** RabbitMQ `/config/connection` is write-only. Future stories referencing known write-only endpoints should include this in dev notes to avoid repeating the discovery cost.

---

## Action Items

### Process Improvements

1. **ObservedGeneration baseline assertion guidance**
- Owner: Bob (Scrum Master) — incorporate into story spec templates
- Description: When story specs describe an update test, explicitly instruct: "Record initial ObservedGeneration BEFORE the update, then assert the post-update value is strictly greater than the recorded baseline."
- Success criteria: Next epic's update tests include baseline checks without code review intervention

2. **Document write-only Vault endpoints when discovered**
- Owner: Dev Agent (ongoing)
- Description: When a new write-only endpoint is discovered, note it in story dev notes and in future story specs referencing that engine type.
- Success criteria: Future stories referencing known write-only endpoints include this in dev notes

### Technical Debt (Carried)

3. **PKI `CreateOrUpdateConfig` dual bug** (CARRIED from Epic 2)
- Owner: Epic 7
- Priority: Medium — confirmed non-blocking for Epics 5 and 6

4. **AC#4 extra-field handling → Story 7-4** (CARRIED from Epic 1)
- Owner: Epic 7
- Priority: Medium — scope growing, all tested types confirmed with this pattern

5. **Story 4.2 `omitempty` workaround revert**
- Owner: Epic 7.5, Story 7.5.1 (Task 1.2)
- Status: Properly tracked

### Dismissed

6. ~~CRD annotation workaround rule in project-context.md~~ — Dismissed per Raffa. Epic 7.5 covers the systematic CRD annotation refactor across all types, making a standalone warning rule redundant.

### Team Agreements

- Continue using Opus-class models for integration test stories — validated across 15 consecutive stories (Epics 2–5)
- Continue the code review process — catches real issues consistently, including cross-story bugs
- Three-tier infrastructure classification is standard practice — proven across Epics 4 and 5
- Story ordering by complexity/dependency remains effective

---

## Epic 6 Preparation

### Dependencies on Epic 5

- No direct dependencies. Epic 6 types (identity, audit) are a different domain from secret engines.
- Integration test patterns and infrastructure from Epics 2–5 carry forward unchanged.

### Infrastructure Requirements

None. All Epic 6 types interact with Vault's internal identity and audit subsystems. No external services needed — Tier 1 across the board.

### Suggested Story Ordering

| Order | Story | Types | Complexity Notes |
|-------|-------|-------|-----------------|
| 1 | 6.1 | Group, GroupAlias | GroupAlias has non-trivial `PrepareInternalValues` (accessor lookup) |
| 2 | 6.2 | IdentityOIDCProvider, Scope, Client, Assignment | 4-type dependency chain |
| 3 | 6.3 | IdentityTokenConfig, Key, Role | 3 types, straightforward |
| 4 | 6.4 | Audit, AuditRequestHeader | New `VaultAuditResource` reconciler variant |

### New Patterns to Watch

- **`VaultAuditResource` reconciler variant** (Story 6.4) — third reconciler type, not yet exercised in integration tests
- **GroupAlias accessor lookup** (Story 6.1) — `PrepareInternalValues` resolves auth mount accessor ID dynamically

### Readiness Assessment

- Testing & Quality: All 63 specs passing, coverage at 46.0%
- Technical Health: Codebase stable, zero regressions
- Infrastructure: None needed for Epic 6
- Unresolved Blockers: None

### Verdict

**Ready to proceed with Epic 6.** No prep work needed. Story ordering: 6.1 → 6.2 → 6.3 → 6.4.

---

## Team Performance

Epic 5 delivered 3 stories covering integration tests for 6 secret engine types (DatabaseSecretEngineConfig/Role, RabbitMQSecretEngineConfig/Role, KubernetesSecretEngineConfig/Role) in ~2 days — the fastest epic to date. 4 types were correctly skipped per the three-tier infrastructure classification. The epic benefited directly from Epic 4's preparation work, with zero new infrastructure surprises beyond the Bitnami paywall pivot. Coverage grew 4.0 percentage points (42.0% → 46.0%), zero regressions, and the code review process caught 8 findings including a cross-story bug. The team is well-positioned for Epic 6.
24 changes: 18 additions & 6 deletions _bmad-output/implementation-artifacts/sprint-status.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,19 @@
# - Dev moves story to 'review', then runs code-review (fresh context, different LLM recommended)

generated: 2026-04-11
last_updated: 2026-04-23
last_updated: 2026-04-29
# Epic 5 done + retrospective completed 2026-04-29
# Story 5.3 done 2026-04-29
# Story 5.3 review 2026-04-29
# Story 5.2 done 2026-04-29
# Story 5.2 review 2026-04-29
# Story 5.2 in-progress 2026-04-28
# Story 5.1 done 2026-04-28
# Story 5.1 review 2026-04-28
# Story 5.3 ready-for-dev 2026-04-28
# Story 5.2 ready-for-dev 2026-04-28
# Story 5.1 ready-for-dev 2026-04-28
# Epic 5 in-progress 2026-04-28
# Epic 4 retrospective completed 2026-04-23
# Story 4.3 done 2026-04-23
# Story 4.3 review 2026-04-23
Expand Down Expand Up @@ -119,11 +131,11 @@ development_status:
epic-4-retrospective: done

# Epic 5: Secret Engine Integration Tests
epic-5: backlog
5-1-integration-tests-for-databasesecretengineconfig-and-databasesecretenginerole: backlog
5-2-integration-tests-for-rabbitmq-secret-engine-types: backlog
5-3-integration-tests-for-remaining-secret-engine-types: backlog
epic-5-retrospective: optional
epic-5: done
5-1-integration-tests-for-databasesecretengineconfig-and-databasesecretenginerole: done
5-2-integration-tests-for-rabbitmq-secret-engine-types: done
5-3-integration-tests-for-remaining-secret-engine-types: done
epic-5-retrospective: done

# Epic 6: Identity & Audit Integration Tests
epic-6: backlog
Expand Down
75 changes: 75 additions & 0 deletions controllers/controllertestutils/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,21 @@ func (d *decoder) GetDatabaseSecretEngineConfigInstance(filename string) (*redha
return nil, errDecode
}

func (d *decoder) GetDatabaseSecretEngineRoleInstance(filename string) (*redhatcopv1alpha1.DatabaseSecretEngineRole, error) {
obj, groupKindVersion, err := d.decodeFile(filename)
if err != nil {
return nil, err
}

kind := reflect.TypeOf(redhatcopv1alpha1.DatabaseSecretEngineRole{}).Name()
if groupKindVersion.Kind == kind {
o := obj.(*redhatcopv1alpha1.DatabaseSecretEngineRole)
return o, nil
}

return nil, errDecode
}

func (d *decoder) GetDatabaseSecretEngineStaticRoleInstance(filename string) (*redhatcopv1alpha1.DatabaseSecretEngineStaticRole, error) {
obj, groupKindVersion, err := d.decodeFile(filename)
if err != nil {
Expand Down Expand Up @@ -292,6 +307,66 @@ func (d *decoder) GetJWTOIDCAuthEngineRoleInstance(filename string) (*redhatcopv
return nil, errDecode
}

func (d *decoder) GetRabbitMQSecretEngineConfigInstance(filename string) (*redhatcopv1alpha1.RabbitMQSecretEngineConfig, error) {
obj, groupKindVersion, err := d.decodeFile(filename)
if err != nil {
return nil, err
}

kind := reflect.TypeOf(redhatcopv1alpha1.RabbitMQSecretEngineConfig{}).Name()
if groupKindVersion.Kind == kind {
o := obj.(*redhatcopv1alpha1.RabbitMQSecretEngineConfig)
return o, nil
}

return nil, errDecode
}

func (d *decoder) GetRabbitMQSecretEngineRoleInstance(filename string) (*redhatcopv1alpha1.RabbitMQSecretEngineRole, error) {
obj, groupKindVersion, err := d.decodeFile(filename)
if err != nil {
return nil, err
}

kind := reflect.TypeOf(redhatcopv1alpha1.RabbitMQSecretEngineRole{}).Name()
if groupKindVersion.Kind == kind {
o := obj.(*redhatcopv1alpha1.RabbitMQSecretEngineRole)
return o, nil
}

return nil, errDecode
}

func (d *decoder) GetKubernetesSecretEngineConfigInstance(filename string) (*redhatcopv1alpha1.KubernetesSecretEngineConfig, error) {
obj, groupKindVersion, err := d.decodeFile(filename)
if err != nil {
return nil, err
}

kind := reflect.TypeOf(redhatcopv1alpha1.KubernetesSecretEngineConfig{}).Name()
if groupKindVersion.Kind == kind {
o := obj.(*redhatcopv1alpha1.KubernetesSecretEngineConfig)
return o, nil
}

return nil, errDecode
}

func (d *decoder) GetKubernetesSecretEngineRoleInstance(filename string) (*redhatcopv1alpha1.KubernetesSecretEngineRole, error) {
obj, groupKindVersion, err := d.decodeFile(filename)
if err != nil {
return nil, err
}

kind := reflect.TypeOf(redhatcopv1alpha1.KubernetesSecretEngineRole{}).Name()
if groupKindVersion.Kind == kind {
o := obj.(*redhatcopv1alpha1.KubernetesSecretEngineRole)
return o, nil
}

return nil, errDecode
}

func (d *decoder) GetEntityAliasInstance(filename string) (*redhatcopv1alpha1.EntityAlias, error) {
obj, groupKindVersion, err := d.decodeFile(filename)
if err != nil {
Expand Down
Loading
Loading