diff --git a/.ai/checkpoints/topic-4-policy-commands.md b/.ai/checkpoints/topic-4-policy-commands.md new file mode 100644 index 0000000..4038ee5 --- /dev/null +++ b/.ai/checkpoints/topic-4-policy-commands.md @@ -0,0 +1,106 @@ +# Checkpoint: Topic 4 — Policy Commands + +- **Branch:** `topic-4-policy-commands` +- **Base:** `baseline` (commit `794930d`) +- **Date:** 2026-03-12 +- **Status:** Complete + +--- + +## Scope + +Topic 4 implements the five policy CRUD commands (create, list, get, update, delete) per spec section 4.4, along with shared command helpers extracted into `helpers.go` for reuse by Topics 5-7. + +### Requirements Addressed + +| ID | Description | Status | +|----|-------------|--------| +| REQ-POL-010 | `dcm policy create --from-file FILE` | Done | +| REQ-POL-020 | `dcm policy create --id ID` optional client-specified ID | Done | +| REQ-POL-030 | `dcm policy list` with filter, order-by, pagination flags | Done | +| REQ-POL-040 | `dcm policy get POLICY_ID` | Done | +| REQ-POL-050 | `dcm policy update POLICY_ID --from-file FILE` (PATCH) | Done | +| REQ-POL-060 | `dcm policy delete POLICY_ID` | Done | +| REQ-XC-CLI-010 | Exit code 0 for success | Done | +| REQ-XC-CLI-020 | Exit code 1 for runtime errors | Done | +| REQ-XC-CLI-030 | Exit code 2 for usage errors | Done | +| REQ-XC-CLI-040 | Context-based request timeout | Done | +| REQ-ERR-010 | RFC 7807 Problem Details error formatting | Done | +| REQ-INP-010 | YAML/JSON input file parsing | Done | + +### Tests Implemented (32 specs) + +| TC ID | Description | Status | +|-------|-------------|--------| +| TC-U026 | `policy create --from-file policy.yaml` sends POST, returns 201 | Pass | +| TC-U027 | `policy create` without `--from-file` exits code 2 | Pass | +| TC-U028 | `policy create --from-file` with nonexistent file returns error | Pass | +| TC-U029 | `policy create --from-file bad.yaml` with invalid YAML returns error | Pass | +| TC-U030 | `policy create --id custom-id` passes `?id=custom-id` query param | Pass | +| TC-U031 | `policy create` with 409 RFC 7807 error formats to stderr | Pass | +| TC-U032 | `policy list` sends GET, returns table with 2 policies | Pass | +| TC-U033 | `policy list --filter` passes filter query parameter | Pass | +| TC-U034 | `policy list --order-by` passes order_by query parameter | Pass | +| TC-U035 | `policy list --page-size --page-token` passes pagination params | Pass | +| TC-U036 | `policy list` with next_page_token shows pagination hint | Pass | +| TC-U037 | `policy list -o json` returns JSON with results array | Pass | +| TC-U038 | `policy get POLICY_ID` sends GET to correct path | Pass | +| TC-U039 | `policy get` without ID exits code 2 | Pass | +| TC-U040 | `policy update POLICY_ID --from-file` sends PATCH | Pass | +| TC-U041 | `policy update` without `--from-file` exits code 2 | Pass | +| TC-U062 | `policy delete POLICY_ID` sends DELETE, returns success message | Pass | +| TC-U063 | `policy delete` without ID exits code 2 | Pass | +| TC-U080 | Connection refused returns user-friendly error with gateway URL | Pass | +| TC-U081 | Request timeout returns timeout-specific error message | Pass | +| TC-U082 | 404 RFC 7807 error formatted to stderr, exit code 1 | Pass | +| TC-U083 | Non-RFC-7807 error body returns `HTTP : ` | Pass | +| TC-U084 | `--from-file` with JSON input sends correct request body | Pass | +| TC-U085 | `--from-file` with YAML list (non-object) returns parse error | Pass | +| TC-U100 | `policy list -o json` returns valid JSON structure | Pass | +| TC-U101 | `policy list -o yaml` returns valid YAML structure | Pass | +| TC-U102 | `policy get -o json` returns valid JSON | Pass | +| TC-U103 | `policy get -o yaml` returns valid YAML | Pass | +| TC-U104 | `policy create -o json` returns valid JSON | Pass | +| TC-U116 | Error response formatted to stderr per output format | Pass | +| TC-U120 | Empty policy list returns headers only (table) / empty array (JSON) | Pass | +| — | `policy list` with `--page-size` includes it in pagination hint | Pass | + +--- + +## Files Created / Modified + +| File | Change | Purpose | +|------|--------|---------| +| `internal/commands/helpers.go` | Created | Shared utilities: `FormattedError`, `newFormatter`, `buildHTTPClient`, `apiBaseURL`, `parseInputFile`, `parseInputFileAs` (generic typed variant), `handleErrorResponse`, `requestContext`, `connectionError`, `isTimeoutError`, `stringifyValue` | +| `internal/commands/policy.go` | Modified | Full CRUD implementation using generated policy-manager client | +| `internal/commands/root.go` | Modified | Added `FormattedError` handling in `Execute()`, `requiredFlagsPreRun` hook | +| `internal/commands/policy_test.go` | Created | 32 Ginkgo test specs with httptest-based mocking | + +--- + +## Key Design Decisions + +1. **Generated client from policy-manager** — Per REQ-XC-CLI-010, all policy operations use the oapi-codegen generated client from `github.com/dcm-project/policy-manager/pkg/client`. Client is instantiated via `policyclient.NewClient(apiBaseURL(cfg), policyclient.WithHTTPClient(buildHTTPClient(cfg)))`. Create and update commands use typed client methods (`CreatePolicy`, `UpdatePolicyWithApplicationMergePatchPlusJSONBody`) with typed request bodies (`CreatePolicyJSONRequestBody`, `UpdatePolicyApplicationMergePatchPlusJSONRequestBody`) for client-side payload validation against the generated schema. + +2. **Shared helpers in `helpers.go`** — HTTP client construction, request context with timeout, error handling, input file parsing, and table cell extraction are shared across all command groups. Extracted into `helpers.go` so Topics 5-7 reuse them without duplication. + +3. **`FormattedError` sentinel type** — When `handleErrorResponse` detects an RFC 7807 error, it formats it via `formatter.FormatError()` (writing to stderr) and returns `&FormattedError{}`. `Execute()` checks for this type and skips printing the error again, preventing double output. + +4. **`MarkFlagRequired` with `requiredFlagsPreRun`** — Required flags use Cobra's `MarkFlagRequired` for help/docs. A `requiredFlagsPreRun` hook (set as `PreRunE`) calls `cmd.ValidateRequiredFlags()` early and wraps errors as `UsageError` for exit code 2. Cobra's own `ValidateRequiredFlags` call that follows is a no-op. + +5. **Context timeout, not `http.Client.Timeout`** — Per REQ-XC-CLI-040, request deadlines use `context.WithTimeout` on the command context, allowing cancellation propagation through the request lifecycle. + +6. **YAML-first parsing with typed unmarshalling** — `parseInputFile` uses YAML unmarshal (which also handles valid JSON) with a nil-result check to reject non-object content (arrays, scalars, empty files). `parseInputFileAs[T]` extends this by further unmarshalling the parsed map into a typed struct via JSON round-trip, enabling client-side schema validation. + +7. **Pagination hint includes `--page-size`** — The list command dynamically builds the hint command string to include `--page-size` if specified. The CLI flag `--page-size` maps to the API's `max_page_size` query parameter via `ListPoliciesParams.MaxPageSize`. + +8. **List response field mapping** — The API returns policies in a `policies` field (per `PolicyList` type), which the CLI re-wraps as `results` in its JSON/YAML output via the formatter's `ListResponse` struct. + +--- + +## What's Next + +- **Topic 5: Catalog Service-Type Commands** — `catalog service-type list` and `catalog service-type get` +- **Topic 6: Catalog Item Commands** — `catalog item create/list/get/delete` +- **Topic 7: Catalog Instance Commands** — `catalog instance create/list/get/delete` +- All future command topics reuse `helpers.go` utilities diff --git a/go.mod b/go.mod index 5148564..e48d84f 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,9 @@ module github.com/dcm-project/cli -go 1.25.4 +go 1.25.5 require ( + github.com/dcm-project/policy-manager v0.0.0-20260310132113-15bd45617e87 github.com/onsi/ginkgo/v2 v2.28.1 github.com/onsi/gomega v1.39.1 github.com/spf13/cobra v1.10.2 @@ -12,24 +13,38 @@ require ( require ( github.com/Masterminds/semver/v3 v3.4.0 // indirect + github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect + github.com/getkin/kin-openapi v0.133.0 // indirect github.com/go-logr/logr v1.4.3 // indirect + github.com/go-openapi/jsonpointer v0.21.0 // indirect + github.com/go-openapi/swag v0.23.0 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/pprof v0.0.0-20260115054156-294ebfa9ad83 // indirect + github.com/google/uuid v1.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/josharian/intern v1.0.0 // indirect + github.com/mailru/easyjson v0.7.7 // indirect + github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect + github.com/oapi-codegen/runtime v1.2.0 // indirect + github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 // indirect + github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect + github.com/perimeterx/marshmallow v1.1.5 // indirect github.com/sagikazarmark/locafero v0.11.0 // indirect github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect github.com/spf13/afero v1.15.0 // indirect github.com/spf13/cast v1.10.0 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect + github.com/woodsbury/decimal128 v1.3.0 // indirect golang.org/x/mod v0.32.0 // indirect golang.org/x/net v0.49.0 // indirect golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.33.0 // indirect golang.org/x/tools v0.41.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 943c8d0..9e69792 100644 --- a/go.sum +++ b/go.sum @@ -1,12 +1,21 @@ github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= +github.com/RaveNoX/go-jsoncommentstrip v1.0.0/go.mod h1:78ihd09MekBnJnxpICcwzCMzGrKSKYe4AqU6PDYYpjk= +github.com/apapsch/go-jsonmerge/v2 v2.0.0 h1:axGnT1gRIfimI7gJifB699GoE/oq+F2MU7Dml6nw9rQ= +github.com/apapsch/go-jsonmerge/v2 v2.0.0/go.mod h1:lvDnEdqiQrp0O42VQGgmlKpxL1AP2+08jFMw88y4klk= +github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dcm-project/policy-manager v0.0.0-20260310132113-15bd45617e87 h1:IgIFK8eWeNHLloVuwbGZLzun8LHA6d5nqVrct7nB+S8= +github.com/dcm-project/policy-manager v0.0.0-20260310132113-15bd45617e87/go.mod h1:a9eT8Ws0Gy/6FJGp+dWmrB4s/hyfVE0PQPats/aQW0E= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= +github.com/getkin/kin-openapi v0.133.0 h1:pJdmNohVIJ97r4AUFtEXRXwESr8b0bD721u/Tz6k8PQ= +github.com/getkin/kin-openapi v0.133.0/go.mod h1:boAciF6cXk5FhPqe/NQeBTeenbjqU4LhWBf09ILVvWE= github.com/gkampitakis/ciinfo v0.3.2 h1:JcuOPk8ZU7nZQjdUhctuhQofk7BGHuIy0c9Ez8BNhXs= github.com/gkampitakis/ciinfo v0.3.2/go.mod h1:1NIwaOcFChN4fa/B0hEBdAb6npDlFL8Bwx4dfRLRqAo= github.com/gkampitakis/go-diff v1.3.2 h1:Qyn0J9XJSDTgnsgHRdz9Zp24RaJeKMUHg2+PDZZdC4M= @@ -15,8 +24,14 @@ github.com/gkampitakis/go-snaps v0.5.15 h1:amyJrvM1D33cPHwVrjo9jQxX8g/7E2wYdZ+01 github.com/gkampitakis/go-snaps v0.5.15/go.mod h1:HNpx/9GoKisdhw9AFOBT1N7DBs9DiHo/hGheFGBZ+mc= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-openapi/jsonpointer v0.21.0 h1:YgdVicSA9vH5RiHs9TZW5oyafXZFc6+2Vc1rr/O9oNQ= +github.com/go-openapi/jsonpointer v0.21.0/go.mod h1:IUyH9l/+uyhIYQ/PXVA41Rexl+kOkAPDdXEYns6fzUY= +github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+GrE= +github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= +github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM= +github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/goccy/go-yaml v1.18.0 h1:8W7wMFS12Pcas7KU+VVkaiCng+kG8QiFeFwzFb+rwuw= @@ -25,24 +40,41 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/pprof v0.0.0-20260115054156-294ebfa9ad83 h1:z2ogiKUYzX5Is6zr/vP9vJGqPwcdqsWjOt+V8J7+bTc= github.com/google/pprof v0.0.0-20260115054156-294ebfa9ad83/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= +github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/joshdk/go-junit v1.0.0 h1:S86cUKIdwBHWwA6xCmFlf3RTLfVXYQfvanM5Uh+K6GE= github.com/joshdk/go-junit v1.0.0/go.mod h1:TiiV0PqkaNfFXjEiyjWM3XXrhVyCa1K4Zfga6W52ung= +github.com/juju/gnuflag v0.0.0-20171113085948-2ce1bb71843d/go.mod h1:2PavIy+JPciBPrBUjwbNvtwB6RQlve+hkpll6QSNmOE= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= +github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/maruel/natural v1.1.1 h1:Hja7XhhmvEFhcByqDoHz9QZbkWey+COd9xWfCfn1ioo= github.com/maruel/natural v1.1.1/go.mod h1:v+Rfd79xlw1AgVBjbO0BEQmptqb5HvL/k9GRHB7ZKEg= github.com/mfridman/tparse v0.18.0 h1:wh6dzOKaIwkUGyKgOntDW4liXSo37qg5AXbIhkMV3vE= github.com/mfridman/tparse v0.18.0/go.mod h1:gEvqZTuCgEhPbYk/2lS3Kcxg1GmTxxU7kTC8DvP0i/A= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8= +github.com/oapi-codegen/runtime v1.2.0 h1:RvKc1CVS1QeKSNzO97FBQbSMZyQ8s6rZd+LpmzwHMP4= +github.com/oapi-codegen/runtime v1.2.0/go.mod h1:Y7ZhmmlE8ikZOmuHRRndiIm7nf3xcVv+YMweKgG1DT0= +github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 h1:G7ERwszslrBzRxj//JalHPu/3yz+De2J+4aLtSRlHiY= +github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037/go.mod h1:2bpvgLBZEtENV5scfDFEtB/5+1M4hkQhDQrccEJ/qGw= +github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 h1:bQx3WeLcUWy+RletIKwUIt4x3t8n2SxavmoclizMb8c= +github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90/go.mod h1:y5+oSEHCPT/DGrS++Wc/479ERge0zTFxaF8PbGKcg2o= github.com/onsi/ginkgo/v2 v2.28.1 h1:S4hj+HbZp40fNKuLUQOYLDgZLwNUVn19N3Atb98NCyI= github.com/onsi/ginkgo/v2 v2.28.1/go.mod h1:CLtbVInNckU3/+gC8LzkGUb9oF+e8W8TdUsxPwvdOgE= github.com/onsi/gomega v1.39.1 h1:1IJLAad4zjPn2PsnhH70V4DKRFlrCzGBNrNaru+Vf28= github.com/onsi/gomega v1.39.1/go.mod h1:hL6yVALoTOxeWudERyfppUcZXjMwIMLnuSfruD2lcfg= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= +github.com/perimeterx/marshmallow v1.1.5 h1:a2LALqQ1BlHM8PZblsDdidgv1mWi1DgC2UmX50IvK2s= +github.com/perimeterx/marshmallow v1.1.5/go.mod h1:dsXbUu8CRzfYP5a87xpp0xq9S3u0Vchtcl8we9tYaXw= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= @@ -63,6 +95,9 @@ github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= +github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad/go.mod h1:qLr4V1qq6nMqFKkMo8ZTx3f+BZEkzsRUY10Xsm2mwU0= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= @@ -75,6 +110,10 @@ github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= +github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE= +github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= +github.com/woodsbury/decimal128 v1.3.0 h1:8pffMNWIlC0O5vbyHWFZAt5yWvWcrHA+3ovIIjVWss0= +github.com/woodsbury/decimal128 v1.3.0/go.mod h1:C5UTmyTjW3JftjUFzOVhC20BEQa2a4ZKOB5I6Zjb+ds= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/mod v0.32.0 h1:9F4d3PHLljb6x//jOyokMv3eX+YDeepZSEo3mFJy93c= @@ -92,7 +131,7 @@ golang.org/x/tools v0.41.0/go.mod h1:XSY6eDqxVNiYgezAVqqCeihT4j1U2CCsqvH3WhQpnlg google.golang.org/protobuf v1.36.7 h1:IgrO7UwFQGJdRNXH/sQux4R1Dj1WAKcLElzeeRaXV2A= google.golang.org/protobuf v1.36.7/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/commands/helpers.go b/internal/commands/helpers.go new file mode 100644 index 0000000..a0448cd --- /dev/null +++ b/internal/commands/helpers.go @@ -0,0 +1,174 @@ +package commands + +import ( + "context" + "crypto/tls" + "crypto/x509" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "os" + "strings" + "time" + + "github.com/dcm-project/cli/internal/config" + "github.com/dcm-project/cli/internal/output" + "github.com/spf13/cobra" + "go.yaml.in/yaml/v3" +) + +// FormattedError indicates an error that has already been formatted and +// written to stderr. Execute() should not print it again. +type FormattedError struct{} + +func (*FormattedError) Error() string { return "" } + +// newFormatter creates a Formatter from the command's resolved configuration. +func newFormatter(cmd *cobra.Command, table *output.TableDef, command string) (*output.Formatter, error) { + cfg := config.FromCommand(cmd) + format, err := output.ParseFormat(cfg.OutputFormat) + if err != nil { + return nil, &UsageError{Err: err} + } + return output.New(format, cmd.OutOrStdout(), cmd.ErrOrStderr(), table, command), nil +} + +// buildHTTPClient creates an HTTP client from the resolved configuration. +// When the API Gateway URL uses https://, TLS is configured using the +// TLS-related settings. When it uses http://, TLS settings are ignored. +func buildHTTPClient(cfg *config.Config) (*http.Client, error) { + if !strings.HasPrefix(cfg.APIGatewayURL, "https://") { + return &http.Client{}, nil + } + + // Validate mTLS pair: both or neither must be set. + if (cfg.TLSClientCert == "") != (cfg.TLSClientKey == "") { + return nil, &UsageError{Err: fmt.Errorf("--tls-client-cert and --tls-client-key must be used together")} + } + + tlsCfg := &tls.Config{ + InsecureSkipVerify: cfg.TLSSkipVerify, + } + + if cfg.TLSCACert != "" { + caCert, err := os.ReadFile(cfg.TLSCACert) + if err != nil { + return nil, fmt.Errorf("reading CA certificate %s: %w", cfg.TLSCACert, err) + } + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM(caCert) { + return nil, fmt.Errorf("failed to parse CA certificate %s", cfg.TLSCACert) + } + tlsCfg.RootCAs = pool + } + + if cfg.TLSClientCert != "" { + cert, err := tls.LoadX509KeyPair(cfg.TLSClientCert, cfg.TLSClientKey) + if err != nil { + return nil, fmt.Errorf("loading client certificate: %w", err) + } + tlsCfg.Certificates = []tls.Certificate{cert} + } + + return &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: tlsCfg, + }, + }, nil +} + +// apiBaseURL returns the API base URL with the /api/v1alpha1 suffix. +func apiBaseURL(cfg *config.Config) string { + return strings.TrimRight(cfg.APIGatewayURL, "/") + "/api/v1alpha1" +} + +// parseInputFile reads a YAML or JSON file and returns its content as a map. +// Format detection attempts YAML parsing first (valid JSON is also valid YAML). +func parseInputFile(path string) (map[string]any, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading file %s: %w", path, err) + } + var result map[string]any + if err := yaml.Unmarshal(data, &result); err != nil { + return nil, fmt.Errorf("parsing file %s: %w", path, err) + } + if result == nil { + return nil, fmt.Errorf("parsing file %s: file does not contain a valid YAML/JSON object", path) + } + return result, nil +} + +// parseInputFileAs reads a YAML or JSON file and unmarshals its content into +// the specified type. This provides client-side validation that the payload +// matches the expected schema. +func parseInputFileAs[T any](path string) (T, error) { + var zero T + data, err := parseInputFile(path) + if err != nil { + return zero, err + } + jsonBytes, err := json.Marshal(data) + if err != nil { + return zero, fmt.Errorf("marshalling input: %w", err) + } + var result T + if err := json.Unmarshal(jsonBytes, &result); err != nil { + return zero, fmt.Errorf("invalid payload in %s: %w", path, err) + } + return result, nil +} + +// handleErrorResponse processes a non-2xx HTTP response. For RFC 7807 +// responses it formats the error via the Formatter; for other responses +// it returns a plain error with the HTTP status and body. +func handleErrorResponse(resp *http.Response, formatter *output.Formatter) error { + body, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("reading error response: %w", err) + } + + var problem output.ProblemDetail + if err := json.Unmarshal(body, &problem); err == nil && problem.Type != "" { + _ = formatter.FormatError(problem) + return &FormattedError{} + } + + // Non-RFC-7807 error response + return fmt.Errorf("HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) +} + +// requestContext returns a context with the configured timeout. +func requestContext(cmd *cobra.Command) (context.Context, context.CancelFunc) { + cfg := config.FromCommand(cmd) + return context.WithTimeout(cmd.Context(), time.Duration(cfg.Timeout)*time.Second) +} + +// connectionError wraps HTTP client errors with a user-friendly message, +// distinguishing timeout errors from connection errors. +func connectionError(err error, cfg *config.Config) error { + if isTimeoutError(err) { + return fmt.Errorf("request timed out after %d seconds", cfg.Timeout) + } + return fmt.Errorf("failed to connect to API Gateway at %s: %w", cfg.APIGatewayURL, err) +} + +// isTimeoutError checks whether an error is a timeout (context deadline or net timeout). +func isTimeoutError(err error) bool { + if errors.Is(err, context.DeadlineExceeded) { + return true + } + var netErr interface{ Timeout() bool } + return errors.As(err, &netErr) && netErr.Timeout() +} + +// stringifyValue extracts a string representation of a map value for table output. +func stringifyValue(m map[string]any, key string) string { + v, ok := m[key] + if !ok || v == nil { + return "" + } + return fmt.Sprintf("%v", v) +} diff --git a/internal/commands/helpers_test.go b/internal/commands/helpers_test.go new file mode 100644 index 0000000..567665b --- /dev/null +++ b/internal/commands/helpers_test.go @@ -0,0 +1,322 @@ +package commands_test + +import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "errors" + "math/big" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/dcm-project/cli/internal/commands" +) + +// testCA holds a self-signed CA and can issue certificates for testing. +type testCA struct { + cert *x509.Certificate + key *ecdsa.PrivateKey + certPEM []byte +} + +func newTestCA() *testCA { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).NotTo(HaveOccurred()) + + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "Test CA"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + } + + certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + Expect(err).NotTo(HaveOccurred()) + + cert, err := x509.ParseCertificate(certDER) + Expect(err).NotTo(HaveOccurred()) + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + + return &testCA{cert: cert, key: key, certPEM: certPEM} +} + +// issueCert creates a server or client certificate signed by the CA. +// Returns PEM-encoded cert and key. +func (ca *testCA) issueCert(cn string, dnsNames []string, ipAddrs ...net.IP) (certPEM, keyPEM []byte) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).NotTo(HaveOccurred()) + + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{CommonName: cn}, + DNSNames: dnsNames, + IPAddresses: ipAddrs, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + } + + certDER, err := x509.CreateCertificate(rand.Reader, tmpl, ca.cert, &key.PublicKey, ca.key) + Expect(err).NotTo(HaveOccurred()) + + keyDER, err := x509.MarshalECPrivateKey(key) + Expect(err).NotTo(HaveOccurred()) + + certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + keyPEM = pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) + return certPEM, keyPEM +} + +// writePEM writes PEM data to a temp file and returns the path. +func writePEM(dir, name string, data []byte) string { + path := filepath.Join(dir, name) + Expect(os.WriteFile(path, data, 0o600)).To(Succeed()) + return path +} + +var _ = Describe("buildHTTPClient (via commands)", func() { + var ( + outBuf *bytes.Buffer + errBuf *bytes.Buffer + ) + + BeforeEach(func() { + clearDCMEnvVars() + }) + + executeWithArgs := func(args ...string) error { + cmd := commands.NewRootCommand() + outBuf = new(bytes.Buffer) + errBuf = new(bytes.Buffer) + cmd.SetOut(outBuf) + cmd.SetErr(errBuf) + + fullArgs := make([]string, 0, 2+len(args)) + fullArgs = append(fullArgs, "--config", nonexistentConfigPath()) + fullArgs = append(fullArgs, args...) + cmd.SetArgs(fullArgs) + + return cmd.Execute() + } + + Describe("http:// URL", func() { + It("should ignore TLS flags and connect successfully", func() { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + defer server.Close() + + // TLS flags are present but should be silently ignored for http:// + err := executeWithArgs( + "--api-gateway-url", server.URL, + "--tls-skip-verify", + "policy", "list", + ) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Describe("https:// URL", func() { + It("should connect to an HTTPS server with --tls-ca-cert", func() { + ca := newTestCA() + serverCert, serverKey := ca.issueCert("localhost", []string{"localhost"}, net.IPv4(127, 0, 0, 1)) + + tlsCert, err := tls.X509KeyPair(serverCert, serverKey) + Expect(err).NotTo(HaveOccurred()) + + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + server.TLS = &tls.Config{Certificates: []tls.Certificate{tlsCert}} + server.StartTLS() + defer server.Close() + + tmpDir := GinkgoT().TempDir() + caFile := writePEM(tmpDir, "ca.pem", ca.certPEM) + + err = executeWithArgs( + "--api-gateway-url", server.URL, + "--tls-ca-cert", caFile, + "policy", "list", + ) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should connect with --tls-skip-verify", func() { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + defer server.Close() + + err := executeWithArgs( + "--api-gateway-url", server.URL, + "--tls-skip-verify", + "policy", "list", + ) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should fail without --tls-ca-cert or --tls-skip-verify against self-signed server", func() { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + defer server.Close() + + err := executeWithArgs( + "--api-gateway-url", server.URL, + "policy", "list", + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to connect")) + }) + + It("should connect with mTLS client certificate", func() { + ca := newTestCA() + serverCert, serverKey := ca.issueCert("localhost", []string{"localhost"}, net.IPv4(127, 0, 0, 1)) + clientCert, clientKey := ca.issueCert("client", nil) + + tlsCert, err := tls.X509KeyPair(serverCert, serverKey) + Expect(err).NotTo(HaveOccurred()) + + caPool := x509.NewCertPool() + caPool.AddCert(ca.cert) + + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + server.TLS = &tls.Config{ + Certificates: []tls.Certificate{tlsCert}, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: caPool, + } + server.StartTLS() + defer server.Close() + + tmpDir := GinkgoT().TempDir() + caFile := writePEM(tmpDir, "ca.pem", ca.certPEM) + certFile := writePEM(tmpDir, "client.pem", clientCert) + keyFile := writePEM(tmpDir, "client-key.pem", clientKey) + + err = executeWithArgs( + "--api-gateway-url", server.URL, + "--tls-ca-cert", caFile, + "--tls-client-cert", certFile, + "--tls-client-key", keyFile, + "policy", "list", + ) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Describe("validation errors", func() { + It("should return UsageError when only --tls-client-cert is set", func() { + tmpDir := GinkgoT().TempDir() + certFile := writePEM(tmpDir, "client.pem", []byte("dummy")) + + err := executeWithArgs( + "--api-gateway-url", "https://localhost:9999", + "--tls-client-cert", certFile, + "--tls-skip-verify", + "policy", "list", + ) + Expect(err).To(HaveOccurred()) + + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("--tls-client-cert and --tls-client-key must be used together")) + }) + + It("should return UsageError when only --tls-client-key is set", func() { + tmpDir := GinkgoT().TempDir() + keyFile := writePEM(tmpDir, "client-key.pem", []byte("dummy")) + + err := executeWithArgs( + "--api-gateway-url", "https://localhost:9999", + "--tls-client-key", keyFile, + "--tls-skip-verify", + "policy", "list", + ) + Expect(err).To(HaveOccurred()) + + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("--tls-client-cert and --tls-client-key must be used together")) + }) + + It("should return error when --tls-ca-cert file does not exist", func() { + err := executeWithArgs( + "--api-gateway-url", "https://localhost:9999", + "--tls-ca-cert", "/nonexistent/ca.pem", + "--tls-skip-verify", + "policy", "list", + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("reading CA certificate")) + + // Should be exit code 1, not 2 + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeFalse()) + }) + + It("should return error when --tls-ca-cert contains invalid PEM", func() { + tmpDir := GinkgoT().TempDir() + caFile := writePEM(tmpDir, "bad-ca.pem", []byte("not a certificate")) + + err := executeWithArgs( + "--api-gateway-url", "https://localhost:9999", + "--tls-ca-cert", caFile, + "--tls-skip-verify", + "policy", "list", + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to parse CA certificate")) + }) + + It("should return error when client cert file does not exist", func() { + tmpDir := GinkgoT().TempDir() + keyFile := writePEM(tmpDir, "client-key.pem", []byte("dummy")) + + err := executeWithArgs( + "--api-gateway-url", "https://localhost:9999", + "--tls-client-cert", "/nonexistent/client.pem", + "--tls-client-key", keyFile, + "--tls-skip-verify", + "policy", "list", + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("loading client certificate")) + }) + + It("should not validate TLS settings for http:// URL", func() { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + defer server.Close() + + // Even with mismatched mTLS flags, http:// should succeed + err := executeWithArgs( + "--api-gateway-url", server.URL, + "--tls-client-cert", "/nonexistent/client.pem", + "policy", "list", + ) + Expect(err).NotTo(HaveOccurred()) + }) + }) +}) diff --git a/internal/commands/policy.go b/internal/commands/policy.go index 9618414..65ac5e7 100644 --- a/internal/commands/policy.go +++ b/internal/commands/policy.go @@ -1,6 +1,43 @@ package commands -import "github.com/spf13/cobra" +import ( + "encoding/json" + "fmt" + "net/http" + + policyapi "github.com/dcm-project/policy-manager/api/v1alpha1" + policyclient "github.com/dcm-project/policy-manager/pkg/client" + + "github.com/dcm-project/cli/internal/config" + "github.com/dcm-project/cli/internal/output" + "github.com/spf13/cobra" +) + +var policyTableDef = &output.TableDef{ + Headers: []string{"ID", "DISPLAY NAME", "TYPE", "PRIORITY", "ENABLED", "CREATED"}, + RowFunc: func(resource any) []string { + m, ok := resource.(map[string]any) + if !ok { + return []string{"", "", "", "", "", ""} + } + return []string{ + stringifyValue(m, "id"), + stringifyValue(m, "display_name"), + stringifyValue(m, "policy_type"), + stringifyValue(m, "priority"), + stringifyValue(m, "enabled"), + stringifyValue(m, "create_time"), + } + }, +} + +func newPolicyClient(cfg *config.Config) (*policyclient.Client, error) { + httpClient, err := buildHTTPClient(cfg) + if err != nil { + return nil, err + } + return policyclient.NewClient(apiBaseURL(cfg), policyclient.WithHTTPClient(httpClient)) +} func newPolicyCommand() *cobra.Command { cmd := &cobra.Command{ @@ -18,23 +55,136 @@ func newPolicyCommand() *cobra.Command { } func newPolicyCreateCommand() *cobra.Command { - return &cobra.Command{ - Use: "create", - Short: "Create a new policy", - RunE: func(_ *cobra.Command, _ []string) error { - return nil + cmd := &cobra.Command{ + Use: "create", + Short: "Create a new policy", + PreRunE: requiredFlagsPreRun, + RunE: func(cmd *cobra.Command, _ []string) error { + fromFile, _ := cmd.Flags().GetString("from-file") + + cfg := config.FromCommand(cmd) + formatter, err := newFormatter(cmd, policyTableDef, "policy create") + if err != nil { + return err + } + + body, err := parseInputFileAs[policyapi.CreatePolicyJSONRequestBody](fromFile) + if err != nil { + return err + } + + params := &policyapi.CreatePolicyParams{} + if id, _ := cmd.Flags().GetString("id"); id != "" { + params.Id = &id + } + + client, err := newPolicyClient(cfg) + if err != nil { + return fmt.Errorf("creating policy client: %w", err) + } + + ctx, cancel := requestContext(cmd) + defer cancel() + + resp, err := client.CreatePolicy(ctx, params, body) + if err != nil { + return connectionError(err, cfg) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + return handleErrorResponse(resp, formatter) + } + + var result map[string]any + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return fmt.Errorf("parsing response: %w", err) + } + + return formatter.FormatOne(result) }, } + + cmd.Flags().String("from-file", "", "Path to policy YAML/JSON file (required)") + _ = cmd.MarkFlagRequired("from-file") + cmd.Flags().String("id", "", "Client-specified policy ID") + + return cmd } func newPolicyListCommand() *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: "list", Short: "List policies", - RunE: func(_ *cobra.Command, _ []string) error { - return nil + RunE: func(cmd *cobra.Command, _ []string) error { + cfg := config.FromCommand(cmd) + + // Build the pagination hint command string. + listCmd := "policy list" + if pageSize, _ := cmd.Flags().GetInt32("page-size"); pageSize > 0 { + listCmd += fmt.Sprintf(" --page-size %d", pageSize) + } + + formatter, err := newFormatter(cmd, policyTableDef, listCmd) + if err != nil { + return err + } + + params := &policyapi.ListPoliciesParams{} + if filter, _ := cmd.Flags().GetString("filter"); filter != "" { + params.Filter = &filter + } + if orderBy, _ := cmd.Flags().GetString("order-by"); orderBy != "" { + params.OrderBy = &orderBy + } + if pageSize, _ := cmd.Flags().GetInt32("page-size"); pageSize > 0 { + params.MaxPageSize = &pageSize + } + if pageToken, _ := cmd.Flags().GetString("page-token"); pageToken != "" { + params.PageToken = &pageToken + } + + client, err := newPolicyClient(cfg) + if err != nil { + return fmt.Errorf("creating policy client: %w", err) + } + + ctx, cancel := requestContext(cmd) + defer cancel() + + resp, err := client.ListPolicies(ctx, params) + if err != nil { + return connectionError(err, cfg) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return handleErrorResponse(resp, formatter) + } + + var listResp struct { + Policies []map[string]any `json:"policies"` + NextPageToken string `json:"next_page_token"` + } + if err := json.NewDecoder(resp.Body).Decode(&listResp); err != nil { + return fmt.Errorf("parsing response: %w", err) + } + + resources := make([]any, len(listResp.Policies)) + for i, r := range listResp.Policies { + resources[i] = r + } + + return formatter.FormatList(resources, listResp.NextPageToken) }, } + + cmd.Flags().String("filter", "", "CEL filter expression") + cmd.Flags().String("order-by", "", "Order field and direction") + cmd.Flags().Int32("page-size", 0, "Maximum results per page") + cmd.Flags().String("page-token", "", "Token for next page") + + return cmd } func newPolicyGetCommand() *cobra.Command { @@ -42,21 +192,92 @@ func newPolicyGetCommand() *cobra.Command { Use: "get POLICY_ID", Short: "Get a policy by ID", Args: ExactArgs(1), - RunE: func(_ *cobra.Command, _ []string) error { - return nil + RunE: func(cmd *cobra.Command, args []string) error { + cfg := config.FromCommand(cmd) + formatter, err := newFormatter(cmd, policyTableDef, "policy get") + if err != nil { + return err + } + + client, err := newPolicyClient(cfg) + if err != nil { + return fmt.Errorf("creating policy client: %w", err) + } + + ctx, cancel := requestContext(cmd) + defer cancel() + + resp, err := client.GetPolicy(ctx, args[0]) + if err != nil { + return connectionError(err, cfg) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return handleErrorResponse(resp, formatter) + } + + var result map[string]any + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return fmt.Errorf("parsing response: %w", err) + } + + return formatter.FormatOne(result) }, } } func newPolicyUpdateCommand() *cobra.Command { - return &cobra.Command{ - Use: "update POLICY_ID", - Short: "Update an existing policy", - Args: ExactArgs(1), - RunE: func(_ *cobra.Command, _ []string) error { - return nil + cmd := &cobra.Command{ + Use: "update POLICY_ID", + Short: "Update an existing policy", + Args: ExactArgs(1), + PreRunE: requiredFlagsPreRun, + RunE: func(cmd *cobra.Command, args []string) error { + fromFile, _ := cmd.Flags().GetString("from-file") + + cfg := config.FromCommand(cmd) + formatter, err := newFormatter(cmd, policyTableDef, "policy update") + if err != nil { + return err + } + + body, err := parseInputFileAs[policyapi.UpdatePolicyApplicationMergePatchPlusJSONRequestBody](fromFile) + if err != nil { + return err + } + + client, err := newPolicyClient(cfg) + if err != nil { + return fmt.Errorf("creating policy client: %w", err) + } + + ctx, cancel := requestContext(cmd) + defer cancel() + + resp, err := client.UpdatePolicyWithApplicationMergePatchPlusJSONBody(ctx, args[0], body) + if err != nil { + return connectionError(err, cfg) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return handleErrorResponse(resp, formatter) + } + + var result map[string]any + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return fmt.Errorf("parsing response: %w", err) + } + + return formatter.FormatOne(result) }, } + + cmd.Flags().String("from-file", "", "Path to patch YAML/JSON file (required)") + _ = cmd.MarkFlagRequired("from-file") + + return cmd } func newPolicyDeleteCommand() *cobra.Command { @@ -64,8 +285,32 @@ func newPolicyDeleteCommand() *cobra.Command { Use: "delete POLICY_ID", Short: "Delete a policy by ID", Args: ExactArgs(1), - RunE: func(_ *cobra.Command, _ []string) error { - return nil + RunE: func(cmd *cobra.Command, args []string) error { + cfg := config.FromCommand(cmd) + formatter, err := newFormatter(cmd, policyTableDef, "policy delete") + if err != nil { + return err + } + + client, err := newPolicyClient(cfg) + if err != nil { + return fmt.Errorf("creating policy client: %w", err) + } + + ctx, cancel := requestContext(cmd) + defer cancel() + + resp, err := client.DeletePolicy(ctx, args[0]) + if err != nil { + return connectionError(err, cfg) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusNoContent { + return handleErrorResponse(resp, formatter) + } + + return formatter.FormatMessage(fmt.Sprintf("Policy %q deleted successfully.", args[0])) }, } } diff --git a/internal/commands/policy_test.go b/internal/commands/policy_test.go new file mode 100644 index 0000000..695f1bf --- /dev/null +++ b/internal/commands/policy_test.go @@ -0,0 +1,649 @@ +package commands_test + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/dcm-project/cli/internal/commands" +) + +// clearDCMEnvVars removes all DCM_* environment variables to isolate tests. +func clearDCMEnvVars() { + for _, env := range []string{ + "DCM_API_GATEWAY_URL", + "DCM_OUTPUT_FORMAT", + "DCM_TIMEOUT", + "DCM_CONFIG", + "DCM_TLS_CA_CERT", + "DCM_TLS_CLIENT_CERT", + "DCM_TLS_CLIENT_KEY", + "DCM_TLS_SKIP_VERIFY", + } { + Expect(os.Unsetenv(env)).To(Succeed()) + } +} + +// nonexistentConfigPath returns a path to a config file that does not exist, +// suitable for isolating tests from the developer's ~/.dcm/config.yaml. +func nonexistentConfigPath() string { + return filepath.Join(GinkgoT().TempDir(), "nonexistent.yaml") +} + +// writeTempFile creates a temporary file with the given content and extension. +func writeTempFile(content, ext string) string { + f, err := os.CreateTemp(GinkgoT().TempDir(), "test-*"+ext) + Expect(err).NotTo(HaveOccurred()) + _, err = f.WriteString(content) + Expect(err).NotTo(HaveOccurred()) + Expect(f.Close()).To(Succeed()) + return f.Name() +} + +// samplePolicyResponse returns a sample policy JSON response body. +func samplePolicyResponse() map[string]any { + return map[string]any{ + "id": "my-policy", + "display_name": "Require CPU Limits", + "policy_type": "GLOBAL", + "priority": float64(100), + "enabled": true, + "create_time": "2026-03-09T10:00:00Z", + } +} + +// writeJSONResponse writes a JSON response with the given status code and body. +func writeJSONResponse(w http.ResponseWriter, status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + Expect(json.NewEncoder(w).Encode(v)).To(Succeed()) +} + +// writeRFC7807 writes an RFC 7807 error response. +func writeRFC7807(w http.ResponseWriter, status int, typ, title, detail string) { + body := map[string]any{ + "type": typ, + "status": status, + "title": title, + "detail": detail, + } + w.Header().Set("Content-Type", "application/problem+json") + w.WriteHeader(status) + Expect(json.NewEncoder(w).Encode(body)).To(Succeed()) +} + +// emptyListResponse returns a standard empty list response body. +func emptyListResponse() map[string]any { + return map[string]any{ + "policies": []any{}, + "next_page_token": "", + } +} + +var _ = Describe("Policy Commands", func() { + var ( + server *httptest.Server + outBuf *bytes.Buffer + errBuf *bytes.Buffer + ) + + BeforeEach(func() { + clearDCMEnvVars() + }) + + AfterEach(func() { + if server != nil { + server.Close() + server = nil + } + }) + + // executeCommand creates a root command, sets up output capture, and executes + // with the given args prepended by --api-gateway-url and --config. + executeCommand := func(args ...string) error { + cmd := commands.NewRootCommand() + outBuf = new(bytes.Buffer) + errBuf = new(bytes.Buffer) + cmd.SetOut(outBuf) + cmd.SetErr(errBuf) + + fullArgs := []string{ + "--config", nonexistentConfigPath(), + } + if server != nil { + fullArgs = append(fullArgs, "--api-gateway-url", server.URL) + } + fullArgs = append(fullArgs, args...) + cmd.SetArgs(fullArgs) + + return cmd.Execute() + } + + // --- Section 5: Policy Commands --- + + Describe("create", func() { + // TC-U026: Create policy from YAML file + It("TC-U026: should create a policy from a YAML file", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.Method).To(Equal(http.MethodPost)) + Expect(r.URL.Path).To(Equal("/api/v1alpha1/policies")) + + var body map[string]any + Expect(json.NewDecoder(r.Body).Decode(&body)).To(Succeed()) + Expect(body["display_name"]).To(Equal("Test Policy")) + + writeJSONResponse(w, http.StatusCreated, samplePolicyResponse()) + })) + + yamlFile := writeTempFile("display_name: Test Policy\npolicy_type: GLOBAL\npriority: 100\nenabled: true\n", ".yaml") + + err := executeCommand("policy", "create", "--from-file", yamlFile) + Expect(err).NotTo(HaveOccurred()) + + out := outBuf.String() + Expect(out).To(ContainSubstring("my-policy")) + Expect(out).To(ContainSubstring("Require CPU Limits")) + }) + + // TC-U027: Create policy from JSON file + It("TC-U027: should create a policy from a JSON file", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.Method).To(Equal(http.MethodPost)) + Expect(r.URL.Path).To(Equal("/api/v1alpha1/policies")) + + var body map[string]any + Expect(json.NewDecoder(r.Body).Decode(&body)).To(Succeed()) + Expect(body["display_name"]).To(Equal("JSON Policy")) + + writeJSONResponse(w, http.StatusCreated, samplePolicyResponse()) + })) + + jsonFile := writeTempFile(`{"display_name":"JSON Policy","policy_type":"GLOBAL"}`, ".json") + + err := executeCommand("policy", "create", "--from-file", jsonFile) + Expect(err).NotTo(HaveOccurred()) + }) + + // TC-U028: Create policy with client-specified ID + It("TC-U028: should send ?id= query parameter when --id is provided", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.Method).To(Equal(http.MethodPost)) + Expect(r.URL.Path).To(Equal("/api/v1alpha1/policies")) + Expect(r.URL.Query().Get("id")).To(Equal("my-policy")) + + writeJSONResponse(w, http.StatusCreated, samplePolicyResponse()) + })) + + yamlFile := writeTempFile("display_name: Test\n", ".yaml") + + err := executeCommand("policy", "create", "--from-file", yamlFile, "--id", "my-policy") + Expect(err).NotTo(HaveOccurred()) + }) + + // TC-U029: Create policy without --from-file fails + It("TC-U029: should return a UsageError when --from-file is not provided", func() { + err := executeCommand("policy", "create") + Expect(err).To(HaveOccurred()) + + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + }) + + // TC-U062: Invalid file produces error (transitively via TC-U026) + It("TC-U062: should return an error when the input file does not exist", func() { + err := executeCommand("policy", "create", "--from-file", "/nonexistent/policy.yaml") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("reading file")) + }) + + // TC-U063: Unreadable file content produces error (transitively via TC-U026) + It("TC-U063: should return an error when the input file contains non-object content", func() { + invalidFile := writeTempFile("- item1\n- item2\n", ".yaml") + + err := executeCommand("policy", "create", "--from-file", invalidFile) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("parsing file")) + }) + + // TC-U104: Create policy server error + It("TC-U104: should display error and exit code 1 on server error", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusInternalServerError, "INTERNAL", "Internal server error", "Something went wrong") + })) + + yamlFile := writeTempFile("display_name: Test\n", ".yaml") + + err := executeCommand("policy", "create", "--from-file", yamlFile) + Expect(err).To(HaveOccurred()) + + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeTrue()) + Expect(errBuf.String()).To(ContainSubstring("INTERNAL")) + }) + }) + + Describe("list", func() { + // TC-U030: List policies + It("TC-U030: should list policies", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.Method).To(Equal(http.MethodGet)) + Expect(r.URL.Path).To(Equal("/api/v1alpha1/policies")) + + writeJSONResponse(w, http.StatusOK, map[string]any{ + "policies": []any{samplePolicyResponse()}, + "next_page_token": "", + }) + })) + + err := executeCommand("policy", "list") + Expect(err).NotTo(HaveOccurred()) + + out := outBuf.String() + Expect(out).To(ContainSubstring("my-policy")) + Expect(out).To(ContainSubstring("Require CPU Limits")) + }) + + // TC-U031: List policies with filter + It("TC-U031: should pass filter query parameter", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.URL.Query().Get("filter")).To(Equal("policy_type='GLOBAL'")) + + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + + err := executeCommand("policy", "list", "--filter", "policy_type='GLOBAL'") + Expect(err).NotTo(HaveOccurred()) + }) + + // TC-U032: List policies with order-by + It("TC-U032: should pass order_by query parameter", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.URL.Query().Get("order_by")).To(Equal("priority asc")) + + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + + err := executeCommand("policy", "list", "--order-by", "priority asc") + Expect(err).NotTo(HaveOccurred()) + }) + + // TC-U033: List policies with pagination + It("TC-U033: should pass max_page_size and page_token query parameters", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.URL.Query().Get("max_page_size")).To(Equal("10")) + Expect(r.URL.Query().Get("page_token")).To(Equal("abc123")) + + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + + err := executeCommand("policy", "list", "--page-size", "10", "--page-token", "abc123") + Expect(err).NotTo(HaveOccurred()) + }) + + // TC-U100: List policies returns empty list + It("TC-U100: should display empty result for empty list", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + + err := executeCommand("policy", "list") + Expect(err).NotTo(HaveOccurred()) + + out := outBuf.String() + // Table output should have headers but no data rows + Expect(out).To(ContainSubstring("ID")) + Expect(out).To(ContainSubstring("DISPLAY NAME")) + Expect(out).NotTo(ContainSubstring("my-policy")) + }) + + // TC-U100 (JSON variant): Empty list in JSON format + It("TC-U100: should display empty results array in JSON format", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + + err := executeCommand("--output", "json", "policy", "list") + Expect(err).NotTo(HaveOccurred()) + + var result map[string]any + Expect(json.Unmarshal(outBuf.Bytes(), &result)).To(Succeed()) + Expect(result["results"]).To(BeAssignableToTypeOf([]any{})) + Expect(result["results"]).To(BeEmpty()) + }) + }) + + Describe("get", func() { + // TC-U034: Get policy + It("TC-U034: should get a policy by ID", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.Method).To(Equal(http.MethodGet)) + Expect(r.URL.Path).To(Equal("/api/v1alpha1/policies/my-policy")) + + writeJSONResponse(w, http.StatusOK, samplePolicyResponse()) + })) + + err := executeCommand("policy", "get", "my-policy") + Expect(err).NotTo(HaveOccurred()) + + out := outBuf.String() + Expect(out).To(ContainSubstring("my-policy")) + Expect(out).To(ContainSubstring("Require CPU Limits")) + }) + + // TC-U035: Get policy without POLICY_ID fails + It("TC-U035: should return a UsageError when POLICY_ID is missing", func() { + err := executeCommand("policy", "get") + Expect(err).To(HaveOccurred()) + + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + }) + + // TC-U101: Get non-existent policy + It("TC-U101: should display error for non-existent policy", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusNotFound, "NOT_FOUND", + `Policy "nonexistent" not found.`, + "The requested policy resource does not exist.") + })) + + err := executeCommand("policy", "get", "nonexistent") + Expect(err).To(HaveOccurred()) + + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeTrue()) + + errOut := errBuf.String() + Expect(errOut).To(ContainSubstring("NOT_FOUND")) + Expect(errOut).To(ContainSubstring("not found")) + Expect(outBuf.String()).To(BeEmpty()) + }) + + // TC-U041: Policy table output columns + It("TC-U041: should display correct table columns", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeJSONResponse(w, http.StatusOK, samplePolicyResponse()) + })) + + err := executeCommand("policy", "get", "my-policy") + Expect(err).NotTo(HaveOccurred()) + + out := outBuf.String() + Expect(out).To(ContainSubstring("ID")) + Expect(out).To(ContainSubstring("DISPLAY NAME")) + Expect(out).To(ContainSubstring("TYPE")) + Expect(out).To(ContainSubstring("PRIORITY")) + Expect(out).To(ContainSubstring("ENABLED")) + Expect(out).To(ContainSubstring("CREATED")) + Expect(out).To(ContainSubstring("my-policy")) + Expect(out).To(ContainSubstring("Require CPU Limits")) + Expect(out).To(ContainSubstring("GLOBAL")) + Expect(out).To(ContainSubstring("100")) + Expect(out).To(ContainSubstring("true")) + Expect(out).To(ContainSubstring("2026-03-09T10:00:00Z")) + }) + }) + + Describe("update", func() { + // TC-U036: Update policy + It("TC-U036: should update a policy with a patch file", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.Method).To(Equal(http.MethodPatch)) + Expect(r.URL.Path).To(Equal("/api/v1alpha1/policies/my-policy")) + + var body map[string]any + Expect(json.NewDecoder(r.Body).Decode(&body)).To(Succeed()) + Expect(body["priority"]).To(Equal(float64(50))) + + resp := samplePolicyResponse() + resp["priority"] = float64(50) + writeJSONResponse(w, http.StatusOK, resp) + })) + + patchFile := writeTempFile("priority: 50\n", ".yaml") + + err := executeCommand("policy", "update", "my-policy", "--from-file", patchFile) + Expect(err).NotTo(HaveOccurred()) + + out := outBuf.String() + Expect(out).To(ContainSubstring("my-policy")) + Expect(out).To(ContainSubstring("50")) + }) + + // TC-U037: Update policy without --from-file fails + It("TC-U037: should return a UsageError when --from-file is not provided", func() { + err := executeCommand("policy", "update", "my-policy") + Expect(err).To(HaveOccurred()) + + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + }) + + // TC-U038: Update policy without POLICY_ID fails + It("TC-U038: should return a UsageError when POLICY_ID is missing", func() { + err := executeCommand("policy", "update") + Expect(err).To(HaveOccurred()) + + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + }) + + // TC-U102: Update non-existent policy + It("TC-U102: should display error for non-existent policy", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusNotFound, "NOT_FOUND", + `Policy "nonexistent" not found.`, + "The requested policy resource does not exist.") + })) + + patchFile := writeTempFile("priority: 50\n", ".yaml") + + err := executeCommand("policy", "update", "nonexistent", "--from-file", patchFile) + Expect(err).To(HaveOccurred()) + + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeTrue()) + Expect(errBuf.String()).To(ContainSubstring("NOT_FOUND")) + }) + }) + + Describe("delete", func() { + // TC-U039: Delete policy + It("TC-U039: should delete a policy and display success message", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.Method).To(Equal(http.MethodDelete)) + Expect(r.URL.Path).To(Equal("/api/v1alpha1/policies/my-policy")) + + w.WriteHeader(http.StatusNoContent) + })) + + err := executeCommand("policy", "delete", "my-policy") + Expect(err).NotTo(HaveOccurred()) + + out := outBuf.String() + Expect(out).To(ContainSubstring(`Policy "my-policy" deleted successfully.`)) + }) + + // TC-U040: Delete policy without POLICY_ID fails + It("TC-U040: should return a UsageError when POLICY_ID is missing", func() { + err := executeCommand("policy", "delete") + Expect(err).To(HaveOccurred()) + + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + }) + + // TC-U103: Delete non-existent policy + It("TC-U103: should display error for non-existent policy", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusNotFound, "NOT_FOUND", + `Policy "nonexistent" not found.`, + "The requested policy resource does not exist.") + })) + + err := executeCommand("policy", "delete", "nonexistent") + Expect(err).To(HaveOccurred()) + + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeTrue()) + Expect(errBuf.String()).To(ContainSubstring("NOT_FOUND")) + }) + }) + + // --- Section 10: Error Handling --- + + Describe("Error Handling", func() { + // TC-U080: API error displayed in table format + It("TC-U080: should display RFC 7807 error in table format", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusNotFound, "NOT_FOUND", + `Policy "nonexistent" not found.`, + "The requested policy resource does not exist.") + })) + + err := executeCommand("policy", "get", "nonexistent") + Expect(err).To(HaveOccurred()) + + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeTrue()) + + errOut := errBuf.String() + Expect(errOut).To(ContainSubstring(`Error: NOT_FOUND - Policy "nonexistent" not found.`)) + Expect(errOut).To(ContainSubstring("Status: 404")) + Expect(errOut).To(ContainSubstring("Detail: The requested policy resource does not exist.")) + Expect(outBuf.String()).To(BeEmpty()) + }) + + // TC-U081: API error displayed in JSON format + It("TC-U081: should display RFC 7807 error in JSON format", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusNotFound, "NOT_FOUND", + `Policy "nonexistent" not found.`, + "The requested policy resource does not exist.") + })) + + err := executeCommand("--output", "json", "policy", "get", "nonexistent") + Expect(err).To(HaveOccurred()) + + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeTrue()) + + // stderr should contain valid JSON with Problem Details + var problem map[string]any + Expect(json.Unmarshal(errBuf.Bytes(), &problem)).To(Succeed()) + Expect(problem["type"]).To(Equal("NOT_FOUND")) + Expect(problem["status"]).To(BeNumerically("==", 404)) + Expect(outBuf.String()).To(BeEmpty()) + }) + + // TC-U082: API error displayed in YAML format + It("TC-U082: should display RFC 7807 error in YAML format", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusNotFound, "NOT_FOUND", + `Policy "nonexistent" not found.`, + "The requested policy resource does not exist.") + })) + + err := executeCommand("--output", "yaml", "policy", "get", "nonexistent") + Expect(err).To(HaveOccurred()) + + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeTrue()) + + errOut := errBuf.String() + Expect(errOut).To(ContainSubstring("type: NOT_FOUND")) + Expect(errOut).To(ContainSubstring("status: 404")) + Expect(outBuf.String()).To(BeEmpty()) + }) + + // TC-U083: Connection error displays clear message + It("TC-U083: should display a connection error when API Gateway is unreachable", func() { + // Use a server that is immediately closed to simulate unreachable + closedServer := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})) + closedURL := closedServer.URL + closedServer.Close() + + cmd := commands.NewRootCommand() + outBuf = new(bytes.Buffer) + errBuf = new(bytes.Buffer) + cmd.SetOut(outBuf) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", nonexistentConfigPath(), + "--api-gateway-url", closedURL, + "policy", "list", + }) + + err := cmd.Execute() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to connect")) + }) + + // TC-U084: Timeout error displays clear message + It("TC-U084: should display a timeout error when request exceeds timeout", func() { + server = httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + // Delay longer than the configured timeout + time.Sleep(3 * time.Second) + })) + + err := executeCommand("--timeout", "1", "policy", "list") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("timed out")) + }) + + // TC-U085: Exit code 1 on runtime error + It("TC-U085: should return a non-UsageError for server errors", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusInternalServerError, "INTERNAL", "Internal error", "Something broke") + })) + + err := executeCommand("policy", "list") + Expect(err).To(HaveOccurred()) + + // Should NOT be a UsageError (exit code 1, not 2) + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeFalse()) + }) + + // TC-U116: Error output written to stderr (command-level verification) + It("TC-U116: should write error output to stderr and nothing to stdout", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeRFC7807(w, http.StatusNotFound, "NOT_FOUND", "Not found", "Resource not found") + })) + + err := executeCommand("policy", "get", "nonexistent") + Expect(err).To(HaveOccurred()) + + Expect(errBuf.String()).NotTo(BeEmpty()) + Expect(outBuf.String()).To(BeEmpty()) + }) + + // TC-U120: Non-RFC-7807 error response + It("TC-U120: should display HTTP status and body for non-RFC-7807 errors", func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadGateway) + _, err := fmt.Fprint(w, "Bad Gateway") + Expect(err).NotTo(HaveOccurred()) + })) + + err := executeCommand("policy", "list") + Expect(err).To(HaveOccurred()) + + // Should NOT be a FormattedError (plain error message) + var fmtErr *commands.FormattedError + Expect(errors.As(err, &fmtErr)).To(BeFalse()) + + Expect(err.Error()).To(ContainSubstring("502")) + Expect(err.Error()).To(ContainSubstring("Bad Gateway")) + }) + }) +}) diff --git a/internal/commands/root.go b/internal/commands/root.go index a190f70..12f15a1 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -65,7 +65,10 @@ func Execute() { cmd := NewRootCommand() err := cmd.Execute() if err != nil { - _, _ = fmt.Fprintln(cmd.ErrOrStderr(), err) + var fmtErr *FormattedError + if !errors.As(err, &fmtErr) { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), err) + } os.Exit(getExitCode(err)) } } @@ -101,3 +104,13 @@ func ExactArgs(n int) cobra.PositionalArgs { return nil } } + +// requiredFlagsPreRun is a PreRunE hook that wraps Cobra's required-flag +// validation errors as UsageError (exit code 2). Cobra's own +// ValidateRequiredFlags call that follows PreRunE becomes a no-op. +func requiredFlagsPreRun(cmd *cobra.Command, _ []string) error { + if err := cmd.ValidateRequiredFlags(); err != nil { + return &UsageError{Err: err} + } + return nil +}