chore(sdk): drop dead TDFConfig fields and deprecate the TDFFormat enum - #3947
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughTDF creation now resolves key access before writing payload segments and delegates segment writing and manifest finalization to a chunked writer. The SDK also removes unused configuration fields and updates policy-binding and encryption tests. ChangesTDF SDK Updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SDK.CreateTDF
participant newTDFChunkedWriter
participant resolveKeyAccess
participant chunkedWriter
participant outputWriter
SDK.CreateTDF->>newTDFChunkedWriter: Create writer before payload writing
newTDFChunkedWriter->>resolveKeyAccess: Resolve policy and key access
newTDFChunkedWriter-->>SDK.CreateTDF: Return chunked writer
SDK.CreateTDF->>chunkedWriter: WriteSegment for each input segment
chunkedWriter-->>SDK.CreateTDF: Return segment TDF data
SDK.CreateTDF->>outputWriter: Copy segment TDF data
SDK.CreateTDF->>chunkedWriter: Finalize with options
chunkedWriter-->>SDK.CreateTDF: Return manifest and written byte count
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change has no identified merge-blocking issue in the supplied evidence; complete the stated build and test checks before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit watched the DEK divide, Comment |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
02e414b to
f6a3cf2
Compare
ed39809 to
905336f
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
905336f to
c8748c6
Compare
X-Test Failure Report |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
79e745d to
a5d2340
Compare
cc3fe0b to
15cdcc8
Compare
X-Test Failure Report |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
a5d2340 to
9318a77
Compare
15cdcc8 to
5002041
Compare
X-Test Failure Report |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
5002041 to
c899390
Compare
TDFConfig.enableEncryption was set to true at construction and never read again; nothing could turn it off and nothing branched on it. tdfFormat was likewise fixed at JSONFormat forever. readActionName was a leftover constant with no references. All three are gone. rootIntegrityAlg and segmentIntegrityAlg go with them: once CreateTDF delegates to the chunked writer, which emits an HS256 root and GMAC segments unconditionally, nothing reads either field and no exported option ever set one. The defaults they carried were exactly those two algorithms, so the manifest is unchanged. TestIntegrityAlgDefaults asserted on the fields; the same invariant is now asserted on a manifest CreateTDF actually produced, in TDFSuite.testEncrypt, so every encrypt case in the suite checks it. TDFFormat, JSONFormat, and XMLFormat are exported, so they are deprecated rather than deleted. XML manifests were never implemented and the enum has no remaining consumer inside the SDK. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com> wip: resolve conflict 18
c899390 to
6a92b71
Compare
X-Test Failure Reportopentdf |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
Proposed Changes
TDFConfig.enableEncryption was set to true at construction and never read
again; nothing could turn it off and nothing branched on it. tdfFormat was
likewise fixed at JSONFormat forever. readActionName was a leftover constant
with no references. All three are gone.
rootIntegrityAlg and segmentIntegrityAlg go with them. #3940 (revised) fixed
the chunked writer's root to HS256 and its segments to GMAC, and #3946 made
CreateTDF delegate to it, so nothing reads either field any more. No exported
option ever set one, and the defaults they held were exactly those two
algorithms, so the manifest is byte-identical. They are unexported, so they
are deleted outright rather than deprecated.
TestIntegrityAlgDefaultsasserted on those fields and could not survivethem. The invariant it guarded is now asserted on a manifest
CreateTDFactually produced —
TDFSuite.testEncryptchecksmanifest.AlgorithmisHS256 and
manifest.SegmentHashAlgorithmis GMAC — so every encrypt case inthe suite carries it rather than one test reading back a struct field.
TDFFormat, JSONFormat, and XMLFormat are exported, so they are deprecated
rather than deleted. XML manifests were never implemented and the enum has no
remaining consumer inside the SDK.
Checklist
Testing Instructions
Nothing reads any of the removed fields; the check is that the tree still
builds across
sdk,service,otdfctl,examplesandtests-bdd, andthat
TDFSuitestill emits HS256/GMAC on every encrypt case.The full DSPX-2604 stack — 20 PRs
mainmainmainmainmainmainmaindspx-2604-base-11= #3932 + #3934 + #3935dspx-2604-base-17= #3944 + #3945dspx-2604-base-19= #3947 + #3939Reviewable in parallel right now, since they sit directly on
mainand depend onnothing else: 01, 02, 04, 05, 06, 07, 08.
Why three PRs have a
dspx-2604-base-*base. A GitHub PR takes one base branch,but 11, 17 and 19 each build on more than one parent. The
base-*branches are emptymerge commits that exist only to join those parents so the PR diff shows exactly its
own change and nothing else. They contain no code, have no PR of their own, and go
away once their parents land — retarget the child onto
mainat that point.Wants a cross-SDK xtest run before merge: 15, 17 (and therefore 20). They touch
the KAS wire format.
Red checks you may see are network flakes, not this stack. Four distinct ones hit
this batch and all clear on re-run:
golangci-lint config verifytiming out onhttps://golangci-lint.run/.../golangci.v2.8.jsonschema.json(fails the wholego (<module>)job and fail-fast cancels its siblings), the bats installer getting a 403,Docker Hub timing out on
keycloak/keycloak:26.4, andbufreporting "the serverhosted at that remote is unavailable" while the Java SDK generates sources. The
govulncheckstep also emits##[error]annotations against the go1.25.11 stdlib, butit is
continue-on-error: trueand never fails a job — 01 bumps the toolchain andclears those annotations.
Summary by CodeRabbit
Bug Fixes
Configuration