Skip to content

Remove FIDES_PBAC_LIB_PATH env var, add importlib fallback#8213

Open
galvana wants to merge 8 commits into
mainfrom
remove-libpbac-env-var
Open

Remove FIDES_PBAC_LIB_PATH env var, add importlib fallback#8213
galvana wants to merge 8 commits into
mainfrom
remove-libpbac-env-var

Conversation

@galvana
Copy link
Copy Markdown
Contributor

@galvana galvana commented May 16, 2026

Description Of Changes

Removes the FIDES_PBAC_LIB_PATH environment variable and makes libpbac discovery fully automatic. When fides source is volume-mounted for hot-reload (e.g. in fidesplus dev containers), __file__ points to the mount instead of site-packages, so the binary isn't found at the __file__-relative path. The new importlib.util.find_spec("fides") fallback resolves this by locating the installed package regardless of __file__ remapping.

Code Changes

  • src/fides/service/pbac/engine.py: Remove FIDES_PBAC_LIB_PATH env var check, add importlib.util.find_spec fallback, rename _find_library to find_library, replace build hint error with searched-paths diagnostic
  • tests/service/pbac/test_engine_unit.py: Rewrite tests to verify real library discovery instead of mocking everything; test importlib path resolution, error diagnostics

Steps to Confirm

  1. Run pytest tests/service/pbac/test_engine_unit.py -- all tests pass
  2. In a fidesplus dev container with fides-pkg mount, verify from fides.service.pbac.engine import find_library; find_library() returns a valid path

Pre-Merge Checklist

  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration label
    • Add a high-risk label
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • No UX review needed
  • Followup issues:
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

Note: The companion fidesplus PR removes the mkdir/cp libpbac docker-compose workarounds that this change makes unnecessary.

galvana and others added 2 commits May 15, 2026 20:57
…y discovery

The libpbac shared library search now uses importlib.util.find_spec as a
fallback when __file__ is a volume mount (e.g. fidesplus dev containers
hot-reloading fides source). This eliminates the need for manual binary
copying in docker-compose and the FIDES_PBAC_LIB_PATH env var.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment May 18, 2026 9:16pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored May 18, 2026 9:16pm

Request Review

@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.61%. Comparing base (ff33f64) to head (16b0e3f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/fides/service/pbac/engine.py 71.42% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8213      +/-   ##
==========================================
- Coverage   85.62%   85.61%   -0.01%     
==========================================
  Files         659      659              
  Lines       42898    42906       +8     
  Branches     5021     5023       +2     
==========================================
+ Hits        36731    36734       +3     
- Misses       5063     5066       +3     
- Partials     1104     1106       +2     

☔ View full report in Codecov by Sentry.
📢 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.

galvana and others added 4 commits May 15, 2026 21:12
CI misc-unit runner doesn't have Go toolchain, so libpbac.so
isn't present. Skip the two tests that need the real binary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The prod/test Dockerfile targets install fides from sdist into
/opt/fides/ (non-editable). The compiled libpbac.so lives at
/fides/src/fides/bin/ from the build stage but isn't included in
the sdist. Copy it into the installed package's bin/ directory so
find_library() can locate it.

Also removes skipif from tests since the binary is now guaranteed
present in all environments.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CI test target mounts the repo checkout over /fides/, shadowing
the libpbac.so that the Dockerfile built into /fides/src/fides/bin/.
Copy the binary to /opt/fides/lib/ (not shadowed) and add it as a
search path in find_library().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Different environments (editable install, Docker, site-packages)
return different paths. What matters is the binary is found and real.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@galvana galvana requested a review from thabofletcher May 17, 2026 09:51
@galvana galvana marked this pull request as ready for review May 17, 2026 09:51
@galvana galvana requested a review from a team as a code owner May 17, 2026 09:51
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.

1 participant