-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Separating the RBAC and Remote related integration tests. #4905
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
name: pr-remote-rbac-integration-tests | ||
# This runs the integration tests related to rbac functionality and remote registry and online features. | ||
|
||
on: | ||
pull_request: | ||
types: | ||
- opened | ||
- synchronize | ||
- labeled | ||
paths-ignore: | ||
- 'community/**' | ||
- 'docs/**' | ||
- 'examples/**' | ||
|
||
jobs: | ||
remote-rbac-integration-tests-python: | ||
if: | ||
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) || | ||
(github.event.action != 'labeled' && (contains(github.event.pull_request.labels.*.name, 'ok-to-test') || contains(github.event.pull_request.labels.*.name, 'approved') || contains(github.event.pull_request.labels.*.name, 'lgtm')))) && | ||
github.event.pull_request.base.repo.full_name == 'feast-dev/feast' | ||
runs-on: ${{ matrix.os }} | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
python-version: [ "3.11" ] | ||
os: [ ubuntu-latest ] | ||
env: | ||
OS: ${{ matrix.os }} | ||
PYTHON: ${{ matrix.python-version }} | ||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
repository: ${{ github.event.repository.full_name }} # Uses the full repository name | ||
ref: ${{ github.ref }} # Uses the ref from the event | ||
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token | ||
submodules: recursive | ||
- name: Setup Python | ||
uses: actions/setup-python@v5 | ||
id: setup-python | ||
with: | ||
python-version: ${{ matrix.python-version }} | ||
architecture: x64 | ||
- name: Install uv | ||
run: curl -LsSf https://astral.sh/uv/install.sh | sh | ||
- name: Get uv cache dir | ||
id: uv-cache | ||
run: | | ||
echo "dir=$(uv cache dir)" >> $GITHUB_OUTPUT | ||
- name: uv cache | ||
uses: actions/cache@v4 | ||
with: | ||
path: ${{ steps.uv-cache.outputs.dir }} | ||
key: ${{ runner.os }}-${{ matrix.python-version }}-uv-${{ hashFiles(format('**/py{0}-ci-requirements.txt', matrix.python-version)) }} | ||
- name: Install dependencies | ||
run: make install-python-dependencies-ci | ||
- name: Test rbac and remote feature integration tests | ||
if: ${{ always() }} # this will guarantee that step won't be canceled and resources won't leak | ||
run: make test-python-integration-rbac-remote |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
import logging | ||
import random | ||
import time | ||
from multiprocessing import Manager | ||
|
||
import pytest | ||
from testcontainers.keycloak import KeycloakContainer | ||
|
@@ -9,14 +12,30 @@ | |
from tests.utils.auth_permissions_util import setup_permissions_on_keycloak | ||
|
||
logger = logging.getLogger(__name__) | ||
logger.setLevel(logging.INFO) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any reason to set log level as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This log is only for integration tests to display info logs. Keeping it info helped to display the important logs during the test runs. These logs purged after the new run per PR only. It does not harm to keep it Info but also if you want we can change it WARN as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can keep Feast default log level, until unless it's required to see the info message. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can update in the next PR as this is merged. |
||
|
||
shared_state = Manager().dict() | ||
|
||
|
||
@pytest.fixture(scope="session") | ||
def start_keycloak_server(): | ||
# Add random sleep between 0 and 2 before checking the state to avoid concurrency issues. | ||
random_sleep_time = random.uniform(0, 2) | ||
time.sleep(random_sleep_time) | ||
|
||
# If the Keycloak instance is already started (in any worker), reuse it | ||
if shared_state.get("keycloak_started", False): | ||
return shared_state["keycloak_url"] | ||
logger.info("Starting keycloak instance") | ||
with KeycloakContainer("quay.io/keycloak/keycloak:24.0.1") as keycloak_container: | ||
setup_permissions_on_keycloak(keycloak_container.get_client()) | ||
yield keycloak_container.get_url() | ||
shared_state["keycloak_started"] = True | ||
shared_state["keycloak_url"] = keycloak_container.get_url() | ||
yield shared_state["keycloak_url"] | ||
|
||
# After the fixture is done, cleanup the shared state | ||
del shared_state["keycloak_started"] | ||
del shared_state["keycloak_url"] | ||
|
||
|
||
@pytest.fixture(scope="session") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only 3.11?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current integration tests are only running on 3.11. so kept the same for this action also. Should we add another python version for all integration tests?