Skip to content

fix: prevent dry-run crash on clientAuthCredentials handler - #1438

Draft
harshithRai wants to merge 5 commits into
masterfrom
ESD-64945
Draft

fix: prevent dry-run crash on clientAuthCredentials handler#1438
harshithRai wants to merge 5 commits into
masterfrom
ESD-64945

Conversation

@harshithRai

@harshithRai harshithRai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🔧 Changes

Fixes a crash during import --dry_run that aborted the entire dry-run preview with handler.dryRunChanges is not a function.

The clientAuthCredentials feature added two handlers that were implemented for the deploy path only and did not define a dryRunChanges() method. Because the dry-run engine calls dryRunChanges() on every registered handler, the run crashed as soon as it reached this resource - regardless of whether the tenant actually used client credentials.

  • Added a dryRunChanges() method to ClientAuthCredentialsHandler and ClientAuthCredentialsPreHandler, each returning empty changes ({ del: [], create: [], conflicts: [], update: [] }) - the same no-op shape the base handler returns when there is nothing to do.
  • Client credentials are per-client sub-resources with no top-level asset list, so they are intentionally not itemized in the dry-run diff for now. Credential changes are still applied correctly during a real import.
  • Hardened the dry-run loop to skip any handler that doesn't implement dryRunChanges (with a warning) instead of crashing, so a future deploy-only handler can't reintroduce this failure.
  • A normal import (without --dry_run) was never affected.

No changes to the YAML/JSON config shape - the clientAuthCredentials / client_authentication_methods structure is unchanged. This only affects dry-run behavior.

🔭 Future scope

  • Itemize client credential creates/deletes in the dry-run diff so the preview reflects the changes a real import would apply, rather than showing no-op.

🔬 Testing

Unit Test

  • Added regression tests confirming both handlers return empty changes without crashing during dry run.
  • Added a conformance test asserting every registered handler implements dryRunChanges, so a non-conforming handler is caught at build time.

Manual:

  • a0deploy import --input_file=./tenant.yaml --dry_run now completes on a tenant with client credentials instead of crashing.

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@codecov-commenter

codecov-commenter commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.27%. Comparing base (7ebf407) to head (dc98a8f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1438      +/-   ##
==========================================
+ Coverage   80.25%   80.27%   +0.01%     
==========================================
  Files         163      163              
  Lines        7553     7559       +6     
  Branches     1667     1668       +1     
==========================================
+ Hits         6062     6068       +6     
  Misses        801      801              
  Partials      690      690              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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