Skip to content

Dev tax service - #1579

Open
tzin1401 wants to merge 95 commits into
nashtech-garage:mainfrom
tzin1401:dev_tax_service
Open

Dev tax service#1579
tzin1401 wants to merge 95 commits into
nashtech-garage:mainfrom
tzin1401:dev_tax_service

Conversation

@tzin1401

@tzin1401 tzin1401 commented Jul 6, 2026

Copy link
Copy Markdown

No description provided.

tzin1401 and others added 30 commits May 1, 2026 14:37
fix(ci): coverage gate awk parsing bug — now properly enforces ≥70%
Handle flaky Snyk CLI failures by capturing exit codes and retrying test/monitor with reduced dependency depth. This keeps CI informative and stable while server-side memory/CLI fixes are pending.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
fix(ci): add resilient Snyk scan fallback for exit -13
…nloads

Run on label yas-build-worker; resolve JDK/Maven once via tool step; verify
gitleaks/snyk on agent with version gate; use system gitleaks (no release curl).
Shorten console noise; strict snyk test/monitor with single fallback.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Pipeline-only changes aligned with ci/setup-jenkins-pipeline for merge to main.

Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid selecting all changed service modules when the diff is only CI files,
so Coverage Gate does not block merges on unrelated service coverage.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ci: Mitigate Snyk OOM; agent label; verify tools; lighter scan chain
…0000

Run Maven dependency tree per scanned module before Snyk; pass -d to snyk
test/monitor so Jenkins logs show child STDERR when exit -13 occurs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Snyk maven plugin spawns ./mvnw; repository wrappers often lack execute bit
after checkout on Linux agents.

Co-authored-by: Cursor <cursoragent@cursor.com>
Force system `mvn` instead of `./mvnw` so scans avoid wrapper permission issues
while keeping module-based test/monitor fallbacks.

Co-authored-by: Cursor <cursoragent@cursor.com>
Match team report style: keep stage green while still running test/monitor
and printing logs; Maven dependency:tree remains strict before scan.

Co-authored-by: Cursor <cursoragent@cursor.com>
ci(snyk): add mvn dependency:tree preflight and snyk -d for SNYK-CLI-…
Use withSonarQubeEnv for analysis and waitForQualityGate so Jenkins can
correlate scans and show gate status; requires matching Jenkins Sonar
installation name and Sonar webhook for timely notifications.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep junit; replace archiveArtifacts with Coverage plugin recordCoverage
pointing at generated jacoco.xml reports.

Co-authored-by: Cursor <cursoragent@cursor.com>
emanhthangngot and others added 29 commits June 26, 2026 17:14
fix(lab2): package payment paypal as boot jar
test(lab2): fix search Kafka integration ack mode
fix(payment): keep paypal dependency jar thin
…tion

ci(lab2): rebuild dependent service modules
…test

ci(lab2): skip snyk monitor during cd validation
ci(lab2): align CQ service release scope
cd(lab2): promote release images by retagging
cd(lab2): deploy dev images by commit tag
…mmit SHA

Incremental CI means overlays/dev holds mixed tags (13 services at a commit
SHA, UIs at 'main'), so requiring every image to exist at the tag's commit
SHA breaks any release from a mixed state (v0.1.1 failed on yas-backoffice).
Release now promotes exactly what dev runs: per-service source tag is read
from yas-cd overlays/dev/kustomization.yaml, with a clear failure when a
service has no dev entry and a log note when source differs from the SHA.
A release tag promotes binaries that already passed every gate in the main
build that published them (images with the source tag only exist after a
green main build). Re-running Test/Coverage/Sonar on the tag re-verifies
source, not the promoted artifacts, and costs ~90% of tag build time.
Tag builds now run: checkout -> gitleaks -> promote -> GitOps only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ci(lab2): promote release images from dev overlay tags, not tagged-co…
@bitdive-review

Copy link
Copy Markdown

BitDive Runtime Review - PR #1579

PR: #1579
Branch: dev_tax_service -> main
Method: BitDive runtime trace comparison + direct HTTP verification + targeted code inspection
Verdict: REQUEST CHANGES
Confidence: High for the payment deployment blocker and checked stable service paths

Summary

PR 1579 changes payment Liquibase data changesets (changelog-0001-provider.sql, changelog-0002-provider.sql) to rename is_enabledenabled in INSERT statements, aligning with the DDL column rename in changelog-0004.sql. It also adds PaymentPaypalApplication.java (compile-only in this review) and a trivial tax config comment.

Critical finding: both SQL files are already-applied changesets modified without runOnChange:true or validCheckSum:any. On any database where they ran before, Liquibase fails at payment startup:

ValidationFailedException: Validation Failed: 2 changesets check sum

Runtime result: payment 502 Bad Gateway on HEAD — service does not start. Baseline payment works (200, PAYPAL + COD providers).

Stable on HEAD: product list, location countries, and customer guest-user show zero contract divergence (TRACE_VERIFIED). CustomerService.java is not modified by this PR — an earlier review plan incorrectly assumed a guest-password change from a different branch.

Runtime Verification Matrix

Area Result Before PR After PR Evidence
Payment providers list Blocked (regression) 200, PAYPAL + COD 502 — service fails Liquibase startup before / after: no trace (startup crash)
Product listing Stable 200, 4 trace nodes 200, zero contract drift before / after
Location countries list Stable 200, 249 trace nodes 200, zero divergence before / after
Guest user create Stable (unchanged code) 200, password GUEST 200, password GUEST (email random only) before / after
Guest Keycloak auth Stable 200, token issued 200, token issued HTTP-only (Keycloak not traced)
Review scope and validation coverage
storefront / backoffice
  -> payment
     -> PaymentProviderController.getAll()     [BLOCKED — Liquibase checksum failure on HEAD]
  -> product
     -> ProductController.getProductsByMultiQuery()   [stable canary]
  -> location
     -> CountryStorefrontController.listCountries()    [stable]
  -> customer
     -> CustomerController.createGuestUser()           [stable — NOT changed by this PR]
  -> Keycloak token endpoint                            [HTTP only]
Behavior Evidence Status
Payment SQL migration Git diff + baseline trace + HEAD 502 Merge blocker
Product list Trace pair Stable
Location countries Trace pair Stable
Guest user create Trace pair + git diff (0 lines in CustomerService) Stable, code unchanged
Guest auth HTTP Stable
PaymentPaypalApplication Compile only Not runtime-tested

Out of scope: CI/CD, frontend, tax application.properties comment-only change.

Change #1 — Liquibase checksum failure blocks payment startup

Files:

  • payment/src/main/resources/db/changelog/data/changelog-0001-provider.sql (::paypal)
  • payment/src/main/resources/db/changelog/data/changelog-0002-provider.sql (::cod)

Both changesets rename is_enabledenabled in INSERT statements. The DDL rename (changelog-0004.sql) already applied the column change; this PR edits historical data changesets instead of adding new ones.

Runtime evidence

Side HTTP Service state
Before PR 200 Payment starts; providers PAYPAL + COD returned in 44ms
After PR 502 ValidationFailedException — 2 changeset checksum mismatches at startup

Baseline trace

before200, SQL uses enabled column, pre-existing ClassCastException in MediaService.getMediaVmMap() with working fallback (unrelated to PR).

Required fix

Add runOnChange:true or validCheckSum:any to both modified changesets, or create new migration changesets instead of editing already-applied ones.

Change #2 — stable services (no collateral regression)

Customer: CustomerService.java has zero diff in this PR. Trace pair shows identical 4-node topology; only guest email differs per invocation. Password remains GUEST on both sides (HTTP-verified).

Product: 4 nodes, 3 SQL both sides — timestamp-only divergence in MediaService.getMedia() headers.

Location: 249 nodes, 1 SQL both sides — zero divergence.

Limitations
Item Notes
PaymentPaypalApplication New module compiles; not in docker-compose — boot path untested
Payment after-trace Impossible — service crashes during Spring/Liquibase init
Pre-existing MediaService ClassCastException On payment baseline only; fallback works; not PR-caused
mergeStateStatus=BLOCKED Branch protection — human action to merge

Follow-Ups

Type Item Blocking
Merge blocker Fix Liquibase checksum conflict on payment data changesets Yes
Non-blocking Runtime-test PaymentPaypalApplication if module is intended for deploy No
Informational GitHub branch protection No

Recommendation

Do not merge until the Liquibase issue is fixed. The column rename direction is correct (enabled matches DDL + JPA entity), but editing already-applied changesets without checksum handling breaks payment on existing databases. Product, location, and customer paths show no behavioral regression.

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.

4 participants