Skip to content

Add jws-diag diff command: compare effective config between two CATALINA_BASE directories - #20

Merged
dsoumis merged 2 commits into
web-servers:mainfrom
onkar717:feat/config-diff
Aug 14, 2026
Merged

Add jws-diag diff command: compare effective config between two CATALINA_BASE directories#20
dsoumis merged 2 commits into
web-servers:mainfrom
onkar717:feat/config-diff

Conversation

@onkar717

@onkar717 onkar717 commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds jws-diag diff --left <base> --right <base> (stretch goal, Week 12)
  • Parses both server.xml files, emits structural diff of connectors, executors, and server-level settings
  • Connectors keyed by port, executors by name; ConfigValue provenance (explicit/default) preserved so users see what changed and whether it was intentional
  • Human and JSON output formats; JSON follows schemaVersion: "1.0" convention
  • Exit codes: 0 = identical, 1 = differences found, 2 = error
  • Accepts CATALINA_BASE directory (resolves conf/server.xml) or direct path to any XML file

New files

File Purpose
diff/model/ChangeType.java ADDED / REMOVED / CHANGED enum
diff/model/DiffEntry.java Single difference: path, type, left value, right value
diff/model/DiffReport.java Full diff result with metadata
diff/ConfigDiffer.java Structural comparison engine
diff/formatter/DiffHumanFormatter.java Groups entries by path section, renders with +/-/~ sigils
diff/formatter/DiffJsonFormatter.java JSON serialization (schemaVersion first)
diff/DiffCommand.java picocli diff subcommand wired into Main

Test plan

  • ConfigDifferTest - 9 tests: identical configs, shutdown port change, connector field change with provenance, connector ADDED/REMOVED, executor ADDED/REMOVED, executor field change, multiple changes, path metadata
  • DiffHumanFormatterTest - 6 tests: no-diff message, CHANGED arrow, ADDED/REMOVED labels, change count header, section grouping
  • DiffJsonFormatterTest - 6 tests: schemaVersion, left/right in root, changeCount, CHANGED entry fields, ADDED null-left, empty diff array
  • All 378 tests pass (mvn verify)
  • jws-diag diff --help renders correctly
  • End-to-end smoke with HUMAN and JSON format

@dsoumis

dsoumis commented Aug 10, 2026

Copy link
Copy Markdown
Member

check on conflicts


private ServerConfig parse(Path serverXml, Path base) {
try {
Path resolverBase = Files.isDirectory(base) ? base : base.getParent().getParent();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NPE when --left/--right is a bare filename (e.g., server.xml). Null-check getParent() first

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. getParent() result is now null-checked before calling getParent() again; PropertyResolver.create(null) is safe (it skips property loading) so a bare filename like server.xml no longer throws NPE.

}
}

private void compareConnectors(String svcPath,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skips sslHostConfigs; TLS certificate/keystore/cipher changes are silently missed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Added compareSslHostConfigs and compareCertificates methods keyed by hostName (with index fallback) and certificate type (with index fallback) respectively. SSL block diff now covers protocols, sslEnabledProtocols, ciphers, certificateVerification, and per-certificate keystoreFile/keystoreType/type fields.

return new DiffReport(leftBase, rightBase, entries);
}

private void compareServer(ServerConfig left, ServerConfig right, List<DiffEntry> out) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skips shutdownCommand; security-relevant change (SEC-003 fix) would go undetected

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Added diffNullable("server.shutdownCommand", ...) call in compareServer. A changed or missing SHUTDOWN token will now appear in the diff output.

@dsoumis

dsoumis commented Aug 12, 2026

Copy link
Copy Markdown
Member

@onkar717 can you add tests for the SSL, certificate and shutdownCommand diff logic?

@dsoumis
dsoumis merged commit 9569756 into web-servers:main Aug 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants