feat(sdk): add WithPolicyFrom re-wrap helper#3476
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a new ChangesPolicy Extraction Feature and Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request adds a convenience builder to the SDK that streamlines the process of carrying over TDF policies during re-wrap operations. By abstracting the extraction and application of data attributes, it reduces boilerplate code for developers and ensures consistent policy propagation without manual manifest handling. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. A helper to bind what we know, To let the new policy flow. With reader in hand, Across all the land, Our TDFs ready to go. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces the WithPolicyFrom function to the SDK, enabling the extraction and application of data attributes from an existing TDF reader to a new TDF configuration. It also includes a unit test to verify error handling for nil readers. Feedback from the reviewer indicates that the documentation incorrectly mandates calling Reader.Init before using this function, which would cause unnecessary network overhead. Additionally, the reviewer provided a suggestion to fix unsafe error handling in the documentation's example code.
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:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/tdf.go`:
- Around line 818-820: The doc comment for WithPolicyFrom is misleading by
stating the reader must be initialized via Reader.Init; update the comment to
remove or reword that requirement and instead document that WithPolicyFrom calls
Reader.DataAttributes which reads r.manifest.Policy directly (so Init is not
required). Locate the WithPolicyFrom function and its preceding comment and
replace the Init requirement with a brief note that DataAttributes accesses the
manifest.Policy field and therefore no network/unwrapping via Reader.Init is
necessary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fb66014-e03d-4f17-a0d2-427fcf38f853
📒 Files selected for processing (3)
.docs-drift.yamlsdk/tdf.gosdk/tdf_rewrap_test.go
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Adds a TDFOption builder that binds the source TDF's policy — its
attribute value FQNs — to a new TDF being created via CreateTDF.
Targets re-wrap pipelines where the source policy should carry forward
without callers handling the manifest's base64+JSON encoding themselves.
Call site is a single line, matching the existing With* option-builder
idiom in this package:
if ok, _ := sdk.IsValidTdf(file); !ok {
// pass through unchanged
}
reader, _ := s.LoadTDF(file)
_ = reader.Init(ctx)
_, _ = s.CreateTDF(out, transformed, sdk.WithPolicyFrom(reader))
Includes a unit test for the nil-Reader error path. Real re-wrap
coverage will come with a TDF fixture test in a follow-up; the helper
delegates to Reader.DataAttributes + WithDataAttributes, both of which
are already well-tested.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Mary Dickson <mary.dickson@virtru.com>
Adds a top-level .docs-drift.yaml that documents the contract used by the docs-drift skill when scanning this repo for SDK changes that should be reflected in opentdf/docs. Most of the file mirrors the skill's built-in OpenTDF defaults — those are committed here so the conventions are discoverable in this repo rather than hidden in the skill source. The `mappings:` section adds routing rules the skill can't infer from file layout alone — e.g., that EntityIdentifier constructor helpers (ForToken, ForEmail, ForRegisteredResource, etc.) belong in authorization.mdx#entityidentifier alongside the existing entries, not in net-new per-symbol files. Without these mappings the skill falls back to a name-only sniff which produces good-but-imperfect placement. First-match-wins, so the more-specific patterns are listed first (e.g., WithPolicyFrom → tdf.mdx wins over the catch-all With* → platform-client.mdx#initializing-the-sdk-client). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mary Dickson <mary.dickson@virtru.com>
DataAttributes() reads from the manifest, which LoadTDF has already populated. The previous comment pushed callers into an unnecessary KAS rewrap. Also tightens the example to handle the LoadTDF error. Signed-off-by: Mary Dickson <mary.dickson@virtru.com>
283cb8e to
0949988
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
Summary
Adds
sdk.WithPolicyFrom(r *Reader) TDFOption— a builder that binds the source TDF's policy (its attribute value FQNs) to a new TDF being created viaCreateTDF. Targets re-wrap pipelines where the source policy should carry forward to the output without callers having to handle the manifest's base64 + JSON encoding themselves.Call site is a single line, matching the existing
With*option-builder idiom in this package (e.g.,WithDataAttributes,WithKasInformation):Implementation
Thin wrapper over
Reader.DataAttributes(which already handles the base64 + JSON decoding) +WithDataAttributes(which already builds the TDF option). The only new logic is thenilReader check.Test plan
sdk/tdf_rewrap_test.go— unit test for thenilReader error pathgo test ./sdk/... -count=1passes (includingTestREADMECodeBlocks)gofumptcleangolangci-lintclean on the changed filesCompanion docs
The corresponding
tdf.mdxsection is drafted on opentdf/docsdemo/docs-drift-with-policy-from. That branch was generated by the docs-drift skill mining this function's godoc example verbatim — the doc PR is a demo of that workflow as well as a real doc addition.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests