From df3c0a6970a1cc7ab4b8aff05708b2eba4a0e95b Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Sun, 1 Mar 2026 22:26:54 +0000 Subject: [PATCH] feat(CC-0004): add common types and pure function packages - Add shared type definitions (ImageSpec, DatabaseSpec, MessagingSpec, CacheSpec, PolicySpec, PluginSpec, PipelineSpec) for reuse across OpenStack operator CRDs - Add conditions package for managing K8s status conditions with LastTransitionTime preservation on no-op updates - Add config package with deterministic INI rendering, user-wins merge semantics, secret injection with RFC 3986 password encoding, and oslo_policy config injection - Add plugins package for generating PasteDeploy api-paste.ini pipelines with positional middleware insertion and plugin config rendering - Add policy package for merging oslo.policy rules with inline-wins precedence, validation via field.ErrorList, and deterministic YAML rendering - Include comprehensive table-driven tests with Gomega for all packages, covering edge cases (nil inputs, empty maps, mutation safety, idempotency) - Add reference documentation for internal/common packages and types AI-assisted: Claude Code On-behalf-of: @SAP christian.berendt@sap.com Signed-off-by: Christian Berendt --- ...mmon-types-and-pure-function-packages.json | 79 ++- ...s-and-pure-function-packages-review-1.json | 112 ++++ ...s-and-pure-function-packages-review-2.json | 171 ++++++ ...-review-by-sourcery-ai-bot-external-1.json | 59 ++ docs/reference/internal-common-packages.md | 306 +++++++++++ docs/reference/internal-common-types.md | 132 +++++ internal/common/conditions/conditions.go | 63 +++ internal/common/conditions/conditions_test.go | 326 +++++++++++ internal/common/conditions/doc.go | 2 + internal/common/config/config.go | 197 +++++++ internal/common/config/config_test.go | 467 ++++++++++++++++ internal/common/config/doc.go | 3 + internal/common/plugins/doc.go | 3 + internal/common/plugins/plugins.go | 142 +++++ internal/common/plugins/plugins_test.go | 275 ++++++++++ internal/common/policy/doc.go | 3 + internal/common/policy/policy.go | 87 +++ internal/common/policy/policy_test.go | 232 ++++++++ internal/common/types/doc.go | 2 + internal/common/types/types.go | 85 +++ internal/common/types/types_test.go | 512 ++++++++++++++++++ 21 files changed, 3256 insertions(+), 2 deletions(-) rename .planwerk/{features => completed}/CC-0004-b001-add-common-types-and-pure-function-packages.json (95%) create mode 100644 .planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-1.json create mode 100644 .planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-2.json create mode 100644 .planwerk/reviews/CC-0004-github-review-by-sourcery-ai-bot-external-1.json create mode 100644 docs/reference/internal-common-packages.md create mode 100644 docs/reference/internal-common-types.md create mode 100644 internal/common/conditions/conditions.go create mode 100644 internal/common/conditions/conditions_test.go create mode 100644 internal/common/conditions/doc.go create mode 100644 internal/common/config/config.go create mode 100644 internal/common/config/config_test.go create mode 100644 internal/common/config/doc.go create mode 100644 internal/common/plugins/doc.go create mode 100644 internal/common/plugins/plugins.go create mode 100644 internal/common/plugins/plugins_test.go create mode 100644 internal/common/policy/doc.go create mode 100644 internal/common/policy/policy.go create mode 100644 internal/common/policy/policy_test.go create mode 100644 internal/common/types/doc.go create mode 100644 internal/common/types/types.go create mode 100644 internal/common/types/types_test.go diff --git a/.planwerk/features/CC-0004-b001-add-common-types-and-pure-function-packages.json b/.planwerk/completed/CC-0004-b001-add-common-types-and-pure-function-packages.json similarity index 95% rename from .planwerk/features/CC-0004-b001-add-common-types-and-pure-function-packages.json rename to .planwerk/completed/CC-0004-b001-add-common-types-and-pure-function-packages.json index d420b6b..238d349 100644 --- a/.planwerk/features/CC-0004-b001-add-common-types-and-pure-function-packages.json +++ b/.planwerk/completed/CC-0004-b001-add-common-types-and-pure-function-packages.json @@ -2,8 +2,8 @@ "feature_id": "CC-0004", "title": "B001: Add common types and pure function packages", "slug": "b001-add-common-types-and-pure-function-packages", - "status": "prepared", - "phase": null, + "status": "completed", + "phase": "awaiting_external_review", "summary": "", "description": "**Size:** πŸ—οΈ large\n**Category:** backend\n**Priority:** high\n\n## Resolved Decisions\n\n| # | Decision | Choice |\n|---|----------|--------|\n| 1 | `PipelinePosition` model | Struct with `Before` and `After` fields β€” `type PipelinePosition struct { Before string; After string }` |\n| 2 | `RenderPastePipeline` input | `PipelineSpec` struct β€” `{ Name, BasePipeline, Filters []FilterSpec, Middleware []MiddlewareSpec }` |\n| 3 | `ValidatePolicyRules` return | `field.ErrorList` β€” webhook-compatible from day one, avoids CC-0011 breaking change |\n| 4 | Kubebuilder markers | No markers on shared types β€” markers belong on CRD embedding sites in CC-0011 |\n\n## Summary\n\nCC-0004 implements the foundational shared type definitions and pure-function utility packages under `internal/common/`. This includes 11 reusable Go types (`ImageSpec`, `DatabaseSpec`, `MessagingSpec`, `CacheSpec`, `SecretRefSpec`, `PolicySpec`, `PluginSpec`, `MiddlewareSpec`, `PipelinePosition`, `PipelineSpec`, `FilterSpec`) and 4 utility packages (`conditions/`, `config/`, `plugins/`, `policy/`) containing pure functions that operate on Go maps, slices, and structs with no Kubernetes client interaction. All functions get table-driven unit tests using `gomega` + `testing.T` following existing codebase conventions.\n\n## Scope\n\n**Included:**\n- `internal/common/types/` β€” 11 shared type structs with JSON tags and godoc (9 original + `PipelineSpec` + `FilterSpec`)\n- `internal/common/conditions/` β€” `SetCondition`, `IsReady`, `GetCondition`, `AllTrue` operating on `[]metav1.Condition`\n- `internal/common/config/` β€” `RenderINI`, `MergeDefaults`, `InjectSecrets`, `InjectOsloPolicyConfig` operating on `map[string]map[string]string`\n- `internal/common/plugins/` β€” `RenderPastePipeline`, `RenderPluginConfig` producing INI text\n- `internal/common/policy/` β€” `RenderPolicyYAML`, `MergePolicies`, `ValidatePolicyRules` (returns `field.ErrorList`)\n- Table-driven unit tests for every exported function (`gomega` + `testing.T`, no build tags)\n\n**Excluded (with rationale):**\n- `CreateImmutableConfigMap` β€” requires K8s client, belongs in CC-0005\n- `LoadPolicyFromConfigMap` β€” requires K8s client, belongs in CC-0005\n- Kubebuilder CEL markers on types β€” belongs in CC-0011 (CRD embedding sites)\n- Webhook validation logic β€” belongs in CC-0011\n- `database/`, `deployment/`, `job/`, `secrets/`, `tls/` packages β€” all require K8s client, CC-0005\n- OpenStack release-specific default values β€” belong in operator-level code (YAGNI)\n\n## Visualization\n\n```mermaid\nflowchart TD\n subgraph types[\"internal/common/types/\"]\n IS[\"ImageSpec\"]\n DS[\"DatabaseSpec\"]\n MS[\"MessagingSpec\"]\n CS[\"CacheSpec\"]\n SRS[\"SecretRefSpec\"]\n PS[\"PolicySpec\"]\n PLS[\"PluginSpec\"]\n MWS[\"MiddlewareSpec\"]\n PP[\"PipelinePosition\"]\n FS[\"FilterSpec\"]\n PSP[\"PipelineSpec\"]\n end\n\n subgraph conditions[\"internal/common/conditions/\"]\n SC[\"SetCondition\"]\n IR[\"IsReady\"]\n GC[\"GetCondition\"]\n AT[\"AllTrue\"]\n end\n\n subgraph config[\"internal/common/config/\"]\n RI[\"RenderINI\"]\n MD[\"MergeDefaults\"]\n ISec[\"InjectSecrets\"]\n IOP[\"InjectOsloPolicyConfig\"]\n end\n\n subgraph plugins[\"internal/common/plugins/\"]\n RPP[\"RenderPastePipeline\"]\n RPC[\"RenderPluginConfig\"]\n end\n\n subgraph policy[\"internal/common/policy/\"]\n RPY[\"RenderPolicyYAML\"]\n MP[\"MergePolicies\"]\n VPR[\"ValidatePolicyRules\"]\n end\n\n DS --> SRS\n MS --> SRS\n MWS --> PP\n PSP --> MWS\n PSP --> FS\n\n RPP --> PSP\n RPC --> PLS\n RPY --> PS\n MP --> PS\n VPR --> PS\n IOP --> RI\n```\n\n```mermaid\nflowchart LR\n CRD[\"CRD Spec Fields\"] --> MD\n Defaults[\"Operator Defaults\"] --> MD[\"MergeDefaults\"]\n MD --> ISec[\"InjectSecrets\"]\n Secrets[\"Resolved Secret Values\"] --> ISec\n ISec --> IOP[\"InjectOsloPolicyConfig\"]\n IOP --> RI[\"RenderINI\"]\n RI --> INI[\"keystone.conf\"]\n\n CRD2[\"CRD Spec Fields\"] --> RPC[\"RenderPluginConfig\"]\n RPC --> ISec\n\n CRD3[\"CRD Spec Fields\"] --> RPP[\"RenderPastePipeline\"]\n RPP --> Paste[\"api-paste.ini\"]\n\n CRD4[\"CRD Spec Fields\"] --> MP[\"MergePolicies\"]\n MP --> VPR[\"ValidatePolicyRules\"]\n VPR --> RPY[\"RenderPolicyYAML\"]\n RPY --> PolicyYAML[\"policy.yaml\"]\n IOP -.->|\"adds oslo_policy section\"| RI\n```\n\n## Key Components\n\n- **`types/`** β€” 11 shared structs. `DatabaseSpec`, `MessagingSpec`, `CacheSpec` follow managed/brownfield dual-mode with `ClusterRef` vs explicit host fields. `SecretRefSpec` is embedded in multiple specs. `PipelinePosition` has `Before`/`After` string fields. `PipelineSpec` aggregates `BasePipeline`, `[]FilterSpec`, and `[]MiddlewareSpec`. `FilterSpec` captures base filter name, factory, and config.\n- **`conditions/`** β€” 4 pure functions for `metav1.Condition` slice manipulation. `AllTrue` gates the aggregate `Ready` condition in reconcilers. Zero K8s client dependency.\n- **`config/`** β€” INI rendering pipeline. `RenderINI` sorts sections/keys deterministically for stable content hashes. `MergeDefaults` implements user-wins-over-operator-defaults precedence. `InjectSecrets` assembles connection strings. `InjectOsloPolicyConfig` conditionally adds `[oslo_policy]` section.\n- **`plugins/`** β€” `RenderPastePipeline` takes `PipelineSpec`, inserts middleware filters at specified `PipelinePosition`s, and emits `[filter:]` blocks as api-paste.ini text. `RenderPluginConfig` converts `[]PluginSpec` into INI section maps for merging into main config.\n- **`policy/`** β€” `MergePolicies` implements inline-wins-over-external precedence. `RenderPolicyYAML` serializes merged rules as YAML. `ValidatePolicyRules` returns `field.ErrorList` for webhook-compatible validation (non-empty keys/values, valid YAML syntax).", "stories": [ @@ -508,6 +508,7 @@ "description": "Create internal/common/types/doc.go and internal/common/types/types.go. Define all 11 structs: ImageSpec, DatabaseSpec, MessagingSpec, CacheSpec, SecretRefSpec, PolicySpec, PluginSpec, MiddlewareSpec, PipelinePosition, PipelineSpec, FilterSpec. Each struct has exported fields with json tags and godoc comments. DatabaseSpec/MessagingSpec/CacheSpec follow managed/brownfield dual-mode. No kubebuilder markers. Import corev1 for LocalObjectReference. Verify go vet passes.", "level": 1, "estimate_minutes": 25, + "status": "done", "requirements": [ "REQ-001", "REQ-002", @@ -520,6 +521,7 @@ "description": "Create internal/common/types/types_test.go with table-driven tests using gomega + testing.T. Test JSON marshaling/unmarshaling for each type: verify json tags produce correct keys, verify optional fields are omitted with omitempty, verify PipelineSpec correctly nests FilterSpec and MiddlewareSpec. Test DatabaseSpec dual-mode (ClusterRef set vs Host set). Grep types.go for +kubebuilder to verify none exist.", "level": 1, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-001", "REQ-002", @@ -532,6 +534,7 @@ "description": "Create internal/common/conditions/doc.go and internal/common/conditions/conditions.go. Implement: SetCondition(*[]metav1.Condition, metav1.Condition) β€” inserts or updates by type, preserves LastTransitionTime when status unchanged. IsReady([]metav1.Condition) bool β€” checks Ready condition. GetCondition([]metav1.Condition, string) *metav1.Condition. AllTrue([]metav1.Condition, ...string) bool. All pure functions, zero K8s client imports.", "level": 1, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-003", "REQ-004", @@ -544,6 +547,7 @@ "description": "Create internal/common/conditions/conditions_test.go. Table-driven tests for: SetCondition (empty slice insert, update status, preserve LastTransitionTime), IsReady (nil slice, no Ready condition, Ready=True, Ready=False), GetCondition (nil, empty, found, not found), AllTrue (nil, empty types, all true, one false, missing type). Use gomega + testing.T pattern.", "level": 1, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-003", "REQ-004", @@ -556,6 +560,7 @@ "description": "Create internal/common/config/doc.go and internal/common/config/config.go. Implement RenderINI(map[string]map[string]string) string β€” sort sections and keys alphabetically, produce [section]\\nkey = value\\n format with blank lines between sections. Implement MergeDefaults(userConfig, defaults map[string]map[string]string) map[string]map[string]string β€” deep-merge where user values take precedence.", "level": 1, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-005", "REQ-006", @@ -568,6 +573,7 @@ "description": "Add to internal/common/config/config.go: InjectSecrets(config map[string]map[string]string, secrets map[string]string) map[string]map[string]string β€” assemble connection strings from secret values, URL-encode special characters in passwords. InjectOsloPolicyConfig(config map[string]map[string]string, policyFilePath string) β€” add [oslo_policy] section when policyFilePath is non-empty, no-op otherwise. Both depend on RenderINI existing from 3.1.", "level": 2, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-007", "REQ-008" @@ -579,6 +585,7 @@ "description": "Create internal/common/config/config_test.go. Table-driven tests: RenderINI β€” single section, multiple sorted sections, empty map, keys sorted within section, idempotent output. MergeDefaults β€” user overrides default, default fills gap, user-only section preserved, both empty, default-only section preserved. gomega + testing.T.", "level": 2, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-005", "REQ-006", @@ -591,6 +598,7 @@ "description": "Add to internal/common/config/config_test.go: Table-driven tests for InjectSecrets β€” valid DB credentials produce connection string, empty secrets is no-op, special characters in password are URL-encoded. InjectOsloPolicyConfig β€” non-empty path adds oslo_policy section, empty path is no-op. gomega + testing.T.", "level": 2, "estimate_minutes": 15, + "status": "done", "requirements": [ "REQ-007", "REQ-008", @@ -603,6 +611,7 @@ "description": "Create internal/common/plugins/doc.go and internal/common/plugins/plugins.go. Implement RenderPastePipeline(PipelineSpec) string β€” parse BasePipeline into filter names, insert middleware at Before/After positions, emit [composite:main] pipeline line and [filter:name] blocks with paste.filter_factory and config entries. Also emit [filter:name] blocks for FilterSpec entries. Imports types package from task 1.1.", "level": 2, "estimate_minutes": 25, + "status": "done", "requirements": [ "REQ-009", "REQ-015" @@ -614,6 +623,7 @@ "description": "Add to internal/common/plugins/plugins.go: RenderPluginConfig([]PluginSpec) map[string]map[string]string β€” iterate plugins, create a map entry per plugin using ConfigSection as key and Config as value. Return the combined map for merging into INI config via MergeDefaults.", "level": 2, "estimate_minutes": 15, + "status": "done", "requirements": [ "REQ-010" ] @@ -624,6 +634,7 @@ "description": "Create internal/common/plugins/plugins_test.go. Table-driven tests for RenderPastePipeline: base pipeline only, single middleware after position, single middleware before position, multiple middleware, middleware with config, empty middleware list. RenderPluginConfig: single plugin, multiple plugins, empty list. gomega + testing.T.", "level": 3, "estimate_minutes": 25, + "status": "done", "requirements": [ "REQ-009", "REQ-010", @@ -636,6 +647,7 @@ "description": "Create internal/common/policy/doc.go and internal/common/policy/policy.go. Implement MergePolicies(inline, external map[string]string) map[string]string β€” start from external (or empty map), overlay inline rules. Implement RenderPolicyYAML(rules map[string]string) (string, error) β€” marshal rules to YAML using gopkg.in/yaml.v3 (already in go.sum). Import types package for PolicySpec reference.", "level": 1, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-011", "REQ-013", @@ -648,6 +660,7 @@ "description": "Add to internal/common/policy/policy.go: ValidatePolicyRules(rules map[string]string, fldPath *field.Path) field.ErrorList β€” iterate rules, check for empty keys (field.Invalid), check for empty values (field.Invalid). Return accumulated field.ErrorList. Import k8s.io/apimachinery/pkg/util/validation/field.", "level": 2, "estimate_minutes": 15, + "status": "done", "requirements": [ "REQ-012" ] @@ -658,6 +671,7 @@ "description": "Create internal/common/policy/policy_test.go. Table-driven tests for MergePolicies: inline wins, non-colliding merged, nil inline, both nil. RenderPolicyYAML: valid rules produce parseable YAML, empty map, special characters. ValidatePolicyRules: valid rules pass, empty key error, empty value error, multiple errors. gomega + testing.T.", "level": 3, "estimate_minutes": 25, + "status": "done", "requirements": [ "REQ-011", "REQ-012", @@ -671,6 +685,7 @@ "description": "Create docs/reference/internal-common-types.md documenting all 11 type structs with field tables (name, type, JSON tag, description, required/optional). Create docs/reference/internal-common-packages.md documenting all 4 utility packages with function signatures, parameters, return values, and brief descriptions. Cross-reference architecture/docs/09-implementation/02-shared-library.md.", "level": 4, "estimate_minutes": 20, + "status": "done", "requirements": [ "REQ-015" ] @@ -887,8 +902,18 @@ "prepared": { "github_account": "berendt", "timestamp": "2026-03-01T21:05:41.417661" + }, + "processing": { + "github_account": "berendt", + "timestamp": "2026-03-01T21:33:37.485467" + }, + "completed": { + "github_account": "berendt", + "timestamp": "2026-03-01T22:26:54.049253" } }, + "github_pr": "https://github.com/C5C3/forge/pull/7", + "pr_number": 7, "execution_history": [ { "run_id": "f941e161-f333-45de-bc42-2e062e5654d5", @@ -903,6 +928,56 @@ "status": "done" } ] + }, + { + "run_id": "8145e29d-d074-4218-bf05-53b924577cdd", + "timestamp": "2026-03-01T22:00:22.914161", + "total_duration": 1465.8437767028809, + "status": "completed", + "timings": [ + { + "name": "Level 1 (6 tasks)", + "duration": 514.3989868164062, + "type": "level", + "status": "done" + }, + { + "name": "Level 2 (6 tasks)", + "duration": 137.56722593307495, + "type": "level", + "status": "done" + }, + { + "name": "Level 3 (2 tasks)", + "duration": 31.218823432922363, + "type": "level", + "status": "done" + }, + { + "name": "Level 4 (1 tasks)", + "duration": 171.0960190296173, + "type": "level", + "status": "done" + }, + { + "name": "[CC-0004] Code Review", + "duration": 149.70509338378906, + "type": "review", + "status": "done" + }, + { + "name": "[CC-0004] Improvements", + "duration": 278.8772928714752, + "type": "improve", + "status": "done" + }, + { + "name": "[CC-0004] Simplify", + "duration": 182.9803352355957, + "type": "simplify", + "status": "done" + } + ] } ] } \ No newline at end of file diff --git a/.planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-1.json b/.planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-1.json new file mode 100644 index 0000000..9f38f8a --- /dev/null +++ b/.planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-1.json @@ -0,0 +1,112 @@ +{ + "feature_id": "CC-0004", + "title": "B001: Add common types and pure function packages", + "date": "2026-03-01", + "summary": "CC-0004 implements 11 shared Go type structs in internal/common/types/ and 4 pure-function utility packages (conditions/, config/, plugins/, policy/) with 13 exported functions. All packages compile cleanly, pass go vet, golangci-lint, and 73 tests. Coverage is 100% for conditions, plugins, and types; 96.9% for policy; 88.9% for config. No kubebuilder markers on types (Decision #4). No K8s client-go imports in pure function packages. ValidatePolicyRules returns field.ErrorList (Decision #3). RenderINI output is deterministic. All doc.go files and reference documentation present.", + "verdict": "NEEDS_CHANGES", + "tests_checklist": [ + {"item": "All tests pass (73 tests, 0 failures)", "checked": true}, + {"item": "Table-driven tests using gomega + testing.T (no ginkgo, no build tags)", "checked": true}, + {"item": "Every exported function has at least one test", "checked": true}, + {"item": "Edge cases covered: nil slices, empty maps, nil pointers, boundary values", "checked": true}, + {"item": "Immutability verified: does-not-mutate-inputs tests for MergeDefaults, InjectSecrets, InjectOsloPolicyConfig, MergePolicies", "checked": true}, + {"item": "InjectSecrets generic section/key injection path tested", "checked": false}, + {"item": "assembleDatabaseConnection default port/name fallbacks tested", "checked": false}, + {"item": "Determinism tests: RenderINI idempotency, RenderPolicyYAML determinism, filter section sorting", "checked": true} + ], + "code_quality_checklist": [ + {"item": "Functions do one thing (SRP)", "checked": true}, + {"item": "Functions small (all under 40 lines)", "checked": true}, + {"item": "No code duplication (copyConfig properly extracted)", "checked": true}, + {"item": "Clear naming (intention-revealing)", "checked": true}, + {"item": "Max nesting under 3 levels", "checked": true}, + {"item": "No dead code in production files", "checked": true}, + {"item": "No magic numbers/strings in production code", "checked": false} + ], + "security_checklist": [ + {"item": "No hardcoded secrets", "checked": true}, + {"item": "No K8s client-go imports in pure function packages", "checked": true}, + {"item": "Special characters in passwords URL-encoded", "checked": true}, + {"item": "No path traversal vulnerabilities", "checked": true}, + {"item": "No sensitive data in logs or errors", "checked": true} + ], + "architecture_checklist": [ + {"item": "All 11 type structs defined with correct fields, types, and JSON tags", "checked": true}, + {"item": "No +kubebuilder markers in types package (Decision #4)", "checked": true}, + {"item": "All 13 exported functions implemented", "checked": true}, + {"item": "ValidatePolicyRules returns field.ErrorList (Decision #3)", "checked": true}, + {"item": "RenderINI output deterministic (sections and keys sorted)", "checked": true}, + {"item": "MergeDefaults implements user-wins precedence", "checked": true}, + {"item": "InjectOsloPolicyConfig is no-op when policyFilePath is empty", "checked": true}, + {"item": "Every package has doc.go with package-level godoc", "checked": true}, + {"item": "Reference docs in docs/reference/ for types and utility packages", "checked": true}, + {"item": "Solution is as simple as possible", "checked": true} + ], + "dry_yagni_checklist": [ + {"item": "No duplicated code", "checked": true}, + {"item": "No over-engineering or unused abstractions", "checked": true}, + {"item": "Excluded items (CreateImmutableConfigMap, LoadPolicyFromConfigMap, etc.) correctly deferred", "checked": true} + ], + "fail_fast_checklist": [ + {"item": "Nil pointer handled gracefully in SetCondition", "checked": true}, + {"item": "Empty input handled gracefully in all functions", "checked": true}, + {"item": "ValidatePolicyRules reports all errors (not just first)", "checked": true} + ], + "defensive_checklist": [ + {"item": "All functions return new maps/slices (no input mutation)", "checked": true}, + {"item": "Nil and empty inputs handled without panics", "checked": true} + ], + "security_findings": [], + "architecture_findings": [], + "issues_found": [ + { + "id": "TE2-1", + "severity": "major", + "check_id": "TE2", + "file": "internal/common/config/config_test.go", + "description": "InjectSecrets generic section/key injection path (config.go:100-113) is untested. The loop that handles arbitrary 'section/key' secrets (non-database parts) has zero test coverage. InjectSecrets is at 68.2% statement coverage.", + "fix": "Add test case to TestInjectSecrets: {name: 'generic section/key secrets injected', config: {}, secrets: {'keystone_authtoken/auth_url': 'https://keystone:5000'}, expected: {'keystone_authtoken': {'auth_url': 'https://keystone:5000'}}}. Also add a case mixing database and generic secrets." + }, + { + "id": "TE2-2", + "severity": "major", + "check_id": "TE2", + "file": "internal/common/config/config_test.go", + "description": "assembleDatabaseConnection default fallbacks (config.go:172-178) untested. When 'database/port' or 'database/name' are omitted from secrets, the function defaults to '3306' and uses the username respectively. These branches have 0 test coverage (function at 80%).", + "fix": "Add test case to TestInjectSecrets: {name: 'default port and database name when omitted', config: {}, secrets: {'database/user': 'admin', 'database/password': 'pass', 'database/host': 'db.host'}, expected: {'database': {'connection': 'mysql+pymysql://admin:pass@db.host:3306/admin'}}}." + }, + { + "id": "Q6-1", + "severity": "minor", + "check_id": "Q6", + "file": "internal/common/config/config.go:173", + "description": "Magic string '3306' as default port in assembleDatabaseConnection.", + "fix": "Extract to a named constant: const defaultDatabasePort = \"3306\"." + }, + { + "id": "D3-1", + "severity": "minor", + "check_id": "Q7", + "file": "internal/common/types/types_test.go:328", + "description": "TestDatabaseSpec_DualMode test struct has an unused 'expectedJSON' field that is never set in any test case.", + "fix": "Remove the 'expectedJSON string' field from the test struct." + }, + { + "id": "Q3-1", + "severity": "minor", + "check_id": "Q3", + "file": "internal/common/plugins/plugins_test.go:271-278", + "description": "Helper function 'indexOf' reimplements strings.Index from the standard library.", + "fix": "Replace with strings.Index(s, substr)." + } + ], + "suggested_improvements": [ + "Consider using url.UserPassword() or url.URL{} for assembling database connection URIs instead of manual string formatting + escapePassword. This handles all RFC 3986 reserved characters in userinfo correctly, including $, +, ;, = which the current escapePassword misses.", + "The escapePassword function uses url.PathEscape which does not escape all URI-reserved characters for the userinfo component. Characters like +, $, ;, = are left unescaped. While unlikely in typical passwords, this could cause parsing issues with connection strings containing these characters." + ], + "next_steps": [ + "Add missing test cases for InjectSecrets generic section/key injection path (TE2-1)", + "Add missing test cases for assembleDatabaseConnection default port/name fallbacks (TE2-2)", + "After test gaps are addressed, re-run coverage to confirm config package reaches >95%" + ] +} diff --git a/.planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-2.json b/.planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-2.json new file mode 100644 index 0000000..108b22f --- /dev/null +++ b/.planwerk/reviews/CC-0004-b001-add-common-types-and-pure-function-packages-review-2.json @@ -0,0 +1,171 @@ +{ + "feature_id": "CC-0004", + "title": "B001: Add common types and pure function packages", + "date": "2026-03-01", + "summary": "CC-0004 implements 11 shared Go type structs in internal/common/types/ and 4 pure-function utility packages (conditions/, config/, plugins/, policy/) with 13 exported functions. All 86 tests pass. Coverage is 100% for conditions, config, and plugins; 96.9% for policy (yaml.Marshal error branch). go build, go vet, and golangci-lint all pass clean. All review-1 issues have been addressed. All sourcery-ai[bot] review issues have been addressed. Two plan-criteria discrepancies found: MessagingSpec missing ClusterRef field and PolicySpec missing SecretRefSpec embedding. One minor code robustness issue in escapePassword. One minor test pattern inconsistency in plugins package.", + "verdict": "NEEDS_CHANGES", + "tests_checklist": [ + {"item": "All 86 tests pass (0 failures)", "checked": true}, + {"item": "Every exported function has at least one test", "checked": true}, + {"item": "Table-driven tests using gomega + testing.T (no ginkgo, no build tags)", "checked": true}, + {"item": "Edge cases covered: nil slices, empty maps, nil pointers, boundary values", "checked": true}, + {"item": "Immutability verified: does-not-mutate-inputs tests for MergeDefaults, InjectSecrets, InjectOsloPolicyConfig, MergePolicies", "checked": true}, + {"item": "InjectSecrets generic section/key injection path tested (review-1 TE2-1 resolved)", "checked": true}, + {"item": "assembleDatabaseConnection default port/name fallbacks tested (review-1 TE2-2 resolved)", "checked": true}, + {"item": "Determinism tests: RenderINI idempotency, RenderPolicyYAML determinism, filter section sorting", "checked": true}, + {"item": "GetCondition mutation test verifies pointer into original slice (sourcery-ai issue resolved)", "checked": true}, + {"item": "InjectSecrets incomplete parts and connection-precedence tests (sourcery-ai issue resolved)", "checked": true}, + {"item": "plugins/ tests use table-driven t.Run pattern consistent with other packages", "checked": false}, + {"item": "TestRenderINI covers nil map input (consistent with other functions in same file)", "checked": false} + ], + "code_quality_checklist": [ + {"item": "Functions do one thing (SRP)", "checked": true}, + {"item": "Functions small (all under 40 lines)", "checked": true}, + {"item": "No code duplication (copyConfig properly extracted)", "checked": true}, + {"item": "Clear naming (intention-revealing)", "checked": true}, + {"item": "Max nesting under 3 levels", "checked": true}, + {"item": "No dead code in production files", "checked": true}, + {"item": "No magic numbers/strings (review-1 Q6-1 resolved: defaultMySQLPort constant)", "checked": true}, + {"item": "No unused test struct fields (review-1 D3-1 resolved: expectedJSON removed)", "checked": true} + ], + "security_checklist": [ + {"item": "No hardcoded secrets", "checked": true}, + {"item": "No K8s client-go imports in pure function packages (conditions/, config/, plugins/, policy/)", "checked": true}, + {"item": "Special characters in passwords URL-encoded via url.UserPassword (RFC 3986 userinfo)", "checked": true}, + {"item": "No path traversal vulnerabilities", "checked": true}, + {"item": "No sensitive data in logs or errors", "checked": true} + ], + "architecture_checklist": [ + {"item": "All 11 type structs defined with correct fields, types, and JSON tags", "checked": true}, + {"item": "No +kubebuilder markers in types package (Decision #4)", "checked": true}, + {"item": "All 13 exported functions implemented", "checked": true}, + {"item": "ValidatePolicyRules returns field.ErrorList (Decision #3)", "checked": true}, + {"item": "PipelinePosition is struct with Before/After string fields (Decision #1)", "checked": true}, + {"item": "RenderINI output deterministic (sections and keys sorted)", "checked": true}, + {"item": "MergeDefaults implements user-wins precedence", "checked": true}, + {"item": "InjectOsloPolicyConfig is no-op when policyFilePath is empty", "checked": true}, + {"item": "Every package has doc.go with package-level godoc", "checked": true}, + {"item": "Reference docs in docs/reference/ for types and utility packages", "checked": true}, + {"item": "MessagingSpec follows managed/brownfield dual-mode (ClusterRef vs explicit host)", "checked": false}, + {"item": "SecretRefSpec embedded in PolicySpec as per plan acceptance criteria", "checked": false}, + {"item": "Solution is as simple as possible", "checked": true} + ], + "dry_yagni_checklist": [ + {"item": "No duplicated code", "checked": true}, + {"item": "No over-engineering or unused abstractions", "checked": true}, + {"item": "Excluded items (CreateImmutableConfigMap, LoadPolicyFromConfigMap, etc.) correctly deferred", "checked": true} + ], + "fail_fast_checklist": [ + {"item": "Nil pointer handled gracefully in SetCondition", "checked": true}, + {"item": "Empty input handled gracefully in all functions", "checked": true}, + {"item": "ValidatePolicyRules reports all errors with deterministic ordering", "checked": true} + ], + "defensive_checklist": [ + {"item": "All functions return new maps/slices (no input mutation)", "checked": true}, + {"item": "Nil and empty inputs handled without panics", "checked": true} + ], + "security_findings": [], + "architecture_findings": [ + { + "severity": "MEDIUM", + "description": "MessagingSpec missing ClusterRef field. Plan criteria (line 17, line 508) explicitly state 'DatabaseSpec, MessagingSpec, CacheSpec follow managed/brownfield dual-mode pattern (ClusterRef vs explicit host fields)'. Current implementation has Host as required (no omitempty) with no ClusterRef pointer field, making it brownfield-only. DatabaseSpec and CacheSpec correctly implement dual-mode. Either add ClusterRef to MessagingSpec and make Host optional, or formally update the plan criteria if this was an intentional design change.", + "location": "internal/common/types/types.go:30-35", + "fix": "Add ClusterRef *corev1.LocalObjectReference field with json:\"clusterRef,omitempty\" and change Host tag to json:\"host,omitempty\". Update docs/reference/internal-common-types.md and types_test.go accordingly. Alternatively, document the intentional deviation in the plan." + }, + { + "severity": "LOW", + "description": "PolicySpec missing SecretRefSpec embedding. Plan acceptance criteria (line 18) state 'SecretRefSpec is embedded in DatabaseSpec, MessagingSpec, and PolicySpec via value/pointer as per architecture docs'. PolicySpec currently has ConfigMapRef and Inline only. This may be an intentional design simplification (policy sources don't naturally need secret references), but the plan criteria were not formally updated.", + "location": "internal/common/types/types.go:43-47", + "fix": "Either add SecretRefSpec to PolicySpec if the architecture docs require it, or formally update the plan acceptance criteria to remove this requirement. Given PolicySpec deals with policy rules (not credentials), dropping this requirement seems reasonable β€” but it should be explicit." + } + ], + "issues_found": [ + { + "id": "FC1-1", + "severity": "major", + "check_id": "FC1", + "file": "internal/common/types/types.go:30-35", + "description": "MessagingSpec does not implement the managed/brownfield dual-mode pattern specified in plan criteria. Missing ClusterRef *corev1.LocalObjectReference field. Host field uses json:\"host\" (required) instead of json:\"host,omitempty\" (optional). DatabaseSpec and CacheSpec both correctly implement dual-mode. Plan references: task 1.1 description, user story criterion line 17.", + "fix": "Add 'ClusterRef *corev1.LocalObjectReference `json:\"clusterRef,omitempty\"`' field to MessagingSpec. Change Host tag to 'json:\"host,omitempty\"'. Update docs/reference/internal-common-types.md to reflect dual-mode. Add JSON round-trip test for MessagingSpec with ClusterRef set. OR: formally update plan criteria if this was intentional." + }, + { + "id": "FC1-2", + "severity": "minor", + "check_id": "FC1", + "file": "internal/common/types/types.go:43-47", + "description": "PolicySpec missing SecretRefSpec embedding per plan acceptance criteria (line 18). Likely an intentional design simplification β€” policy sources don't need secret references. Docs and implementation are internally consistent. Plan criteria need formal update to match.", + "fix": "Update plan acceptance criteria to remove SecretRefSpec requirement from PolicySpec, or add the field if the architecture docs require it." + }, + { + "id": "P1-1", + "severity": "minor", + "check_id": "P1", + "file": "internal/common/plugins/plugins_test.go", + "description": "plugins/ test file uses standalone top-level test functions (TestRenderPastePipeline_BasePipelineOnly, etc.) instead of the table-driven t.Run pattern used consistently in conditions/, config/, and policy/ test files. Functionally correct but inconsistent with project conventions.", + "fix": "Consolidate into table-driven TestRenderPastePipeline(t *testing.T) and TestRenderPluginConfig(t *testing.T) with t.Run subtests." + }, + { + "id": "TE2-1", + "severity": "minor", + "check_id": "TE2", + "file": "internal/common/config/config_test.go", + "description": "TestRenderINI does not include a nil-map input test case. Every other function in the same file (MergeDefaults, InjectSecrets, InjectOsloPolicyConfig) has an explicit nil-input test. The code handles nil correctly (len(nil) == 0), but the contract is unverified by tests.", + "fix": "Add table case: {name: \"nil map returns empty string\", config: nil, expected: \"\"}." + }, + { + "id": "Q4-1", + "severity": "minor", + "check_id": "Q4", + "file": "internal/common/config/config.go:189-193", + "description": "escapePassword uses fragile byte-index slicing (userinfo[2:]) to extract the encoded password from url.UserPassword(\"x\", password).String(). The offset depends on the placeholder \"x\" always encoding to exactly 1 byte. While correct today, strings.Cut would be more robust and self-documenting.", + "fix": "Replace 'return userinfo[2:]' with '_, encoded, _ := strings.Cut(userinfo, \":\"); return encoded'." + }, + { + "id": "C3-1", + "severity": "minor", + "check_id": "C3", + "file": "internal/common/config/config.go:127-129", + "description": "InjectOsloPolicyConfig unconditionally replaces the entire oslo_policy section. If the caller's config already has oslo_policy keys (e.g. enforce_scope, enforce_new_defaults), they are silently dropped. This violates the user-wins principle established by MergeDefaults.", + "fix": "Merge into existing section instead of replacing: if result[\"oslo_policy\"] == nil { result[\"oslo_policy\"] = make(map[string]string) }; result[\"oslo_policy\"][\"policy_file\"] = policyFilePath." + } + ], + "suggested_improvements": [ + "Consider adding a test case for InjectOsloPolicyConfig where the input config already has an oslo_policy section with additional keys, to verify merge-vs-replace behavior.", + "ValidatePolicyRules uses fldPath.Key(\"\") when k is empty, producing a confusing webhook error path like 'spec.rules[\"\"]'. Consider using fldPath directly or field.Required for a clearer error message." + ], + "next_steps": [ + "Resolve MessagingSpec dual-mode discrepancy (FC1-1): either add ClusterRef field or formally update plan criteria", + "Resolve PolicySpec SecretRefSpec discrepancy (FC1-2): formally update plan criteria to match implementation", + "Fix InjectOsloPolicyConfig section-replace behavior to merge instead (C3-1)", + "Optional: consolidate plugins/ tests into table-driven pattern (P1-1)", + "Optional: add nil-map test to TestRenderINI (TE2-1)", + "Optional: use strings.Cut in escapePassword (Q4-1)" + ], + "review_1_issues_resolved": [ + "TE2-1: InjectSecrets generic section/key injection path β€” now tested with 'generic section/key secrets injected' and 'generic section/key merges into existing section' cases", + "TE2-2: assembleDatabaseConnection default port/name fallbacks β€” now tested with 'default port 3306' and 'default name equals user' cases", + "Q6-1: Magic string '3306' β€” extracted to const defaultMySQLPort", + "D3-1: Unused expectedJSON field in types_test.go β€” removed", + "Q3-1: indexOf helper reimplementing strings.Index β€” removed" + ], + "sourcery_ai_issues_resolved": [ + "insertToken docstring expanded to document After>Before precedence and fallback-append behavior", + "GetCondition mutation test added (TestGetCondition_MutationUpdatesOriginalSlice)", + "InjectSecrets incomplete-parts and connection-precedence tests added", + "Typo 'Empty secrets is a no-op' corrected to 'Empty secrets are a no-op'", + "RenderPolicyYAML comment now correctly says '{}\n' not '{}'" + ], + "automated_verification": { + "go_build": "PASS β€” go build ./internal/common/... clean", + "go_test": "PASS β€” 86 tests, 0 failures", + "go_vet": "PASS β€” go vet ./internal/common/... clean", + "golangci_lint": "PASS β€” 0 issues", + "coverage": { + "conditions": "100.0%", + "config": "100.0%", + "plugins": "100.0%", + "policy": "96.9%", + "types": "no statements (struct definitions only)" + } + } +} diff --git a/.planwerk/reviews/CC-0004-github-review-by-sourcery-ai-bot-external-1.json b/.planwerk/reviews/CC-0004-github-review-by-sourcery-ai-bot-external-1.json new file mode 100644 index 0000000..29573fd --- /dev/null +++ b/.planwerk/reviews/CC-0004-github-review-by-sourcery-ai-bot-external-1.json @@ -0,0 +1,59 @@ +{ + "feature_id": "CC-0004", + "title": "GitHub Review by sourcery-ai[bot]", + "summary": "Hey - I've found 4 issues, and left some high level feedback:\n\n- In `RenderPolicyYAML`, the comment says \"Empty map returns empty YAML (\"{}\")\" but the function actually returns `\"{}\\n\"`; consider aligning the comment (and any reference docs) with the actual behavior to avoid confusion.\n\n
\nPrompt for AI Agents\n\n~~~markdown\nPlease address the comments from this code review:\n\n## Overall Comments\n- In `RenderPolicyYAML`, the comment says \"Empty map returns empty YAML (\"{}\")\" but the function actually returns `\"{}\\n\"`; consider aligning the comment (and any reference docs) with the actual behavior to avoid confusion.\n\n## Individual Comments\n\n### Comment 1\n\n\n+}\n+\n+// insertToken inserts a token into the pipeline at the position specified.\n+func insertToken(tokens []string, name string, pos types.PipelinePosition) []string {\n+\tif pos.After != \"\" {\n+\t\tfor i, t := range tokens {\n+\t\t\tif t == pos.After {\n+\t\t\t\tresult := make([]string, 0, len(tokens)+1)\n+\t\t\t\tresult = append(result, tokens[:i+1]...)\n+\t\t\t\tresult = append(result, name)\n+\t\t\t\tresult = append(result, tokens[i+1:]...)\n+\t\t\t\treturn result\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif pos.Before != \"\" {\n+\t\tfor i, t := range tokens {\n+\t\t\tif t == pos.Before {\n+\t\t\t\tresult := make([]string, 0, len(tokens)+1)\n+\t\t\t\tresult = append(result, tokens[:i]...)\n+\t\t\t\tresult = append(result, name)\n+\t\t\t\tresult = append(result, tokens[i:]...)\n+\t\t\t\treturn result\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t// Default: append to end.\n+\treturn append(tokens, name)\n+}\n\n\n**suggestion (bug_risk):** insertToken prioritizes After over Before and silently appends when no anchor is found, which might hide configuration mistakes\n\nConsider either failing when a requested `Before`/`After` anchor is not found (so configuration errors surface early) or clearly documenting that invalid anchors fall back to an append so callers understand the behavior and its risks.\n\n```suggestion\n// insertToken inserts a token into the pipeline at the position specified.\n// If both After and Before are set, After takes precedence.\n// If the requested anchor token (After/Before) is not found in the existing\n// pipeline, the token is appended to the end. This fallback behavior is\n// intentional; callers that require strict placement should validate their\n// configuration before invoking insertToken.\n```\n\n\n### Comment 2\n\n\n+func TestGetCondition(t *testing.T) {\n\n\n**issue (testing):** Add a test that mutating the condition returned by GetCondition also updates the underlying slice element\n\n`TestGetCondition` currently only checks the fields of the returned pointer, but not that it references the original slice element. Please also verify that mutating the returned condition (e.g. `c.Status = ...`) updates the original `[]metav1.Condition`, ensuring `GetCondition` returns a pointer into the slice rather than a copy, e.g.:\n\n```go\ng := NewGomegaWithT(t)\nconditions := []metav1.Condition{{Type: \"Ready\", Status: metav1.ConditionFalse}}\n\nc := GetCondition(conditions, \"Ready\")\ng.Expect(c).NotTo(BeNil())\nc.Status = metav1.ConditionTrue\n\ng.Expect(conditions[0].Status).To(Equal(metav1.ConditionTrue))\n```\n\n\n### Comment 3\n\n\n+\t})\n+}\n+\n+func TestInjectSecrets(t *testing.T) {\n+\ttests := []struct {\n+\t\tname string\n\n\n**suggestion (testing):** Add tests for incomplete or conflicting database secret inputs in InjectSecrets\n\nThe current `TestInjectSecrets` table is strong, but two edge cases are missing:\n\n1. When only some of `database/user`, `database/password`, and `database/host` are set, `hasDatabaseParts` is false and `database.connection` is not produced. Please add a case that asserts this behavior so we don’t accidentally start emitting partial connection strings.\n2. When both `database/connection` and the individual parts are provided, the code appears to prefer `database/connection`. A test that captures this precedence would help prevent regressions.\n\nNew table entries for these will better document and lock in the intended behavior for borderline configurations.\n\n\n### Comment 4\n\n\n+| `config` | `map[string]map[string]string` | Current configuration to inject into |\n+| `secrets` | `map[string]string` | Resolved secret values keyed by `section/key` |\n+\n+**Returns:** `map[string]map[string]string` β€” new config with secrets injected. Empty secrets is a no-op.\n+\n+**Database connection assembly:** When individual secret parts (`database/user`, `database/password`, `database/host`, and optionally `database/port`, `database/name`) are present, they are assembled into a connection string:\n\n\n**issue (typo):** Consider changing \"Empty secrets is a no-op\" to \"Empty secrets are a no-op\".\n\nBecause \"secrets\" is plural, use \"are\" instead of \"is\" in that sentence.\n\n```suggestion\n**Returns:** `map[string]map[string]string` β€” new config with secrets injected. Empty secrets are a no-op.\n```\n\n~~~\n\n
\n\n***\n\n
\nSourcery is free for open source - if you like our reviews please consider sharing them ✨\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20got%20an%20instant%20code%20review%20from%20%40SourceryAI%2C%20and%20it%20was%20brilliant%21%20It%27s%20free%20for%20open%20source%20and%20has%20a%20free%20trial%20for%20private%20code.%20Check%20it%20out%20https%3A//sourcery.ai)\n- [Mastodon](https://mastodon.social/share?text=I%20just%20got%20an%20instant%20code%20review%20from%20%40SourceryAI%2C%20and%20it%20was%20brilliant%21%20It%27s%20free%20for%20open%20source%20and%20has%20a%20free%20trial%20for%20private%20code.%20Check%20it%20out%20https%3A//sourcery.ai)\n- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https://sourcery.ai)\n- [Facebook](https://www.facebook.com/sharer/sharer.php?u=https://sourcery.ai)\n\n
\n\n\nHelp me be more useful! Please click πŸ‘ or πŸ‘Ž on each comment and I'll use the feedback to improve your reviews.\n", + "verdict": "ADDRESSED", + "tests_checklist": [], + "code_quality_checklist": [], + "security_checklist": [], + "architecture_checklist": [], + "dry_yagni_checklist": [], + "fail_fast_checklist": [], + "defensive_checklist": [], + "security_findings": [], + "architecture_findings": [], + "issues_found": [], + "suggested_improvements": [], + "next_steps": [], + "reviewer": "sourcery-ai[bot]", + "date": "2026-03-01T22:03:34Z", + "commit_reference": "", + "review_type": "external", + "sequence_number": 1, + "external_feedback": "Hey - I've found 4 issues, and left some high level feedback:\n\n- In `RenderPolicyYAML`, the comment says \"Empty map returns empty YAML (\"{}\")\" but the function actually returns `\"{}\\n\"`; consider aligning the comment (and any reference docs) with the actual behavior to avoid confusion.\n\n
\nPrompt for AI Agents\n\n~~~markdown\nPlease address the comments from this code review:\n\n## Overall Comments\n- In `RenderPolicyYAML`, the comment says \"Empty map returns empty YAML (\"{}\")\" but the function actually returns `\"{}\\n\"`; consider aligning the comment (and any reference docs) with the actual behavior to avoid confusion.\n\n## Individual Comments\n\n### Comment 1\n\n\n+}\n+\n+// insertToken inserts a token into the pipeline at the position specified.\n+func insertToken(tokens []string, name string, pos types.PipelinePosition) []string {\n+\tif pos.After != \"\" {\n+\t\tfor i, t := range tokens {\n+\t\t\tif t == pos.After {\n+\t\t\t\tresult := make([]string, 0, len(tokens)+1)\n+\t\t\t\tresult = append(result, tokens[:i+1]...)\n+\t\t\t\tresult = append(result, name)\n+\t\t\t\tresult = append(result, tokens[i+1:]...)\n+\t\t\t\treturn result\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif pos.Before != \"\" {\n+\t\tfor i, t := range tokens {\n+\t\t\tif t == pos.Before {\n+\t\t\t\tresult := make([]string, 0, len(tokens)+1)\n+\t\t\t\tresult = append(result, tokens[:i]...)\n+\t\t\t\tresult = append(result, name)\n+\t\t\t\tresult = append(result, tokens[i:]...)\n+\t\t\t\treturn result\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t// Default: append to end.\n+\treturn append(tokens, name)\n+}\n\n\n**suggestion (bug_risk):** insertToken prioritizes After over Before and silently appends when no anchor is found, which might hide configuration mistakes\n\nConsider either failing when a requested `Before`/`After` anchor is not found (so configuration errors surface early) or clearly documenting that invalid anchors fall back to an append so callers understand the behavior and its risks.\n\n```suggestion\n// insertToken inserts a token into the pipeline at the position specified.\n// If both After and Before are set, After takes precedence.\n// If the requested anchor token (After/Before) is not found in the existing\n// pipeline, the token is appended to the end. This fallback behavior is\n// intentional; callers that require strict placement should validate their\n// configuration before invoking insertToken.\n```\n\n\n### Comment 2\n\n\n+func TestGetCondition(t *testing.T) {\n\n\n**issue (testing):** Add a test that mutating the condition returned by GetCondition also updates the underlying slice element\n\n`TestGetCondition` currently only checks the fields of the returned pointer, but not that it references the original slice element. Please also verify that mutating the returned condition (e.g. `c.Status = ...`) updates the original `[]metav1.Condition`, ensuring `GetCondition` returns a pointer into the slice rather than a copy, e.g.:\n\n```go\ng := NewGomegaWithT(t)\nconditions := []metav1.Condition{{Type: \"Ready\", Status: metav1.ConditionFalse}}\n\nc := GetCondition(conditions, \"Ready\")\ng.Expect(c).NotTo(BeNil())\nc.Status = metav1.ConditionTrue\n\ng.Expect(conditions[0].Status).To(Equal(metav1.ConditionTrue))\n```\n\n\n### Comment 3\n\n\n+\t})\n+}\n+\n+func TestInjectSecrets(t *testing.T) {\n+\ttests := []struct {\n+\t\tname string\n\n\n**suggestion (testing):** Add tests for incomplete or conflicting database secret inputs in InjectSecrets\n\nThe current `TestInjectSecrets` table is strong, but two edge cases are missing:\n\n1. When only some of `database/user`, `database/password`, and `database/host` are set, `hasDatabaseParts` is false and `database.connection` is not produced. Please add a case that asserts this behavior so we don’t accidentally start emitting partial connection strings.\n2. When both `database/connection` and the individual parts are provided, the code appears to prefer `database/connection`. A test that captures this precedence would help prevent regressions.\n\nNew table entries for these will better document and lock in the intended behavior for borderline configurations.\n\n\n### Comment 4\n\n\n+| `config` | `map[string]map[string]string` | Current configuration to inject into |\n+| `secrets` | `map[string]string` | Resolved secret values keyed by `section/key` |\n+\n+**Returns:** `map[string]map[string]string` β€” new config with secrets injected. Empty secrets is a no-op.\n+\n+**Database connection assembly:** When individual secret parts (`database/user`, `database/password`, `database/host`, and optionally `database/port`, `database/name`) are present, they are assembled into a connection string:\n\n\n**issue (typo):** Consider changing \"Empty secrets is a no-op\" to \"Empty secrets are a no-op\".\n\nBecause \"secrets\" is plural, use \"are\" instead of \"is\" in that sentence.\n\n```suggestion\n**Returns:** `map[string]map[string]string` β€” new config with secrets injected. Empty secrets are a no-op.\n```\n\n~~~\n\n
\n\n***\n\n
\nSourcery is free for open source - if you like our reviews please consider sharing them ✨\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20got%20an%20instant%20code%20review%20from%20%40SourceryAI%2C%20and%20it%20was%20brilliant%21%20It%27s%20free%20for%20open%20source%20and%20has%20a%20free%20trial%20for%20private%20code.%20Check%20it%20out%20https%3A//sourcery.ai)\n- [Mastodon](https://mastodon.social/share?text=I%20just%20got%20an%20instant%20code%20review%20from%20%40SourceryAI%2C%20and%20it%20was%20brilliant%21%20It%27s%20free%20for%20open%20source%20and%20has%20a%20free%20trial%20for%20private%20code.%20Check%20it%20out%20https%3A//sourcery.ai)\n- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https://sourcery.ai)\n- [Facebook](https://www.facebook.com/sharer/sharer.php?u=https://sourcery.ai)\n\n
\n\n\nHelp me be more useful! Please click πŸ‘ or πŸ‘Ž on each comment and I'll use the feedback to improve your reviews.\n", + "code_comments": [ + { + "body": "**suggestion (bug_risk):** insertToken prioritizes After over Before and silently appends when no anchor is found, which might hide configuration mistakes\n\nConsider either failing when a requested `Before`/`After` anchor is not found (so configuration errors surface early) or clearly documenting that invalid anchors fall back to an append so callers understand the behavior and its risks.\n\n```suggestion\n// insertToken inserts a token into the pipeline at the position specified.\n// If both After and Before are set, After takes precedence.\n// If the requested anchor token (After/Before) is not found in the existing\n// pipeline, the token is appended to the end. This fallback behavior is\n// intentional; callers that require strict placement should validate their\n// configuration before invoking insertToken.\n```", + "path": "internal/common/plugins/plugins.go", + "line": 89, + "author": "sourcery-ai[bot]", + "created_at": "2026-03-01T22:03:34Z", + "id": null + }, + { + "body": "**issue (testing):** Add a test that mutating the condition returned by GetCondition also updates the underlying slice element\n\n`TestGetCondition` currently only checks the fields of the returned pointer, but not that it references the original slice element. Please also verify that mutating the returned condition (e.g. `c.Status = ...`) updates the original `[]metav1.Condition`, ensuring `GetCondition` returns a pointer into the slice rather than a copy, e.g.:\n\n```go\ng := NewGomegaWithT(t)\nconditions := []metav1.Condition{{Type: \"Ready\", Status: metav1.ConditionFalse}}\n\nc := GetCondition(conditions, \"Ready\")\ng.Expect(c).NotTo(BeNil())\nc.Status = metav1.ConditionTrue\n\ng.Expect(conditions[0].Status).To(Equal(metav1.ConditionTrue))\n```", + "path": "internal/common/conditions/conditions_test.go", + "line": 148, + "author": "sourcery-ai[bot]", + "created_at": "2026-03-01T22:03:34Z", + "id": null + }, + { + "body": "**suggestion (testing):** Add tests for incomplete or conflicting database secret inputs in InjectSecrets\n\nThe current `TestInjectSecrets` table is strong, but two edge cases are missing:\n\n1. When only some of `database/user`, `database/password`, and `database/host` are set, `hasDatabaseParts` is false and `database.connection` is not produced. Please add a case that asserts this behavior so we don’t accidentally start emitting partial connection strings.\n2. When both `database/connection` and the individual parts are provided, the code appears to prefer `database/connection`. A test that captures this precedence would help prevent regressions.\n\nNew table entries for these will better document and lock in the intended behavior for borderline configurations.", + "path": "internal/common/config/config_test.go", + "line": 183, + "author": "sourcery-ai[bot]", + "created_at": "2026-03-01T22:03:34Z", + "id": null + }, + { + "body": "**issue (typo):** Consider changing \"Empty secrets is a no-op\" to \"Empty secrets are a no-op\".\n\nBecause \"secrets\" is plural, use \"are\" instead of \"is\" in that sentence.\n\n```suggestion\n**Returns:** `map[string]map[string]string` β€” new config with secrets injected. Empty secrets are a no-op.\n```", + "path": "docs/reference/internal-common-packages.md", + "line": 143, + "author": "sourcery-ai[bot]", + "created_at": "2026-03-01T22:03:34Z", + "id": null + } + ], + "github_review_id": 3873449127 +} \ No newline at end of file diff --git a/docs/reference/internal-common-packages.md b/docs/reference/internal-common-packages.md new file mode 100644 index 0000000..a8e9d58 --- /dev/null +++ b/docs/reference/internal-common-packages.md @@ -0,0 +1,306 @@ +--- +title: Shared Utility Packages +quadrant: backend +--- + +# Shared Utility Packages + +**Module:** `internal/common` +**Feature:** CC-0004 + +Pure-function utility packages for OpenStack operator reconcilers. All functions operate on Go maps, slices, and structs with no Kubernetes client dependency (except `conditions/` which uses `metav1.Condition`). + +## conditions + +**Package:** `internal/common/conditions` +**Import:** `"github.com/c5c3/forge/internal/common/conditions"` + +Helper functions for managing Kubernetes status conditions on `[]metav1.Condition`. Zero K8s client dependency β€” operates on in-memory slices only. + +### SetCondition + +```go +func SetCondition(conditions *[]metav1.Condition, conditionType string, status metav1.ConditionStatus, reason, message string) +``` + +Inserts or updates a condition by type. When the status is unchanged, `LastTransitionTime` is preserved to avoid spurious reconcile triggers. + +| Parameter | Type | Description | +|-----------------|---------------------------|------------------------------------------| +| `conditions` | `*[]metav1.Condition` | Pointer to the conditions slice to modify| +| `conditionType` | `string` | Condition type (e.g. `"Ready"`, `"DatabaseReady"`) | +| `status` | `metav1.ConditionStatus` | One of `metav1.ConditionTrue`, `ConditionFalse`, `ConditionUnknown` | +| `reason` | `string` | CamelCase reason token | +| `message` | `string` | Human-readable message | + +**Behavior:** If a condition with the given type exists, its fields are updated in place. If the status changed, `LastTransitionTime` is set to the current time. If no matching condition exists, a new one is appended. A nil `conditions` pointer is a no-op. + +### GetCondition + +```go +func GetCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition +``` + +Returns a pointer to the condition with the given type, or `nil` if not found. Safe to call with nil or empty slices. + +| Parameter | Type | Description | +|-----------------|-----------------------|---------------------------------| +| `conditions` | `[]metav1.Condition` | Conditions slice to search | +| `conditionType` | `string` | Condition type to find | + +**Returns:** `*metav1.Condition` β€” pointer into the original slice, or `nil`. + +### IsReady + +```go +func IsReady(conditions []metav1.Condition) bool +``` + +Returns `true` if a condition with type `"Ready"` exists and has status `True`. Shorthand for checking the aggregate readiness gate. + +| Parameter | Type | Description | +|--------------|-----------------------|----------------------------| +| `conditions` | `[]metav1.Condition` | Conditions slice to check | + +**Returns:** `bool` β€” `true` only when `Ready` condition has status `True`. + +### AllTrue + +```go +func AllTrue(conditions []metav1.Condition, types ...string) bool +``` + +Returns `true` if every named condition type has status `True`. Returns `true` (vacuously) when no types are provided. Useful for gating the aggregate `Ready` condition on multiple sub-conditions. + +| Parameter | Type | Description | +|--------------|-----------------------|---------------------------------------| +| `conditions` | `[]metav1.Condition` | Conditions slice to check | +| `types` | `...string` | Condition types that must all be True | + +**Returns:** `bool` β€” `false` if any named type is missing or not `True`. + +--- + +## config + +**Package:** `internal/common/config` +**Import:** `"github.com/c5c3/forge/internal/common/config"` + +INI configuration rendering pipeline for OpenStack services. All functions are pure β€” they return new maps without mutating inputs. No Kubernetes package imports. + +### RenderINI + +```go +func RenderINI(config map[string]map[string]string) string +``` + +Produces a deterministic INI-format string from a nested config map. Sections and keys are sorted alphabetically to ensure stable ConfigMap content hashes. + +| Parameter | Type | Description | +|-----------|---------------------------------|------------------------------------| +| `config` | `map[string]map[string]string` | Nested map of section β†’ key β†’ value| + +**Returns:** `string` β€” valid INI text with `[section]` headers and `key = value` lines. Empty map returns `""`. + +**Example output:** + +```ini +[DEFAULT] +debug = false + +[database] +connection = mysql+pymysql://user:pass@host:3306/db +``` + +### MergeDefaults + +```go +func MergeDefaults(userConfig, defaults map[string]map[string]string) map[string]map[string]string +``` + +Merges defaults into user config with user-wins precedence. User values always override defaults; sections and keys from defaults fill gaps. + +| Parameter | Type | Description | +|--------------|---------------------------------|-----------------------------| +| `userConfig` | `map[string]map[string]string` | User-supplied configuration | +| `defaults` | `map[string]map[string]string` | Operator default values | + +**Returns:** `map[string]map[string]string` β€” new merged map. + +### InjectSecrets + +```go +func InjectSecrets(config map[string]map[string]string, secrets map[string]string) map[string]map[string]string +``` + +Assembles connection strings from resolved secret values and injects them into the config. Secret keys follow the `"section/key"` pattern. + +| Parameter | Type | Description | +|-----------|---------------------------------|------------------------------------------------| +| `config` | `map[string]map[string]string` | Current configuration to inject into | +| `secrets` | `map[string]string` | Resolved secret values keyed by `section/key` | + +**Returns:** `map[string]map[string]string` β€” new config with secrets injected. Empty secrets are a no-op. + +**Database connection assembly:** When individual secret parts (`database/user`, `database/password`, `database/host`, and optionally `database/port`, `database/name`) are present, they are assembled into a connection string: + +``` +mysql+pymysql://{user}:{password}@{host}:{port}/{database} +``` + +Special characters in the password (`@`, `:`, `/`) are URL-encoded. A pre-assembled `database/connection` value takes priority over individual parts. + +### InjectOsloPolicyConfig + +```go +func InjectOsloPolicyConfig(config map[string]map[string]string, policyFilePath string) map[string]map[string]string +``` + +Adds the `[oslo_policy]` section pointing to the policy file. No-op when `policyFilePath` is empty. + +| Parameter | Type | Description | +|------------------|---------------------------------|-------------------------------------------| +| `config` | `map[string]map[string]string` | Current configuration | +| `policyFilePath` | `string` | Path to the policy.yaml file on the pod | + +**Returns:** `map[string]map[string]string` β€” new config with `oslo_policy.policy_file` set (or unchanged if path is empty). + +--- + +## plugins + +**Package:** `internal/common/plugins` +**Import:** `"github.com/c5c3/forge/internal/common/plugins"` + +PasteDeploy pipeline configuration generation for OpenStack api-paste.ini files. Depends on `internal/common/types`. + +### RenderPastePipeline + +```go +func RenderPastePipeline(spec types.PipelineSpec) string +``` + +Generates a PasteDeploy api-paste.ini configuration from a `PipelineSpec`. Renders the `[pipeline:name]` section with middleware inserted at their specified positions, followed by sorted `[filter:name]` blocks. + +| Parameter | Type | Description | +|-----------|----------------------|--------------------------------| +| `spec` | `types.PipelineSpec` | Pipeline specification | + +**Returns:** `string` β€” valid api-paste.ini text. + +**Middleware insertion:** Each middleware's `Position` determines where its name is inserted in the base pipeline string: + +- `Position.After = "tokenX"` β€” inserted immediately after `tokenX` +- `Position.Before = "tokenY"` β€” inserted immediately before `tokenY` +- Both empty β€” appended to the end of the pipeline + +**Example output:** + +```ini +[pipeline:keystone] +pipeline = cors request_id token_auth admin_service + +[filter:cors] +paste.filter_factory = oslo_middleware.cors:filter_factory + +[filter:request_id] +paste.filter_factory = oslo_middleware:RequestId + +[filter:token_auth] +paste.filter_factory = keystonemiddleware.auth_token:filter_factory +auth_type = password +``` + +### RenderPluginConfig + +```go +func RenderPluginConfig(plugins []types.PluginSpec) map[string]map[string]string +``` + +Converts a slice of `PluginSpec` into a config map suitable for merging with the main INI configuration via `config.MergeDefaults`. + +| Parameter | Type | Description | +|-----------|---------------------|------------------------------------| +| `plugins` | `[]types.PluginSpec`| Plugin specifications to convert | + +**Returns:** `map[string]map[string]string` β€” one section per plugin, keyed by the plugin's `Section` field. Empty slice returns an empty map. + +--- + +## policy + +**Package:** `internal/common/policy` +**Import:** `"github.com/c5c3/forge/internal/common/policy"` + +oslo.policy rule merging, validation, and YAML rendering. Depends on `gopkg.in/yaml.v3` and `k8s.io/apimachinery/pkg/util/validation/field`. + +### MergePolicies + +```go +func MergePolicies(external, inline map[string]string) map[string]string +``` + +Merges inline policy rules over external rules with inline-wins precedence. Returns a new map without mutating inputs. + +| Parameter | Type | Description | +|------------|---------------------|--------------------------------------| +| `external` | `map[string]string` | Rules from external ConfigMap source | +| `inline` | `map[string]string` | Inline rule overrides from CRD spec | + +**Returns:** `map[string]string` β€” merged rules. Nil inputs are treated as empty. + +### ValidatePolicyRules + +```go +func ValidatePolicyRules(rules map[string]string, fldPath *field.Path) field.ErrorList +``` + +Validates policy rules for webhook compatibility. Returns `field.ErrorList` (Decision #3) with errors for empty keys and empty values. Error ordering is deterministic (sorted by key). + +| Parameter | Type | Description | +|-----------|---------------|-----------------------------------------------| +| `rules` | `map[string]string` | Policy rules to validate | +| `fldPath` | `*field.Path` | Field path for error reporting in webhooks | + +**Returns:** `field.ErrorList` β€” empty list for valid input; one error per violation. + +**Validation rules:** + +- Empty key β†’ `field.Invalid` error +- Empty value β†’ `field.Invalid` error + +### RenderPolicyYAML + +```go +func RenderPolicyYAML(rules map[string]string) (string, error) +``` + +Renders policy rules as a YAML document suitable for oslo.policy. Output is deterministic (keys sorted alphabetically). + +| Parameter | Type | Description | +|-----------|---------------------|--------------------------| +| `rules` | `map[string]string` | Policy rules to render | + +**Returns:** `(string, error)` β€” YAML text and any marshaling error. Empty map produces `"{}\n"`. + +--- + +## Config Pipeline Usage + +The typical reconciler config pipeline chains these functions: + +``` +CRD Spec Fields ──→ MergeDefaults ──→ InjectSecrets ──→ InjectOsloPolicyConfig ──→ RenderINI ──→ ConfigMap + ↑ ↑ ↑ + Operator Defaults Resolved Secrets Policy File Path + +CRD Spec Fields ──→ RenderPluginConfig ──→ MergeDefaults (merged into main config) +CRD Spec Fields ──→ RenderPastePipeline ──→ api-paste.ini ConfigMap +CRD Spec Fields ──→ MergePolicies ──→ ValidatePolicyRules ──→ RenderPolicyYAML ──→ policy.yaml ConfigMap +``` + +## Dependencies + +- **CC-0004** β€” implements all packages +- **CC-0005** β€” will add K8s-client-dependent functions (`CreateImmutableConfigMap`, `LoadPolicyFromConfigMap`) +- **CC-0011** β€” will consume types in CRD definitions with kubebuilder markers diff --git a/docs/reference/internal-common-types.md b/docs/reference/internal-common-types.md new file mode 100644 index 0000000..1fc4b59 --- /dev/null +++ b/docs/reference/internal-common-types.md @@ -0,0 +1,132 @@ +--- +title: Shared Types +quadrant: backend +--- + +# Shared Types + +**Package:** `internal/common/types` +**Feature:** CC-0004 + +Shared Go struct types for embedding in operator CRD specs. These types carry no kubebuilder markers (Decision #4) β€” markers belong on CRD embedding sites. The only external dependency is `k8s.io/api/core/v1` for `LocalObjectReference`. + +## ImageSpec + +Container image reference. + +| Field | Type | JSON Tag | Required | Description | +|--------------|----------|--------------|----------|--------------------------------------| +| `Repository` | `string` | `repository` | Yes | Container image repository URL | +| `Tag` | `string` | `tag` | Yes | Container image tag or digest | + +## SecretRefSpec + +References a Kubernetes Secret by name and key. Embedded as a value type in `DatabaseSpec`, `MessagingSpec`, and other specs that need secret references. + +| Field | Type | JSON Tag | Required | Description | +|--------|----------|----------|----------|---------------------------------| +| `Name` | `string` | `name` | Yes | Name of the Kubernetes Secret | +| `Key` | `string` | `key` | Yes | Key within the Secret's data | + +## DatabaseSpec + +Database connection parameters. Supports two mutually exclusive modes: + +- **Managed mode:** Set `ClusterRef` to reference a managed MariaDB cluster. +- **Brownfield mode:** Set `Host` and `Port` to connect to an external database. + +| Field | Type | JSON Tag | Required | Description | +|--------------|-----------------------------------|-----------------------|----------|------------------------------------------------------| +| `ClusterRef` | `*corev1.LocalObjectReference` | `clusterRef,omitempty`| No | Reference to a managed MariaDB cluster | +| `Host` | `string` | `host,omitempty` | No | Hostname for brownfield database | +| `Port` | `int32` | `port,omitempty` | No | Port for brownfield database | +| `Database` | `string` | `database` | Yes | Database name | +| `SecretRef` | `SecretRefSpec` | `secretRef` | Yes | Secret containing database credentials | + +## MessagingSpec + +RabbitMQ messaging parameters. + +| Field | Type | JSON Tag | Required | Description | +|-------------|-----------------|-------------------|----------|---------------------------------------| +| `Host` | `string` | `host` | Yes | RabbitMQ hostname | +| `Port` | `int32` | `port,omitempty` | No | RabbitMQ port (defaults to 5672) | +| `SecretRef` | `SecretRefSpec` | `secretRef` | Yes | Secret containing messaging credentials| +| `Vhost` | `string` | `vhost,omitempty` | No | RabbitMQ virtual host | + +## CacheSpec + +Memcached cache parameters. Supports two mutually exclusive modes: + +- **Managed mode:** Set `ClusterRef` to reference a managed Memcached cluster. +- **Brownfield mode:** Set `Servers` to provide explicit server addresses. + +| Field | Type | JSON Tag | Required | Description | +|--------------|--------------------------------|------------------------|----------|-------------------------------------------| +| `ClusterRef` | `*corev1.LocalObjectReference` | `clusterRef,omitempty` | No | Reference to a managed Memcached cluster | +| `Servers` | `[]string` | `servers,omitempty` | No | Explicit server addresses (host:port) | + +## PolicySpec + +Policy override sources for oslo.policy. + +| Field | Type | JSON Tag | Required | Description | +|----------------|--------------------------------|---------------------------|----------|------------------------------------------| +| `ConfigMapRef` | `*corev1.LocalObjectReference` | `configMapRef,omitempty` | No | ConfigMap containing external policy rules| +| `Inline` | `map[string]string` | `inline,omitempty` | No | Inline policy rule overrides | + +## PluginSpec + +Plugin configuration section for merging into the main INI config. + +| Field | Type | JSON Tag | Required | Description | +|-----------|---------------------|-------------------|----------|----------------------------------------------| +| `Name` | `string` | `name` | Yes | Plugin name | +| `Section` | `string` | `section` | Yes | INI section name for this plugin's config | +| `Config` | `map[string]string` | `config,omitempty`| No | Key-value pairs for the plugin section | + +## PipelinePosition + +Specifies where middleware is inserted in the PasteDeploy pipeline. At most one of `Before` or `After` should be set. If both are empty, the middleware is appended to the end. + +| Field | Type | JSON Tag | Required | Description | +|----------|----------|-------------------|----------|------------------------------------------------| +| `Before` | `string` | `before,omitempty`| No | Insert before this pipeline token | +| `After` | `string` | `after,omitempty` | No | Insert after this pipeline token | + +## MiddlewareSpec + +PasteDeploy middleware/filter to insert into the pipeline. + +| Field | Type | JSON Tag | Required | Description | +|-----------------|---------------------|-------------------|----------|--------------------------------------------| +| `Name` | `string` | `name` | Yes | Middleware name (used in pipeline and filter section) | +| `FilterFactory` | `string` | `filterFactory` | Yes | PasteDeploy filter factory entry point | +| `Config` | `map[string]string` | `config,omitempty`| No | Additional filter configuration keys | +| `Position` | `PipelinePosition` | `position` | Yes | Where to insert in the pipeline | + +## FilterSpec + +PasteDeploy filter entry for the base pipeline. + +| Field | Type | JSON Tag | Required | Description | +|-----------|---------------------|-------------------|----------|--------------------------------------------| +| `Name` | `string` | `name` | Yes | Filter name (used in `[filter:name]` section)| +| `Factory` | `string` | `factory` | Yes | PasteDeploy filter factory entry point | +| `Config` | `map[string]string` | `config,omitempty`| No | Additional filter configuration keys | + +## PipelineSpec + +Complete PasteDeploy pipeline configuration. Aggregates a base pipeline string with filters and middleware. + +| Field | Type | JSON Tag | Required | Description | +|----------------|--------------------|------------------------|----------|---------------------------------------------| +| `Name` | `string` | `name` | Yes | Pipeline name (used in `[pipeline:name]`) | +| `BasePipeline` | `string` | `basePipeline` | Yes | Space-separated base pipeline token list | +| `Filters` | `[]FilterSpec` | `filters,omitempty` | No | Base pipeline filter definitions | +| `Middleware` | `[]MiddlewareSpec` | `middleware,omitempty` | No | Middleware to insert into the pipeline | + +## Dependencies + +- **CC-0004** β€” implements all 11 types +- **CC-0011** β€” will add kubebuilder markers when embedding in CRD specs diff --git a/internal/common/conditions/conditions.go b/internal/common/conditions/conditions.go new file mode 100644 index 0000000..62f7995 --- /dev/null +++ b/internal/common/conditions/conditions.go @@ -0,0 +1,63 @@ +package conditions + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// SetCondition inserts or updates a condition by type. When the status is unchanged, +// LastTransitionTime is preserved. (CC-0004, REQ-003) +func SetCondition(conditions *[]metav1.Condition, conditionType string, status metav1.ConditionStatus, reason, message string) { + if conditions == nil { + return + } + + now := metav1.Now() + + for i := range *conditions { + if (*conditions)[i].Type == conditionType { + if (*conditions)[i].Status != status { + (*conditions)[i].LastTransitionTime = now + } + (*conditions)[i].Status = status + (*conditions)[i].Reason = reason + (*conditions)[i].Message = message + return + } + } + + *conditions = append(*conditions, metav1.Condition{ + Type: conditionType, + Status: status, + Reason: reason, + Message: message, + LastTransitionTime: now, + }) +} + +// GetCondition returns the condition with the given type, or nil if not found. +func GetCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition { + for i := range conditions { + if conditions[i].Type == conditionType { + return &conditions[i] + } + } + return nil +} + +// IsReady returns true if a "Ready" condition exists with status True. +func IsReady(conditions []metav1.Condition) bool { + c := GetCondition(conditions, "Ready") + return c != nil && c.Status == metav1.ConditionTrue +} + +// AllTrue returns true if every named condition type has status True. +// Returns true (vacuously) when no types are provided. +func AllTrue(conditions []metav1.Condition, types ...string) bool { + for _, t := range types { + c := GetCondition(conditions, t) + if c == nil || c.Status != metav1.ConditionTrue { + return false + } + } + return true +} diff --git a/internal/common/conditions/conditions_test.go b/internal/common/conditions/conditions_test.go new file mode 100644 index 0000000..93e6e97 --- /dev/null +++ b/internal/common/conditions/conditions_test.go @@ -0,0 +1,326 @@ +package conditions + +import ( + "testing" + "time" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestSetCondition(t *testing.T) { + tests := []struct { + name string + initial []metav1.Condition + condType string + status metav1.ConditionStatus + reason string + message string + expectLen int + expectStatus metav1.ConditionStatus + expectTimeChanged bool + otherTypeUnchanged string + }{ + { + name: "insert into empty slice", + initial: []metav1.Condition{}, + condType: "Ready", + status: metav1.ConditionTrue, + reason: "AllGood", + message: "everything is fine", + expectLen: 1, + expectStatus: metav1.ConditionTrue, + expectTimeChanged: true, + }, + { + name: "update with new status", + initial: []metav1.Condition{ + { + Type: "Ready", + Status: metav1.ConditionFalse, + Reason: "NotReady", + Message: "not ready yet", + LastTransitionTime: metav1.NewTime(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)), + }, + }, + condType: "Ready", + status: metav1.ConditionTrue, + reason: "NowReady", + message: "now ready", + expectLen: 1, + expectStatus: metav1.ConditionTrue, + expectTimeChanged: true, + }, + { + name: "no-op when status unchanged preserves LastTransitionTime", + initial: []metav1.Condition{ + { + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "AllGood", + Message: "fine", + LastTransitionTime: metav1.NewTime(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)), + }, + }, + condType: "Ready", + status: metav1.ConditionTrue, + reason: "StillGood", + message: "still fine", + expectLen: 1, + expectStatus: metav1.ConditionTrue, + expectTimeChanged: false, + }, + { + name: "multiple conditions, only target type updated", + initial: []metav1.Condition{ + { + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "OK", + Message: "ok", + LastTransitionTime: metav1.NewTime(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)), + }, + { + Type: "Available", + Status: metav1.ConditionFalse, + Reason: "NotAvailable", + Message: "not available", + LastTransitionTime: metav1.NewTime(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)), + }, + }, + condType: "Available", + status: metav1.ConditionTrue, + reason: "NowAvailable", + message: "available now", + expectLen: 2, + expectStatus: metav1.ConditionTrue, + expectTimeChanged: true, + otherTypeUnchanged: "Ready", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + conditions := make([]metav1.Condition, len(tc.initial)) + copy(conditions, tc.initial) + + var originalTime metav1.Time + for _, c := range conditions { + if c.Type == tc.condType { + originalTime = c.LastTransitionTime + } + } + + var otherOriginalTime metav1.Time + if tc.otherTypeUnchanged != "" { + for _, c := range conditions { + if c.Type == tc.otherTypeUnchanged { + otherOriginalTime = c.LastTransitionTime + } + } + } + + SetCondition(&conditions, tc.condType, tc.status, tc.reason, tc.message) + + g.Expect(conditions).To(HaveLen(tc.expectLen)) + + found := GetCondition(conditions, tc.condType) + g.Expect(found).NotTo(BeNil()) + g.Expect(found.Status).To(Equal(tc.expectStatus)) + g.Expect(found.Reason).To(Equal(tc.reason)) + g.Expect(found.Message).To(Equal(tc.message)) + + if tc.expectTimeChanged { + g.Expect(found.LastTransitionTime.Time).NotTo(Equal(originalTime.Time)) + } else { + g.Expect(found.LastTransitionTime.Time).To(Equal(originalTime.Time)) + } + + if tc.otherTypeUnchanged != "" { + other := GetCondition(conditions, tc.otherTypeUnchanged) + g.Expect(other).NotTo(BeNil()) + g.Expect(other.LastTransitionTime.Time).To(Equal(otherOriginalTime.Time)) + } + }) + } +} + +func TestSetCondition_NilPointer(t *testing.T) { + g := NewGomegaWithT(t) + + // Should not panic on nil pointer. + g.Expect(func() { + SetCondition(nil, "Ready", metav1.ConditionTrue, "OK", "ok") + }).NotTo(Panic()) +} + +func TestGetCondition(t *testing.T) { + tests := []struct { + name string + conditions []metav1.Condition + condType string + expectNil bool + }{ + { + name: "returns matching condition", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue}, + {Type: "Available", Status: metav1.ConditionFalse}, + }, + condType: "Ready", + expectNil: false, + }, + { + name: "returns nil when not found", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue}, + }, + condType: "Missing", + expectNil: true, + }, + { + name: "returns nil on empty slice", + conditions: []metav1.Condition{}, + condType: "Ready", + expectNil: true, + }, + { + name: "returns nil on nil slice", + conditions: nil, + condType: "Ready", + expectNil: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + result := GetCondition(tc.conditions, tc.condType) + if tc.expectNil { + g.Expect(result).To(BeNil()) + } else { + g.Expect(result).NotTo(BeNil()) + g.Expect(result.Type).To(Equal(tc.condType)) + } + }) + } +} + +func TestGetCondition_MutationUpdatesOriginalSlice(t *testing.T) { + g := NewGomegaWithT(t) + conditions := []metav1.Condition{{Type: "Ready", Status: metav1.ConditionFalse}} + + c := GetCondition(conditions, "Ready") + g.Expect(c).NotTo(BeNil()) + c.Status = metav1.ConditionTrue + + g.Expect(conditions[0].Status).To(Equal(metav1.ConditionTrue)) +} + +func TestIsReady(t *testing.T) { + tests := []struct { + name string + conditions []metav1.Condition + expected bool + }{ + { + name: "true when Ready=True", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue}, + }, + expected: true, + }, + { + name: "false when Ready=False", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionFalse}, + }, + expected: false, + }, + { + name: "false when no Ready condition", + conditions: []metav1.Condition{ + {Type: "Available", Status: metav1.ConditionTrue}, + }, + expected: false, + }, + { + name: "false on nil slice", + conditions: nil, + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + g.Expect(IsReady(tc.conditions)).To(Equal(tc.expected)) + }) + } +} + +func TestAllTrue(t *testing.T) { + tests := []struct { + name string + conditions []metav1.Condition + types []string + expected bool + }{ + { + name: "all requested types True", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue}, + {Type: "Available", Status: metav1.ConditionTrue}, + }, + types: []string{"Ready", "Available"}, + expected: true, + }, + { + name: "one type False", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue}, + {Type: "Available", Status: metav1.ConditionFalse}, + }, + types: []string{"Ready", "Available"}, + expected: false, + }, + { + name: "missing type", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue}, + }, + types: []string{"Ready", "Missing"}, + expected: false, + }, + { + name: "no types requested (vacuous truth)", + conditions: []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionFalse}, + }, + types: []string{}, + expected: true, + }, + { + name: "empty conditions slice with types requested", + conditions: []metav1.Condition{}, + types: []string{"Ready"}, + expected: false, + }, + { + name: "nil conditions with no types (vacuous truth)", + conditions: nil, + types: []string{}, + expected: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + g.Expect(AllTrue(tc.conditions, tc.types...)).To(Equal(tc.expected)) + }) + } +} diff --git a/internal/common/conditions/doc.go b/internal/common/conditions/doc.go new file mode 100644 index 0000000..f646704 --- /dev/null +++ b/internal/common/conditions/doc.go @@ -0,0 +1,2 @@ +// Package conditions provides helper functions for managing Kubernetes status conditions. +package conditions diff --git a/internal/common/config/config.go b/internal/common/config/config.go new file mode 100644 index 0000000..e12ca96 --- /dev/null +++ b/internal/common/config/config.go @@ -0,0 +1,197 @@ +package config + +import ( + "fmt" + "net/url" + "sort" + "strings" +) + +// RenderINI produces a deterministic INI-format string from a nested config map. +// Sections and keys are sorted alphabetically to ensure stable ConfigMap content hashes. (CC-0004, REQ-005) +func RenderINI(config map[string]map[string]string) string { + if len(config) == 0 { + return "" + } + + sections := make([]string, 0, len(config)) + for section := range config { + sections = append(sections, section) + } + sort.Strings(sections) + + var b strings.Builder + for i, section := range sections { + if i > 0 { + b.WriteString("\n") + } + fmt.Fprintf(&b, "[%s]\n", section) + + keys := make([]string, 0, len(config[section])) + for key := range config[section] { + keys = append(keys, key) + } + sort.Strings(keys) + + for _, key := range keys { + fmt.Fprintf(&b, "%s = %s\n", key, config[section][key]) + } + } + + return strings.TrimRight(b.String(), "\n") +} + +// MergeDefaults merges defaults into userConfig with user-wins precedence. +// User values always override defaults. Sections and keys from defaults fill gaps. (CC-0004, REQ-006) +func MergeDefaults(userConfig, defaults map[string]map[string]string) map[string]map[string]string { + result := make(map[string]map[string]string) + + // Copy all defaults first. + for section, kvs := range defaults { + result[section] = make(map[string]string, len(kvs)) + for k, v := range kvs { + result[section][k] = v + } + } + + // Overlay user config (user wins). + for section, kvs := range userConfig { + if _, ok := result[section]; !ok { + result[section] = make(map[string]string, len(kvs)) + } + for k, v := range kvs { + result[section][k] = v + } + } + + return result +} + +// InjectSecrets assembles connection strings from resolved secret values and injects +// them into the config. The secrets map keys follow the pattern "section/key" and values +// are the resolved secret values. (CC-0004, REQ-007) +// +// Supported connection patterns: +// - database/connection: mysql+pymysql://{user}:{password}@{host}:{port}/{database} +// +// Special characters in password are URL-encoded. +func InjectSecrets(config map[string]map[string]string, secrets map[string]string) map[string]map[string]string { + result := copyConfig(config) + + if len(secrets) == 0 { + return result + } + + // Check if a pre-assembled database/connection string is provided. + if connStr, ok := secrets["database/connection"]; ok { + if result["database"] == nil { + result["database"] = make(map[string]string) + } + result["database"]["connection"] = connStr + } else if hasDatabaseParts(secrets) { + // Assemble from individual parts. + if result["database"] == nil { + result["database"] = make(map[string]string) + } + result["database"]["connection"] = assembleDatabaseConnection(secrets) + } + + // Inject all other "section/key" entries. + for secretKey, secretVal := range secrets { + if isDatabasePart(secretKey) { + continue + } + parts := strings.SplitN(secretKey, "/", 2) + if len(parts) != 2 { + continue + } + section, key := parts[0], parts[1] + if result[section] == nil { + result[section] = make(map[string]string) + } + result[section][key] = secretVal + } + + return result +} + +// InjectOsloPolicyConfig merges the policy_file key into the [oslo_policy] section, +// preserving any existing keys (e.g. enforce_scope, enforce_new_defaults). +// Does nothing if policyFilePath is empty. (CC-0004, REQ-008) +func InjectOsloPolicyConfig(config map[string]map[string]string, policyFilePath string) map[string]map[string]string { + result := copyConfig(config) + + if policyFilePath == "" { + return result + } + + if result["oslo_policy"] == nil { + result["oslo_policy"] = make(map[string]string) + } + result["oslo_policy"]["policy_file"] = policyFilePath + + return result +} + +// copyConfig returns a deep copy of the config map. +func copyConfig(config map[string]map[string]string) map[string]map[string]string { + result := make(map[string]map[string]string) + for section, kvs := range config { + result[section] = make(map[string]string, len(kvs)) + for k, v := range kvs { + result[section][k] = v + } + } + return result +} + +// defaultMySQLPort is the default port for MySQL/MariaDB connections. +const defaultMySQLPort = "3306" + +var databasePartKeys = map[string]bool{ + "database/user": true, + "database/password": true, + "database/host": true, + "database/port": true, + "database/name": true, +} + +func isDatabasePart(key string) bool { + return databasePartKeys[key] || key == "database/connection" +} + +func hasDatabaseParts(secrets map[string]string) bool { + _, hasUser := secrets["database/user"] + _, hasPassword := secrets["database/password"] + _, hasHost := secrets["database/host"] + return hasUser && hasPassword && hasHost +} + +func assembleDatabaseConnection(secrets map[string]string) string { + user := secrets["database/user"] + password := escapePassword(secrets["database/password"]) + host := secrets["database/host"] + port := secrets["database/port"] + name := secrets["database/name"] + + if port == "" { + port = defaultMySQLPort + } + if name == "" { + name = user + } + + return fmt.Sprintf("mysql+pymysql://%s:%s@%s:%s/%s", user, password, host, port, name) +} + +// escapePassword percent-encodes special characters in a password for safe +// embedding in a connection URI userinfo component per RFC 3986. +// Uses url.UserPassword to correctly handle all reserved characters +// including @, :, /, +, $, ;, and =. +func escapePassword(password string) string { + // url.UserPassword("x", password).String() produces "x:encoded_password". + // We extract just the encoded password portion after the first colon. + userinfo := url.UserPassword("x", password).String() + _, encoded, _ := strings.Cut(userinfo, ":") + return encoded +} diff --git a/internal/common/config/config_test.go b/internal/common/config/config_test.go new file mode 100644 index 0000000..6b45b04 --- /dev/null +++ b/internal/common/config/config_test.go @@ -0,0 +1,467 @@ +package config + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestRenderINI(t *testing.T) { + tests := []struct { + name string + config map[string]map[string]string + expected string + }{ + { + name: "single section single key", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + expected: "[DEFAULT]\ndebug = true", + }, + { + name: "multiple sections sorted alphabetically", + config: map[string]map[string]string{ + "zeta": {"key": "z"}, + "alpha": {"key": "a"}, + "middle": {"key": "m"}, + }, + expected: "[alpha]\nkey = a\n\n[middle]\nkey = m\n\n[zeta]\nkey = z", + }, + { + name: "multiple keys sorted alphabetically", + config: map[string]map[string]string{ + "section": { + "zebra": "z", + "apple": "a", + "mango": "m", + }, + }, + expected: "[section]\napple = a\nmango = m\nzebra = z", + }, + { + name: "empty map returns empty string", + config: map[string]map[string]string{}, + expected: "", + }, + { + name: "nil map returns empty string", + config: nil, + expected: "", + }, + { + name: "idempotent output", + config: map[string]map[string]string{ + "b": {"y": "2", "x": "1"}, + "a": {"z": "3"}, + }, + expected: "[a]\nz = 3\n\n[b]\nx = 1\ny = 2", + }, + { + name: "special characters in values preserved", + config: map[string]map[string]string{ + "database": {"connection": "mysql+pymysql://user:p%40ss@host:3306/db"}, + }, + expected: "[database]\nconnection = mysql+pymysql://user:p%40ss@host:3306/db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := RenderINI(tt.config) + g.Expect(result).To(Equal(tt.expected)) + }) + } + + // Verify idempotency by calling twice with the same input. + t.Run("idempotent across calls", func(t *testing.T) { + g := NewGomegaWithT(t) + config := map[string]map[string]string{ + "b": {"y": "2", "x": "1"}, + "a": {"z": "3"}, + } + g.Expect(RenderINI(config)).To(Equal(RenderINI(config))) + }) +} + +func TestMergeDefaults(t *testing.T) { + tests := []struct { + name string + userConfig map[string]map[string]string + defaults map[string]map[string]string + expected map[string]map[string]string + }{ + { + name: "user overrides default same key", + userConfig: map[string]map[string]string{ + "DEFAULT": {"debug": "false"}, + }, + defaults: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "false"}, + }, + }, + { + name: "default fills missing key", + userConfig: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + defaults: map[string]map[string]string{ + "DEFAULT": {"log_level": "INFO"}, + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true", "log_level": "INFO"}, + }, + }, + { + name: "user-only section preserved", + userConfig: map[string]map[string]string{ + "custom": {"key": "val"}, + }, + defaults: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + "custom": {"key": "val"}, + }, + }, + { + name: "default-only section added", + userConfig: map[string]map[string]string{}, + defaults: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + }, + { + name: "both empty returns empty map", + userConfig: map[string]map[string]string{}, + defaults: map[string]map[string]string{}, + expected: map[string]map[string]string{}, + }, + { + name: "nil inputs return empty map", + userConfig: nil, + defaults: nil, + expected: map[string]map[string]string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := MergeDefaults(tt.userConfig, tt.defaults) + g.Expect(result).To(Equal(tt.expected)) + }) + } + + // Verify inputs are not mutated. + t.Run("does not mutate original inputs", func(t *testing.T) { + g := NewGomegaWithT(t) + + userConfig := map[string]map[string]string{ + "DEFAULT": {"debug": "false"}, + } + defaults := map[string]map[string]string{ + "DEFAULT": {"log_level": "INFO"}, + } + + result := MergeDefaults(userConfig, defaults) + + // Result has merged data. + g.Expect(result["DEFAULT"]).To(HaveLen(2)) + + // Originals are unchanged. + g.Expect(userConfig["DEFAULT"]).To(HaveLen(1)) + g.Expect(userConfig["DEFAULT"]["debug"]).To(Equal("false")) + g.Expect(defaults["DEFAULT"]).To(HaveLen(1)) + g.Expect(defaults["DEFAULT"]["log_level"]).To(Equal("INFO")) + }) +} + +func TestInjectSecrets(t *testing.T) { + tests := []struct { + name string + config map[string]map[string]string + secrets map[string]string + expected map[string]map[string]string + }{ + { + name: "pre-assembled connection string injected as-is", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + secrets: map[string]string{ + "database/connection": "mysql+pymysql://admin:secret@db.host:3306/mydb", + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + "database": {"connection": "mysql+pymysql://admin:secret@db.host:3306/mydb"}, + }, + }, + { + name: "individual parts assembled into connection string", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + secrets: map[string]string{ + "database/user": "admin", + "database/password": "secret", + "database/host": "db.host", + "database/port": "3306", + "database/name": "mydb", + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + "database": {"connection": "mysql+pymysql://admin:secret@db.host:3306/mydb"}, + }, + }, + { + name: "special characters in password are URL-encoded per RFC 3986 userinfo", + config: map[string]map[string]string{}, + secrets: map[string]string{ + "database/user": "admin", + "database/password": "p@ss:w/rd", + "database/host": "db.host", + "database/port": "3306", + "database/name": "mydb", + }, + expected: map[string]map[string]string{ + "database": {"connection": "mysql+pymysql://admin:p%40ss%3Aw%2Frd@db.host:3306/mydb"}, + }, + }, + { + name: "empty secrets returns config unchanged", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + secrets: map[string]string{}, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + }, + { + name: "nil config works without panic", + config: nil, + secrets: map[string]string{"database/connection": "mysql+pymysql://u:p@h:3306/d"}, + expected: map[string]map[string]string{"database": {"connection": "mysql+pymysql://u:p@h:3306/d"}}, + }, + { + name: "generic section/key secrets injected into config", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + secrets: map[string]string{ + "keystone_authtoken/password": "auth-secret", + "cache/backend": "dogpile.cache.memcached", + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + "keystone_authtoken": {"password": "auth-secret"}, + "cache": {"backend": "dogpile.cache.memcached"}, + }, + }, + { + name: "generic section/key merges into existing section", + config: map[string]map[string]string{ + "cache": {"enabled": "true"}, + }, + secrets: map[string]string{ + "cache/backend": "dogpile.cache.memcached", + }, + expected: map[string]map[string]string{ + "cache": {"enabled": "true", "backend": "dogpile.cache.memcached"}, + }, + }, + { + name: "malformed secret key without slash is skipped", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + secrets: map[string]string{ + "noSlashKey": "ignored", + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + }, + { + name: "generic secrets alongside database parts", + config: map[string]map[string]string{}, + secrets: map[string]string{ + "database/user": "admin", + "database/password": "secret", + "database/host": "db.host", + "database/port": "3306", + "database/name": "mydb", + "keystone_authtoken/password": "auth-secret", + }, + expected: map[string]map[string]string{ + "database": {"connection": "mysql+pymysql://admin:secret@db.host:3306/mydb"}, + "keystone_authtoken": {"password": "auth-secret"}, + }, + }, + { + name: "default port 3306 when database/port omitted", + config: map[string]map[string]string{}, + secrets: map[string]string{ + "database/user": "admin", + "database/password": "secret", + "database/host": "db.host", + "database/name": "mydb", + }, + expected: map[string]map[string]string{ + "database": {"connection": "mysql+pymysql://admin:secret@db.host:3306/mydb"}, + }, + }, + { + name: "default name equals user when database/name omitted", + config: map[string]map[string]string{}, + secrets: map[string]string{ + "database/user": "admin", + "database/password": "secret", + "database/host": "db.host", + "database/port": "3306", + }, + expected: map[string]map[string]string{ + "database": {"connection": "mysql+pymysql://admin:secret@db.host:3306/admin"}, + }, + }, + { + name: "incomplete database parts do not produce connection string", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + secrets: map[string]string{ + "database/user": "admin", + "database/host": "db.host", + // Missing database/password β€” hasDatabaseParts requires user, password, and host. + }, + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + }, + { + name: "pre-assembled connection takes precedence over individual parts", + config: map[string]map[string]string{}, + secrets: map[string]string{ + "database/connection": "mysql+pymysql://override:conn@other:3306/db", + "database/user": "admin", + "database/password": "secret", + "database/host": "db.host", + }, + expected: map[string]map[string]string{ + "database": {"connection": "mysql+pymysql://override:conn@other:3306/db"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := InjectSecrets(tt.config, tt.secrets) + g.Expect(result).To(Equal(tt.expected)) + }) + } + + // Verify inputs are not mutated. + t.Run("does not mutate original config", func(t *testing.T) { + g := NewGomegaWithT(t) + + config := map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + } + secrets := map[string]string{ + "database/connection": "mysql+pymysql://u:p@h:3306/d", + } + + result := InjectSecrets(config, secrets) + g.Expect(result).To(HaveKey("database")) + g.Expect(config).NotTo(HaveKey("database")) + }) +} + +func TestInjectOsloPolicyConfig(t *testing.T) { + tests := []struct { + name string + config map[string]map[string]string + policyFilePath string + expected map[string]map[string]string + }{ + { + name: "non-empty path adds oslo_policy section", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + policyFilePath: "/etc/nova/policy.yaml", + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + "oslo_policy": {"policy_file": "/etc/nova/policy.yaml"}, + }, + }, + { + name: "empty path returns config unchanged", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + policyFilePath: "", + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + }, + }, + { + name: "nil config works without panic", + config: nil, + policyFilePath: "/etc/nova/policy.yaml", + expected: map[string]map[string]string{ + "oslo_policy": {"policy_file": "/etc/nova/policy.yaml"}, + }, + }, + { + name: "merges into existing oslo_policy section preserving caller keys", + config: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + "oslo_policy": { + "enforce_scope": "true", + "enforce_new_defaults": "true", + }, + }, + policyFilePath: "/etc/nova/policy.yaml", + expected: map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + "oslo_policy": { + "policy_file": "/etc/nova/policy.yaml", + "enforce_scope": "true", + "enforce_new_defaults": "true", + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := InjectOsloPolicyConfig(tt.config, tt.policyFilePath) + g.Expect(result).To(Equal(tt.expected)) + }) + } + + // Verify inputs are not mutated. + t.Run("does not mutate original config", func(t *testing.T) { + g := NewGomegaWithT(t) + + config := map[string]map[string]string{ + "DEFAULT": {"debug": "true"}, + } + + result := InjectOsloPolicyConfig(config, "/etc/nova/policy.yaml") + g.Expect(result).To(HaveKey("oslo_policy")) + g.Expect(config).NotTo(HaveKey("oslo_policy")) + }) +} diff --git a/internal/common/config/doc.go b/internal/common/config/doc.go new file mode 100644 index 0000000..501339f --- /dev/null +++ b/internal/common/config/doc.go @@ -0,0 +1,3 @@ +// Package config provides pure functions for building and merging OpenStack INI +// configuration files. (CC-0004) +package config diff --git a/internal/common/plugins/doc.go b/internal/common/plugins/doc.go new file mode 100644 index 0000000..e678ca7 --- /dev/null +++ b/internal/common/plugins/doc.go @@ -0,0 +1,3 @@ +// Package plugins provides pure functions for generating OpenStack PasteDeploy +// pipeline configurations and plugin INI sections. (CC-0004) +package plugins diff --git a/internal/common/plugins/plugins.go b/internal/common/plugins/plugins.go new file mode 100644 index 0000000..c1a613a --- /dev/null +++ b/internal/common/plugins/plugins.go @@ -0,0 +1,142 @@ +package plugins + +import ( + "fmt" + "sort" + "strings" + + "github.com/c5c3/forge/internal/common/types" +) + +// RenderPastePipeline generates a PasteDeploy api-paste.ini configuration from a PipelineSpec. +// It renders the [pipeline:name] section with middleware inserted at specified positions, +// followed by [filter:name] blocks for each middleware and filter. (CC-0004, REQ-009) +func RenderPastePipeline(spec types.PipelineSpec) string { + var sections []string + + // Build the pipeline string by inserting middleware into the base pipeline. + pipeline := insertMiddleware(spec.BasePipeline, spec.Middleware) + + // Render the pipeline section. + sections = append(sections, fmt.Sprintf("[pipeline:%s]\npipeline = %s", spec.Name, pipeline)) + + // Collect all filter sections (middleware + filters) and sort by name for determinism. + type filterEntry struct { + name string + factory string + config map[string]string + } + + var filters []filterEntry + for _, mw := range spec.Middleware { + filters = append(filters, filterEntry{ + name: mw.Name, + factory: mw.FilterFactory, + config: mw.Config, + }) + } + for _, f := range spec.Filters { + filters = append(filters, filterEntry{ + name: f.Name, + factory: f.Factory, + config: f.Config, + }) + } + + sort.Slice(filters, func(i, j int) bool { + return filters[i].name < filters[j].name + }) + + for _, f := range filters { + sections = append(sections, renderFilterSection(f.name, f.factory, f.config)) + } + + return strings.Join(sections, "\n\n") + "\n" +} + +// RenderPluginConfig converts a slice of PluginSpec into a config map suitable for +// merging with the main INI configuration via MergeDefaults. Each plugin produces one +// section keyed by its Section field. (CC-0004, REQ-010) +func RenderPluginConfig(plugins []types.PluginSpec) map[string]map[string]string { + result := make(map[string]map[string]string) + + for _, p := range plugins { + existing, ok := result[p.Section] + if !ok { + existing = make(map[string]string) + } + for k, v := range p.Config { + existing[k] = v + } + result[p.Section] = existing + } + + return result +} + +// insertMiddleware inserts middleware names into the base pipeline string at the +// positions specified by each middleware's Position field. +func insertMiddleware(basePipeline string, middleware []types.MiddlewareSpec) string { + tokens := strings.Fields(basePipeline) + + for _, mw := range middleware { + tokens = insertToken(tokens, mw.Name, mw.Position) + } + + return strings.Join(tokens, " ") +} + +// insertToken inserts a token into the pipeline at the position specified. +// If both After and Before are set, After takes precedence. +// If the requested anchor token (After/Before) is not found in the existing +// pipeline, the token is appended to the end. This fallback behavior is +// intentional; callers that require strict placement should validate their +// configuration before invoking insertToken. +func insertToken(tokens []string, name string, pos types.PipelinePosition) []string { + if pos.After != "" { + for i, t := range tokens { + if t == pos.After { + result := make([]string, 0, len(tokens)+1) + result = append(result, tokens[:i+1]...) + result = append(result, name) + result = append(result, tokens[i+1:]...) + return result + } + } + } + + if pos.Before != "" { + for i, t := range tokens { + if t == pos.Before { + result := make([]string, 0, len(tokens)+1) + result = append(result, tokens[:i]...) + result = append(result, name) + result = append(result, tokens[i:]...) + return result + } + } + } + + // Default: append to end. + return append(tokens, name) +} + +// renderFilterSection renders a [filter:name] INI section with sorted config keys. +func renderFilterSection(name, factory string, config map[string]string) string { + var b strings.Builder + fmt.Fprintf(&b, "[filter:%s]\npaste.filter_factory = %s", name, factory) + + if len(config) > 0 { + keys := make([]string, 0, len(config)) + for k := range config { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + fmt.Fprintf(&b, "\n%s = %s", k, config[k]) + } + } + + return b.String() +} diff --git a/internal/common/plugins/plugins_test.go b/internal/common/plugins/plugins_test.go new file mode 100644 index 0000000..920eadd --- /dev/null +++ b/internal/common/plugins/plugins_test.go @@ -0,0 +1,275 @@ +package plugins + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/c5c3/forge/internal/common/types" +) + +func TestRenderPastePipeline(t *testing.T) { + tests := []struct { + name string + spec types.PipelineSpec + contains []string + exact string // if non-empty, assert exact match instead of contains + }{ + { + name: "base pipeline only", + spec: types.PipelineSpec{ + Name: "api_v3", + BasePipeline: "cors request_id service_v3", + }, + exact: "[pipeline:api_v3]\npipeline = cors request_id service_v3\n", + }, + { + name: "after position insertion", + spec: types.PipelineSpec{ + Name: "api_v3", + BasePipeline: "cors request_id authtoken keystonecontext service_v3", + Middleware: []types.MiddlewareSpec{ + { + Name: "audit", + FilterFactory: "audit_middleware:filter_factory", + Position: types.PipelinePosition{After: "authtoken"}, + Config: map[string]string{"audit_map": "/etc/keystone/audit_map.yaml"}, + }, + }, + }, + contains: []string{ + "pipeline = cors request_id authtoken audit keystonecontext service_v3", + "[filter:audit]", + "paste.filter_factory = audit_middleware:filter_factory", + "audit_map = /etc/keystone/audit_map.yaml", + }, + }, + { + name: "before position insertion", + spec: types.PipelineSpec{ + Name: "api_v3", + BasePipeline: "cors request_id authtoken keystonecontext service_v3", + Middleware: []types.MiddlewareSpec{ + { + Name: "rate_limit", + FilterFactory: "rate_limit:filter_factory", + Position: types.PipelinePosition{Before: "authtoken"}, + }, + }, + }, + contains: []string{ + "pipeline = cors request_id rate_limit authtoken keystonecontext service_v3", + }, + }, + { + name: "multiple middleware", + spec: types.PipelineSpec{ + Name: "api_v3", + BasePipeline: "cors request_id authtoken keystonecontext service_v3", + Middleware: []types.MiddlewareSpec{ + { + Name: "audit", + FilterFactory: "audit_middleware:filter_factory", + Position: types.PipelinePosition{After: "authtoken"}, + }, + { + Name: "rate_limit", + FilterFactory: "rate_limit:filter_factory", + Position: types.PipelinePosition{Before: "keystonecontext"}, + }, + }, + }, + contains: []string{ + "pipeline = cors request_id authtoken audit rate_limit keystonecontext service_v3", + }, + }, + { + name: "filter blocks with sorted config keys", + spec: types.PipelineSpec{ + Name: "api_v3", + BasePipeline: "cors request_id service_v3", + Filters: []types.FilterSpec{ + { + Name: "cors", + Factory: "oslo_middleware:filter_factory", + Config: map[string]string{ + "oslo_config_project": "keystone", + "allowed_origin": "http://example.com", + }, + }, + }, + }, + contains: []string{ + "[filter:cors]", + "paste.filter_factory = oslo_middleware:filter_factory", + "allowed_origin = http://example.com", + "oslo_config_project = keystone", + }, + }, + { + name: "empty base pipeline appends middleware", + spec: types.PipelineSpec{ + Name: "api_v3", + BasePipeline: "", + Middleware: []types.MiddlewareSpec{ + { + Name: "audit", + FilterFactory: "audit_middleware:filter_factory", + Position: types.PipelinePosition{}, + }, + { + Name: "cors", + FilterFactory: "cors:filter_factory", + Position: types.PipelinePosition{}, + }, + }, + }, + contains: []string{ + "pipeline = audit cors", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := RenderPastePipeline(tt.spec) + + if tt.exact != "" { + g.Expect(result).To(Equal(tt.exact)) + } + for _, substr := range tt.contains { + g.Expect(result).To(ContainSubstring(substr)) + } + }) + } + + // Filter sections are sorted by name (cross-section ordering). + t.Run("filter sections sorted by name", func(t *testing.T) { + g := NewGomegaWithT(t) + + spec := types.PipelineSpec{ + Name: "api_v3", + BasePipeline: "service_v3", + Middleware: []types.MiddlewareSpec{ + { + Name: "zebra", + FilterFactory: "zebra:factory", + Position: types.PipelinePosition{}, + }, + }, + Filters: []types.FilterSpec{ + { + Name: "alpha", + Factory: "alpha:factory", + }, + }, + } + + result := RenderPastePipeline(spec) + + alphaIdx := strings.Index(result, "[filter:alpha]") + zebraIdx := strings.Index(result, "[filter:zebra]") + g.Expect(alphaIdx).To(BeNumerically("<", zebraIdx)) + }) +} + +func TestRenderPluginConfig(t *testing.T) { + tests := []struct { + name string + plugins []types.PluginSpec + expectedLen int + expected map[string]map[string]string + }{ + { + name: "single plugin", + plugins: []types.PluginSpec{ + { + Name: "keycloak-backend", + Section: "keycloak", + Config: map[string]string{ + "url": "https://keycloak.example.com", + "realm": "openstack", + }, + }, + }, + expectedLen: 1, + expected: map[string]map[string]string{ + "keycloak": {"url": "https://keycloak.example.com", "realm": "openstack"}, + }, + }, + { + name: "multiple plugins in different sections", + plugins: []types.PluginSpec{ + { + Name: "keycloak-backend", + Section: "keycloak", + Config: map[string]string{"url": "https://keycloak.example.com"}, + }, + { + Name: "audit", + Section: "audit_middleware", + Config: map[string]string{"enabled": "true"}, + }, + }, + expectedLen: 2, + expected: map[string]map[string]string{ + "keycloak": {"url": "https://keycloak.example.com"}, + "audit_middleware": {"enabled": "true"}, + }, + }, + { + name: "empty slice returns empty map", + plugins: []types.PluginSpec{}, + expectedLen: 0, + }, + { + name: "nil slice returns empty map", + plugins: nil, + expectedLen: 0, + }, + { + name: "same section merges with last-wins", + plugins: []types.PluginSpec{ + { + Name: "plugin-a", + Section: "shared", + Config: map[string]string{ + "key1": "value1", + "key2": "value2-a", + }, + }, + { + Name: "plugin-b", + Section: "shared", + Config: map[string]string{ + "key2": "value2-b", + "key3": "value3", + }, + }, + }, + expectedLen: 1, + expected: map[string]map[string]string{ + "shared": {"key1": "value1", "key2": "value2-b", "key3": "value3"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := RenderPluginConfig(tt.plugins) + + g.Expect(result).NotTo(BeNil()) + g.Expect(result).To(HaveLen(tt.expectedLen)) + + for section, expectedKVs := range tt.expected { + g.Expect(result).To(HaveKey(section)) + for k, v := range expectedKVs { + g.Expect(result[section]).To(HaveKeyWithValue(k, v)) + } + } + }) + } +} diff --git a/internal/common/policy/doc.go b/internal/common/policy/doc.go new file mode 100644 index 0000000..060414d --- /dev/null +++ b/internal/common/policy/doc.go @@ -0,0 +1,3 @@ +// Package policy provides pure functions for merging and rendering OpenStack +// oslo.policy policy rules. (CC-0004) +package policy diff --git a/internal/common/policy/policy.go b/internal/common/policy/policy.go new file mode 100644 index 0000000..a8c57e5 --- /dev/null +++ b/internal/common/policy/policy.go @@ -0,0 +1,87 @@ +package policy + +import ( + "sort" + + "gopkg.in/yaml.v3" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// MergePolicies merges inline policy rules over external rules with inline-wins precedence. +// Returns a new map without mutating inputs. (CC-0004, REQ-011) +func MergePolicies(external, inline map[string]string) map[string]string { + result := make(map[string]string) + + for k, v := range external { + result[k] = v + } + for k, v := range inline { + result[k] = v + } + + return result +} + +// ValidatePolicyRules validates policy rules and returns a field.ErrorList for webhook compatibility. +// Each rule must have a non-empty key and non-empty value. (CC-0004, REQ-012) +func ValidatePolicyRules(rules map[string]string, fldPath *field.Path) field.ErrorList { + var errs field.ErrorList + + // Sort keys for deterministic error ordering. + keys := make([]string, 0, len(rules)) + for k := range rules { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + v := rules[k] + if k == "" { + errs = append(errs, field.Required(fldPath, "policy rule key must not be empty")) + } + if v == "" { + errs = append(errs, field.Invalid(fldPath.Key(k), v, "policy rule value must not be empty")) + } + } + + return errs +} + +// RenderPolicyYAML renders policy rules as a YAML document suitable for oslo.policy. +// Returns valid YAML. Empty map returns empty YAML ("{}\n"). (CC-0004, REQ-013) +func RenderPolicyYAML(rules map[string]string) (string, error) { + if len(rules) == 0 { + return "{}\n", nil + } + + // Sort keys for deterministic output. + keys := make([]string, 0, len(rules)) + for k := range rules { + keys = append(keys, k) + } + sort.Strings(keys) + + // Build an ordered map using yaml.Node for sorted key output. + mapping := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + } + for _, k := range keys { + mapping.Content = append(mapping.Content, + &yaml.Node{Kind: yaml.ScalarNode, Value: k, Tag: "!!str"}, + &yaml.Node{Kind: yaml.ScalarNode, Value: rules[k], Tag: "!!str"}, + ) + } + + doc := &yaml.Node{ + Kind: yaml.DocumentNode, + Content: []*yaml.Node{mapping}, + } + + out, err := yaml.Marshal(doc) + if err != nil { + return "", err + } + + return string(out), nil +} diff --git a/internal/common/policy/policy_test.go b/internal/common/policy/policy_test.go new file mode 100644 index 0000000..bb743fd --- /dev/null +++ b/internal/common/policy/policy_test.go @@ -0,0 +1,232 @@ +package policy + +import ( + "testing" + + . "github.com/onsi/gomega" + "gopkg.in/yaml.v3" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestMergePolicies_InlineWinsOnCollision(t *testing.T) { + g := NewGomegaWithT(t) + + external := map[string]string{ + "compute:create": "role:member", + "compute:delete": "role:admin", + } + inline := map[string]string{ + "compute:create": "role:admin", + } + + result := MergePolicies(external, inline) + + g.Expect(result).To(HaveKeyWithValue("compute:create", "role:admin")) + g.Expect(result).To(HaveKeyWithValue("compute:delete", "role:admin")) +} + +func TestMergePolicies_NonCollidingMerged(t *testing.T) { + g := NewGomegaWithT(t) + + external := map[string]string{"compute:create": "role:member"} + inline := map[string]string{"identity:list_users": "role:admin"} + + result := MergePolicies(external, inline) + + g.Expect(result).To(HaveLen(2)) + g.Expect(result).To(HaveKeyWithValue("compute:create", "role:member")) + g.Expect(result).To(HaveKeyWithValue("identity:list_users", "role:admin")) +} + +func TestMergePolicies_NilInline(t *testing.T) { + g := NewGomegaWithT(t) + + external := map[string]string{"compute:create": "role:member"} + + result := MergePolicies(external, nil) + + g.Expect(result).To(HaveLen(1)) + g.Expect(result).To(HaveKeyWithValue("compute:create", "role:member")) +} + +func TestMergePolicies_NilExternal(t *testing.T) { + g := NewGomegaWithT(t) + + inline := map[string]string{"compute:create": "role:admin"} + + result := MergePolicies(nil, inline) + + g.Expect(result).To(HaveLen(1)) + g.Expect(result).To(HaveKeyWithValue("compute:create", "role:admin")) +} + +func TestMergePolicies_BothNil(t *testing.T) { + g := NewGomegaWithT(t) + + result := MergePolicies(nil, nil) + + g.Expect(result).NotTo(BeNil()) + g.Expect(result).To(BeEmpty()) +} + +func TestMergePolicies_DoesNotMutateInputs(t *testing.T) { + g := NewGomegaWithT(t) + + external := map[string]string{"compute:create": "role:member"} + inline := map[string]string{"compute:create": "role:admin"} + + externalCopy := map[string]string{"compute:create": "role:member"} + inlineCopy := map[string]string{"compute:create": "role:admin"} + + _ = MergePolicies(external, inline) + + g.Expect(external).To(Equal(externalCopy)) + g.Expect(inline).To(Equal(inlineCopy)) +} + +func TestRenderPolicyYAML_ValidRules(t *testing.T) { + g := NewGomegaWithT(t) + + rules := map[string]string{ + "compute:create": "role:member", + "compute:delete": "role:admin", + } + + result, err := RenderPolicyYAML(rules) + + g.Expect(err).NotTo(HaveOccurred()) + + // Verify it is valid YAML. + var parsed map[string]string + g.Expect(yaml.Unmarshal([]byte(result), &parsed)).To(Succeed()) + g.Expect(parsed).To(HaveKeyWithValue("compute:create", "role:member")) + g.Expect(parsed).To(HaveKeyWithValue("compute:delete", "role:admin")) +} + +func TestRenderPolicyYAML_EmptyMap(t *testing.T) { + g := NewGomegaWithT(t) + + result, err := RenderPolicyYAML(map[string]string{}) + + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).To(Equal("{}\n")) + + // Verify it is valid YAML. + var parsed map[string]string + g.Expect(yaml.Unmarshal([]byte(result), &parsed)).To(Succeed()) +} + +func TestRenderPolicyYAML_SpecialCharacters(t *testing.T) { + g := NewGomegaWithT(t) + + rules := map[string]string{ + "identity:create_user": "role:admin or (role:member and project_id:%(target.project.id)s)", + "identity:list_users": "rule:admin_required", + } + + result, err := RenderPolicyYAML(rules) + + g.Expect(err).NotTo(HaveOccurred()) + + var parsed map[string]string + g.Expect(yaml.Unmarshal([]byte(result), &parsed)).To(Succeed()) + g.Expect(parsed).To(HaveKeyWithValue("identity:create_user", "role:admin or (role:member and project_id:%(target.project.id)s)")) +} + +func TestRenderPolicyYAML_Deterministic(t *testing.T) { + g := NewGomegaWithT(t) + + rules := map[string]string{ + "z_rule": "value_z", + "a_rule": "value_a", + "m_rule": "value_m", + } + + result1, err := RenderPolicyYAML(rules) + g.Expect(err).NotTo(HaveOccurred()) + + result2, err := RenderPolicyYAML(rules) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(result1).To(Equal(result2)) +} + +func TestValidatePolicyRules_Valid(t *testing.T) { + g := NewGomegaWithT(t) + + rules := map[string]string{ + "compute:create": "role:member", + "compute:delete": "role:admin", + } + fldPath := field.NewPath("spec", "policyOverrides", "rules") + + errs := ValidatePolicyRules(rules, fldPath) + + g.Expect(errs).To(BeEmpty()) +} + +func TestValidatePolicyRules_EmptyKey(t *testing.T) { + g := NewGomegaWithT(t) + + rules := map[string]string{ + "": "role:admin", + "compute:create": "role:member", + } + fldPath := field.NewPath("spec", "policyOverrides", "rules") + + errs := ValidatePolicyRules(rules, fldPath) + + g.Expect(errs).To(HaveLen(1)) + g.Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired)) + g.Expect(errs[0].Detail).To(ContainSubstring("key must not be empty")) +} + +func TestValidatePolicyRules_EmptyValue(t *testing.T) { + g := NewGomegaWithT(t) + + rules := map[string]string{ + "compute:create": "", + } + fldPath := field.NewPath("spec", "policyOverrides", "rules") + + errs := ValidatePolicyRules(rules, fldPath) + + g.Expect(errs).To(HaveLen(1)) + g.Expect(errs[0].Type).To(Equal(field.ErrorTypeInvalid)) + g.Expect(errs[0].Detail).To(ContainSubstring("value must not be empty")) +} + +func TestValidatePolicyRules_MultipleErrors(t *testing.T) { + g := NewGomegaWithT(t) + + rules := map[string]string{ + "": "role:admin", + "compute:create": "", + "valid:rule": "role:member", + } + fldPath := field.NewPath("spec", "policyOverrides", "rules") + + errs := ValidatePolicyRules(rules, fldPath) + + g.Expect(errs).To(HaveLen(2)) +} + +func TestValidatePolicyRules_NilMap(t *testing.T) { + g := NewGomegaWithT(t) + + fldPath := field.NewPath("spec", "policyOverrides", "rules") + + errs := ValidatePolicyRules(nil, fldPath) + + g.Expect(errs).To(BeEmpty()) +} + +func TestValidatePolicyRules_EmptyMap(t *testing.T) { + g := NewGomegaWithT(t) + + fldPath := field.NewPath("spec", "policyOverrides", "rules") + + errs := ValidatePolicyRules(map[string]string{}, fldPath) + + g.Expect(errs).To(BeEmpty()) +} diff --git a/internal/common/types/doc.go b/internal/common/types/doc.go new file mode 100644 index 0000000..07c16f1 --- /dev/null +++ b/internal/common/types/doc.go @@ -0,0 +1,2 @@ +// Package types provides shared type definitions for CobaltCore operators. +package types diff --git a/internal/common/types/types.go b/internal/common/types/types.go new file mode 100644 index 0000000..28c9ed0 --- /dev/null +++ b/internal/common/types/types.go @@ -0,0 +1,85 @@ +package types + +import ( + corev1 "k8s.io/api/core/v1" +) + +// ImageSpec defines a container image reference. +type ImageSpec struct { + Repository string `json:"repository"` + Tag string `json:"tag"` +} + +// SecretRefSpec references a Kubernetes Secret by name and key. +type SecretRefSpec struct { + Name string `json:"name"` + Key string `json:"key"` +} + +// DatabaseSpec defines database connection parameters. +// ClusterRef and Host are mutually exclusive: ClusterRef for managed MariaDB, Host for brownfield. +type DatabaseSpec struct { + ClusterRef *corev1.LocalObjectReference `json:"clusterRef,omitempty"` + Host string `json:"host,omitempty"` + Port int32 `json:"port,omitempty"` + Database string `json:"database"` + SecretRef SecretRefSpec `json:"secretRef"` +} + +// MessagingSpec defines RabbitMQ messaging parameters. +// ClusterRef and Host are mutually exclusive: ClusterRef for managed RabbitMQ, Host for brownfield. +type MessagingSpec struct { + ClusterRef *corev1.LocalObjectReference `json:"clusterRef,omitempty"` + Host string `json:"host,omitempty"` + Port int32 `json:"port,omitempty"` + SecretRef SecretRefSpec `json:"secretRef"` + Vhost string `json:"vhost,omitempty"` +} + +// CacheSpec defines Memcached cache parameters. +type CacheSpec struct { + ClusterRef *corev1.LocalObjectReference `json:"clusterRef,omitempty"` + Servers []string `json:"servers,omitempty"` +} + +// PolicySpec defines policy override sources. +type PolicySpec struct { + ConfigMapRef *corev1.LocalObjectReference `json:"configMapRef,omitempty"` + Inline map[string]string `json:"inline,omitempty"` +} + +// PluginSpec defines a plugin configuration section. +type PluginSpec struct { + Name string `json:"name"` + Section string `json:"section"` + Config map[string]string `json:"config,omitempty"` +} + +// PipelinePosition specifies where middleware is inserted in the pipeline. +type PipelinePosition struct { + Before string `json:"before,omitempty"` + After string `json:"after,omitempty"` +} + +// MiddlewareSpec defines a PasteDeploy middleware/filter to insert. +type MiddlewareSpec struct { + Name string `json:"name"` + FilterFactory string `json:"filterFactory"` + Config map[string]string `json:"config,omitempty"` + Position PipelinePosition `json:"position"` +} + +// FilterSpec defines a PasteDeploy filter entry. +type FilterSpec struct { + Name string `json:"name"` + Factory string `json:"factory"` + Config map[string]string `json:"config,omitempty"` +} + +// PipelineSpec defines a complete PasteDeploy pipeline configuration. +type PipelineSpec struct { + Name string `json:"name"` + BasePipeline string `json:"basePipeline"` + Filters []FilterSpec `json:"filters,omitempty"` + Middleware []MiddlewareSpec `json:"middleware,omitempty"` +} diff --git a/internal/common/types/types_test.go b/internal/common/types/types_test.go new file mode 100644 index 0000000..382ac11 --- /dev/null +++ b/internal/common/types/types_test.go @@ -0,0 +1,512 @@ +package types + +import ( + "encoding/json" + "reflect" + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +func TestImageSpec_JSONRoundTrip(t *testing.T) { + g := NewGomegaWithT(t) + + original := ImageSpec{Repository: "ghcr.io/example/keystone", Tag: "2025.2"} + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded ImageSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) +} + +func TestSecretRefSpec_JSONRoundTrip(t *testing.T) { + g := NewGomegaWithT(t) + + original := SecretRefSpec{Name: "db-credentials", Key: "password"} + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded SecretRefSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) +} + +func TestDatabaseSpec_JSONRoundTrip(t *testing.T) { + tests := []struct { + name string + original DatabaseSpec + }{ + { + name: "with ClusterRef (managed MariaDB)", + original: DatabaseSpec{ + ClusterRef: &corev1.LocalObjectReference{Name: "mariadb-cluster"}, + Port: 3306, + Database: "keystone", + SecretRef: SecretRefSpec{Name: "db-secret", Key: "password"}, + }, + }, + { + name: "with Host (brownfield)", + original: DatabaseSpec{ + Host: "db.example.com", + Port: 3306, + Database: "keystone", + SecretRef: SecretRefSpec{Name: "db-secret", Key: "password"}, + }, + }, + { + name: "minimal (neither ClusterRef nor Host)", + original: DatabaseSpec{ + Database: "keystone", + SecretRef: SecretRefSpec{Name: "db-secret", Key: "password"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + data, err := json.Marshal(tc.original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded DatabaseSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(tc.original)) + }) + } +} + +func TestMessagingSpec_JSONRoundTrip(t *testing.T) { + tests := []struct { + name string + original MessagingSpec + }{ + { + name: "with ClusterRef (managed RabbitMQ)", + original: MessagingSpec{ + ClusterRef: &corev1.LocalObjectReference{Name: "rabbitmq-cluster"}, + Port: 5672, + SecretRef: SecretRefSpec{Name: "mq-secret", Key: "password"}, + Vhost: "/openstack", + }, + }, + { + name: "with Host (brownfield)", + original: MessagingSpec{ + Host: "rabbitmq.example.com", + Port: 5672, + SecretRef: SecretRefSpec{Name: "mq-secret", Key: "password"}, + Vhost: "/openstack", + }, + }, + { + name: "minimal (neither ClusterRef nor Host)", + original: MessagingSpec{ + SecretRef: SecretRefSpec{Name: "mq-secret", Key: "password"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + data, err := json.Marshal(tc.original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded MessagingSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(tc.original)) + }) + } +} + +func TestMessagingSpec_DualMode(t *testing.T) { + tests := []struct { + name string + spec MessagingSpec + hasCluster bool + hasHost bool + }{ + { + name: "ClusterRef set", + spec: MessagingSpec{ + ClusterRef: &corev1.LocalObjectReference{Name: "rabbitmq"}, + SecretRef: SecretRefSpec{Name: "s", Key: "k"}, + }, + hasCluster: true, + hasHost: false, + }, + { + name: "Host set", + spec: MessagingSpec{ + Host: "rabbitmq.example.com", + SecretRef: SecretRefSpec{Name: "s", Key: "k"}, + }, + hasCluster: false, + hasHost: true, + }, + { + name: "neither set", + spec: MessagingSpec{ + SecretRef: SecretRefSpec{Name: "s", Key: "k"}, + }, + hasCluster: false, + hasHost: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + data, err := json.Marshal(tc.spec) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded MessagingSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + + if tc.hasCluster { + g.Expect(decoded.ClusterRef).NotTo(BeNil()) + } else { + g.Expect(decoded.ClusterRef).To(BeNil()) + } + + if tc.hasHost { + g.Expect(decoded.Host).NotTo(BeEmpty()) + } else { + g.Expect(decoded.Host).To(BeEmpty()) + } + }) + } +} + +func TestCacheSpec_JSONRoundTrip(t *testing.T) { + tests := []struct { + name string + original CacheSpec + }{ + { + name: "with ClusterRef", + original: CacheSpec{ + ClusterRef: &corev1.LocalObjectReference{Name: "memcached-cluster"}, + }, + }, + { + name: "with Servers", + original: CacheSpec{ + Servers: []string{"memcached-0:11211", "memcached-1:11211"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + data, err := json.Marshal(tc.original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded CacheSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(tc.original)) + }) + } +} + +func TestPolicySpec_JSONRoundTrip(t *testing.T) { + g := NewGomegaWithT(t) + + original := PolicySpec{ + ConfigMapRef: &corev1.LocalObjectReference{Name: "policy-overrides"}, + Inline: map[string]string{"identity:list_users": "role:admin"}, + } + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded PolicySpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) +} + +func TestPluginSpec_JSONRoundTrip(t *testing.T) { + g := NewGomegaWithT(t) + + original := PluginSpec{ + Name: "ldap", + Section: "identity", + Config: map[string]string{"url": "ldap://ldap.example.com", "user": "cn=admin"}, + } + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded PluginSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) +} + +func TestMiddlewareSpec_JSONRoundTrip(t *testing.T) { + g := NewGomegaWithT(t) + + original := MiddlewareSpec{ + Name: "cors", + FilterFactory: "oslo_middleware.cors:filter_factory", + Config: map[string]string{"allowed_origin": "https://example.com"}, + Position: PipelinePosition{Before: "keystoneauth"}, + } + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded MiddlewareSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) +} + +func TestFilterSpec_JSONRoundTrip(t *testing.T) { + g := NewGomegaWithT(t) + + original := FilterSpec{ + Name: "healthcheck", + Factory: "oslo_middleware:Healthcheck.app_factory", + Config: map[string]string{"path": "/healthcheck"}, + } + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded FilterSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) +} + +func TestPipelineSpec_JSONRoundTrip(t *testing.T) { + g := NewGomegaWithT(t) + + original := PipelineSpec{ + Name: "keystone-public-api", + BasePipeline: "public_api", + Filters: []FilterSpec{ + { + Name: "healthcheck", + Factory: "oslo_middleware:Healthcheck.app_factory", + Config: map[string]string{"path": "/healthcheck"}, + }, + }, + Middleware: []MiddlewareSpec{ + { + Name: "cors", + FilterFactory: "oslo_middleware.cors:filter_factory", + Config: map[string]string{"allowed_origin": "https://example.com"}, + Position: PipelinePosition{After: "debug"}, + }, + }, + } + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded PipelineSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) +} + +func TestPipelinePosition_JSONRoundTrip(t *testing.T) { + tests := []struct { + name string + original PipelinePosition + }{ + { + name: "before only", + original: PipelinePosition{Before: "keystoneauth"}, + }, + { + name: "after only", + original: PipelinePosition{After: "debug"}, + }, + { + name: "both", + original: PipelinePosition{Before: "keystoneauth", After: "debug"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + data, err := json.Marshal(tc.original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded PipelinePosition + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(tc.original)) + }) + } +} + +func TestStructFieldJSONTags(t *testing.T) { + tests := []struct { + name string + structType reflect.Type + field string + tag string + }{ + {"ImageSpec.Repository", reflect.TypeOf(ImageSpec{}), "Repository", `json:"repository"`}, + {"ImageSpec.Tag", reflect.TypeOf(ImageSpec{}), "Tag", `json:"tag"`}, + {"SecretRefSpec.Name", reflect.TypeOf(SecretRefSpec{}), "Name", `json:"name"`}, + {"SecretRefSpec.Key", reflect.TypeOf(SecretRefSpec{}), "Key", `json:"key"`}, + {"DatabaseSpec.ClusterRef", reflect.TypeOf(DatabaseSpec{}), "ClusterRef", `json:"clusterRef,omitempty"`}, + {"DatabaseSpec.Host", reflect.TypeOf(DatabaseSpec{}), "Host", `json:"host,omitempty"`}, + {"DatabaseSpec.Port", reflect.TypeOf(DatabaseSpec{}), "Port", `json:"port,omitempty"`}, + {"DatabaseSpec.Database", reflect.TypeOf(DatabaseSpec{}), "Database", `json:"database"`}, + {"DatabaseSpec.SecretRef", reflect.TypeOf(DatabaseSpec{}), "SecretRef", `json:"secretRef"`}, + {"MessagingSpec.ClusterRef", reflect.TypeOf(MessagingSpec{}), "ClusterRef", `json:"clusterRef,omitempty"`}, + {"MessagingSpec.Host", reflect.TypeOf(MessagingSpec{}), "Host", `json:"host,omitempty"`}, + {"MessagingSpec.Port", reflect.TypeOf(MessagingSpec{}), "Port", `json:"port,omitempty"`}, + {"MessagingSpec.SecretRef", reflect.TypeOf(MessagingSpec{}), "SecretRef", `json:"secretRef"`}, + {"MessagingSpec.Vhost", reflect.TypeOf(MessagingSpec{}), "Vhost", `json:"vhost,omitempty"`}, + {"CacheSpec.ClusterRef", reflect.TypeOf(CacheSpec{}), "ClusterRef", `json:"clusterRef,omitempty"`}, + {"CacheSpec.Servers", reflect.TypeOf(CacheSpec{}), "Servers", `json:"servers,omitempty"`}, + {"PolicySpec.ConfigMapRef", reflect.TypeOf(PolicySpec{}), "ConfigMapRef", `json:"configMapRef,omitempty"`}, + {"PolicySpec.Inline", reflect.TypeOf(PolicySpec{}), "Inline", `json:"inline,omitempty"`}, + {"PluginSpec.Name", reflect.TypeOf(PluginSpec{}), "Name", `json:"name"`}, + {"PluginSpec.Section", reflect.TypeOf(PluginSpec{}), "Section", `json:"section"`}, + {"PluginSpec.Config", reflect.TypeOf(PluginSpec{}), "Config", `json:"config,omitempty"`}, + {"MiddlewareSpec.Name", reflect.TypeOf(MiddlewareSpec{}), "Name", `json:"name"`}, + {"MiddlewareSpec.FilterFactory", reflect.TypeOf(MiddlewareSpec{}), "FilterFactory", `json:"filterFactory"`}, + {"MiddlewareSpec.Config", reflect.TypeOf(MiddlewareSpec{}), "Config", `json:"config,omitempty"`}, + {"MiddlewareSpec.Position", reflect.TypeOf(MiddlewareSpec{}), "Position", `json:"position"`}, + {"FilterSpec.Name", reflect.TypeOf(FilterSpec{}), "Name", `json:"name"`}, + {"FilterSpec.Factory", reflect.TypeOf(FilterSpec{}), "Factory", `json:"factory"`}, + {"FilterSpec.Config", reflect.TypeOf(FilterSpec{}), "Config", `json:"config,omitempty"`}, + {"PipelineSpec.Name", reflect.TypeOf(PipelineSpec{}), "Name", `json:"name"`}, + {"PipelineSpec.BasePipeline", reflect.TypeOf(PipelineSpec{}), "BasePipeline", `json:"basePipeline"`}, + {"PipelineSpec.Filters", reflect.TypeOf(PipelineSpec{}), "Filters", `json:"filters,omitempty"`}, + {"PipelineSpec.Middleware", reflect.TypeOf(PipelineSpec{}), "Middleware", `json:"middleware,omitempty"`}, + {"PipelinePosition.Before", reflect.TypeOf(PipelinePosition{}), "Before", `json:"before,omitempty"`}, + {"PipelinePosition.After", reflect.TypeOf(PipelinePosition{}), "After", `json:"after,omitempty"`}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + field, found := tc.structType.FieldByName(tc.field) + g.Expect(found).To(BeTrue(), "field %s not found on %s", tc.field, tc.structType.Name()) + g.Expect(string(field.Tag)).To(ContainSubstring(tc.tag)) + }) + } +} + +func TestDatabaseSpec_DualMode(t *testing.T) { + tests := []struct { + name string + spec DatabaseSpec + hasCluster bool + hasHost bool + }{ + { + name: "ClusterRef set", + spec: DatabaseSpec{ + ClusterRef: &corev1.LocalObjectReference{Name: "mariadb"}, + Database: "keystone", + SecretRef: SecretRefSpec{Name: "s", Key: "k"}, + }, + hasCluster: true, + hasHost: false, + }, + { + name: "Host set", + spec: DatabaseSpec{ + Host: "db.example.com", + Database: "keystone", + SecretRef: SecretRefSpec{Name: "s", Key: "k"}, + }, + hasCluster: false, + hasHost: true, + }, + { + name: "neither set", + spec: DatabaseSpec{ + Database: "keystone", + SecretRef: SecretRefSpec{Name: "s", Key: "k"}, + }, + hasCluster: false, + hasHost: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + data, err := json.Marshal(tc.spec) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded DatabaseSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + + if tc.hasCluster { + g.Expect(decoded.ClusterRef).NotTo(BeNil()) + } else { + g.Expect(decoded.ClusterRef).To(BeNil()) + } + + if tc.hasHost { + g.Expect(decoded.Host).NotTo(BeEmpty()) + } else { + g.Expect(decoded.Host).To(BeEmpty()) + } + }) + } +} + +func TestPipelineSpec_Aggregation(t *testing.T) { + g := NewGomegaWithT(t) + + original := PipelineSpec{ + Name: "nova-api", + BasePipeline: "default", + Filters: []FilterSpec{ + {Name: "f1", Factory: "factory1", Config: map[string]string{"k": "v"}}, + {Name: "f2", Factory: "factory2"}, + }, + Middleware: []MiddlewareSpec{ + { + Name: "m1", + FilterFactory: "mf1", + Config: map[string]string{"mk": "mv"}, + Position: PipelinePosition{Before: "auth"}, + }, + }, + } + + data, err := json.Marshal(original) + g.Expect(err).NotTo(HaveOccurred()) + + var decoded PipelineSpec + err = json.Unmarshal(data, &decoded) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(decoded).To(Equal(original)) + + g.Expect(decoded.Name).To(Equal("nova-api")) + g.Expect(decoded.BasePipeline).To(Equal("default")) + g.Expect(decoded.Filters).To(HaveLen(2)) + g.Expect(decoded.Filters[0].Name).To(Equal("f1")) + g.Expect(decoded.Filters[0].Config).To(HaveKeyWithValue("k", "v")) + g.Expect(decoded.Middleware).To(HaveLen(1)) + g.Expect(decoded.Middleware[0].Position.Before).To(Equal("auth")) +}