diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md new file mode 100644 index 0000000..0714d04 --- /dev/null +++ b/.claude/skills/commit/SKILL.md @@ -0,0 +1,56 @@ +--- +name: commit +description: Commit work in the iics-cli repo with the project's checks - branch guard (dev only), gofmt, go vet, golangci-lint, tests, docs and completions, CR and bug file moves, then a conventional commit without a Co-Authored-By trailer. Use when asked to commit, save, or check in changes in this repo, including "commit and push". +--- + +# Commit + +Follow these steps in order. Stop and report on any failure. + +## 1. Branch guard + +Run `git branch --show-current`. Continue only on `dev`; otherwise stop and ask. + +## 2. Checks + +```bash +gofmt -s -w . +go vet ./... +golangci-lint run ./... +go test ./... +``` + +Fix what they report, then run them again. If `go.mod` changed, check that its +Go version matches `go-version` in `.github/workflows/*.yml`. + +## 3. Completeness + +Check the diff (`git status --short`, `git diff --stat`) against these rules: + +- A command or flag was added or changed: `docs/documentation/.md` + and the `README.md` commands table are updated, and `make completions` was + run. Commit the `completions/` changes with the code. +- A user-visible fix or feature: `CHANGELOG.md` has an entry under + `## [Unreleased]`. +- A CR was implemented: its file moves from `docs/ChangeRequests/new/` to + `pending/` in the same commit. A bug was fixed: same for `docs/issues/`. + Move files to `completed/` only when the user confirms. +- Markdown files were changed: they lint clean (`markdown-lint` skill). + +Report anything missing and fix it, or ask if it needs a decision. + +## 4. Stage and commit + +- Stage the files of this change by path (`git add `). Do not use + `git add -A`; leave unrelated or unknown files unstaged and mention them. +- Write a conventional commit message (`feat:`, `fix:`, `docs:`, `chore:`, + `refactor:`, `test:`; scope optional, CR number in the subject when there is + one, for example `feat(agent): ... (CR-0049)`). The body says why. +- No `Co-Authored-By` trailer (project rule; it overrides the default + attribution). +- Commit with a heredoc (`git commit -F - <<'EOF'`), then show + `git log --oneline -1` and `git status --short`. + +## 5. Push + +Push only when the user asked for it: `git push origin dev`. diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md new file mode 100644 index 0000000..59c4fa2 --- /dev/null +++ b/.claude/skills/release/SKILL.md @@ -0,0 +1,114 @@ +--- +name: release +description: Release a new iics-cli version from dev - checks, CHANGELOG, a release PR to main, fast-forward (keeps every commit) or squash merge, tag, GoReleaser, and CI monitoring. Use when asked to release, cut, tag or publish a version, or to merge dev into main for a release. +--- + +# Release + +The release goes from `dev` to `main` through a pull request, which records the +release and runs CI on it. How it lands on `main`: + +- **Fast-forward (default):** push `dev` to `main`. Every commit is kept, + `main` and `dev` stay identical, and GitHub marks the PR as merged because + its commits are now on `main`. Needs your bypass of the `main` ruleset while + it requires pull requests. +- **Squash (when the user asks for one commit):** `gh pr merge --squash`, then + merge `main` back into `dev`. + +Stop and report on any failure. Ask before every step that pushes. + +## 1. Branch guard and sync + +```bash +git branch --show-current # must be dev +git fetch origin +git status --short # must be clean +git merge-base --is-ancestor origin/main dev # fast-forward possible +``` + +If `dev` is behind `origin/dev`, stop. If `main` is not an ancestor of `dev` +(a squash release or a commit made on `main`), a fast-forward is impossible: +report it and offer squash, or `git merge origin/main` into `dev` first. + +## 2. Checks + +```bash +gofmt -s -l . # must print nothing +go vet ./... +golangci-lint run ./... +go test -race ./... +``` + +Check that the Go version in `go.mod` matches `go-version` in +`.github/workflows/*.yml`, and that `make completions` leaves no changes. + +## 3. Pending CRs and bugs + +List the files in `docs/ChangeRequests/pending/` and `docs/issues/pending/` +with the commit that implemented each (`git log --oneline -- `). Ask the +user which ones are confirmed. Move only those with `git mv` to `completed/` +and commit: `chore: move to completed`. + +## 4. Version + +Show the latest tag (`git tag --sort=-version:refname | head -1`) and the +commits since it (`git log --oneline ..dev`). Suggest a version: minor +for features or breaking changes, patch for fixes only. Ask the user to +confirm; it must match `v\d+\.\d+\.\d+`. + +## 5. CHANGELOG + +In `CHANGELOG.md`, rename `## [Unreleased]` to `## [X.Y.Z] - YYYY-MM-DD` +(today) and add an empty `## [Unreleased]` above it. Edit in place; do not +rewrite the file. Check that breaking changes to commands, flags or output +fields are called out. Lint the file (`markdown-lint` skill), then commit +`chore: update CHANGELOG for vX.Y.Z release` (no `Co-Authored-By` trailer) +and push `dev`. + +## 6. Release PR + +```bash +gh pr create --base main --head dev --title "Release vX.Y.Z" --body-file +``` + +The body: one line on what the release is, the merge method, and the +CHANGELOG section for the version. End it with the PR attribution line. Wait +for the PR checks (`gh pr checks --watch`); stop if any fail. + +## 7. Land on main + +Fast-forward: + +```bash +git push origin dev:main +gh pr view --json state # MERGED +``` + +Squash, only when asked: + +```bash +gh pr merge --squash --subject "Release vX.Y.Z (#)" +git fetch origin +git merge --no-edit origin/main # on dev +git push origin dev +``` + +## 8. Tag + +Tag the commit now on `main` and push the tag; it starts the GoReleaser +workflow. + +```bash +git tag -a vX.Y.Z -m "Release vX.Y.Z" origin/main +git push origin vX.Y.Z +``` + +## 9. Monitor + +```bash +gh run list --workflow Release --limit 3 +gh run watch --exit-status +gh release view vX.Y.Z +``` + +Report the result of the Release job and the release URL with its assets. diff --git a/.gitignore b/.gitignore index 2ae3407..d9c0202 100644 --- a/.gitignore +++ b/.gitignore @@ -44,8 +44,9 @@ dist/ # Debug binaries __debug_bin* -# Claude Code runtime artifacts -.claude/ +# Claude Code runtime artifacts; the project skills are versioned +.claude/* +!.claude/skills/ target/ # Demo recording masking rules (they name the values they hide) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e852dd..eee912d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.6.1] - 2026-10-23 + +### Fixed + +- `agent configs set`, `agent configs edit` and `agent rename` failed with + HTTP 500 after an Administrator UI change. The agent form's property rows + are now sent in the `configFormJson` field, as the UI sends them. +- `agent configs` no longer reports values that differ from the default only + in line endings or whitespace as overridden. Setting such a value, or + resetting a property that is not overridden, is no longer shown as a + change. +- `agent restart --blocking` and `agent start --blocking` timed out while + Process Server was still starting. `--max-wait-time` now defaults to 1200 + seconds (20 minutes) for `start` and `restart` (`stop` keeps 300), poll + lines label subStates (`-1 - Initializing`, `2 - Starting`, + `0 - Operational`), and a timeout reports the last state seen. +- `agent details --full` showed every property as not customized, because the + API's `customized` flag only marks custom properties. The `CUSTOMIZED` + column is replaced by `OVERRIDDEN` (`custom`, `yes` or empty), computed from + value and default as in `agent configs get`, and JSON/YAML output adds an + `overridden` field. + ## [0.6.0] - 2026-10-20 First production-ready release. The core functionality (profiles and diff --git a/CLAUDE.md b/CLAUDE.md index 65525d7..49bb1e4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,6 +85,8 @@ golangci-lint run ./... ## Commit Style +- Commit with the `commit` skill and release with the `release` skill + (`.claude/skills/`); they carry the full checklists below. - Do **not** include `Co-Authored-By: Claude ...` trailers in commit messages. - **Before every commit**, run the following and fix all reported issues: @@ -146,7 +148,9 @@ Commit the updated files in `completions/` together with the code change. ## Markdown Rules -- Generate Markdown that follows the [Markdown Lint Rules](https://github.com/markdownlint/markdownlint/blob/main/docs/RULES.md) +- Generate Markdown that follows the [Markdown Lint Rules](https://github.com/markdownlint/markdownlint/blob/main/docs/RULES.md). + `.markdownlint-cli2.yaml` holds the project rules; a global hook formats and + lints every Markdown file Claude writes (prettier, then markdownlint-cli2). - Do **not** use em dashes (`-`) in generated Markdown; use a regular hyphen (`-`) instead See [docs/CLAUDE.md](docs/CLAUDE.md) for complete code templates for all four files. diff --git a/cmd/agent.go b/cmd/agent.go index d3d160e..e14692e 100644 --- a/cmd/agent.go +++ b/cmd/agent.go @@ -264,12 +264,15 @@ var agentEngineConfigCols = []output.Column{ {Header: "NAME", Field: "name", Width: 30}, {Header: "VALUE", Field: "value", Width: 40, Func: wrapConfigField("value")}, {Header: "DEFAULT", Field: "defaultValue", Width: 40, Func: wrapConfigField("defaultValue")}, - {Header: "CUSTOMIZED", Field: "customized", Width: 10, Func: func(v interface{}) string { + {Header: "OVERRIDDEN", Field: "overridden", Width: 10, Func: func(v interface{}) string { row, _ := v.(map[string]interface{}) if b, _ := row["customized"].(bool); b { + return "custom" + } + if b, _ := row["overridden"].(bool); b { return "yes" } - return "no" + return "" }}, } diff --git a/cmd/agent_service.go b/cmd/agent_service.go index 938df81..7ab75a1 100644 --- a/cmd/agent_service.go +++ b/cmd/agent_service.go @@ -24,7 +24,14 @@ type agentServiceFlags struct { maxWaitTime int } -func (f *agentServiceFlags) bind(cmd *cobra.Command, past string) { +// Default --max-wait-time values. Process Server can take well over five +// minutes to become operational after a start. +const ( + defaultStopWaitSec = 300 + defaultStartWaitSec = 1200 +) + +func (f *agentServiceFlags) bind(cmd *cobra.Command, past string, maxWaitSec int) { cmd.Flags().StringVar(&f.id, "id", "", "agent ID") cmd.Flags().StringVar(&f.name, "name", "", "agent name") cmd.Flags().StringVar(&f.hostname, "hostname", "", "agent host name") @@ -32,7 +39,7 @@ func (f *agentServiceFlags) bind(cmd *cobra.Command, past string) { cmd.Flags().BoolVarP(&f.interactive, "interactive", "i", false, "select the agent and service interactively") cmd.Flags().BoolVar(&f.blocking, "blocking", false, fmt.Sprintf("poll the service status until it has %s", past)) cmd.Flags().IntVar(&f.pollInterval, "poll-interval", 10, "seconds between status polls (with --blocking)") - cmd.Flags().IntVar(&f.maxWaitTime, "max-wait-time", 300, "maximum seconds to wait (with --blocking)") + cmd.Flags().IntVar(&f.maxWaitTime, "max-wait-time", maxWaitSec, "maximum seconds to wait (with --blocking)") cmd.MarkFlagsMutuallyExclusive("id", "name", "hostname") } @@ -149,7 +156,11 @@ func newAgentServiceCmd(use, short string, action client.AgentServiceAction, pas return nil }, } - f.bind(cmd, past) + maxWait := defaultStopWaitSec + if action == client.AgentServiceStart { + maxWait = defaultStartWaitSec + } + f.bind(cmd, past, maxWait) return cmd } @@ -163,7 +174,9 @@ func newAgentRestartCmd() *cobra.Command { The service is stopped and the command always waits for it to leave the agent's service listing, then it is started again. With --blocking (or after the interactive prompt) it also waits for the service to come back up (RUNNING with -subState 0). --max-wait-time bounds the whole operation.`, +subState 0). --max-wait-time bounds the whole operation; the default of 20 +minutes allows for services such as Process Server that take well over five +minutes to start.`, Example: ` iics agent restart --id --service "Data Integration Server" iics agent restart -i`, RunE: func(cmd *cobra.Command, args []string) error { @@ -223,7 +236,7 @@ subState 0). --max-wait-time bounds the whole operation.`, return nil }, } - f.bind(cmd, "restarted") + f.bind(cmd, "restarted", defaultStartWaitSec) return cmd } @@ -304,6 +317,20 @@ func serviceStates(d *client.AgentDetails, svc string) []client.AgentEngineStatu return out } +// subStateLabel describes an engine subState in the poll log. "2" is shown as +// a warning in the Administrator UI, but the service is still starting. +func subStateLabel(sub string) string { + switch sub { + case "0": + return "Operational" + case "-1": + return "Initializing" + case "2": + return "Starting" + } + return "" +} + func servicePending(status string) bool { switch strings.ToUpper(status) { case "NEED_RUNNING", "NEED_STOP", "DEPLOYING", "STARTING", "STOPPING": @@ -328,8 +355,8 @@ func serviceReached(verb string, states []client.AgentEngineStatus) (bool, error return false, nil } if strings.EqualFold(s.Status, "RUNNING") { - // subState "0" means the engine is fully operational; "-1" - // (and other values) mean it is still initializing. + // subState "0" means the engine is fully operational; "-1", + // "2" and other values mean it is still initializing. if s.SubState == "0" { operational = true } else { @@ -372,8 +399,15 @@ func waitForAgentService(ctx context.Context, c *client.Client, agentID, svc, ve if len(states) == 0 { _, _ = fmt.Fprintf(w, "%s%s: (not reported)\n", ts(), svc) } + var last []string for _, s := range states { - _, _ = fmt.Fprintf(w, "%s%s: %s (desired %s, subState %s)\n", ts(), svc, s.Status, s.DesiredStatus, s.SubState) + sub := s.SubState + if l := subStateLabel(sub); l != "" { + sub += " - " + l + } + line := fmt.Sprintf("%s (desired %s, subState %s)", s.Status, s.DesiredStatus, sub) + last = append(last, line) + _, _ = fmt.Fprintf(w, "%s%s: %s\n", ts(), svc, line) } done, rerr := serviceReached(verb, states) @@ -385,7 +419,11 @@ func waitForAgentService(ctx context.Context, c *client.Client, agentID, svc, ve return nil } if time.Now().After(deadline) { - return fmt.Errorf("timed out waiting for service %q to %s", svc, verb) + if len(last) == 0 { + last = []string{"not reported"} + } + return fmt.Errorf("timed out waiting for service %q to %s; last state: %s (raise --max-wait-time to wait longer)", + svc, verb, strings.Join(last, "; ")) } } } diff --git a/docs/ChangeRequests/pending/CR-0049-agent-level-service-properties.md b/docs/ChangeRequests/completed/CR-0049-agent-level-service-properties.md similarity index 100% rename from docs/ChangeRequests/pending/CR-0049-agent-level-service-properties.md rename to docs/ChangeRequests/completed/CR-0049-agent-level-service-properties.md diff --git a/docs/documentation/agent.md b/docs/documentation/agent.md index 6bc8470..6915761 100644 --- a/docs/documentation/agent.md +++ b/docs/documentation/agent.md @@ -185,13 +185,20 @@ Table mode prints, in order: a vertical status listing, and (with `--full`) a configuration table. Configuration tables have columns `TYPE`, `NAME`, `VALUE`, `DEFAULT`, -`CUSTOMIZED`. Long `VALUE` / `DEFAULT` cells are wrapped onto multiple lines. +`OVERRIDDEN`. Long `VALUE` / `DEFAULT` cells are wrapped onto multiple lines. +`OVERRIDDEN` follows [`agent configs get`](#agent-configs-get): `custom` for a +custom property (Custom Configuration Details), `yes` for a property whose +value differs from its default (ignoring line endings and whitespace), empty +otherwise. The API's own `customized` flag marks custom properties only, so it +is not used for overrides. With `--services`, only the `Agent:` summary and a single `Services (N):` table are printed, one row per service with columns `SERVICE`, `APP NAME`, `VERSION`, `STATUS`, `DESIRED`, `SUBSTATE`, `REPLACE`, `UPDATED`. -`--output json` / `yaml` render the full nested details document. +`--output json` / `yaml` render the full nested details document. Each +configuration property also has a computed `overridden` field, next to the +API's `customized` field. ### Examples @@ -328,6 +335,11 @@ Table mode prints the layout of `agent details --full`: an `Agent: header and a table with columns `TYPE` (with `/subType` for `agent` properties), `NAME`, `VALUE`, `DEFAULT`, `OVERRIDDEN`. Long values are wrapped. +A value that differs from its default only in line endings, trailing +whitespace on a line, or leading and trailing blank lines is shown as not +overridden, even when the server flags it as overridden. Indentation counts, +as it is significant in YAML values. + `--output csv` prints one flat table with an extra `SERVICE` column. `--output json` / `yaml` list the properties with `service`, `serviceName`, `type`, `subType`, `name`, `value`, `defaultValue`, `overridden`, @@ -393,6 +405,11 @@ Rules: on top. Check the list with `--dry-run` first. - Setting a property to its current value changes nothing, so a default does not turn into an override. Use `--reset` to go back to the default. + Values are compared ignoring line endings, trailing whitespace on a line, + and leading and trailing blank lines, so a YAML file with an extra trailing + newline is not a change. +- Setting a property to its default value resets it, and resetting a + property that is not overridden changes nothing. - A property can be changed only once per command. - If the agent's Secure Agent group has group-level properties, the group's settings replace the agent's when the agent is added to or restarted in the @@ -680,7 +697,7 @@ Start or stop a service on a Secure Agent. Uses | `--interactive`, `-i` | bool | false | Select the agent and service interactively | | `--blocking` | bool | false | Poll the service status until it has started/stopped, printing each poll | | `--poll-interval` | int | 10 | Seconds between status polls (with `--blocking`) | -| `--max-wait-time` | int | 300 | Maximum seconds to wait (with `--blocking`) | +| `--max-wait-time` | int | 1200 | Maximum seconds to wait (with `--blocking`); `stop` defaults to 300 | Exactly one of `--id` / `--name` / `--hostname` is required, unless `--interactive` is used. `--interactive` lists the Secure Agents to choose from, @@ -694,8 +711,26 @@ asks for confirmation, and (unless `--blocking` was passed explicitly) asks With `--blocking` the command returns only once the service is fully started (`RUNNING` with `subState 0`) or stopped (gone from the listing), or -`--max-wait-time` elapses (non-zero exit). A service in `ERROR` also fails the -wait. +`--max-wait-time` elapses (non-zero exit, with the last state seen). A service +in `ERROR` also fails the wait. The default wait is 20 minutes for `start` and +5 minutes for `stop`, because services such as Process Server can take well +over five minutes to become operational. + +Each poll prints the status, desired status and subState. Known subStates are +labeled: + +| subState | Label | Meaning | +| -------- | -------------- | ---------------------------------------------------------------------------------------------------------- | +| `-1` | `Initializing` | The service has been deployed and is initializing | +| `2` | `Starting` | The service is still starting; the Administrator UI shows a warning state, but the wait continues | +| `0` | `Operational` | The service is fully started | + +```text +[2026-10-23 09:55:20.451] Process Server: RUNNING (desired RUNNING, subState -1 - Initializing) +[2026-10-23 09:56:24.102] Process Server: RUNNING (desired RUNNING, subState 2 - Starting) +[2026-10-23 10:01:44.530] Process Server: RUNNING (desired RUNNING, subState 0 - Operational) +[2026-10-23 10:01:44.530] Service "Process Server" started. +``` All [global flags](../../README.md#global-flags) apply. @@ -731,8 +766,9 @@ Same as `agent start` / `agent stop` The wait for the **stop** to complete is always performed. The wait for the **start** to complete happens with `--blocking`, or after answering yes to the interactive `Wait for service "" to start` prompt. `--max-wait-time` -bounds the whole operation (stop wait + start wait combined). "Fully started" -means the engine reports `RUNNING` with `subState 0`. +bounds the whole operation (stop wait + start wait combined) and defaults to +1200 seconds (20 minutes). "Fully started" means the engine reports `RUNNING` +with `subState 0`; see [subState labels](#agent-start--agent-stop). ### Examples diff --git a/docs/issues/completed/Bug-agent-configs-set-http-500-configFormJson.md b/docs/issues/completed/Bug-agent-configs-set-http-500-configFormJson.md new file mode 100644 index 0000000..c6994f1 --- /dev/null +++ b/docs/issues/completed/Bug-agent-configs-set-http-500-configFormJson.md @@ -0,0 +1,93 @@ +# Bug: agent configs set fails with HTTP 500 - form rows must be packed into configFormJson + +## Summary + +`iics agent configs set` (and `agent configs edit`, `agent rename`) fail with +HTTP 500 `CMN_001` when saving the agent form. Informatica changed the +Administrator UI form at `/runtime/app/agent.do` (internal ticket POP-88806): +the property rows are no longer submitted as flat form parameters but bundled +into one hidden JSON field, `configFormJson`, because the flat form exceeded +Tomcat's `maxParameterCount`. The CLI still sent about 1,100 flat parameters, +so the server rejected the POST. + +## Error + +```text +Changes for agent CAI-A03 (01D23X0800000000008G): +... +Error: IICS API error (HTTP 500): +HTTP 500 Internal Server Error +... + { + "error": { + "code": "CMN_001", + "message": "Internal system error. Contact Informatica Global Customer Support.", + "requestId": "1Z71Pf2BP2gdRKnkdT7SIs" + } + } +``` + +The form GET still works; only the POST fails, with an upstream service time +of a few milliseconds. + +## Root Cause + +The UI's `doUpdate` now calls `pnAgent.packConfigFormParams(form)` from +`/runtime/js/pnAgent.js` before submitting. It collects every element named +in `configFormParamNames` (`sysConfig*`, `customConfig*`, `engineConfig*`, +`engineCustomConfigIdInput`, `isSensitiveConfig`), stores +`{"name": ["value", ...]}` in `configFormJson`, and disables the original +elements so they are not posted. Unchecked checkboxes are left out, and +names without elements are omitted. The server expands the JSON with +`ConfigFormRequestWrapper`. + +`AgentConfigForm.encode()` in `internal/client/agentform.go` posted all rows as +flat parameters and left `configFormJson` empty. + +## Fix + +`encode()` now passes the submitted fields through `packConfigFields`. When the +form contains a `configFormJson` field, the row fields are moved into it as +JSON in the UI's key order, with LF line endings as the browser's +`element.value` has. Forms without the field are still sent flat. + +Verified live on tst agent CAI-A03: setting +`process-engine:connectors:https-maxThreads=601` saves and reads back. All +other property values are unchanged. Four multi-line Process Server properties +(`server:ldap-properties`, `db:connection-properties`, +`db:logConnection-properties`, `logging:additional-logging`) whose values +equal their defaults were flagged overridden before the save and are now +reported as not overridden, the same as a save from the UI. + +## Follow-up: whitespace-only overrides + +The server flags a value as overridden when it differs from the default only +in line endings or whitespace. On tst CAI-A01 and CAI-A02 the four multi-line +Process Server properties above were listed by `--overridden` and exported by +`--export` for that reason alone. + +`client.SameValue` compares values ignoring CRLF/LF differences, trailing +whitespace on a line, and leading and trailing blank lines. Indentation +counts, as it is significant in YAML. It is used: + +- when the form is parsed, to clear the overridden flag of such values, so + `get --overridden`, `--export`, `--reset-all` and the editor agree +- in `Apply`, so a whitespace-only `--set` is no change, a value equal to the + default resets the property, and resetting a property that is not + overridden is no change +- when verifying a save, and in the TUI editor's pending-change check + +## Steps to Reproduce + +```bash +iics --profile tst agent configs set --name CAI-A03 \ + --set "process-engine:connectors:https-maxThreads=601" +``` + +## Expected Behaviour + +The property is saved and the restart hint is printed. + +## Actual Behaviour + +HTTP 500 `CMN_001` from `POST /runtime/app/agent.do`. diff --git a/internal/client/agentform.go b/internal/client/agentform.go index f6e55db..f33309f 100644 --- a/internal/client/agentform.go +++ b/internal/client/agentform.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "encoding/base64" + "encoding/json" "errors" "fmt" "html" @@ -117,6 +118,20 @@ func normalizeNewlines(s string) string { return strings.ReplaceAll(strings.ReplaceAll(s, "\r\n", "\n"), "\r", "\n") } +// SameValue reports whether a and b differ only in line endings, trailing +// whitespace on a line, or leading and trailing blank lines. Indentation +// counts, as it is significant in YAML values. +func SameValue(a, b string) bool { + norm := func(s string) string { + lines := strings.Split(normalizeNewlines(s), "\n") + for i, l := range lines { + lines[i] = strings.TrimRight(l, " \t") + } + return strings.Trim(strings.Join(lines, "\n"), "\n") + } + return a == b || norm(a) == norm(b) +} + func parseAttrs(s string) map[string]string { attrs := map[string]string{} for _, m := range formAttrRe.FindAllStringSubmatch(s, -1) { @@ -312,6 +327,12 @@ func parseAgentConfigForm(page string) (*AgentConfigForm, error) { if p.Custom && p.ServiceName == "" { p.ServiceName = form.serviceNames[p.serviceKey] } + // The server flags a value as overridden when it differs from the + // default only in line endings or whitespace, for example after a + // save with CRLF line breaks. Such values are the default. + if !p.Custom && p.Overridden && !p.Sensitive && SameValue(p.Value, p.DefaultValue) { + p.Overridden = false + } } return form, nil } @@ -413,10 +434,75 @@ func (f *AgentConfigForm) Reset(i int) { f.fields[p.overIdx].value = "false" } +// configFormJSONField is the hidden field that carries the property rows. +// The UI (pnAgent.packConfigFormParams) bundles the fields named in +// configFormParamNames into it as {"name": ["value", ...]} and does not send +// them as form parameters, because the flat form exceeds Tomcat's +// maxParameterCount and the server answers HTTP 500. +const configFormJSONField = "configFormJson" + +var configFormParamNames = []string{"sysConfigType", "sysConfigSubType", "sysConfigName", "sysConfigValue", + "sysConfigOverriden", "sysConfigDefault", "sysConfigSubTypeSel", "customConfigEngine", "customConfigType", + "customConfigSubType", "customConfigName", "customConfigValue", "customConfigTypeEngine", + "customConfigSensitive", "engineCustomConfigIdInput", "engineConfigKey", "engineConfigValue", + "engineConfigSensitive", "engineConfigDefault", "engineConfigOverriden", "isSensitiveConfig"} + +// packConfigFields moves the property row fields into the configFormJson +// field, as the UI does. Forms without that field are returned unchanged. +func packConfigFields(fields []formField) []formField { + jsonIdx := -1 + for i, fld := range fields { + if fld.name == configFormJSONField { + jsonIdx = i + break + } + } + if jsonIdx < 0 { + return fields + } + packed := map[string]bool{} + for _, n := range configFormParamNames { + packed[n] = true + } + values := map[string][]string{} + out := make([]formField, 0, len(fields)) + for _, fld := range fields { + if packed[fld.name] { + values[fld.name] = append(values[fld.name], normalizeNewlines(fld.value)) + continue + } + out = append(out, fld) + } + // Keys in the UI's order; names without fields are left out. + var b strings.Builder + b.WriteByte('{') + for _, n := range configFormParamNames { + v, ok := values[n] + if !ok { + continue + } + if b.Len() > 1 { + b.WriteByte(',') + } + k, _ := json.Marshal(n) + a, _ := json.Marshal(v) + b.Write(k) + b.WriteByte(':') + b.Write(a) + } + b.WriteByte('}') + for i := range out { + if out[i].name == configFormJSONField { + out[i].value = b.String() + } + } + return out +} + // encode builds the form body for actionMethod=update, keeping field order. func (f *AgentConfigForm) encode() string { var b strings.Builder - for i, fld := range f.submitFields() { + for i, fld := range packConfigFields(f.submitFields()) { v := fld.value switch fld.name { case "actionMethod": @@ -693,7 +779,7 @@ func (f *AgentConfigForm) ApplyWith(changes []AgentConfigChange, opts ApplyOptio p.removed = true diffs = append(diffs, AgentConfigDiff{Property: p.Key(), ServiceName: p.ServiceName, OldValue: old.Value, Sensitive: p.Sensitive, Custom: true, Change: "remove"}) - case value != old.Value: + case !SameValue(value, old.Value): p.Value = value if p.rowStart >= 0 { f.fields[p.valueIdx].value = value @@ -707,9 +793,13 @@ func (f *AgentConfigForm) ApplyWith(changes []AgentConfigChange, opts ApplyOptio return nil, fmt.Errorf("%s is a standard property and cannot be removed; use reset", p.Key()) } switch value := normalizeNewlines(ch.Value); { - case ch.Reset: - f.Reset(i) - case value != old.Value: + case ch.Reset || !p.Sensitive && SameValue(value, p.DefaultValue): + // A value equal to the default is reported as not overridden + // when read back, so it is stored as the default. + if old.Overridden { + f.Reset(i) + } + case !SameValue(value, old.Value): f.Set(i, value) } if p.Value == old.Value && p.Overridden == old.Overridden { @@ -749,7 +839,7 @@ func (c *Client) VerifyAgentConfigDiffs(ctx context.Context, agentID string, dif continue } p := form.Properties[i] - if p.Overridden != d.NewOverridden || !d.Sensitive && p.Value != d.NewValue { + if p.Overridden != d.NewOverridden || !d.Sensitive && !SameValue(p.Value, d.NewValue) { failed = append(failed, fmt.Sprintf("%s (now %q, overridden %t)", d.Property, p.Value, p.Overridden)) } } diff --git a/internal/client/agentform_test.go b/internal/client/agentform_test.go index 1eeb40d..a012b15 100644 --- a/internal/client/agentform_test.go +++ b/internal/client/agentform_test.go @@ -1,6 +1,7 @@ package client import ( + "encoding/json" "io" "net/http" "net/url" @@ -161,6 +162,126 @@ func TestAgentConfigFormApply(t *testing.T) { } } +func TestAgentConfigFormEncodePacked(t *testing.T) { + page := strings.Replace(agentPage("tok", "false", ""), `", "- key: testOnBorrow \r\n value: true\r\n\r\n", 1) + return strings.Replace(page, ``, + ``, 1) +} + +func TestWhitespaceOnlyOverride(t *testing.T) { + form, err := parseAgentConfigForm(whitespaceOverridePage()) + if err != nil { + t.Fatal(err) + } + i, err := form.Find(PropertySelector{Service: "process-engine", Type: "db", Name: "connection-properties"}) + if err != nil { + t.Fatal(err) + } + if form.Properties[i].Overridden { + t.Error("whitespace-only difference reported as overridden") + } + props, err := form.Filter(AgentConfigFilter{Service: "process-engine", Overridden: true}) + if err != nil { + t.Fatal(err) + } + if len(props) != 0 { + t.Errorf("--overridden lists %d properties, want 0", len(props)) + } + + diffs, err := form.Apply([]AgentConfigChange{ + {Property: "process-engine:db:connection-properties", Value: "- key: testOnBorrow\n value: true"}, + {Property: "process-engine:cluster:primary-node", Reset: true}, + }) + if err != nil { + t.Fatal(err) + } + if len(diffs) != 0 { + t.Errorf("got diffs for whitespace-only changes and a no-op reset: %+v", diffs) + } +} + +func TestSetToDefaultResets(t *testing.T) { + form, err := parseAgentConfigForm(agentPage("tok", "false", "")) + if err != nil { + t.Fatal(err) + } + if _, err = form.Apply([]AgentConfigChange{{Property: "process-engine:cluster:primary-node", Value: "false"}}); err != nil { + t.Fatal(err) + } + // primary-node is now overridden with false; setting the default back + // resets it. + diffs, err := form.Apply([]AgentConfigChange{{Property: "process-engine:cluster:primary-node", Value: "true\n"}}) + if err != nil { + t.Fatal(err) + } + if len(diffs) != 1 || diffs[0].NewValue != "true" || diffs[0].NewOverridden { + t.Errorf("diffs = %+v, want a reset to the default", diffs) + } +} + func TestAgentConfigFormApplyErrors(t *testing.T) { form, err := parseAgentConfigForm(agentPage("tok", "false", "")) if err != nil { diff --git a/internal/client/agents.go b/internal/client/agents.go index d451cc4..e5a9f27 100644 --- a/internal/client/agents.go +++ b/internal/client/agents.go @@ -50,7 +50,20 @@ type AgentEngineConfig struct { Value string `json:"value,omitempty"` Platform string `json:"platform,omitempty"` DefaultValue string `json:"defaultValue,omitempty"` - Customized bool `json:"customized,omitempty"` + // Customized is set by the API for custom properties (Custom + // Configuration Details), not for overridden standard properties. + Customized bool `json:"customized,omitempty"` + // Overridden is computed: the value differs from the default, ignoring + // line endings and whitespace (see SameValue). The API has no such flag. + Overridden bool `json:"overridden"` +} + +// markOverridden sets Overridden on standard properties whose value differs +// from the default, as agent configs get reports them. +func markOverridden(cfgs []AgentEngineConfig) { + for i := range cfgs { + cfgs[i].Overridden = !cfgs[i].Customized && !SameValue(cfgs[i].Value, cfgs[i].DefaultValue) + } } // AgentEngineStatus represents the status of a service (engine) running on the agent. @@ -144,6 +157,10 @@ func (c *Client) GetAgentDetails(ctx context.Context, id string, full bool) (*Ag if err := c.doJSONWithQuery(ctx, http.MethodGet, fmt.Sprintf("%s/agent/details/%s", BaseAPIPathV2, id), query, nil, &resp); err != nil { return nil, err } + markOverridden(resp.AgentConfigs) + for i := range resp.AgentEngines { + markOverridden(resp.AgentEngines[i].AgentEngineConfigs) + } return &resp, nil } diff --git a/internal/client/agents_test.go b/internal/client/agents_test.go index 6cc04ad..1d40acb 100644 --- a/internal/client/agents_test.go +++ b/internal/client/agents_test.go @@ -100,6 +100,33 @@ func TestGetAgentDetailsNested(t *testing.T) { if len(d.AgentConfigs) != 1 || !d.AgentConfigs[0].Customized { t.Errorf("agent configs not parsed: %+v", d.AgentConfigs) } + if !e.AgentEngineConfigs[0].Overridden { + t.Error("host-name differs from its default but is not overridden") + } + if d.AgentConfigs[0].Overridden { + t.Error("custom property reported as overridden") + } +} + +func TestGetAgentDetailsOverridden(t *testing.T) { + const body = `{"id":"a1","agentEngines":[{"agentEngineStatus":{"appDisplayName":"Process Server"}, + "agentEngineConfigs":[ + {"type":"connectors","name":"https-maxThreads","value":"601","defaultValue":"600","customized":false}, + {"type":"connectors","name":"http-maxThreads","value":"600","defaultValue":"600","customized":false}, + {"type":"db","name":"connection-properties","value":"- key: a \r\n value: b\r\n","defaultValue":"- key: a\n value: b","customized":false}]}]}` + c := newTestClient(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(body)) + })) + d, err := c.GetAgentDetails(context.Background(), "a1", true) + if err != nil { + t.Fatal(err) + } + want := []bool{true, false, false} + for i, cfg := range d.AgentEngines[0].AgentEngineConfigs { + if cfg.Overridden != want[i] { + t.Errorf("%s overridden = %t, want %t", cfg.Name, cfg.Overridden, want[i]) + } + } } func TestSetAgentServiceState(t *testing.T) { diff --git a/internal/tui/agentconfigeditor.go b/internal/tui/agentconfigeditor.go index 9451b22..b10760e 100644 --- a/internal/tui/agentconfigeditor.go +++ b/internal/tui/agentconfigeditor.go @@ -128,7 +128,7 @@ func (e *agentConfigEditor) record(i int, ch client.AgentConfigChange) { e.pending[p.Key()] = client.AgentConfigChange{Property: p.Key(), Remove: true} return } - if ch.Reset && !p.Overridden && p.Value == p.DefaultValue || !ch.Reset && ch.Value == p.Value { + if ch.Reset && !p.Overridden || !ch.Reset && client.SameValue(ch.Value, p.Value) { delete(e.pending, p.Key()) return }