From 2ab83f61bcf0b87bd854730b9dc31ae13c81002f Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 12:23:57 +0200 Subject: [PATCH 01/15] Add manual release process to PyPi --- .bumpversion.cfg | 2 +- .pypirc | 14 ++++ GEMMA_RELEASE.md | 161 +++++++++++++++++++++++++++++++++++++ VERSION | 2 +- pyproject.toml | 73 +++++++++++++++++ scripts/publish_prod.sh | 52 ++++++++++++ scripts/publish_test.sh | 42 ++++++++++ setup.py | 10 +-- src/permifrost/__init__.py | 2 +- 9 files changed, 350 insertions(+), 8 deletions(-) create mode 100644 .pypirc create mode 100644 GEMMA_RELEASE.md create mode 100644 pyproject.toml create mode 100755 scripts/publish_prod.sh create mode 100755 scripts/publish_test.sh diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 224b7cdfb..76b135a12 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.15.4 +current_version = 0.1.0 commit = True tag = False parse = (?P\d+)\.(?P\d+)\.(?P\d+)(\-(?P[a-z]+)(?P\d+))? diff --git a/.pypirc b/.pypirc new file mode 100644 index 000000000..4ef03b5b5 --- /dev/null +++ b/.pypirc @@ -0,0 +1,14 @@ +[distutils] +index-servers = + pypi + testpypi + +[pypi] +repository = https://upload.pypi.org/legacy/ +username = __token__ +password = YOUR_PYPI_API_TOKEN_HERE + +[testpypi] +repository = https://test.pypi.org/legacy/ +username = __token__ +password = YOUR_TEST_PYPI_API_TOKEN_HERE diff --git a/GEMMA_RELEASE.md b/GEMMA_RELEASE.md new file mode 100644 index 000000000..14fbfe00c --- /dev/null +++ b/GEMMA_RELEASE.md @@ -0,0 +1,161 @@ +# Gemma Analytics Permifrost Release Guide + +This guide explains how to release the `gemma.permifrost` package to PyPI. + +## Prerequisites + +1. **PyPI Account**: Create an account on [PyPI](https://pypi.org/account/register/) +2. **Test PyPI Account**: Create an account on [Test PyPI](https://test.pypi.org/account/register/) +3. **API Tokens**: Generate API tokens for both PyPI and Test PyPI + - Go to your account settings + - Create a new API token + - Copy the token (it starts with `pypi-`) + +## Setup + +1. **Configure credentials** (choose one method): + + **Method A: Environment Variables** + ```bash + export TWINE_USERNAME=__token__ + export TWINE_PASSWORD=your_api_token_here + ``` + + **Method B: .pypirc file** + ```bash + # Edit .pypirc and replace YOUR_PYPI_API_TOKEN_HERE with your actual token + cp .pypirc ~/.pypirc + chmod 600 ~/.pypirc + ``` + +2. **Install build tools**: + ```bash + pip install --upgrade build twine + ``` + +## Release Process + +### Step 1: Test Release to Test PyPI + +1. **Update version** (if needed): + ```bash + # Update VERSION file + echo "0.1.0" > VERSION + + # Update .bumpversion.cfg + # Update src/permifrost/__init__.py + ``` + +2. **Build and test locally**: + ```bash + # Clean previous builds + rm -rf dist/ build/ *.egg-info/ + + # Build package + python -m build + + # Check package + twine check dist/* + ``` + +3. **Publish to Test PyPI**: + ```bash + ./scripts/publish_test.sh + ``` + +4. **Test installation**: + ```bash + # Create a new virtual environment for testing + python -m venv test_env + source test_env/bin/activate + + # Install from Test PyPI + pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ gemma.permifrost + + # Test the installation + permifrost --help + ``` + +### Step 2: Production Release to PyPI + +1. **Verify everything works** from Test PyPI +2. **Publish to Production PyPI**: + ```bash + ./scripts/publish_prod.sh + ``` + +3. **Verify the release**: + - Check [PyPI project page](https://pypi.org/project/gemma.permifrost/) + - Test installation: `pip install gemma.permifrost` + +## Version Management + +### Bumping Versions + +Use the existing bumpversion setup: + +```bash +# For patch release (0.1.0 -> 0.1.1) +bumpversion patch + +# For minor release (0.1.0 -> 0.2.0) +bumpversion minor + +# For major release (0.1.0 -> 1.0.0) +bumpversion major +``` + +### Manual Version Updates + +If you need to manually update versions: + +1. Update `VERSION` file +2. Update `.bumpversion.cfg` current_version +3. Update `src/permifrost/__init__.py` __version__ +4. Update `pyproject.toml` version + +## Troubleshooting + +### Common Issues + +1. **Authentication Errors**: + - Ensure your API token is correct + - Check that `.pypirc` has correct permissions (600) + - Verify environment variables are set + +2. **Package Name Conflicts**: + - The package name `gemma.permifrost` should be unique + - If there's a conflict, consider using a different name + +3. **Build Errors**: + - Ensure all dependencies are installed + - Check that `pyproject.toml` is properly formatted + - Verify all required files are included in `MANIFEST.in` + +### Testing Before Release + +```bash +# Test build +python -m build + +# Test package check +twine check dist/* + +# Test upload to Test PyPI (dry run) +twine upload --repository testpypi --dry-run dist/* +``` + +## Package Information + +- **Package Name**: `gemma.permifrost` +- **Description**: Permifrost Permissions - Fork by Gemma Analytics +- **Author**: Gemma Analytics +- **License**: MIT +- **Python Version**: >=3.8 +- **Entry Point**: `permifrost` command + +## Links + +- [PyPI Project](https://pypi.org/project/gemma.permifrost/) +- [Test PyPI Project](https://test.pypi.org/project/gemma.permifrost/) +- [GitHub Repository](https://github.com/Gemma-Analytics/permifrost) diff --git a/VERSION b/VERSION index 3b1c79408..6e8bf73aa 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.15.7 +0.1.0 diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 000000000..f2c876b1c --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,73 @@ +[build-system] +requires = ["setuptools>=45", "wheel"] +build-backend = "setuptools.build_meta" + +[project] +name = "gemma.permifrost" +version = "0.1.0" +description = "Permifrost Permissions - Fork by Gemma Analytics" +readme = "README.md" +license = {text = "MIT"} +authors = [ + {name = "Gemma Analytics", email = "info@gemmaanalytics.com"} +] +maintainers = [ + {name = "Gemma Analytics", email = "info@gemmaanalytics.com"} +] +keywords = ["snowflake", "permissions", "database", "security"] +classifiers = [ + "Development Status :: 4 - Beta", + "Intended Audience :: Developers", + "License :: OSI Approved :: MIT License", + "Operating System :: OS Independent", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Topic :: Database", + "Topic :: Security", +] +requires-python = ">=3.8" +dependencies = [ + "cerberus", + "colorama", + "coloredlogs", + "click", + "click-default-group", + "pyyaml", + "snowflake-connector-python[secure-local-storage]", + "snowflake-sqlalchemy==1.5.3", + "sqlalchemy", +] + +[project.optional-dependencies] +dev = [ + "black", + "bumpversion", + "changelog-cli", + "coverage", + "flake8", + "isort", + "mypy", + "pre-commit", + "pytest", + "pytest-cov", + "pytest-mock", + "types-PyYAML", +] + +[project.scripts] +permifrost = "permifrost.cli:main" + +[project.urls] +Homepage = "https://github.com/Gemma-Analytics/permifrost" +Repository = "https://github.com/Gemma-Analytics/permifrost" +Documentation = "https://github.com/Gemma-Analytics/permifrost" +Issues = "https://github.com/Gemma-Analytics/permifrost/issues" + +[tool.setuptools.packages.find] +where = ["src"] + +[tool.setuptools.package-data] +"*" = ["*.txt", "*.md", "*.yml", "*.yaml"] diff --git a/scripts/publish_prod.sh b/scripts/publish_prod.sh new file mode 100755 index 000000000..2c5138e23 --- /dev/null +++ b/scripts/publish_prod.sh @@ -0,0 +1,52 @@ +#!/bin/bash + +# Script to publish gemma.permifrost to Production PyPI +# Usage: ./scripts/publish_prod.sh + +set -e + +echo "๐Ÿš€ Publishing gemma.permifrost to Production PyPI..." + +# Check if we're in a virtual environment +if [[ "$VIRTUAL_ENV" == "" ]]; then + echo "โš ๏ธ Warning: Not in a virtual environment. Consider activating one." +fi + +# Install build tools if not present +echo "๐Ÿ“ฆ Installing build tools..." +pip install --upgrade build twine + +# Clean previous builds +echo "๐Ÿงน Cleaning previous builds..." +rm -rf dist/ build/ *.egg-info/ + +# Build the package +echo "๐Ÿ”จ Building package..." +python -m build + +# Check the built package +echo "๐Ÿ” Checking built package..." +twine check dist/* + +# Confirm before uploading to production +echo "โš ๏ธ WARNING: You are about to publish to PRODUCTION PyPI!" +echo "This will make the package publicly available." +read -p "Are you sure you want to continue? (y/N): " -n 1 -r +echo +if [[ ! $REPLY =~ ^[Yy]$ ]]; then + echo "โŒ Publishing cancelled." + exit 1 +fi + +# Upload to Production PyPI +echo "๐Ÿ“ค Uploading to Production PyPI..." +echo "You will be prompted for your PyPI credentials." +echo "Username: __token__" +echo "Password: Your PyPI API token (starts with pypi-)" +twine upload dist/* + +echo "โœ… Successfully published to Production PyPI!" +echo "๐Ÿ”— PyPI URL: https://pypi.org/project/gemma.permifrost/" +echo "" +echo "To install from PyPI:" +echo "pip install gemma.permifrost" diff --git a/scripts/publish_test.sh b/scripts/publish_test.sh new file mode 100755 index 000000000..28019bafd --- /dev/null +++ b/scripts/publish_test.sh @@ -0,0 +1,42 @@ +#!/bin/bash + +# Script to publish gemma.permifrost to Test PyPI +# Usage: ./scripts/publish_test.sh + +set -e + +echo "๐Ÿš€ Publishing gemma.permifrost to Test PyPI..." + +# Check if we're in a virtual environment +if [[ "$VIRTUAL_ENV" == "" ]]; then + echo "โš ๏ธ Warning: Not in a virtual environment. Consider activating one." +fi + +# Install build tools if not present +echo "๐Ÿ“ฆ Installing build tools..." +pip install --upgrade build twine + +# Clean previous builds +echo "๐Ÿงน Cleaning previous builds..." +rm -rf dist/ build/ *.egg-info/ + +# Build the package +echo "๐Ÿ”จ Building package..." +python -m build + +# Check the built package +echo "๐Ÿ” Checking built package..." +twine check dist/* + +# Upload to Test PyPI +echo "๐Ÿ“ค Uploading to Test PyPI..." +echo "You will be prompted for your Test PyPI credentials." +echo "Username: __token__" +echo "Password: Your Test PyPI API token (starts with pypi-)" +twine upload --repository testpypi dist/* + +echo "โœ… Successfully published to Test PyPI!" +echo "๐Ÿ”— Test PyPI URL: https://test.pypi.org/project/gemma.permifrost/" +echo "" +echo "To install from Test PyPI:" +echo "pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ gemma.permifrost" diff --git a/setup.py b/setup.py index 9a6c37292..58e002ba1 100755 --- a/setup.py +++ b/setup.py @@ -35,14 +35,14 @@ ] setup( - name="permifrost", + name="gemma.permifrost", version=version, - author="GitLab Data Team, Meltano Team, & Contributors", - author_email="permifrost@gitlab.com", - description="Permifrost Permissions", + author="Gemma Analytics", + author_email="info@gemmaanalytics.com", + description="Permifrost Permissions - Fork by Gemma Analytics", long_description=long_description, long_description_content_type="text/markdown", - url="https://gitlab.com/gitlab-data/permifrost", + url="https://github.com/Gemma-Analytics/permifrost", package_dir={"": "src"}, packages=find_packages(where="src"), include_package_data=True, diff --git a/src/permifrost/__init__.py b/src/permifrost/__init__.py index f1723352c..ad9277255 100644 --- a/src/permifrost/__init__.py +++ b/src/permifrost/__init__.py @@ -1,5 +1,5 @@ # Managed by bumpversion -__version__ = "0.15.4" +__version__ = "0.1.0" from permifrost.error import SpecLoadingError From d1fe2873f2f348a0f84a72a61cb4a4af1d946d19 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 15:32:30 +0200 Subject: [PATCH 02/15] Add GitHub workflows to release to PyPi --- .github/workflows/manual-release.yml | 121 +++++++++++++ .github/workflows/release.yml | 161 ++++++++++++++++++ .github/workflows/test.yml | 104 ++++++++++++ GITHUB_ACTIONS.md | 244 +++++++++++++++++++++++++++ scripts/bump_version.sh | 73 ++++++++ 5 files changed, 703 insertions(+) create mode 100644 .github/workflows/manual-release.yml create mode 100644 .github/workflows/release.yml create mode 100644 .github/workflows/test.yml create mode 100644 GITHUB_ACTIONS.md create mode 100755 scripts/bump_version.sh diff --git a/.github/workflows/manual-release.yml b/.github/workflows/manual-release.yml new file mode 100644 index 000000000..6dcb53883 --- /dev/null +++ b/.github/workflows/manual-release.yml @@ -0,0 +1,121 @@ +name: Manual Release + +on: + workflow_dispatch: + inputs: + version: + description: 'Version to release (e.g., 0.1.0)' + required: true + type: string + target: + description: 'Target PyPI repository' + required: true + default: 'test' + type: choice + options: + - test + - production + +env: + PYTHON_VERSION: "3.11" + +jobs: + manual-release: + name: Manual Release to ${{ inputs.target }} PyPI + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Update VERSION file + run: | + echo "${{ inputs.version }}" > VERSION + echo "Updated VERSION to ${{ inputs.version }}" + + - name: Update version in files + run: | + # Update .bumpversion.cfg + sed -i "s/current_version = .*/current_version = ${{ inputs.version }}/" .bumpversion.cfg + + # Update src/permifrost/__init__.py + sed -i "s/__version__ = \".*\"/__version__ = \"${{ inputs.version }}\"/" src/permifrost/__init__.py + + # Update pyproject.toml + sed -i "s/version = \".*\"/version = \"${{ inputs.version }}\"/" pyproject.toml + + - name: Install build dependencies + run: | + python -m pip install --upgrade pip + pip install build twine + + - name: Build package + run: | + python -m build + + - name: Check package + run: | + twine check dist/* + + - name: Publish to Test PyPI + if: inputs.target == 'test' + uses: pypa/gh-action-pypi-publish@release/v1 + with: + password: ${{ secrets.TEST_PYPI_API_TOKEN }} + repository-url: https://test.pypi.org/legacy/ + skip-existing: true + + - name: Publish to Production PyPI + if: inputs.target == 'production' + uses: pypa/gh-action-pypi-publish@release/v1 + with: + password: ${{ secrets.PYPI_API_TOKEN }} + skip-existing: true + + - name: Create Release + if: inputs.target == 'production' + uses: actions/create-release@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + tag_name: v${{ inputs.version }} + release_name: Release v${{ inputs.version }} + body: | + ## Release v${{ inputs.version }} + + This release has been manually published to PyPI. + + **Install:** + ```bash + pip install gemma.permifrost + ``` + + **PyPI URL:** https://pypi.org/project/gemma.permifrost/ + draft: false + prerelease: false + + - name: Commit version changes + run: | + git config --local user.email "action@github.com" + git config --local user.name "GitHub Action" + git add VERSION .bumpversion.cfg src/permifrost/__init__.py pyproject.toml + git commit -m "Bump version to ${{ inputs.version }}" + git tag v${{ inputs.version }} + git push origin HEAD + git push origin v${{ inputs.version }} + + - name: Success message + run: | + if [ "${{ inputs.target }}" = "test" ]; then + echo "โœ… Successfully published version ${{ inputs.version }} to Test PyPI" + echo "๐Ÿ”— Test PyPI URL: https://test.pypi.org/project/gemma.permifrost/" + echo "๐Ÿ“ฆ Install: pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ gemma.permifrost" + else + echo "โœ… Successfully published version ${{ inputs.version }} to Production PyPI" + echo "๐Ÿ”— PyPI URL: https://pypi.org/project/gemma.permifrost/" + echo "๐Ÿ“ฆ Install: pip install gemma.permifrost" + fi diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 000000000..617150227 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,161 @@ +name: Release to PyPI + +on: + pull_request: + types: [opened, synchronize, reopened, closed] + paths: + - 'VERSION' + push: + branches: + - main + - master + paths: + - 'VERSION' + +env: + PYTHON_VERSION: "3.11" + +jobs: + test-release: + name: Release to Test PyPI + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' && github.event.pull_request.state == 'open' + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.ref }} + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Install build dependencies + run: | + python -m pip install --upgrade pip + pip install build twine + + - name: Check if VERSION file changed + id: check_version + run: | + if git diff --name-only ${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }} | grep -q "VERSION"; then + echo "version_changed=true" >> $GITHUB_OUTPUT + else + echo "version_changed=false" >> $GITHUB_OUTPUT + fi + + - name: Build package + if: steps.check_version.outputs.version_changed == 'true' + run: | + python -m build + + - name: Check package + if: steps.check_version.outputs.version_changed == 'true' + run: | + twine check dist/* + + - name: Publish to Test PyPI + if: steps.check_version.outputs.version_changed == 'true' + uses: pypa/gh-action-pypi-publish@release/v1 + with: + password: ${{ secrets.TEST_PYPI_API_TOKEN }} + repository-url: https://test.pypi.org/legacy/ + skip-existing: true + + - name: Comment on PR + if: steps.check_version.outputs.version_changed == 'true' + uses: actions/github-script@v7 + with: + script: | + const version = require('fs').readFileSync('VERSION', 'utf8').trim(); + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: `๐Ÿš€ **Test Release Published!** + + Version \`${version}\` has been published to Test PyPI. + + **Install from Test PyPI:** + \`\`\`bash + pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ gemma.permifrost + \`\`\` + + **Test PyPI URL:** https://test.pypi.org/project/gemma.permifrost/ + + Once this PR is merged, it will be automatically published to production PyPI.` + }); + + production-release: + name: Release to Production PyPI + runs-on: ubuntu-latest + if: github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master') + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Install build dependencies + run: | + python -m pip install --upgrade pip + pip install build twine + + - name: Check if VERSION file changed + id: check_version + run: | + if git diff --name-only HEAD~1..HEAD | grep -q "VERSION"; then + echo "version_changed=true" >> $GITHUB_OUTPUT + else + echo "version_changed=false" >> $GITHUB_OUTPUT + fi + + - name: Build package + if: steps.check_version.outputs.version_changed == 'true' + run: | + python -m build + + - name: Check package + if: steps.check_version.outputs.version_changed == 'true' + run: | + twine check dist/* + + - name: Publish to Production PyPI + if: steps.check_version.outputs.version_changed == 'true' + uses: pypa/gh-action-pypi-publish@release/v1 + with: + password: ${{ secrets.PYPI_API_TOKEN }} + skip-existing: true + + - name: Create Release + if: steps.check_version.outputs.version_changed == 'true' + uses: actions/create-release@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + tag_name: v${{ steps.get_version.outputs.version }} + release_name: Release v${{ steps.get_version.outputs.version }} + body: | + ## Release v${{ steps.get_version.outputs.version }} + + This release has been automatically published to PyPI. + + **Install:** + ```bash + pip install gemma.permifrost + ``` + + **PyPI URL:** https://pypi.org/project/gemma.permifrost/ + draft: false + prerelease: false + + - name: Get version + if: steps.check_version.outputs.version_changed == 'true' + id: get_version + run: | + version=$(cat VERSION) + echo "version=$version" >> $GITHUB_OUTPUT diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 000000000..0dc23561e --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,104 @@ +name: Test Package + +on: + pull_request: + paths: + - 'VERSION' + - 'src/**' + - 'tests/**' + - 'setup.py' + - 'pyproject.toml' + push: + branches: + - main + - master + paths: + - 'VERSION' + - 'src/**' + - 'tests/**' + - 'setup.py' + - 'pyproject.toml' + +env: + PYTHON_VERSION: "3.11" + +jobs: + test: + name: Run Tests + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.8", "3.9", "3.10", "3.11"] + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -e ".[dev]" + + - name: Run linting + run: | + flake8 src/ tests/ + isort --check-only src/ tests/ + black --check src/ tests/ + + - name: Run type checking + run: | + mypy src/ + + - name: Run tests + run: | + pytest tests/ -v --cov=src/ --cov-report=xml + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v3 + with: + file: ./coverage.xml + flags: unittests + name: codecov-umbrella + + build-test: + name: Test Build + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' || github.event_name == 'push' + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Install build dependencies + run: | + python -m pip install --upgrade pip + pip install build twine + + - name: Build package + run: | + python -m build + + - name: Check package + run: | + twine check dist/* + + - name: Test package installation + run: | + pip install dist/*.whl + permifrost --help + + - name: Upload build artifacts + uses: actions/upload-artifact@v3 + with: + name: dist-files + path: dist/ + retention-days: 1 diff --git a/GITHUB_ACTIONS.md b/GITHUB_ACTIONS.md new file mode 100644 index 000000000..083570d26 --- /dev/null +++ b/GITHUB_ACTIONS.md @@ -0,0 +1,244 @@ +# GitHub Actions Workflows for gemma.permifrost + +This document explains the automated release workflows for the `gemma.permifrost` package. + +## Overview + +We have three GitHub Actions workflows that handle different aspects of the release process: + +1. **Test Workflow** (`test.yml`) - Runs tests and checks before releases +2. **Release Workflow** (`release.yml`) - Automatically releases to Test PyPI and Production PyPI +3. **Manual Release Workflow** (`manual-release.yml`) - Allows manual releases via GitHub UI + +## Workflow Details + +### 1. Test Workflow (`test.yml`) + +**Triggers:** +- Pull requests that modify `VERSION`, `src/`, `tests/`, `setup.py`, or `pyproject.toml` +- Pushes to `main`/`master` that modify the same files + +**What it does:** +- Runs tests on multiple Python versions (3.8, 3.9, 3.10, 3.11) +- Performs linting checks (flake8, isort, black) +- Runs type checking with mypy +- Executes pytest with coverage +- Tests package building and installation +- Uploads build artifacts for inspection + +### 2. Release Workflow (`release.yml`) + +**Triggers:** +- Pull requests that modify `VERSION` file +- Pushes to `main`/`master` that modify `VERSION` file + +**Jobs:** + +#### Test Release Job +- **When:** PR is opened/updated with VERSION changes +- **What:** Publishes to Test PyPI +- **Features:** + - Only runs if VERSION file actually changed + - Builds and validates the package + - Publishes to Test PyPI + - Comments on the PR with installation instructions + - Uses `skip-existing: true` to avoid conflicts + +#### Production Release Job +- **When:** PR with VERSION changes is merged to main/master +- **What:** Publishes to Production PyPI +- **Features:** + - Only runs if VERSION file actually changed + - Builds and validates the package + - Publishes to Production PyPI + - Creates a GitHub release with tag + - Uses `skip-existing: true` to avoid conflicts + +### 3. Manual Release Workflow (`manual-release.yml`) + +**Triggers:** +- Manual trigger via GitHub Actions UI + +**Features:** +- Allows specifying version and target (test/production) +- Updates all version files automatically +- Commits and tags the changes +- Creates GitHub releases for production releases + +## Setup Instructions + +### 1. Repository Secrets + +You need to set up the following secrets in your GitHub repository: + +1. Go to your repository โ†’ Settings โ†’ Secrets and variables โ†’ Actions +2. Add the following secrets: + +#### Required Secrets: +- `TEST_PYPI_API_TOKEN` - Your Test PyPI API token (starts with `pypi-`) +- `PYPI_API_TOKEN` - Your Production PyPI API token (starts with `pypi-`) + +#### Optional Secrets: +- `GITHUB_TOKEN` - Automatically provided by GitHub + +### 2. Getting PyPI API Tokens + +#### Test PyPI Token: +1. Go to [https://test.pypi.org/account/register/](https://test.pypi.org/account/register/) +2. Create an account if you don't have one +3. Go to Account Settings โ†’ API tokens +4. Create a new token with "Entire account" scope +5. Copy the token (starts with `pypi-`) + +#### Production PyPI Token: +1. Go to [https://pypi.org/account/register/](https://pypi.org/account/register/) +2. Create an account if you don't have one +3. Go to Account Settings โ†’ API tokens +4. Create a new token with "Entire account" scope +5. Copy the token (starts with `pypi-`) + +### 3. Branch Protection (Recommended) + +Set up branch protection for `main`/`master`: + +1. Go to Settings โ†’ Branches +2. Add rule for `main`/`master` +3. Enable: + - Require status checks to pass before merging + - Require branches to be up to date before merging + - Select the "test" workflow as required + +## Usage Examples + +### Automatic Release Process + +1. **Create a feature branch:** + ```bash + git checkout -b feature/new-version + ``` + +2. **Update VERSION file:** + ```bash + echo "0.1.1" > VERSION + git add VERSION + git commit -m "Bump version to 0.1.1" + git push origin feature/new-version + ``` + +3. **Create a Pull Request:** + - The test workflow will run automatically + - If VERSION changed, it will publish to Test PyPI + - A comment will be added to the PR with installation instructions + +4. **Merge the PR:** + - The production workflow will run automatically + - If VERSION changed, it will publish to Production PyPI + - A GitHub release will be created + +### Manual Release + +1. Go to Actions โ†’ Manual Release +2. Click "Run workflow" +3. Enter the version (e.g., "0.1.2") +4. Select target (test or production) +5. Click "Run workflow" + +## Workflow Behavior + +### VERSION File Detection + +The workflows use git diff to detect if the VERSION file actually changed: + +- **For PRs:** Compares base branch with PR branch +- **For pushes:** Compares current commit with previous commit + +This prevents unnecessary releases when VERSION hasn't changed. + +### Skip Existing + +All PyPI uploads use `skip-existing: true`, which means: +- If a version already exists on PyPI, the upload will be skipped +- No errors will occur for duplicate versions +- Useful for re-running workflows + +### Error Handling + +- Build failures will stop the workflow +- PyPI upload failures will be reported +- Test failures will prevent releases +- All steps have proper error reporting + +## Monitoring + +### Workflow Status + +- Check the Actions tab in your repository +- Green checkmarks indicate success +- Red X marks indicate failures +- Click on any workflow run for detailed logs + +### Release Tracking + +- Test releases: Check Test PyPI project page +- Production releases: Check PyPI project page +- GitHub releases: Check the Releases tab + +## Troubleshooting + +### Common Issues + +1. **Authentication Errors:** + - Verify API tokens are correct + - Ensure tokens have upload permissions + - Check that secrets are properly set + +2. **Version Conflicts:** + - The workflow uses `skip-existing: true` to handle duplicates + - If you need to force upload, manually delete the version from PyPI first + +3. **Build Failures:** + - Check the build logs for specific errors + - Ensure all dependencies are properly specified + - Verify package structure is correct + +4. **Test Failures:** + - Fix any linting issues (flake8, isort, black) + - Address mypy type checking errors + - Fix failing tests + +### Debugging + +- All workflows provide detailed logs +- Check the "Actions" tab for workflow runs +- Look for specific error messages in the logs +- Use the manual release workflow for testing + +## Security Considerations + +- API tokens are stored as GitHub secrets +- Tokens should have minimal required permissions +- Consider using scoped tokens for production +- Regularly rotate API tokens +- Monitor PyPI for unauthorized uploads + +## Best Practices + +1. **Version Management:** + - Always update VERSION file for releases + - Use semantic versioning + - Keep version files in sync + +2. **Testing:** + - Test locally before pushing + - Use Test PyPI for validation + - Verify installation works + +3. **Documentation:** + - Update CHANGELOG.md for releases + - Document breaking changes + - Keep release notes current + +4. **Monitoring:** + - Check workflow status regularly + - Monitor PyPI for successful uploads + - Verify GitHub releases are created diff --git a/scripts/bump_version.sh b/scripts/bump_version.sh new file mode 100755 index 000000000..a06de2ab0 --- /dev/null +++ b/scripts/bump_version.sh @@ -0,0 +1,73 @@ +#!/bin/bash + +# Script to bump version across all files +# Usage: ./scripts/bump_version.sh [patch|minor|major|version] + +set -e + +if [ $# -eq 0 ]; then + echo "Usage: $0 [patch|minor|major|version]" + echo " patch: bump patch version (0.1.0 -> 0.1.1)" + echo " minor: bump minor version (0.1.0 -> 0.2.0)" + echo " major: bump major version (0.1.0 -> 1.0.0)" + echo " version: specify exact version (e.g., 0.1.2)" + exit 1 +fi + +BUMP_TYPE=$1 + +# Get current version +CURRENT_VERSION=$(cat VERSION) +echo "Current version: $CURRENT_VERSION" + +# Calculate new version +if [ "$BUMP_TYPE" = "patch" ] || [ "$BUMP_TYPE" = "minor" ] || [ "$BUMP_TYPE" = "major" ]; then + # Use bumpversion to calculate new version + NEW_VERSION=$(bumpversion --dry-run --list $BUMP_TYPE | grep new_version | cut -d= -f2) +else + # Use specified version + NEW_VERSION=$BUMP_TYPE +fi + +echo "New version: $NEW_VERSION" + +# Confirm +read -p "Do you want to bump version from $CURRENT_VERSION to $NEW_VERSION? (y/N): " -n 1 -r +echo +if [[ ! $REPLY =~ ^[Yy]$ ]]; then + echo "Version bump cancelled." + exit 1 +fi + +# Update files +echo "Updating version files..." + +# Update VERSION file +echo "$NEW_VERSION" > VERSION + +# Update .bumpversion.cfg +sed -i "s/current_version = .*/current_version = $NEW_VERSION/" .bumpversion.cfg + +# Update src/permifrost/__init__.py +sed -i "s/__version__ = \".*\"/__version__ = \"$NEW_VERSION\"/" src/permifrost/__init__.py + +# Update pyproject.toml +sed -i "s/version = \".*\"/version = \"$NEW_VERSION\"/" pyproject.toml + +echo "โœ… Version bumped to $NEW_VERSION" +echo "" +echo "Files updated:" +echo " - VERSION" +echo " - .bumpversion.cfg" +echo " - src/permifrost/__init__.py" +echo " - pyproject.toml" +echo "" +echo "Next steps:" +echo " 1. Commit the changes:" +echo " git add VERSION .bumpversion.cfg src/permifrost/__init__.py pyproject.toml" +echo " git commit -m \"Bump version to $NEW_VERSION\"" +echo "" +echo " 2. Push to trigger GitHub Actions:" +echo " git push origin " +echo "" +echo " 3. Create a Pull Request to trigger Test PyPI release" From aed9c4ea39ebdb38056cb66f3f7703bd425b53c4 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 15:32:57 +0200 Subject: [PATCH 03/15] Bump version to test release flow --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 6e8bf73aa..17e51c385 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.0 +0.1.1 From 537db550da22c34104294ebd2e9e69eb2180f84c Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 15:36:25 +0200 Subject: [PATCH 04/15] Back to previous version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 17e51c385..6e8bf73aa 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.1 +0.1.0 From 2db68651094fead97ad302239ec4149a5e02161f Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 15:37:06 +0200 Subject: [PATCH 05/15] Bump version to 0.1.1 --- .bumpversion.cfg | 2 +- VERSION | 2 +- pyproject.toml | 2 +- src/permifrost/__init__.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 76b135a12..7c7a0e8fd 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.1.0 +current_version = 0.1.1 commit = True tag = False parse = (?P\d+)\.(?P\d+)\.(?P\d+)(\-(?P[a-z]+)(?P\d+))? diff --git a/VERSION b/VERSION index 6e8bf73aa..17e51c385 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.0 +0.1.1 diff --git a/pyproject.toml b/pyproject.toml index f2c876b1c..49eba7462 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "gemma.permifrost" -version = "0.1.0" +version = "0.1.1" description = "Permifrost Permissions - Fork by Gemma Analytics" readme = "README.md" license = {text = "MIT"} diff --git a/src/permifrost/__init__.py b/src/permifrost/__init__.py index ad9277255..bcc5c572a 100644 --- a/src/permifrost/__init__.py +++ b/src/permifrost/__init__.py @@ -1,5 +1,5 @@ # Managed by bumpversion -__version__ = "0.1.0" +__version__ = "0.1.1" from permifrost.error import SpecLoadingError From 44759a04b852a49a0ee8cf404a0453360fab62c8 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:28:54 +0200 Subject: [PATCH 06/15] Fix lint errors --- src/permifrost/snowflake_connector.py | 2 +- src/permifrost/snowflake_spec_loader.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/permifrost/snowflake_connector.py b/src/permifrost/snowflake_connector.py index bd43c9696..3b812e1d3 100644 --- a/src/permifrost/snowflake_connector.py +++ b/src/permifrost/snowflake_connector.py @@ -386,7 +386,7 @@ def snowflaky(name: str) -> str: "Object identifier is Null", SyntaxWarning, ) - + new_name_parts = [] for part in name_parts: diff --git a/src/permifrost/snowflake_spec_loader.py b/src/permifrost/snowflake_spec_loader.py index 227a66501..6dc06c507 100644 --- a/src/permifrost/snowflake_spec_loader.py +++ b/src/permifrost/snowflake_spec_loader.py @@ -77,7 +77,7 @@ def check_permissions_on_snowflake_server( click.secho(f" Current user is: {conn.get_current_user()}.", fg="green") current_role = conn.get_current_role() - if not current_role in ["securityadmin", "accountadmin"]: + if current_role not in ["securityadmin", "accountadmin"]: error_messages.append( "Current role is not securityadmin or accountadmin! " "Permifrost expects to run as securityadmin or accountadmin, please update your connection settings." @@ -242,11 +242,11 @@ def remove_missing_entities( for missing_entity in missing_entities["dbs"]: click.secho(f"Ignored missing db {missing_entity}") self.entities["databases"].remove(missing_entity) - + # Some listed databases might not be present in the database_refs list if missing_entity in self.entities["database_refs"]: self.entities["database_refs"].remove(missing_entity) - + self.spec["databases"] = [ item for item in self.spec["databases"] @@ -287,7 +287,7 @@ def check_entities_on_snowflake_server( # noqa Raises a SpecLoadingError with all the errors found while checking Snowflake for missing entities. - If `ignore_missing_entities` is True, the missing entities are removed from the + If `ignore_missing_entities` is True, the missing entities are removed from the spec and entities objects, so that downstream code can continue to run normally. """ missing = {} @@ -310,7 +310,7 @@ def check_entities_on_snowflake_server( # noqa self.remove_missing_entities(missing) return - errors = list( + all_errors = list( itertools.chain( errors["warehouses"], errors["integrations"], @@ -322,8 +322,8 @@ def check_entities_on_snowflake_server( # noqa ) ) - if errors: - raise SpecLoadingError("\n".join(errors)) + if all_errors: + raise SpecLoadingError("\n".join(all_errors)) def get_role_privileges_from_snowflake_server( self, From 9f7118d371598dc3ac99cb881ba3adc0471f1502 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:29:04 +0200 Subject: [PATCH 07/15] Adjust mypy config --- mypy.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mypy.ini b/mypy.ini index f0b3bca6c..a14b6d7b2 100644 --- a/mypy.ini +++ b/mypy.ini @@ -7,6 +7,9 @@ ignore_missing_imports = True [mypy-sqlalchemy] ignore_missing_imports = True +[mypy-sqlalchemy.exc] +ignore_missing_imports = True + [mypy-snowflake.sqlalchemy] ignore_missing_imports = True From 17166d13bd1494af44ddac482e185d34e8be45f1 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:29:21 +0200 Subject: [PATCH 08/15] Bump actions step version --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0dc23561e..77432f619 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -97,7 +97,7 @@ jobs: permifrost --help - name: Upload build artifacts - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: dist-files path: dist/ From 1fb90a55361214d348377efb5142db7c64af3177 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:31:41 +0200 Subject: [PATCH 09/15] Test only 3.10 --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 77432f619..ba91661ac 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,7 +28,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.8", "3.9", "3.10", "3.11"] + python-version: ["3.10"] steps: - name: Checkout code From 182b8236ec4e14fad27ba2008550e72f85df8702 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:36:30 +0200 Subject: [PATCH 10/15] Adjust type comparison --- src/permifrost/entities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/permifrost/entities.py b/src/permifrost/entities.py index a6a8cbf76..e019efa9d 100644 --- a/src/permifrost/entities.py +++ b/src/permifrost/entities.py @@ -314,7 +314,7 @@ def generate_databases(self, db_list: List[Dict[str, Dict]]) -> None: for db_name, config in db_entry.items(): self.entities["databases"].add(db_name) if "shared" in config: - if type(config["shared"]) == bool: + if isinstance(config["shared"], bool): if config["shared"]: self.entities["shared_databases"].add(db_name) else: From 383aa19808e17d5cbba8191f638b2264917c890a Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:42:17 +0200 Subject: [PATCH 11/15] Adjust test workflow to use make recipe --- .github/workflows/test.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ba91661ac..5514520c0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -46,13 +46,9 @@ jobs: - name: Run linting run: | - flake8 src/ tests/ - isort --check-only src/ tests/ - black --check src/ tests/ + make ci-show-lint + - - name: Run type checking - run: | - mypy src/ - name: Run tests run: | From b81fefa4f3c1790398213ce76eae73746f199984 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:43:50 +0200 Subject: [PATCH 12/15] Change to Python 3.10 --- .github/workflows/manual-release.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/test.yml | 2 +- GITHUB_ACTIONS.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/manual-release.yml b/.github/workflows/manual-release.yml index 6dcb53883..f96156ca0 100644 --- a/.github/workflows/manual-release.yml +++ b/.github/workflows/manual-release.yml @@ -17,7 +17,7 @@ on: - production env: - PYTHON_VERSION: "3.11" + PYTHON_VERSION: "3.10" jobs: manual-release: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 617150227..8b7fc1cec 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,7 +13,7 @@ on: - 'VERSION' env: - PYTHON_VERSION: "3.11" + PYTHON_VERSION: "3.10" jobs: test-release: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5514520c0..dde31c546 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,7 +20,7 @@ on: - 'pyproject.toml' env: - PYTHON_VERSION: "3.11" + PYTHON_VERSION: "3.10" jobs: test: diff --git a/GITHUB_ACTIONS.md b/GITHUB_ACTIONS.md index 083570d26..6f57c0d64 100644 --- a/GITHUB_ACTIONS.md +++ b/GITHUB_ACTIONS.md @@ -19,7 +19,7 @@ We have three GitHub Actions workflows that handle different aspects of the rele - Pushes to `main`/`master` that modify the same files **What it does:** -- Runs tests on multiple Python versions (3.8, 3.9, 3.10, 3.11) +- Runs tests on Python 3.10 - Performs linting checks (flake8, isort, black) - Runs type checking with mypy - Executes pytest with coverage From fd1d7c003b32660591ef07a7b16d3b681dc0a7ef Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 16:50:03 +0200 Subject: [PATCH 13/15] Apply black --- pyproject.toml | 20 +- src/permifrost/cli/cli.py | 5 +- src/permifrost/cli/permissions.py | 12 +- src/permifrost/entities.py | 51 ++- src/permifrost/snowflake_connector.py | 31 +- src/permifrost/snowflake_grants.py | 378 +++++++++++++----- .../snowflake_role_grant_checker.py | 9 +- src/permifrost/snowflake_spec_loader.py | 69 +++- tests/conftest.py | 6 +- tests/permifrost/cli/test_cli.py | 8 +- tests/permifrost/test_entities.py | 22 +- tests/permifrost/test_snowflake_connector.py | 88 +++- tests/permifrost/test_snowflake_grants.py | 135 +++++-- .../test_snowflake_role_grant_checker.py | 50 ++- .../permifrost/test_snowflake_spec_loader.py | 285 +++++++++---- tests/permifrost/test_snowflake_user.py | 96 +++-- .../snowflake_schema_builder.py | 80 +++- 17 files changed, 1004 insertions(+), 341 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 49eba7462..c927cada6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,7 +43,7 @@ dependencies = [ [project.optional-dependencies] dev = [ - "black", + "black==22.3.0", "bumpversion", "changelog-cli", "coverage", @@ -71,3 +71,21 @@ where = ["src"] [tool.setuptools.package-data] "*" = ["*.txt", "*.md", "*.yml", "*.yaml"] + +[tool.black] +line-length = 80 +target-version = ['py38'] +include = '\.pyi?$' +extend-exclude = ''' +/( + # directories + \.eggs + | \.git + | \.hg + | \.mypy_cache + | \.tox + | \.venv + | build + | dist +)/ +''' diff --git a/src/permifrost/cli/cli.py b/src/permifrost/cli/cli.py index 7179eb21b..0cbeb6a9a 100644 --- a/src/permifrost/cli/cli.py +++ b/src/permifrost/cli/cli.py @@ -8,7 +8,10 @@ @click.group(invoke_without_command=True, no_args_is_help=True) @click.option( - "-v", "--verbose", help="Increases log level with count, e.g -vv", count=True + "-v", + "--verbose", + help="Increases log level with count, e.g -vv", + count=True, ) @click.version_option(version=permifrost.__version__, prog_name="permifrost") @click.pass_context diff --git a/src/permifrost/cli/permissions.py b/src/permifrost/cli/permissions.py index a99e48bd0..7d9c2f2fe 100644 --- a/src/permifrost/cli/permissions.py +++ b/src/permifrost/cli/permissions.py @@ -34,14 +34,18 @@ def print_command(command, diff, dry=False): foreground_color = "red" run_prefix = "[ERROR] " - click.secho(f"{diff_prefix}{run_prefix}{command['sql']};", fg=foreground_color) + click.secho( + f"{diff_prefix}{run_prefix}{command['sql']};", fg=foreground_color + ) @cli.command() # type: ignore @click.argument("spec") @click.option("--dry", help="Do not actually run, just check.", is_flag=True) @click.option( - "--diff", help="Show full diff, both new and existing permissions.", is_flag=True + "--diff", + help="Show full diff, both new and existing permissions.", + is_flag=True, ) @click.option( "--role", @@ -128,7 +132,9 @@ def run( default=["roles", "users"], help="Run grants for specific users. Usage: --user testuser --user testuser2.", ) -def spec_test(spec, role, user, run_list, ignore_memberships, ignore_missing_entities): +def spec_test( + spec, role, user, run_list, ignore_memberships, ignore_missing_entities +): """ Load SnowFlake spec based on the roles.yml provided. CLI use only for confirming specifications are valid. """ diff --git a/src/permifrost/entities.py b/src/permifrost/entities.py index e019efa9d..76525a9ab 100644 --- a/src/permifrost/entities.py +++ b/src/permifrost/entities.py @@ -51,7 +51,9 @@ def inspect_entities(self) -> EntitySchema: or a user given access to a database not defined in databases """ self.generate() - self.error_messages.extend(self.ensure_valid_entity_names(self.entities)) + self.error_messages.extend( + self.ensure_valid_entity_names(self.entities) + ) self.error_messages.extend( self.ensure_valid_spec_for_conditional_settings(self.entities) @@ -65,7 +67,9 @@ def inspect_entities(self) -> EntitySchema: return self.entities @staticmethod - def filter_grouped_entities_by_type(grouped_entities: List, type: str) -> List: + def filter_grouped_entities_by_type( + grouped_entities: List, type: str + ) -> List: """ Takes a list of grouped entities and filters them for a particular entity_type @@ -75,7 +79,9 @@ def filter_grouped_entities_by_type(grouped_entities: List, type: str) -> List: """ filtered_entities = [ - entries for entity_type, entries in grouped_entities if entity_type == type + entries + for entity_type, entries in grouped_entities + if entity_type == type ] # Avoid returning a nested list if there are entities @@ -147,7 +153,9 @@ def generate(self) -> EntitySchema: self.filter_grouped_entities_by_type(entities_by_type, "warehouses") ) self.generate_integrations( - self.filter_grouped_entities_by_type(entities_by_type, "integrations") + self.filter_grouped_entities_by_type( + entities_by_type, "integrations" + ) ) self.generate_users( self.filter_grouped_entities_by_type(entities_by_type, "users") @@ -184,7 +192,9 @@ def generate_implicit_refs_from_tables(self): self.entities["database_refs"].add(name_parts[0]) if name_parts[1] != "*": - self.entities["schema_refs"].add(f"{name_parts[0]}.{name_parts[1]}") + self.entities["schema_refs"].add( + f"{name_parts[0]}.{name_parts[1]}" + ) def ensure_valid_entity_names(self, entities: EntitySchema) -> List[str]: """ @@ -299,12 +309,16 @@ def check_entities_define_owner(self) -> List[str]: return error_messages - def generate_warehouses(self, warehouse_list: List[Dict[str, Dict]]) -> None: + def generate_warehouses( + self, warehouse_list: List[Dict[str, Dict]] + ) -> None: for warehouse_entry in warehouse_list: for warehouse_name, _ in warehouse_entry.items(): self.entities["warehouses"].add(warehouse_name) - def generate_integrations(self, integration_list: List[Dict[str, Dict]]) -> None: + def generate_integrations( + self, integration_list: List[Dict[str, Dict]] + ) -> None: for integration_entry in integration_list: for integration_name, _ in integration_entry.items(): self.entities["integrations"].add(integration_name) @@ -395,9 +409,10 @@ def generate_read_write_database_names(self, config): return (read_databases, write_databases) def generate_schema_roles(self, config, role_name): - read_databases, write_databases = self.generate_read_write_database_names( - config - ) + ( + read_databases, + write_databases, + ) = self.generate_read_write_database_names(config) try: for schema in config["privileges"]["schemas"]["read"]: @@ -434,9 +449,10 @@ def generate_schema_roles(self, config, role_name): ) def generate_table_roles(self, config, role_name): - read_databases, write_databases = self.generate_read_write_database_names( - config - ) + ( + read_databases, + write_databases, + ) = self.generate_read_write_database_names(config) try: for table in config["privileges"]["tables"]["read"]: @@ -541,9 +557,14 @@ def generate_users(self, user_list): for user_entry in user_list: for user_name, config in user_entry.items(): self.entities["users"].add(user_name) - self.generate_user_fn(config, "member_of", "role_refs", user_name) self.generate_user_fn( - config.get("owns", {}), "database", "database_refs", user_name + config, "member_of", "role_refs", user_name + ) + self.generate_user_fn( + config.get("owns", {}), + "database", + "database_refs", + user_name, ) self.generate_user_fn( config.get("owns", {}), "schemas", "schema_refs", user_name diff --git a/src/permifrost/snowflake_connector.py b/src/permifrost/snowflake_connector.py index 3b812e1d3..6ee847657 100644 --- a/src/permifrost/snowflake_connector.py +++ b/src/permifrost/snowflake_connector.py @@ -25,7 +25,9 @@ def __init__(self, config: Optional[Dict] = None) -> None: if not config: config = { "user": os.getenv("PERMISSION_BOT_USER"), - "password": quote_plus(os.getenv("PERMISSION_BOT_PASSWORD", "")), + "password": quote_plus( + os.getenv("PERMISSION_BOT_PASSWORD", "") + ), "account": os.getenv("PERMISSION_BOT_ACCOUNT"), "database": os.getenv("PERMISSION_BOT_DATABASE"), "role": os.getenv("PERMISSION_BOT_ROLE"), @@ -166,9 +168,7 @@ def show_tables( results = self.run_query(query).fetchall() for result in results: - table_identifier = ( - f"{result['database_name']}.{result['schema_name']}.{result['name']}" - ) + table_identifier = f"{result['database_name']}.{result['schema_name']}.{result['name']}" names.append(SnowflakeConnector.snowflaky(table_identifier)) return names @@ -188,9 +188,7 @@ def show_views( results = self.run_query(query).fetchall() for result in results: - view_identifier = ( - f"{result['database_name']}.{result['schema_name']}.{result['name']}" - ) + view_identifier = f"{result['database_name']}.{result['schema_name']}.{result['name']}" names.append(SnowflakeConnector.snowflaky(view_identifier)) return names @@ -215,9 +213,11 @@ def show_future_grants( privilege = result["privilege"].lower() granted_on = result["grant_on"].lower() - future_grants.setdefault(role, {}).setdefault(privilege, {}).setdefault( - granted_on, [] - ).append(SnowflakeConnector.snowflaky(result["name"])) + future_grants.setdefault(role, {}).setdefault( + privilege, {} + ).setdefault(granted_on, []).append( + SnowflakeConnector.snowflaky(result["name"]) + ) else: continue @@ -255,9 +255,9 @@ def show_grants_to_role_with_grant_option(self, role) -> Dict[str, Any]: grant_option = result["grant_option"].lower() == "true" name = SnowflakeConnector.snowflaky(result["name"]) - grants.setdefault(privilege, {}).setdefault(granted_on, {}).setdefault( - name, {} - ).update({"grant_option": grant_option}) + grants.setdefault(privilege, {}).setdefault( + granted_on, {} + ).setdefault(name, {}).update({"grant_option": grant_option}) return grants @@ -395,7 +395,10 @@ def snowflaky(name: str) -> str: new_name_parts.append(part) # If a future object, return in lower case - no need to quote - elif re.match("<(table|view|schema)>", part, re.IGNORECASE) is not None: + elif ( + re.match("<(table|view|schema)>", part, re.IGNORECASE) + is not None + ): new_name_parts.append(part.lower()) # If does not meet requirements for unquoted object identifiers or collides with reserved keywords, diff --git a/src/permifrost/snowflake_grants.py b/src/permifrost/snowflake_grants.py index d070d65f6..c1405acbb 100644 --- a/src/permifrost/snowflake_grants.py +++ b/src/permifrost/snowflake_grants.py @@ -65,7 +65,9 @@ def is_granted_privilege( """ grants = ( - self.grants_to_role.get(role, {}).get(privilege, {}).get(entity_type, []) + self.grants_to_role.get(role, {}) + .get(privilege, {}) + .get(entity_type, []) ) if SnowflakeConnector.snowflaky(entity_name) in grants: @@ -73,7 +75,9 @@ def is_granted_privilege( return False - def _generate_member_lists(self, config: Dict) -> Tuple[List[str], List[str]]: + def _generate_member_lists( + self, config: Dict + ) -> Tuple[List[str], List[str]]: """ Generate a tuple with the member_include_list (e.g. roles that should be granted) and member_exclude_list (e.g. roles that should not be granted) @@ -106,7 +110,9 @@ def _generate_member_lists(self, config: Dict) -> Tuple[List[str], List[str]]: return (member_include_list, member_exclude_list) - def _generate_member_star_lists(self, all_entities: List, entity: str) -> List[str]: + def _generate_member_star_lists( + self, all_entities: List, entity: str + ) -> List[str]: """ Generates the member include list when a * privilege is granted @@ -118,7 +124,9 @@ def _generate_member_star_lists(self, all_entities: List, entity: str) -> List[s conn = SnowflakeConnector() show_roles = conn.show_roles() member_include_list = [ - role for role in show_roles if role in all_entities and role != entity + role + for role in show_roles + if role in all_entities and role != entity ] return member_include_list @@ -150,7 +158,9 @@ def _generate_sql_commands_for_member_of_list( and granted_role in self.roles_granted_to_user[entity] ) or ( entity_type == "roles" - and self.is_granted_privilege(entity, "usage", "role", member_role) + and self.is_granted_privilege( + entity, "usage", "role", member_role + ) ): already_granted = True @@ -172,9 +182,13 @@ def _generate_sql_commands_for_member_of_list( { "already_granted": already_granted, "sql": GRANT_ROLE_TEMPLATE.format( - role_name=SnowflakeConnector.snowflaky_user_role(member_role), + role_name=SnowflakeConnector.snowflaky_user_role( + member_role + ), type=grant_type, - entity_name=SnowflakeConnector.snowflaky_user_role(entity), + entity_name=SnowflakeConnector.snowflaky_user_role( + entity + ), ), } ) @@ -209,7 +223,9 @@ def _generate_revoke_sql_commands_for_user( def _generate_revoke_sql_commands_for_role(self, rolename, member_of_list): sql_commands = [] for granted_role in ( - self.grants_to_role.get(rolename, {}).get("usage", {}).get("role", []) + self.grants_to_role.get(rolename, {}) + .get("usage", {}) + .get("role", []) ): if granted_role not in member_of_list: snowflake_default_roles = [ @@ -262,17 +278,23 @@ def generate_grant_roles( if self.ignore_memberships: return sql_commands - member_include_list, member_exclude_list = self._generate_member_lists(config) + member_include_list, member_exclude_list = self._generate_member_lists( + config + ) if len(member_include_list) == 1 and member_include_list[0] == '"*"': if not all_entities: raise ValueError( "Cannot generate grant roles if all_entities not provided" ) - member_include_list = self._generate_member_star_lists(all_entities, entity) + member_include_list = self._generate_member_star_lists( + all_entities, entity + ) member_of_list = [ - role for role in member_include_list if role not in member_exclude_list + role + for role in member_include_list + if role not in member_exclude_list ] sql_commands.extend( @@ -282,19 +304,27 @@ def generate_grant_roles( ) if entity_type == "users": sql_commands.extend( - self._generate_revoke_sql_commands_for_user(entity, member_of_list) + self._generate_revoke_sql_commands_for_user( + entity, member_of_list + ) ) if entity_type == "roles": sql_commands.extend( - self._generate_revoke_sql_commands_for_role(entity, member_of_list) + self._generate_revoke_sql_commands_for_role( + entity, member_of_list + ) ) return sql_commands def _generate_database_commands(self, role, config, shared_dbs, spec_dbs): databases = { - "read": config.get("privileges", {}).get("databases", {}).get("read", []), - "write": config.get("privileges", {}).get("databases", {}).get("write", []), + "read": config.get("privileges", {}) + .get("databases", {}) + .get("read", []), + "write": config.get("privileges", {}) + .get("databases", {}) + .get("write", []), } if len(databases.get("read", "")) == 0: @@ -312,14 +342,21 @@ def _generate_database_commands(self, role, config, shared_dbs, spec_dbs): ) database_commands = self.generate_database_grants( - role=role, databases=databases, shared_dbs=shared_dbs, spec_dbs=spec_dbs + role=role, + databases=databases, + shared_dbs=shared_dbs, + spec_dbs=spec_dbs, ) return database_commands def _generate_schema_commands(self, role, config, shared_dbs, spec_dbs): schemas = { - "read": config.get("privileges", {}).get("schemas", {}).get("read", []), - "write": config.get("privileges", {}).get("schemas", {}).get("write", []), + "read": config.get("privileges", {}) + .get("schemas", {}) + .get("read", []), + "write": config.get("privileges", {}) + .get("schemas", {}) + .get("write", []), } if len(schemas.get("read", "")) == 0: @@ -343,8 +380,12 @@ def _generate_schema_commands(self, role, config, shared_dbs, spec_dbs): def _generate_table_commands(self, role, config, shared_dbs, spec_dbs): tables = { - "read": config.get("privileges", {}).get("tables", {}).get("read", []), - "write": config.get("privileges", {}).get("tables", {}).get("write", []), + "read": config.get("privileges", {}) + .get("tables", {}) + .get("read", []), + "write": config.get("privileges", {}) + .get("tables", {}) + .get("write", []), } if len(tables.get("read", "")) == 0: @@ -457,14 +498,18 @@ def generate_warehouse_grants( "sql": GRANT_PRIVILEGES_TEMPLATE.format( privileges=priv, resource_type="warehouse", - resource_name=SnowflakeConnector.snowflaky(warehouse), + resource_name=SnowflakeConnector.snowflaky( + warehouse + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } ) for priv in ["usage", "operate", "monitor"]: for granted_warehouse in ( - self.grants_to_role.get(role, {}).get(priv, {}).get("warehouse", []) + self.grants_to_role.get(role, {}) + .get(priv, {}) + .get("warehouse", []) ): if granted_warehouse not in warehouses: sql_commands.append( @@ -476,7 +521,9 @@ def generate_warehouse_grants( resource_name=SnowflakeConnector.snowflaky( granted_warehouse ), - role=SnowflakeConnector.snowflaky_user_role(role), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -512,9 +559,15 @@ def generate_integration_grants( ), } ) - print(self.grants_to_role.get(role, {}).get("usage", {}).get("integration", [])) + print( + self.grants_to_role.get(role, {}) + .get("usage", {}) + .get("integration", []) + ) for granted_integration in ( - self.grants_to_role.get(role, {}).get("usage", {}).get("integration", []) + self.grants_to_role.get(role, {}) + .get("usage", {}) + .get("integration", []) ): if granted_integration not in integrations: sql_commands.append( @@ -534,9 +587,15 @@ def generate_integration_grants( return sql_commands def _generate_database_read_privs( - self, database: str, role: str, shared_dbs: Set[str], read_privileges: str + self, + database: str, + role: str, + shared_dbs: Set[str], + read_privileges: str, ) -> Dict: - already_granted = self.is_granted_privilege(role, "usage", "database", database) + already_granted = self.is_granted_privilege( + role, "usage", "database", database + ) # If this is a shared database, we have to grant the "imported privileges" # privilege to the user and skip granting the specific permissions as @@ -563,7 +622,11 @@ def _generate_database_read_privs( } def generate_database_grants( - self, role: str, databases: Dict[str, List], shared_dbs: Set, spec_dbs: Set + self, + role: str, + databases: Dict[str, List], + shared_dbs: Set, + spec_dbs: Set, ) -> List[Dict[str, Any]]: """ Generate the GRANT and REVOKE statements for Databases @@ -594,7 +657,9 @@ def generate_database_grants( for database in databases.get("write", []): already_granted = ( self.is_granted_privilege(role, "usage", "database", database) - and self.is_granted_privilege(role, "monitor", "database", database) + and self.is_granted_privilege( + role, "monitor", "database", database + ) and self.is_granted_privilege( role, "create schema", "database", database ) @@ -610,7 +675,9 @@ def generate_database_grants( "sql": GRANT_PRIVILEGES_TEMPLATE.format( privileges="imported privileges", resource_type="database", - resource_name=SnowflakeConnector.snowflaky(database), + resource_name=SnowflakeConnector.snowflaky( + database + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -635,19 +702,24 @@ def generate_database_grants( # Compare granted usage to full read/write usage set # and revoke missing ones usage_privs_on_db = ( - self.grants_to_role.get(role, {}).get("usage", {}).get("database", []) + self.grants_to_role.get(role, {}) + .get("usage", {}) + .get("database", []) ) for granted_database in usage_privs_on_db: # If it's a shared database, only revoke imported # We'll only know if it's a shared DB based on the spec - all_databases = databases.get("read", []) + databases.get("write", []) + all_databases = databases.get("read", []) + databases.get( + "write", [] + ) if granted_database not in spec_dbs: # Skip revocation on database that are not defined in spec continue # Revoke read/write permissions on shared databases elif ( - granted_database not in all_databases and granted_database in shared_dbs + granted_database not in all_databases + and granted_database in shared_dbs ): sql_commands.append( { @@ -682,7 +754,9 @@ def generate_database_grants( # This also preserves the case where somebody switches write access # for read access monitor_privs_on_db = ( - self.grants_to_role.get(role, {}).get("monitor", {}).get("database", []) + self.grants_to_role.get(role, {}) + .get("monitor", {}) + .get("database", []) ) create_privs_on_db = ( @@ -769,7 +843,9 @@ def _generate_schema_read_grants( privileges=read_privileges, resource_type="schema", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database), + grouping_name=SnowflakeConnector.snowflaky( + database + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -789,7 +865,9 @@ def _generate_schema_read_grants( "sql": GRANT_PRIVILEGES_TEMPLATE.format( privileges=read_privileges, resource_type="schema", - resource_name=SnowflakeConnector.snowflaky(db_schema), + resource_name=SnowflakeConnector.snowflaky( + db_schema + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -848,7 +926,9 @@ def _generate_schema_write_grants( privileges=write_privileges, resource_type="schema", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database), + grouping_name=SnowflakeConnector.snowflaky( + database + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -870,7 +950,9 @@ def _generate_schema_write_grants( "sql": GRANT_PRIVILEGES_TEMPLATE.format( privileges=write_privileges, resource_type="schema", - resource_name=SnowflakeConnector.snowflaky(db_schema), + resource_name=SnowflakeConnector.snowflaky( + db_schema + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -903,7 +985,9 @@ def _generate_schema_revokes( privileges=read_privileges, resource_type="schema", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -920,7 +1004,9 @@ def _generate_schema_revokes( "sql": REVOKE_PRIVILEGES_TEMPLATE.format( privileges=read_privileges, resource_type="schema", - resource_name=SnowflakeConnector.snowflaky(granted_schema), + resource_name=SnowflakeConnector.snowflaky( + granted_schema + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -930,7 +1016,11 @@ def _generate_schema_revokes( # TODO: This method is too complex, consider refactoring def generate_schema_grants( - self, role: str, schemas: Dict[str, List], shared_dbs: Set, spec_dbs: Set + self, + role: str, + schemas: Dict[str, List], + shared_dbs: Set, + spec_dbs: Set, ) -> List[Dict]: """ Generate the GRANT and REVOKE statements for schemas @@ -1005,7 +1095,9 @@ def generate_schema_grants( other_schema_grants = list() for privilege in other_privileges: other_schema_grants.extend( - self.grants_to_role.get(role, {}).get(privilege, {}).get("schema", []) + self.grants_to_role.get(role, {}) + .get(privilege, {}) + .get("schema", []) ) for granted_schema in other_schema_grants: @@ -1028,7 +1120,9 @@ def generate_schema_grants( privileges=partial_write_privileges, resource_type="schema", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1044,7 +1138,9 @@ def generate_schema_grants( "sql": REVOKE_PRIVILEGES_TEMPLATE.format( privileges=partial_write_privileges, resource_type="schema", - resource_name=SnowflakeConnector.snowflaky(granted_schema), + resource_name=SnowflakeConnector.snowflaky( + granted_schema + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1080,17 +1176,23 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): read_table_list = [] read_view_list = [] - fetched_schemas = conn.full_schema_list(f"{database_name}.{schema_name}") + fetched_schemas = conn.full_schema_list( + f"{database_name}.{schema_name}" + ) # For grants at the database level for tables - future_database_table = "{database}.".format(database=database_name) + future_database_table = "{database}.
".format( + database=database_name + ) table_already_granted = self.is_granted_privilege( role, read_privileges, "table", future_database_table ) read_grant_tables_full.append(future_database_table) # For grants at the database level for views - future_database_view = "{database}.".format(database=database_name) + future_database_view = "{database}.".format( + database=database_name + ) view_already_granted = self.is_granted_privilege( role, read_privileges, "view", future_database_view ) @@ -1105,7 +1207,9 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="table", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1118,7 +1222,9 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="table", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1132,7 +1238,9 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="view", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1145,7 +1253,9 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="view", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1188,8 +1298,12 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="table", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1202,8 +1316,12 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="table", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1220,8 +1338,12 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="view", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1234,8 +1356,12 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): privileges=read_privileges, resource_type="view", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1266,7 +1392,9 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): "sql": GRANT_PRIVILEGES_TEMPLATE.format( privileges=read_privileges, resource_type="table", - resource_name=SnowflakeConnector.snowflaky(db_table), + resource_name=SnowflakeConnector.snowflaky( + db_table + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1293,13 +1421,17 @@ def _generate_table_read_grants(self, conn, tables, shared_dbs, role): return (sql_commands, read_grant_tables_full, read_grant_views_full) # TODO: This method remains complex, could use extra refactoring - def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa + def _generate_table_write_grants( + self, conn, tables, shared_dbs, role + ): # noqa sql_commands = [] write_grant_tables_full = [] write_grant_views_full = [] read_privileges = "select" - write_partial_privileges = "insert, update, delete, truncate, references" + write_partial_privileges = ( + "insert, update, delete, truncate, references" + ) write_privileges = f"{read_privileges}, {write_partial_privileges}" write_privileges_array = write_privileges.split(", ") @@ -1324,11 +1456,17 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa write_table_list = [] write_view_list = [] - fetched_schemas = conn.full_schema_list(f"{database_name}.{name_parts[1]}") + fetched_schemas = conn.full_schema_list( + f"{database_name}.{name_parts[1]}" + ) # For grants at the database level - future_database_table = "{database}.
".format(database=database_name) - future_database_view = "{database}.".format(database=database_name) + future_database_table = "{database}.
".format( + database=database_name + ) + future_database_view = "{database}.".format( + database=database_name + ) table_already_granted = True for privilege in write_privileges_array: @@ -1353,7 +1491,9 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges=write_privileges, resource_type="table", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1366,7 +1506,9 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges=write_privileges, resource_type="table", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1380,7 +1522,9 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges="select", resource_type="view", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1393,7 +1537,9 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges="select", resource_type="view", grouping_type="database", - grouping_name=SnowflakeConnector.snowflaky(database_name), + grouping_name=SnowflakeConnector.snowflaky( + database_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1439,8 +1585,12 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges=write_privileges, resource_type="table", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1453,8 +1603,12 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges=write_privileges, resource_type="table", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1470,8 +1624,12 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges="select", resource_type="view", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1484,8 +1642,12 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa privileges="select", resource_type="view", grouping_type="schema", - grouping_name=SnowflakeConnector.snowflaky(schema), - role=SnowflakeConnector.snowflaky_user_role(role), + grouping_name=SnowflakeConnector.snowflaky( + schema + ), + role=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1519,7 +1681,9 @@ def _generate_table_write_grants(self, conn, tables, shared_dbs, role): # noqa "sql": GRANT_PRIVILEGES_TEMPLATE.format( privileges=write_privileges, resource_type="table", - resource_name=SnowflakeConnector.snowflaky(db_table), + resource_name=SnowflakeConnector.snowflaky( + db_table + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1584,7 +1748,9 @@ def _generate_revoke_select_privs( grouping_type = "database" grouping_name = database_name else: - future_resource = f"{database_name}.{schema_name}.<{resource_type}>" + future_resource = ( + f"{database_name}.{schema_name}.<{resource_type}>" + ) grouping_type = "schema" grouping_name = f"{database_name}.{schema_name}" @@ -1606,7 +1772,9 @@ def _generate_revoke_select_privs( privileges=privilege_set, resource_type=resource_type, grouping_type=grouping_type, - grouping_name=SnowflakeConnector.snowflaky(grouping_name), + grouping_name=SnowflakeConnector.snowflaky( + grouping_name + ), role=SnowflakeConnector.snowflaky_user_role(role), ), } @@ -1642,10 +1810,16 @@ def generate_revoke_privs( write_grant_tables_full: List[str], ) -> List[Dict[str, Any]]: read_privileges = "select" - write_partial_privileges = "insert, update, delete, truncate, references" + write_partial_privileges = ( + "insert, update, delete, truncate, references" + ) sql_commands = [] granted_resources = list( - set(self.grants_to_role.get(role, {}).get("select", {}).get("table", [])) + set( + self.grants_to_role.get(role, {}) + .get("select", {}) + .get("table", []) + ) ) sql_commands.extend( @@ -1660,7 +1834,11 @@ def generate_revoke_privs( ) ) granted_resources = list( - set(self.grants_to_role.get(role, {}).get("select", {}).get("view", [])) + set( + self.grants_to_role.get(role, {}) + .get("select", {}) + .get("view", []) + ) ) sql_commands.extend( self._generate_revoke_select_privs( @@ -1677,10 +1855,14 @@ def generate_revoke_privs( all_write_privs_granted_tables = [] for privilege in write_partial_privileges.split(", "): table_names = ( - self.grants_to_role.get(role, {}).get(privilege, {}).get("table", []) + self.grants_to_role.get(role, {}) + .get(privilege, {}) + .get("table", []) ) all_write_privs_granted_tables += table_names - all_write_privs_granted_tables = list(set(all_write_privs_granted_tables)) + all_write_privs_granted_tables = list( + set(all_write_privs_granted_tables) + ) # Write Privileges # Only need to revoke write privileges for tables since SELECT is the @@ -1736,7 +1918,11 @@ def generate_table_and_view_grants( read_grant_views_full.extend(read_views) write_tables = tables.get("write", []) - write_command, write_table, write_views = self._generate_table_write_grants( + ( + write_command, + write_table, + write_views, + ) = self._generate_table_write_grants( conn, write_tables, shared_dbs, role ) sql_commands.extend(write_command) @@ -1758,7 +1944,9 @@ def generate_table_and_view_grants( ) return sql_commands - def generate_alter_user(self, user: str, config: Dict[str, Any]) -> List[Dict]: + def generate_alter_user( + self, user: str, config: Dict[str, Any] + ) -> List[Dict]: """ Generate the ALTER statements for USERs. @@ -1831,7 +2019,9 @@ def _generate_ownership_grant_database( ) return sql_commands - def _generate_ownership_grant_schema(self, conn, role, schema_refs) -> List[Dict]: + def _generate_ownership_grant_schema( + self, conn, role, schema_refs + ) -> List[Dict]: sql_commands = [] for schema in schema_refs: name_parts = schema.split(".") @@ -1858,8 +2048,12 @@ def _generate_ownership_grant_schema(self, conn, role, schema_refs) -> List[Dict "already_granted": already_granted, "sql": GRANT_OWNERSHIP_TEMPLATE.format( resource_type="schema", - resource_name=SnowflakeConnector.snowflaky(db_schema), - role_name=SnowflakeConnector.snowflaky_user_role(role), + resource_name=SnowflakeConnector.snowflaky( + db_schema + ), + role_name=SnowflakeConnector.snowflaky_user_role( + role + ), ), } ) @@ -1937,7 +2131,9 @@ def generate_grant_ownership( # noqa db_refs = config.get("owns", {}).get("databases") if db_refs: - db_ownership_grants = self._generate_ownership_grant_database(role, db_refs) + db_ownership_grants = self._generate_ownership_grant_database( + role, db_refs + ) sql_commands.extend(db_ownership_grants) schema_refs = config.get("owns", {}).get("schemas") diff --git a/src/permifrost/snowflake_role_grant_checker.py b/src/permifrost/snowflake_role_grant_checker.py index f6ae0be25..e4d225841 100644 --- a/src/permifrost/snowflake_role_grant_checker.py +++ b/src/permifrost/snowflake_role_grant_checker.py @@ -39,7 +39,10 @@ def get_permissions(self, role: str) -> List[SnowflakePermission]: name = entity_name if entity_type != "account" else "*" role_permissions.append( SnowflakePermission( - name, entity_type, [privilege], options["grant_option"] + name, + entity_type, + [privilege], + options["grant_option"], ) ) return role_permissions @@ -61,7 +64,9 @@ def _has_permission( if permission.as_owner() in role_permissions: # Since we don't check the grant_option of a permission when checking equality, we want to make sure # to return the actual permission value that was stored in the database to be most correct. - return role_permissions[role_permissions.index(permission.as_owner())] + return role_permissions[ + role_permissions.index(permission.as_owner()) + ] if permission in role_permissions: return role_permissions[role_permissions.index(permission)] return None diff --git a/src/permifrost/snowflake_spec_loader.py b/src/permifrost/snowflake_spec_loader.py index 6dc06c507..3486a0826 100644 --- a/src/permifrost/snowflake_spec_loader.py +++ b/src/permifrost/snowflake_spec_loader.py @@ -40,7 +40,9 @@ def __init__( # Connect to Snowflake to make sure that the current user has correct # permissions - click.secho("Checking permissions on current snowflake connection", fg="green") + click.secho( + "Checking permissions on current snowflake connection", fg="green" + ) self.check_permissions_on_snowflake_server(conn) # Connect to Snowflake to make sure that all entities defined in the @@ -58,7 +60,9 @@ def __init__( self.roles_granted_to_user: Dict[str, Any] = {} if not spec_test: - click.secho("Fetching granted privileges from Snowflake", fg="green") + click.secho( + "Fetching granted privileges from Snowflake", fg="green" + ) self.get_privileges_from_snowflake_server( conn, roles=roles, @@ -74,7 +78,9 @@ def check_permissions_on_snowflake_server( conn = SnowflakeConnector() error_messages = [] - click.secho(f" Current user is: {conn.get_current_user()}.", fg="green") + click.secho( + f" Current user is: {conn.get_current_user()}.", fg="green" + ) current_role = conn.get_current_role() if current_role not in ["securityadmin", "accountadmin"]: @@ -136,7 +142,9 @@ def check_database_entities(self, conn): " Snowflake Server. Please create it before continuing." ) else: - logger.debug("`databases` not found in spec, skipping SHOW DATABASES call.") + logger.debug( + "`databases` not found in spec, skipping SHOW DATABASES call." + ) return missing_entities, error_messages def check_schema_ref_entities(self, conn): @@ -152,7 +160,9 @@ def check_schema_ref_entities(self, conn): " Snowflake Server. Please create it before continuing." ) else: - logger.debug("`schemas` not found in spec, skipping SHOW SCHEMAS call.") + logger.debug( + "`schemas` not found in spec, skipping SHOW SCHEMAS call." + ) return missing_entities, error_messages def check_table_ref_entities(self, conn): @@ -182,7 +192,9 @@ def check_table_ref_entities(self, conn): " Snowflake Server. Please create it before continuing." ) else: - logger.debug("`tables` not found in spec, skipping SHOW TABLES/VIEWS call.") + logger.debug( + "`tables` not found in spec, skipping SHOW TABLES/VIEWS call." + ) return missing_entities, error_messages def check_role_entities(self, conn): @@ -262,23 +274,31 @@ def remove_missing_entities( click.secho(f"Ignored missing role {missing_entity}") self.entities["roles"].remove(missing_entity) self.spec["roles"] = [ - item for item in self.spec["roles"] if missing_entity not in item.keys() + item + for item in self.spec["roles"] + if missing_entity not in item.keys() ] for role_name in self.spec["roles"]: role = role_name[list(role_name.keys())[0]] if "member_of" in role.keys(): role["member_of"] = [ - item for item in role["member_of"] if item != missing_entity + item + for item in role["member_of"] + if item != missing_entity ] for missing_entity in missing_entities["users"]: click.secho(f"Ignored missing user {missing_entity}") self.entities["users"].remove(missing_entity) self.spec["users"] = [ - item for item in self.spec["users"] if missing_entity not in item.keys() + item + for item in self.spec["users"] + if missing_entity not in item.keys() ] def check_entities_on_snowflake_server( # noqa - self, conn: Optional[SnowflakeConnector] = None, ignore_missing_entities=False + self, + conn: Optional[SnowflakeConnector] = None, + ignore_missing_entities=False, ) -> None: """ Make sure that all [warehouses, integrations, dbs, schemas, tables, users, roles] @@ -356,7 +376,9 @@ def get_role_privileges_from_snowflake_server( .extend( self.filter_to_database_refs( grant_on=grant_on, - filter_set=grant_results[role][privilege][grant_on], + filter_set=grant_results[role][privilege][ + grant_on + ], ) ) ) @@ -387,9 +409,9 @@ def get_role_privileges_from_snowflake_server( .extend( self.filter_to_database_refs( grant_on=grant_on, - filter_set=grant_results[role][privilege][ - grant_on - ], + filter_set=grant_results[role][ + privilege + ][grant_on], ) ) ) @@ -424,7 +446,9 @@ def get_user_privileges_from_snowflake_server( if users and user not in users: continue logger.info(f"Fetching user privileges for user: {user}") - self.roles_granted_to_user[user] = conn.show_roles_granted_to_user(user) + self.roles_granted_to_user[ + user + ] = conn.show_roles_granted_to_user(user) def get_privileges_from_snowflake_server( self, @@ -445,7 +469,9 @@ def get_privileges_from_snowflake_server( if "users" in run_list and not ignore_memberships: logger.info("Fetching user privileges from Snowflake") - self.get_user_privileges_from_snowflake_server(conn=conn, users=users) + self.get_user_privileges_from_snowflake_server( + conn=conn, users=users + ) if "roles" in run_list: logger.info("Fetching role privileges from Snowflake") @@ -490,7 +516,8 @@ def filter_to_database_refs( return [ item for item in filter_set - if item and ("." not in item or item.split(".")[0] in database_refs) + if item + and ("." not in item or item.split(".")[0] in database_refs) ] def generate_permission_queries( @@ -572,7 +599,9 @@ def generate_permission_queries( # TODO: These functions are part of a refactor of the previous module, # but this still requires a fair bit of attention to cleanup - def process_roles(self, generator, entity_type, entity_name, config, all_entities): + def process_roles( + self, generator, entity_type, entity_name, config, all_entities + ): sql_commands = [] click.secho(f" Processing role {entity_name}", fg="green") sql_commands.extend( @@ -581,7 +610,9 @@ def process_roles(self, generator, entity_type, entity_name, config, all_entitie ) ) - sql_commands.extend(generator.generate_grant_ownership(entity_name, config)) + sql_commands.extend( + generator.generate_grant_ownership(entity_name, config) + ) sql_commands.extend( generator.generate_grant_privileges_to_role( diff --git a/tests/conftest.py b/tests/conftest.py index e79efe9fa..7c05f4493 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,5 +48,9 @@ def pytest_itemcollected(item): suf = suf = " ".join(node.__doc__.split()) + " " if node.__doc__ else "" if pref or suf: item._nodeid = ( - Fore.YELLOW + "".join((pref, suf)) + "\n" + Style.RESET_ALL + item._nodeid + Fore.YELLOW + + "".join((pref, suf)) + + "\n" + + Style.RESET_ALL + + item._nodeid ) diff --git a/tests/permifrost/cli/test_cli.py b/tests/permifrost/cli/test_cli.py index 956b368e2..3ed6efe2c 100644 --- a/tests/permifrost/cli/test_cli.py +++ b/tests/permifrost/cli/test_cli.py @@ -5,7 +5,9 @@ def test_version(cli_runner): cli_version = cli_runner.invoke(cli, ["--version"]) - assert cli_version.output == f"permifrost, version {permifrost.__version__}\n" + assert ( + cli_version.output == f"permifrost, version {permifrost.__version__}\n" + ) def test_run_command(cli_runner): @@ -16,7 +18,9 @@ def test_run_command(cli_runner): def test_load_command(cli_runner): - cli_spec_test_command = cli_runner.invoke(cli.commands["spec-test"], ["--help"]) + cli_spec_test_command = cli_runner.invoke( + cli.commands["spec-test"], ["--help"] + ) cli_output = cli_spec_test_command.output assert (len(cli_output) >= 5) and (cli_output[:5] == "Usage") diff --git a/tests/permifrost/test_entities.py b/tests/permifrost/test_entities.py index 33458bb94..85bcef98e 100644 --- a/tests/permifrost/test_entities.py +++ b/tests/permifrost/test_entities.py @@ -41,7 +41,14 @@ def test_db_refs(self, entities): Expect all actionable references from the roles section in snowflake_spec_reference_roles.yml spec """ - expected = {"demodb", "demodb2", "demodb3", "demodb4", "demodb5", "demodb6"} + expected = { + "demodb", + "demodb2", + "demodb3", + "demodb4", + "demodb5", + "demodb6", + } assert entities["database_refs"] == expected def test_schema_refs(self, entities): @@ -138,9 +145,18 @@ def test_entity_integrations(self, entities): def test_filter_by_type(entities): - expected = {"demo", "sysadmin", "accountadmin", "useradmin", "securityadmin", "*"} + expected = { + "demo", + "sysadmin", + "accountadmin", + "useradmin", + "securityadmin", + "*", + } grouped_entities = EntityGenerator.group_spec_by_type(entities) assert ( - EntityGenerator.filter_grouped_entities_by_type(grouped_entities, "roles") + EntityGenerator.filter_grouped_entities_by_type( + grouped_entities, "roles" + ) == expected ) diff --git a/tests/permifrost/test_snowflake_connector.py b/tests/permifrost/test_snowflake_connector.py index 0d5197a54..e4fd1de3b 100644 --- a/tests/permifrost/test_snowflake_connector.py +++ b/tests/permifrost/test_snowflake_connector.py @@ -39,13 +39,26 @@ def test_snowflaky(self): assert SnowflakeConnector.snowflaky(db1) == 'analytics."schema"."table"' assert SnowflakeConnector.snowflaky(db2) == '"1234raw"."schema"."table"' - assert SnowflakeConnector.snowflaky(db3) == '"123-with-quotes"."schema"."table"' - assert SnowflakeConnector.snowflaky(db4) == '"1_db-9-RANDOM"."schema"."table"' - assert SnowflakeConnector.snowflaky(db5) == "database_1.schema_1.table_1" - assert SnowflakeConnector.snowflaky(db6) == "database_1.schema_1.table$a" - assert SnowflakeConnector.snowflaky(db7) == 'database_1.schema_1."GROUP"' assert ( - SnowflakeConnector.snowflaky(db8) == 'database_1.schema_1."1_LEADING_DIGIT"' + SnowflakeConnector.snowflaky(db3) + == '"123-with-quotes"."schema"."table"' + ) + assert ( + SnowflakeConnector.snowflaky(db4) + == '"1_db-9-RANDOM"."schema"."table"' + ) + assert ( + SnowflakeConnector.snowflaky(db5) == "database_1.schema_1.table_1" + ) + assert ( + SnowflakeConnector.snowflaky(db6) == "database_1.schema_1.table$a" + ) + assert ( + SnowflakeConnector.snowflaky(db7) == 'database_1.schema_1."GROUP"' + ) + assert ( + SnowflakeConnector.snowflaky(db8) + == 'database_1.schema_1."1_LEADING_DIGIT"' ) assert ( SnowflakeConnector.snowflaky(db9) @@ -64,21 +77,27 @@ def test_snowflaky(self): == 'database_1.schema_1."Case_Sensitive_Table_Name"' ) - assert SnowflakeConnector.snowflaky(db13) == "database_1.schema_1.
" + assert ( + SnowflakeConnector.snowflaky(db13) == "database_1.schema_1.
" + ) assert ( - SnowflakeConnector.snowflaky(db14) == 'database_1."1_LEADING_DIGIT".
' + SnowflakeConnector.snowflaky(db14) + == 'database_1."1_LEADING_DIGIT".
' ) assert ( - SnowflakeConnector.snowflaky(db15) == 'database_1."1_LEADING_DIGIT".
' + SnowflakeConnector.snowflaky(db15) + == 'database_1."1_LEADING_DIGIT".
' ) with pytest.warns(SyntaxWarning): SnowflakeConnector.snowflaky(db16) SnowflakeConnector.snowflaky(db17) - assert SnowflakeConnector.snowflaky(db18) == 'database_1.schema_1."GROUP"' + assert ( + SnowflakeConnector.snowflaky(db18) == 'database_1.schema_1."GROUP"' + ) assert SnowflakeConnector.snowflaky(db19) == "" @@ -96,7 +115,9 @@ def test_uses_key_pair_if_available(self, mocker, snowflake_connector_env): test_private_key = "TEST_PK" mocker.patch.object( - SnowflakeConnector, "generate_private_key", return_value=test_private_key + SnowflakeConnector, + "generate_private_key", + return_value=test_private_key, ) os.environ["PERMISSION_BOT_KEY_PATH"] = "TEST" @@ -112,7 +133,9 @@ def test_uses_key_pair_if_available(self, mocker, snowflake_connector_env): connect_args={"private_key": test_private_key}, ) - def test_uses_authenticator_if_available(self, mocker, snowflake_connector_env): + def test_uses_authenticator_if_available( + self, mocker, snowflake_connector_env + ): mocker.patch("sqlalchemy.create_engine") os.environ["PERMISSION_BOT_AUTHENTICATOR"] = "TEST" SnowflakeConnector() @@ -121,7 +144,9 @@ def test_uses_authenticator_if_available(self, mocker, snowflake_connector_env): "snowflake://TEST:@TEST/TEST?authenticator=TEST&role=TEST&warehouse=TEST" ) - def test_uses_username_password_by_default(self, mocker, snowflake_connector_env): + def test_uses_username_password_by_default( + self, mocker, snowflake_connector_env + ): mocker.patch("sqlalchemy.create_engine") SnowflakeConnector() sqlalchemy.create_engine.assert_called_with( @@ -135,14 +160,18 @@ def test_run_query_executes_desired_query(self, mocker): conn.run_query(query) - conn.engine.assert_has_calls([mocker.call.connect().__enter__().execute(query)]) + conn.engine.assert_has_calls( + [mocker.call.connect().__enter__().execute(query)] + ) def test_run_query_returns_results(self, mocker): mocker.patch("sqlalchemy.create_engine") conn = SnowflakeConnector() expectedResult = "MY DATABASE RESULT" mocker.patch.object( - conn.engine.connect().__enter__(), "execute", return_value=expectedResult + conn.engine.connect().__enter__(), + "execute", + return_value=expectedResult, ) result = conn.run_query("query") @@ -159,7 +188,9 @@ def test_get_current_user(self, mocker): user = conn.get_current_user() - conn.run_query.assert_has_calls([mocker.call("SELECT CURRENT_USER() AS USER")]) + conn.run_query.assert_has_calls( + [mocker.call("SELECT CURRENT_USER() AS USER")] + ) assert user == "test_user" def test_get_current_role(self, mocker): @@ -172,7 +203,9 @@ def test_get_current_role(self, mocker): role = conn.get_current_role() - conn.run_query.assert_has_calls([mocker.call("SELECT CURRENT_ROLE() AS ROLE")]) + conn.run_query.assert_has_calls( + [mocker.call("SELECT CURRENT_ROLE() AS ROLE")] + ) assert role == "test_role" def test_show_schemas(self, mocker): @@ -299,7 +332,9 @@ def test_show_future_grants_in_schema(self, mocker): ], ) - future_grants = conn.show_future_grants("database_1", "database_1.schema_1") + future_grants = conn.show_future_grants( + "database_1", "database_1.schema_1" + ) conn.run_query.assert_has_calls( [mocker.call("SHOW FUTURE GRANTS IN SCHEMA database_1.schema_1")] @@ -444,7 +479,9 @@ def test_show_roles_granted_to_user(self, mocker): '"lowercase role with spaces"', ] - conn.run_query.assert_has_calls([mocker.call("SHOW GRANTS TO USER test_user")]) + conn.run_query.assert_has_calls( + [mocker.call("SHOW GRANTS TO USER test_user")] + ) assert roles_granted_to_user == expected_roles_granted_to_user def test_show_grants_to_role(self, mocker): @@ -470,10 +507,15 @@ def test_show_grants_to_role(self, mocker): grants = conn.show_grants_to_role("test_role") - conn.run_query.assert_has_calls([mocker.call("SHOW GRANTS TO ROLE test_role")]) + conn.run_query.assert_has_calls( + [mocker.call("SHOW GRANTS TO ROLE test_role")] + ) assert grants == { "select": { - "table": ["database_1.schema_1.table_1", "database_1.schema_1.table_2"] + "table": [ + "database_1.schema_1.table_1", + "database_1.schema_1.table_2", + ] } } @@ -510,7 +552,9 @@ def test_show_grants_to_role_quoted_name(self, mocker): grants = conn.show_grants_to_role("test_role") - conn.run_query.assert_has_calls([mocker.call("SHOW GRANTS TO ROLE test_role")]) + conn.run_query.assert_has_calls( + [mocker.call("SHOW GRANTS TO ROLE test_role")] + ) assert grants == { "select": { "table": [ diff --git a/tests/permifrost/test_snowflake_grants.py b/tests/permifrost/test_snowflake_grants.py index 75a2cde86..248317ca0 100644 --- a/tests/permifrost/test_snowflake_grants.py +++ b/tests/permifrost/test_snowflake_grants.py @@ -79,7 +79,10 @@ def test_role_config(): @pytest.fixture(scope="class") def test_user_config(): config = { - "user_name": {"can_login": True, "member_of": ["object_role", "user_role"]} + "user_name": { + "can_login": True, + "member_of": ["object_role", "user_role"], + } } return config @@ -231,7 +234,8 @@ def test_generate_integration_grants( ) integration_command_list = generator.generate_integration_grants( - "functional_role", test_role_config["functional_role"]["integrations"] + "functional_role", + test_role_config["functional_role"]["integrations"], ) integration_lower_list = [ @@ -478,7 +482,9 @@ def generate_multi_role_revokes(): """ entity_type = "roles" entity = "role_1" - test_grants_to_role = {"role_1": {"usage": {"role": ["role_2", "role_3"]}}} + test_grants_to_role = { + "role_1": {"usage": {"role": ["role_2", "role_3"]}} + } test_roles_granted_to_user = {} ignore_membership = False grants_spec = {} @@ -529,7 +535,9 @@ def generate_multi_role_revokes_ignore(): """ entity_type = "roles" entity = "role_1" - test_grants_to_role = {"role_1": {"usage": {"role": ["role_2", "role_3"]}}} + test_grants_to_role = { + "role_1": {"usage": {"role": ["role_2", "role_3"]}} + } test_roles_granted_to_user = {} ignore_membership = True grants_spec = {} @@ -614,7 +622,9 @@ def test_no_revoke_with_no_member_of_but_ignore_membership( self, test_grants_to_role, test_roles_granted_to_user ): generator = SnowflakeGrantsGenerator( - test_grants_to_role, test_roles_granted_to_user, ignore_memberships=True + test_grants_to_role, + test_roles_granted_to_user, + ignore_memberships=True, ) role_command_list = generator.generate_grant_roles( @@ -689,7 +699,9 @@ def single_table_rw_config(mocker): "show_tables", return_value=["database_1.schema_1.table_1"], ) - mocker.patch.object(MockSnowflakeConnector, "show_views", return_value=[]) + mocker.patch.object( + MockSnowflakeConnector, "show_views", return_value=[] + ) config = { "read": ["database_1.schema_1.table_1"], @@ -714,7 +726,9 @@ def single_table_rw_shared_db_config(mocker): "show_tables", return_value=["shared_database_1.public.table_1"], ) - mocker.patch.object(MockSnowflakeConnector, "show_views", return_value=[]) + mocker.patch.object( + MockSnowflakeConnector, "show_views", return_value=[] + ) config = { "read": ["shared_database_1.public.table_1"], @@ -734,9 +748,14 @@ def future_tables_r_single_schema_config(mocker): mocker.patch.object( MockSnowflakeConnector, "show_tables", - return_value=["database_1.schema_1.table_1", "database_1.schema_1.table_2"], + return_value=[ + "database_1.schema_1.table_1", + "database_1.schema_1.table_2", + ], + ) + mocker.patch.object( + MockSnowflakeConnector, "show_views", return_value=[] ) - mocker.patch.object(MockSnowflakeConnector, "show_views", return_value=[]) config = { "read": [ @@ -764,9 +783,14 @@ def future_tables_w_single_schema_config(mocker): mocker.patch.object( MockSnowflakeConnector, "show_tables", - return_value=["database_1.schema_1.table_1", "database_1.schema_1.table_2"], + return_value=[ + "database_1.schema_1.table_1", + "database_1.schema_1.table_2", + ], + ) + mocker.patch.object( + MockSnowflakeConnector, "show_views", return_value=[] ) - mocker.patch.object(MockSnowflakeConnector, "show_views", return_value=[]) config = { "read": [], @@ -794,9 +818,14 @@ def future_tables_rw_single_schema_config(mocker): mocker.patch.object( MockSnowflakeConnector, "show_tables", - return_value=["database_1.schema_1.table_1", "database_1.schema_1.table_2"], + return_value=[ + "database_1.schema_1.table_1", + "database_1.schema_1.table_2", + ], + ) + mocker.patch.object( + MockSnowflakeConnector, "show_views", return_value=[] ) - mocker.patch.object(MockSnowflakeConnector, "show_views", return_value=[]) config = { "read": [ @@ -830,7 +859,10 @@ def future_tables_views_rw_config(mocker): mocker.patch.object( MockSnowflakeConnector, "show_tables", - return_value=["database_1.schema_1.table_1", "database_1.schema_1.table_2"], + return_value=[ + "database_1.schema_1.table_1", + "database_1.schema_1.table_2", + ], ) mocker.patch.object( MockSnowflakeConnector, @@ -1154,9 +1186,12 @@ def test_generate_table_and_view_grants( test_roles_granted_to_user, ) - mock_connector, test_tables_config, test_grants_to_role_role, expected = config( - mocker - ) + ( + mock_connector, + test_tables_config, + test_grants_to_role_role, + expected, + ) = config(mocker) mocker.patch( "permifrost.snowflake_grants.SnowflakeConnector.show_tables", @@ -1197,7 +1232,9 @@ def single_r_schema_config(mocker): "write": [], } - expected = ["GRANT usage ON schema database_1.schema_1 TO ROLE functional_role"] + expected = [ + "GRANT usage ON schema database_1.schema_1 TO ROLE functional_role" + ] return [MockSnowflakeConnector, config, expected] def single_rw_schema_config(mocker): @@ -1304,8 +1341,14 @@ def multi_diff_shared_db_rw_schema_config(mocker): read/write on DATABASE_1.SCHEMA_1 """ config = { - "read": ["shared_database_1.shared_schema_1", "database_1.schema_1"], - "write": ["shared_database_1.shared_schema_1", "database_1.schema_1"], + "read": [ + "shared_database_1.shared_schema_1", + "database_1.schema_1", + ], + "write": [ + "shared_database_1.shared_schema_1", + "database_1.schema_1", + ], } expected = [ @@ -1753,9 +1796,12 @@ def test_generate_database_grants_v2( ): mocker.patch.object(SnowflakeConnector, "__init__", lambda x: None) - mock_connector, test_database_config, expected, test_grants_to_role = config( - mocker - ) + ( + mock_connector, + test_database_config, + expected, + test_grants_to_role, + ) = config(mocker) # Generation of database grants should be identical while ignoring or not # ignoring memberships generator = SnowflakeGrantsGenerator( @@ -1855,7 +1901,9 @@ def revoke_single_r_table_config(mocker): } test_grants_to_role = { - "functional_role": {"select": {"table": ["database_1.schema_1.table_1"]}}, + "functional_role": { + "select": {"table": ["database_1.schema_1.table_1"]} + }, } expected = [ @@ -1878,7 +1926,9 @@ def revoke_single_w_table_config(mocker): } test_grants_to_role = { - "functional_role": {"insert": {"table": ["database_1.schema_1.table_1"]}}, + "functional_role": { + "insert": {"table": ["database_1.schema_1.table_1"]} + }, } expected = [ @@ -2396,7 +2446,9 @@ def revoke_single_r_database_config(mocker): "functional_role": {"usage": {"database": ["database_1"]}}, } - expected = ["REVOKE usage ON database database_1 FROM ROLE functional_role"] + expected = [ + "REVOKE usage ON database database_1 FROM ROLE functional_role" + ] return [MockSnowflakeConnector, config, expected, test_grants_to_role] @@ -2589,7 +2641,9 @@ def revoke_multi_rw_shared_db_config(mocker): test_grants_to_role = { "functional_role": { "usage": {"database": ["database_1", "shared_database_1"]}, - "create schema": {"database": ["database_1", "shared_database_1"]}, + "create schema": { + "database": ["database_1", "shared_database_1"] + }, } } @@ -2629,9 +2683,12 @@ def test_generate_database_revokes( ): mocker.patch.object(SnowflakeConnector, "__init__", lambda x: None) - mock_connector, test_database_config, expected, test_grants_to_role = config( - mocker - ) + ( + mock_connector, + test_database_config, + expected, + test_grants_to_role, + ) = config(mocker) # Generation of database grants should be identical while ignoring or not # ignoring memberships generator = SnowflakeGrantsGenerator( @@ -2660,7 +2717,9 @@ def test_generate_database_revokes( class TestSnowflakeOwnershipGrants: - def generate_ownership_on_warehouse(self, entity, entity_name, is_view=False): + def generate_ownership_on_warehouse( + self, entity, entity_name, is_view=False + ): """ Test that SnowflakeSpecLoader generates ownership grant for an entity. Because we treat views and tables as the same entity in the spec, but @@ -2707,15 +2766,23 @@ def test_generate_ownership_grants(self, entity, mocker): grants_to_role, roles_granted_to_user, expectation, - ) = self.generate_ownership_on_warehouse(entity[0], entity[1], entity[2]) - generator = SnowflakeGrantsGenerator(grants_to_role, roles_granted_to_user) + ) = self.generate_ownership_on_warehouse( + entity[0], entity[1], entity[2] + ) + generator = SnowflakeGrantsGenerator( + grants_to_role, roles_granted_to_user + ) generator.conn = mock_connector sql_commands = generator.generate_grant_ownership(role, spec) assert sql_commands[0]["sql"] == expectation def test_generate_database_ownership_grants( - self, test_grants_to_role, test_roles_granted_to_user, test_role_config, mocker + self, + test_grants_to_role, + test_roles_granted_to_user, + test_role_config, + mocker, ): """Test that SnowflakeGrantsGenerator generates ownership grants for for multiple grant definitions""" mock_connector = MockSnowflakeConnector() diff --git a/tests/permifrost/test_snowflake_role_grant_checker.py b/tests/permifrost/test_snowflake_role_grant_checker.py index a3a80e777..2af85c0e8 100644 --- a/tests/permifrost/test_snowflake_role_grant_checker.py +++ b/tests/permifrost/test_snowflake_role_grant_checker.py @@ -35,7 +35,9 @@ def mock_connector(mocker): "wh 2": {"grant_option": True}, } }, - "manage grants": {"account": {"Account_Name": {"grant_option": False}}}, + "manage grants": { + "account": {"Account_Name": {"grant_option": False}} + }, }, ) return MockSnowflakeConnector() @@ -53,22 +55,36 @@ def test_get_permissions(self, grant_checker, mock_connector): "my_role" ) - assert SnowflakePermission("db 1", "database", ["use"], False) in permissions assert ( - SnowflakePermission("wh 2", "warehouse", ["monitor"], False) in permissions + SnowflakePermission("db 1", "database", ["use"], False) + in permissions + ) + assert ( + SnowflakePermission("wh 2", "warehouse", ["monitor"], False) + in permissions ) assert ( - SnowflakePermission("table 3", "table", ["ownership"], False) in permissions + SnowflakePermission("table 3", "table", ["ownership"], False) + in permissions ) @pytest.mark.parametrize( "permission,expected", [ - (SnowflakePermission("table 4", "table", ["ownership"], False), False), - (SnowflakePermission("table 1", "table", ["ownership"], False), True), + ( + SnowflakePermission("table 4", "table", ["ownership"], False), + False, + ), + ( + SnowflakePermission("table 1", "table", ["ownership"], False), + True, + ), (SnowflakePermission("table 1", "table", ["select"], False), True), (SnowflakePermission("*", "account", ["ownership"], False), False), - (SnowflakePermission("*", "account", ["manage grants"], False), True), + ( + SnowflakePermission("*", "account", ["manage grants"], False), + True, + ), ], ) def test_has_permissions(self, grant_checker, permission, expected): @@ -84,18 +100,30 @@ def test_has_permission_no_role(self, grant_checker): "permission,expected", [ (SnowflakePermission("table 4", "table", ["select"], True), False), - (SnowflakePermission("schema 1", "schema", ["ownership"], False), True), + ( + SnowflakePermission("schema 1", "schema", ["ownership"], False), + True, + ), (SnowflakePermission("table 1", "table", ["select"], False), True), (SnowflakePermission("*", "account", ["ownership"], False), False), - (SnowflakePermission("wh 2", "warehouse", ["monitor"], False), True), - (SnowflakePermission("wh 1", "warehouse", ["monitor"], False), False), + ( + SnowflakePermission("wh 2", "warehouse", ["monitor"], False), + True, + ), + ( + SnowflakePermission("wh 1", "warehouse", ["monitor"], False), + False, + ), ], ) def test_can_grant_permission(self, grant_checker, permission, expected): print("Permissions for role:") for p in grant_checker.get_permissions("my_role"): print(p) - assert grant_checker.can_grant_permission("my_role", permission) == expected + assert ( + grant_checker.can_grant_permission("my_role", permission) + == expected + ) def test_can_grant_permission_no_role(self, grant_checker): permission = SnowflakePermission("*", "account", ["ownership"], True) diff --git a/tests/permifrost/test_snowflake_spec_loader.py b/tests/permifrost/test_snowflake_spec_loader.py index 55c0a1a33..1e0ca4a40 100644 --- a/tests/permifrost/test_snowflake_spec_loader.py +++ b/tests/permifrost/test_snowflake_spec_loader.py @@ -5,7 +5,9 @@ from permifrost.snowflake_spec_loader import SnowflakeSpecLoader from permifrost.snowflake_connector import SnowflakeConnector from permifrost.snowflake_grants import SnowflakeGrantsGenerator -from permifrost_test_utils.snowflake_schema_builder import SnowflakeSchemaBuilder +from permifrost_test_utils.snowflake_schema_builder import ( + SnowflakeSchemaBuilder, +) from permifrost_test_utils.snowflake_connector import MockSnowflakeConnector @@ -59,7 +61,9 @@ def test_grants_roles_mock_connection(mocker, mock_method, return_value): mocker.patch.object( mock_connector, "get_current_role", return_value="securityadmin" ) - mocker.patch.object(mock_connector, "get_current_user", return_value="testuser") + mocker.patch.object( + mock_connector, "get_current_user", return_value="testuser" + ) mocker.patch.object( mock_connector, "show_warehouses", @@ -71,7 +75,9 @@ def test_grants_roles_mock_connection(mocker, mock_method, return_value): return_value=["primaryintegration", "secondaryintegration"], ) mocker.patch.object( - mock_connector, "show_databases", return_value=["primarydb", "secondarydb"] + mock_connector, + "show_databases", + return_value=["primarydb", "secondarydb"], ) mocker.patch.object( mock_connector, "show_roles", return_value=["testrole", "securityadmin"] @@ -93,7 +99,9 @@ def test_roles_mock_connector(mocker): mocker.patch.object( mock_connector, "get_current_role", return_value="securityadmin" ) - mocker.patch.object(mock_connector, "get_current_user", return_value="testuser") + mocker.patch.object( + mock_connector, "get_current_user", return_value="testuser" + ) mocker.patch.object( mock_connector, "show_warehouses", @@ -105,7 +113,9 @@ def test_roles_mock_connector(mocker): return_value=["primaryintegration", "secondaryintegration"], ) mocker.patch.object( - mock_connector, "show_databases", return_value=["primarydb", "secondarydb"] + mock_connector, + "show_databases", + return_value=["primarydb", "secondarydb"], ) mocker.patch.object( mock_connector, @@ -211,7 +221,9 @@ def test_check_entities_on_snowflake_server_checks_role_owner( spec_file_data, method, return_value = config() print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) mocker.patch.object(mock_connector, method, return_value=return_value) SnowflakeSpecLoader("", mock_connector) @@ -227,9 +239,7 @@ def spec_loader_error_handling_case_one(): ) method = "show_roles" return_value = ["role_1"] - expected_error = ( - 'Spec error: roles "role_1", field "member_of": no definitions validate' - ) + expected_error = 'Spec error: roles "role_1", field "member_of": no definitions validate' return [spec_file_data, method, return_value, expected_error] @@ -251,7 +261,9 @@ def test_spec_loader_errors( """ print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) mocker.patch.object(mock_connector, method, return_value=return_value) with pytest.raises(SpecLoadingError) as context: SnowflakeSpecLoader("", mock_connector) @@ -287,9 +299,7 @@ def require_owner_error_handling_case_two(): ) method = "show_roles" return_value = {"some-other-role": "none"} - expected_error = ( - "Missing Entity Error: Role testrole was not found on Snowflake Server" - ) + expected_error = "Missing Entity Error: Role testrole was not found on Snowflake Server" return [spec_file_data, method, return_value, expected_error] @@ -303,9 +313,7 @@ def require_owner_error_handling_case_three(): ) method = "show_roles" return_value = {} - expected_error = ( - "Missing Entity Error: Role testrole was not found on Snowflake Server" - ) + expected_error = "Missing Entity Error: Role testrole was not found on Snowflake Server" return [spec_file_data, method, return_value, expected_error] @@ -321,9 +329,7 @@ def require_owner_error_handling_case_four(): ) method = "show_roles" return_value = {} - expected_error = ( - "Missing Entity Error: Role testrole was not found on Snowflake Server" - ) + expected_error = "Missing Entity Error: Role testrole was not found on Snowflake Server" return [spec_file_data, method, return_value, expected_error] @@ -348,7 +354,9 @@ def test_check_entities_on_snowflake_server_errors_if_role_owner_does_not_match( """ print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) mocker.patch.object(mock_connector, method, return_value=return_value) with pytest.raises(SpecLoadingError) as context: SnowflakeSpecLoader("", mock_connector) @@ -411,7 +419,9 @@ def test_check_entities_on_snowflake_server_filters_grants_to_role_to_items_defi self, test_grants_roles_mock_connection, mocker, mock_method, config ): spec_file_data, expected_value = config - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) spec_loader = SnowflakeSpecLoader( spec_path="", conn=test_grants_roles_mock_connection ) @@ -580,7 +590,9 @@ def test_filter_to_database_refs( "warehouse_refs": warehouse_refs, "integration_refs": integration_refs, } - spec_loader.filter_to_database_refs(grant_on=grant_on, filter_set=filter_set) + spec_loader.filter_to_database_refs( + grant_on=grant_on, filter_set=filter_set + ) # edge case for PascalCase table entities def load_spec_file_case_one(): @@ -599,7 +611,9 @@ def load_spec_file_case_one(): ) method = "show_tables" return_value = ["database_1.schema_1.tableone"] - expected_error = "Missing Entity Error: Table/View database_1.schema_1.TableOne" + expected_error = ( + "Missing Entity Error: Table/View database_1.schema_1.TableOne" + ) return [spec_file_data, method, return_value, expected_error] # edge case for table entities that do not exist @@ -619,7 +633,9 @@ def load_spec_file_case_two(): ) method = "show_tables" return_value = [] - expected_error = "Missing Entity Error: Table/View database_1.schema_1.tableone" + expected_error = ( + "Missing Entity Error: Table/View database_1.schema_1.tableone" + ) return [spec_file_data, method, return_value, expected_error] # non edge case for table entities that do exist @@ -644,7 +660,11 @@ def load_spec_file_case_three(): @pytest.mark.parametrize( "config", - [load_spec_file_case_one, load_spec_file_case_two, load_spec_file_case_three], + [ + load_spec_file_case_one, + load_spec_file_case_two, + load_spec_file_case_three, + ], ) def test_load_spec_with_edge_case_tables( self, @@ -655,7 +675,9 @@ def test_load_spec_with_edge_case_tables( spec_file_data, method, return_value, expected_error = config() print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) mocker.patch.object(mock_connector, method, return_value=return_value) with pytest.raises(SpecLoadingError) as context: SnowflakeSpecLoader("", mock_connector) @@ -665,7 +687,9 @@ def test_load_spec_with_edge_case_tables( def test_remove_duplicate_queries(self): sql_command_1 = {"sql": "GRANT OWNERSHIP ON SCHEMA PIZZA TO ROLE LIZZY"} sql_command_2 = sql_command_1.copy() - sql_command_3 = {"sql": "REVOKE ALL PRIVILEGES ON SCHEMA PIZZA FROM ROLE LIZZY"} + sql_command_3 = { + "sql": "REVOKE ALL PRIVILEGES ON SCHEMA PIZZA FROM ROLE LIZZY" + } sql_command_4 = sql_command_3.copy() result = SnowflakeSpecLoader.remove_duplicate_queries( @@ -725,7 +749,9 @@ def load_spec_with_owner_case_four(): """ SnowflakeSpecLoader loads without error for warehouse with owner """ - spec_file_data = SnowflakeSchemaBuilder().add_warehouse(owner="user").build() + spec_file_data = ( + SnowflakeSchemaBuilder().add_warehouse(owner="user").build() + ) method = "show_warehouses" return_value = ["testwarehouse"] @@ -737,7 +763,10 @@ def load_spec_with_owner_case_five(): and require-owner: True """ spec_file_data = ( - SnowflakeSchemaBuilder().require_owner().add_db(owner="user").build() + SnowflakeSchemaBuilder() + .require_owner() + .add_db(owner="user") + .build() ) method = "show_databases" return_value = ["testdb"] @@ -766,7 +795,10 @@ def load_spec_with_owner_case_seven(): and require-owner: True """ spec_file_data = ( - SnowflakeSchemaBuilder().require_owner().add_user(owner="user").build() + SnowflakeSchemaBuilder() + .require_owner() + .add_user(owner="user") + .build() ) method = "show_users" return_value = ["testusername"] @@ -779,7 +811,10 @@ def load_spec_with_owner_case_eight(): and require-owner: True """ spec_file_data = ( - SnowflakeSchemaBuilder().require_owner().add_warehouse(owner="user").build() + SnowflakeSchemaBuilder() + .require_owner() + .add_warehouse(owner="user") + .build() ) method = "show_warehouses" return_value = ["testwarehouse"] @@ -792,7 +827,9 @@ def load_spec_with_owner_case_nine(): """ print("spec_file_data") - spec_file_data = SnowflakeSchemaBuilder().add_integration(owner="user").build() + spec_file_data = ( + SnowflakeSchemaBuilder().add_integration(owner="user").build() + ) print(spec_file_data) method = "show_integrations" return_value = ["testintegration"] @@ -839,7 +876,9 @@ def test_load_spec_with_owner( spec_file_data, method, return_value = config() print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) mocker.patch.object(mock_connector, method, return_value=return_value) SnowflakeSpecLoader("", mock_connector) @@ -849,7 +888,9 @@ def load_spec_file_error_case_one(): Raise 'Owner not defined' error on show_databases with no owner and require-owner = True """ - spec_file_data = SnowflakeSchemaBuilder().require_owner().add_db().build() + spec_file_data = ( + SnowflakeSchemaBuilder().require_owner().add_db().build() + ) method = "show_databases" return_value = ["testdb"] return [spec_file_data, method, return_value] @@ -876,7 +917,9 @@ def load_spec_file_error_case_three(): Raise 'Owner not defined' error on show_users with no owner and require-owner = True """ - spec_file_data = SnowflakeSchemaBuilder().require_owner().add_user().build() + spec_file_data = ( + SnowflakeSchemaBuilder().require_owner().add_user().build() + ) method = "show_users" return_value = ["testusername"] return [spec_file_data, method, return_value] @@ -926,7 +969,9 @@ def test_load_spec_owner_required_with_no_owner( spec_file_data, method, return_value = config() print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) mocker.patch.object(mock_connector, method, return_value=return_value) with pytest.raises(SpecLoadingError) as context: SnowflakeSpecLoader("", mock_connector) @@ -944,7 +989,9 @@ def test_generate_permission_queries_with_requires_owner( ) print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) spec_loader = SnowflakeSpecLoader("", mock_connector) queries = spec_loader.generate_permission_queries() @@ -952,17 +999,26 @@ def test_generate_permission_queries_with_requires_owner( class TestSnowflakeSpecLoaderUserRoleFilters: - def test_role_filter(self, mocker, test_roles_mock_connector, test_roles_spec_file): + def test_role_filter( + self, mocker, test_roles_mock_connector, test_roles_spec_file + ): """GRANT queries list filtered by role.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) results = spec_loader.generate_permission_queries( roles=["primary"], run_list=["roles"] ) expected = [ - {"already_granted": False, "sql": "GRANT ROLE testrole TO role primary"} + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role primary", + } ] assert results == expected @@ -972,14 +1028,24 @@ def test_role_filter_multiple( """Make sure that the GRANT queries list can be filtered by multiple roles.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) results = spec_loader.generate_permission_queries( roles=["primary", "secondary"], run_list=["roles"] ) expected_results = [ - {"already_granted": False, "sql": "GRANT ROLE testrole TO role primary"}, - {"already_granted": False, "sql": "GRANT ROLE testrole TO role secondary"}, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role primary", + }, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role secondary", + }, ] assert results == expected_results @@ -992,16 +1058,26 @@ def test_role_filter_and_user_filter( """ print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) results = spec_loader.generate_permission_queries( roles=["primary", "secondary"], users=["testusername"], run_list=["roles", "users"], ) expected_results = [ - {"already_granted": False, "sql": "GRANT ROLE testrole TO role primary"}, - {"already_granted": False, "sql": "GRANT ROLE testrole TO role secondary"}, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role primary", + }, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role secondary", + }, { "already_granted": False, "sql": "ALTER USER testusername SET DISABLED = FALSE", @@ -1015,17 +1091,30 @@ def test_no_role_or_user_filter( """Test that the generate_permissions_query does no filtering on when users and roles are not defined.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) expected_sql_queries = [ - {"already_granted": False, "sql": "GRANT ROLE testrole TO role testrole"}, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role testrole", + }, { "already_granted": False, "sql": "GRANT ROLE testrole TO role securityadmin", }, - {"already_granted": False, "sql": "GRANT ROLE testrole TO role primary"}, - {"already_granted": False, "sql": "GRANT ROLE testrole TO role secondary"}, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role primary", + }, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role secondary", + }, { "already_granted": False, "sql": "ALTER USER testusername SET DISABLED = FALSE", @@ -1038,12 +1127,18 @@ def test_no_role_or_user_filter( assert spec_loader.generate_permission_queries() == expected_sql_queries - def test_user_filter(self, mocker, test_roles_mock_connector, test_roles_spec_file): + def test_user_filter( + self, mocker, test_roles_mock_connector, test_roles_spec_file + ): """Make sure that the grant queries list can be filtered by user.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) assert spec_loader.generate_permission_queries( users=["testusername"], run_list=["users"] ) == [ @@ -1059,8 +1154,12 @@ def test_user_filter_multiple( """Make sure that the grant queries list can be filtered by multiple users.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) results = spec_loader.generate_permission_queries( users=["testusername", "testuser"], run_list=["users"] ) @@ -1082,15 +1181,22 @@ def test_user_filter_and_roles_filter( """Make sure that the grant queries list can be filtered by multiple users and a single role ignores the role""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) results = spec_loader.generate_permission_queries( users=["testusername", "testuser"], roles=["primary"], run_list=["roles", "users"], ) expected_results = [ - {"already_granted": False, "sql": "GRANT ROLE testrole TO role primary"}, + { + "already_granted": False, + "sql": "GRANT ROLE testrole TO role primary", + }, { "already_granted": False, "sql": "ALTER USER testusername SET DISABLED = FALSE", @@ -1275,7 +1381,9 @@ def test_get_privileges_from_snowflake_server( """Verify correct calls when getting privs from server:""" users, roles, run_list, expected_calls = config() print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) mock_get_role_privileges_from_snowflake_server = mocker.patch.object( SnowflakeSpecLoader, "get_role_privileges_from_snowflake_server", @@ -1320,7 +1428,8 @@ def test_check_entities_on_snowflake_server_no_warehouses( ): mocker.patch.object(mock_connector, "show_warehouses") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.show_warehouses.assert_not_called() @@ -1329,7 +1438,8 @@ def test_check_entities_on_snowflake_server_no_integrations( ): mocker.patch.object(mock_connector, "show_integrations") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.show_integrations.assert_not_called() @@ -1338,7 +1448,8 @@ def test_check_entities_on_snowflake_server_no_databases( ): mocker.patch.object(mock_connector, "show_databases") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.show_databases.assert_not_called() @@ -1347,7 +1458,8 @@ def test_check_entities_on_snowflake_server_no_schemas( ): mocker.patch.object(mock_connector, "show_schemas") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.show_schemas.assert_not_called() @@ -1357,7 +1469,8 @@ def test_check_entities_on_snowflake_server_no_tables( mocker.patch.object(mock_connector, "show_tables") mocker.patch.object(mock_connector, "show_views") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.show_tables.assert_not_called() mock_connector.show_views.assert_not_called() @@ -1367,7 +1480,8 @@ def test_check_entities_on_snowflake_server_no_roles( ): mocker.patch.object(mock_connector, "show_roles") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.show_roles.assert_not_called() @@ -1376,7 +1490,8 @@ def test_check_entities_on_snowflake_server_no_users( ): mocker.patch.object(mock_connector, "show_users") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.show_users.assert_not_called() @@ -1384,10 +1499,13 @@ def test_check_permissions_on_snowflake_server_as_securityadmin( self, test_dir, mocker, mock_connector ): mocker.patch.object( - MockSnowflakeConnector, "get_current_role", return_value="securityadmin" + MockSnowflakeConnector, + "get_current_role", + return_value="securityadmin", ) SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.get_current_role.assert_called() @@ -1398,7 +1516,9 @@ def test_check_permissions_on_snowflake_server_not_as_securityadmin( Validates that an error is raised if not using SECURITYADMIN """ mocker.patch.object( - MockSnowflakeConnector, "get_current_role", return_value="notsecurityadmin" + MockSnowflakeConnector, + "get_current_role", + return_value="notsecurityadmin", ) with pytest.raises(SpecLoadingError): SnowflakeSpecLoader( @@ -1412,7 +1532,8 @@ def test_check_permissions_on_snowflake_server_gets_current_user_info( ): mocker.patch.object(mock_connector, "get_current_user") SnowflakeSpecLoader( - os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), mock_connector + os.path.join(test_dir, "specs", "snowflake_spec_blank.yml"), + mock_connector, ) mock_connector.get_current_user.assert_called() @@ -1420,7 +1541,9 @@ def test_edge_case_entities_generate_correct_statements( self, test_dir, mocker, mock_connector ): mocker.patch.object( - SnowflakeSpecLoader, "check_entities_on_snowflake_server", return_value=None + SnowflakeSpecLoader, + "check_entities_on_snowflake_server", + return_value=None, ) mocker.patch( "permifrost.snowflake_connector.SnowflakeConnector", @@ -1479,7 +1602,9 @@ def generate_grants_for_spec_case_one(mocker): return_value=["role_1", "role_2", "role_3"], ) - spec_file_data = SnowflakeSchemaBuilder().add_role(member_of=['"*"']).build() + spec_file_data = ( + SnowflakeSchemaBuilder().add_role(member_of=['"*"']).build() + ) expected = [ "GRANT ROLE role_1 TO role testrole", "GRANT ROLE role_2 TO role testrole", @@ -1635,7 +1760,9 @@ def test_generate_grants_for_snowflake_spec_loader( ): spec_file_data, expected = config(mocker) mocker.patch.object( - SnowflakeSpecLoader, "check_entities_on_snowflake_server", return_value=None + SnowflakeSpecLoader, + "check_entities_on_snowflake_server", + return_value=None, ) mocker.patch( "permifrost.snowflake_connector.SnowflakeConnector", @@ -1658,7 +1785,9 @@ def test_generate_grants_for_snowflake_spec_loader( print("Spec file is: ") print(spec_file_data) - mocker.patch("builtins.open", mocker.mock_open(read_data=spec_file_data)) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=spec_file_data) + ) mocker.patch.object(mock_connector, method, return_value=return_value) spec_loader = SnowflakeSpecLoader("", mock_connector) sql_queries = spec_loader.generate_permission_queries() diff --git a/tests/permifrost/test_snowflake_user.py b/tests/permifrost/test_snowflake_user.py index e29d72271..130b70088 100644 --- a/tests/permifrost/test_snowflake_user.py +++ b/tests/permifrost/test_snowflake_user.py @@ -2,7 +2,9 @@ import os from permifrost.snowflake_spec_loader import SnowflakeSpecLoader -from permifrost_test_utils.snowflake_schema_builder import SnowflakeSchemaBuilder +from permifrost_test_utils.snowflake_schema_builder import ( + SnowflakeSchemaBuilder, +) from permifrost_test_utils.snowflake_connector import MockSnowflakeConnector THIS_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -99,12 +101,18 @@ def test_roles_mock_connector(mocker): class TestSnowflakeUserProperties: - def test_user_base(self, mocker, test_roles_mock_connector, test_roles_spec_file): + def test_user_base( + self, mocker, test_roles_mock_connector, test_roles_spec_file + ): """Make sure that the user without any property only have the DISABLED property altered""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["base_user_test"], run_list=["users"] ) @@ -121,8 +129,12 @@ def test_user_with_password_disabled( """Make sure that the user password get set to null if the has_password property is False""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["test_user_with_password_disabled"], run_list=["users"] ) @@ -139,8 +151,12 @@ def test_user_with_display_name( """Make sure that the user display_name get set if the display_name property is set""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["test_user_with_display_name"], run_list=["users"] ) @@ -158,8 +174,12 @@ def test_user_with_comment( """Make sure that the user COMMENT get set if the comment property is set""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["test_user_with_comment"], run_list=["users"] ) @@ -177,10 +197,15 @@ def test_user_with_comment_password_disabled( """Make sure if multiple properties is set (PASSWORD, COMMENT) then all if them are set.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( - users=["test_user_with_comment_password_disabled"], run_list=["users"] + users=["test_user_with_comment_password_disabled"], + run_list=["users"], ) assert queries == [ @@ -196,8 +221,12 @@ def test_user_with_full_name( """Make sure if multiple properties is set (FIRST_NAME, MIDDLE_NAME, LAST_NAME) then all if them are set.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["test_user_with_full_name"], run_list=["users"] ) @@ -216,8 +245,12 @@ def test_user_with_email( """Make sure if the EMAIL property is set then the EMAIL is set.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["test_user_with_email"], run_list=["users"] ) @@ -235,10 +268,15 @@ def test_user_with_password_display_name_comment( """Make sure if multiple properties is set (DISPLAY_NAME, COMMENT) then all if them are set.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( - users=["test_user_with_password_display_name_comment"], run_list=["users"] + users=["test_user_with_password_display_name_comment"], + run_list=["users"], ) assert queries == [ @@ -257,8 +295,12 @@ def test_user_with_multiple_properties( EMAIL COMMENT) then all if them are set.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["test_user_with_multiple_properties"], run_list=["users"] ) @@ -284,8 +326,12 @@ def test_user_defaults( then all if them are set.""" print(f"Spec File Data is:\n{test_roles_spec_file}") - mocker.patch("builtins.open", mocker.mock_open(read_data=test_roles_spec_file)) - spec_loader = SnowflakeSpecLoader(spec_path="", conn=test_roles_mock_connector) + mocker.patch( + "builtins.open", mocker.mock_open(read_data=test_roles_spec_file) + ) + spec_loader = SnowflakeSpecLoader( + spec_path="", conn=test_roles_mock_connector + ) queries = spec_loader.generate_permission_queries( users=["test_user_defaults"], run_list=["users"] ) @@ -298,4 +344,4 @@ def test_user_defaults( "DEFAULT_NAMESPACE = 'public', " "DEFAULT_ROLE = 'role1'", } - ] + ] diff --git a/tests/permifrost_test_utils/snowflake_schema_builder.py b/tests/permifrost_test_utils/snowflake_schema_builder.py index 71d64d4a7..ec835d3c5 100644 --- a/tests/permifrost_test_utils/snowflake_schema_builder.py +++ b/tests/permifrost_test_utils/snowflake_schema_builder.py @@ -60,14 +60,18 @@ def build(self): if len(self.warehouses) > 0: spec_yaml.append("warehouses:") for warehouse in self.warehouses: - spec_yaml.extend([f" - {warehouse['name']}:", " size: x-small"]) + spec_yaml.extend( + [f" - {warehouse['name']}:", " size: x-small"] + ) if warehouse["owner"] is not None: spec_yaml.append(f" owner: {warehouse['owner']}") if len(self.integrations) > 0: spec_yaml.append("integrations:") for integration in self.integrations: - spec_yaml.extend([f" - {integration['name']}:", " category: storage"]) + spec_yaml.extend( + [f" - {integration['name']}:", " category: storage"] + ) if integration["owner"] is not None: spec_yaml.append(f" owner: {integration['owner']}") @@ -77,36 +81,60 @@ def build(self): def _roles_generator(self, roles): spec_yaml = [] for role in roles: - if (role["member_of_exclude"] or role["member_of_include"]) and role[ - "member_of" - ]: + if ( + role["member_of_exclude"] or role["member_of_include"] + ) and role["member_of"]: raise KeyError( "'member_of_include' and 'member_of_exclude' cannot be defined if 'member_of' is defined" ) elif role["member_of_exclude"] and role["member_of_include"]: spec_yaml.extend( - [f" - {role['name']}:", " member_of:", " include:"] + [ + f" - {role['name']}:", + " member_of:", + " include:", + ] ) spec_yaml.extend( - [f" - {member}" for member in role["member_of_include"]] + [ + f" - {member}" + for member in role["member_of_include"] + ] ) spec_yaml.extend([" exclude:"]) spec_yaml.extend( - [f" - {member}" for member in role["member_of_exclude"]] + [ + f" - {member}" + for member in role["member_of_exclude"] + ] ) elif role["member_of_exclude"] and not role["member_of_include"]: spec_yaml.extend( - [f" - {role['name']}:", " member_of:", " exclude:"] + [ + f" - {role['name']}:", + " member_of:", + " exclude:", + ] ) spec_yaml.extend( - [f" - {member}" for member in role["member_of_exclude"]] + [ + f" - {member}" + for member in role["member_of_exclude"] + ] ) elif role["member_of_include"] and not role["member_of_exclude"]: spec_yaml.extend( - [f" - {role['name']}:", " member_of:", " include:"] + [ + f" - {role['name']}:", + " member_of:", + " include:", + ] ) spec_yaml.extend( - [f" - {member}" for member in role["member_of_include"]] + [ + f" - {member}" + for member in role["member_of_include"] + ] ) elif role["member_of"]: spec_yaml.extend([f" - {role['name']}:", " member_of:"]) @@ -143,7 +171,9 @@ def _build_tables(self, role): for full_table_name in role["tables"] ] ) - elif role["permission_set"] == ["read", "write"] or role["permission_set"] == [ + elif role["permission_set"] == ["read", "write"] or role[ + "permission_set" + ] == [ "write", "read", ]: @@ -176,15 +206,21 @@ def _build_table_schemas(self, role): name_parts = full_table_name.split(".") database_name = name_parts[0] if 0 < len(name_parts) else None schema_name = name_parts[1] if 1 < len(name_parts) else None - spec_yaml.extend([f" - {database_name}.{schema_name}"]) + spec_yaml.extend( + [f" - {database_name}.{schema_name}"] + ) elif role["permission_set"] == ["write"]: spec_yaml.extend([" write:"]) for full_table_name in role["tables"]: name_parts = full_table_name.split(".") database_name = name_parts[0] if 0 < len(name_parts) else None schema_name = name_parts[1] if 1 < len(name_parts) else None - spec_yaml.extend([f" - {database_name}.{schema_name}"]) - elif role["permission_set"] == ["read", "write"] or role["permission_set"] == [ + spec_yaml.extend( + [f" - {database_name}.{schema_name}"] + ) + elif role["permission_set"] == ["read", "write"] or role[ + "permission_set" + ] == [ "write", "read", ]: @@ -193,13 +229,17 @@ def _build_table_schemas(self, role): name_parts = full_table_name.split(".") database_name = name_parts[0] if 0 < len(name_parts) else None schema_name = name_parts[1] if 1 < len(name_parts) else None - spec_yaml.extend([f" - {database_name}.{schema_name}"]) + spec_yaml.extend( + [f" - {database_name}.{schema_name}"] + ) spec_yaml.extend([" write:"]) for full_table_name in role["tables"]: name_parts = full_table_name.split(".") database_name = name_parts[0] if 0 < len(name_parts) else None schema_name = name_parts[1] if 1 < len(name_parts) else None - spec_yaml.extend([f" - {database_name}.{schema_name}"]) + spec_yaml.extend( + [f" - {database_name}.{schema_name}"] + ) else: raise ValueError("Must set the permission_set for table generation") return spec_yaml @@ -218,7 +258,9 @@ def _build_table_databases(self, role): name_parts = full_table_name.split(".") database_name = name_parts[0] if 0 < len(name_parts) else None spec_yaml.extend([f" - {database_name}"]) - elif role["permission_set"] == ["read", "write"] or role["permission_set"] == [ + elif role["permission_set"] == ["read", "write"] or role[ + "permission_set" + ] == [ "write", "read", ]: From 33b09f52edef2d72edcd1de14a90c99f77ca491e Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 17:00:17 +0200 Subject: [PATCH 14/15] Adjust to use make recipes in github actions --- .github/workflows/manual-release.yml | 4 ++-- .github/workflows/release.yml | 8 ++++---- .github/workflows/test.yml | 12 ++++++------ Makefile | 3 +++ pyproject.toml | 8 ++++---- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/.github/workflows/manual-release.yml b/.github/workflows/manual-release.yml index f96156ca0..b545042b5 100644 --- a/.github/workflows/manual-release.yml +++ b/.github/workflows/manual-release.yml @@ -51,11 +51,11 @@ jobs: - name: Install build dependencies run: | python -m pip install --upgrade pip - pip install build twine + pip install '.[dev]' - name: Build package run: | - python -m build + make dist-local - name: Check package run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8b7fc1cec..59cae21fe 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -34,7 +34,7 @@ jobs: - name: Install build dependencies run: | python -m pip install --upgrade pip - pip install build twine + pip install '.[dev]' - name: Check if VERSION file changed id: check_version @@ -48,7 +48,7 @@ jobs: - name: Build package if: steps.check_version.outputs.version_changed == 'true' run: | - python -m build + make dist-local - name: Check package if: steps.check_version.outputs.version_changed == 'true' @@ -103,7 +103,7 @@ jobs: - name: Install build dependencies run: | python -m pip install --upgrade pip - pip install build twine + pip install '.[dev]' - name: Check if VERSION file changed id: check_version @@ -117,7 +117,7 @@ jobs: - name: Build package if: steps.check_version.outputs.version_changed == 'true' run: | - python -m build + make dist-local - name: Check package if: steps.check_version.outputs.version_changed == 'true' diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dde31c546..137cdaf44 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,17 +42,17 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install -e ".[dev]" + pip install '.[dev]' - name: Run linting run: | make ci-show-lint - - - name: Run tests run: | - pytest tests/ -v --cov=src/ --cov-report=xml + pytest -v --cov-report= --cov permifrost -m "$PYTEST_MARKERS" + coverage report + coverage xml - name: Upload coverage to Codecov uses: codecov/codecov-action@v3 @@ -77,11 +77,11 @@ jobs: - name: Install build dependencies run: | python -m pip install --upgrade pip - pip install build twine + pip install '.[dev]' - name: Build package run: | - python -m build + make dist-local - name: Check package run: | diff --git a/Makefile b/Makefile index e76d943b5..f1d3eccf0 100644 --- a/Makefile +++ b/Makefile @@ -165,6 +165,9 @@ dist: compose-build @docker-compose run permifrost /bin/bash \ -c "python setup.py sdist" +dist-local: + python -m build + docker-clean: @docker-compose run permifrost /bin/bash \ -c "rm -rf dist" diff --git a/pyproject.toml b/pyproject.toml index c927cada6..824e3ad63 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,7 +33,7 @@ dependencies = [ "cerberus", "colorama", "coloredlogs", - "click", + "click==8.0.1", "click-default-group", "pyyaml", "snowflake-connector-python[secure-local-storage]", @@ -51,9 +51,9 @@ dev = [ "isort", "mypy", "pre-commit", - "pytest", - "pytest-cov", - "pytest-mock", + "pytest==6.2.5", + "pytest-cov==2.12.1", + "pytest-mock==3.6.1", "types-PyYAML", ] From 9568d73b8547e971f7345a0b64773047d961d8b9 Mon Sep 17 00:00:00 2001 From: Lui Pillmann Date: Fri, 15 Aug 2025 17:03:18 +0200 Subject: [PATCH 15/15] Add missing dependencies --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 824e3ad63..00a2bbccb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,7 @@ dependencies = [ dev = [ "black==22.3.0", "bumpversion", + "build", "changelog-cli", "coverage", "flake8", @@ -54,6 +55,7 @@ dev = [ "pytest==6.2.5", "pytest-cov==2.12.1", "pytest-mock==3.6.1", + "twine", "types-PyYAML", ]