chore(sdk): drop dead TDFConfig fields and deprecate the TDFFormat enum - #3947
dmihalcik-virtru wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. 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
d0e2cb4 to
727a76f
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:
|
727a76f to
7600ada
Compare
a7fe096 to
270c8a6
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:
|
7600ada to
c6542e0
Compare
270c8a6 to
3e82a64
Compare
X-Test Failure Report |
3e82a64 to
de42cb5
Compare
c6542e0 to
a6d1c1f
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:
|
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>
a6d1c1f to
bdea4f1
Compare
de42cb5 to
4a8db7b
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:
|
|
|
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.