Skip to content

test: add extension PipelinePolicy tests#991

Open
silvi-t wants to merge 21 commits into
Kuadrant:mainfrom
silvi-t:extension-PipelinePolicy-tests
Open

test: add extension PipelinePolicy tests#991
silvi-t wants to merge 21 commits into
Kuadrant:mainfrom
silvi-t:extension-PipelinePolicy-tests

Conversation

@silvi-t

@silvi-t silvi-t commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Important

This PR modifies shared/core testsuite code that could potentially affect multiple test areas. 2 reviewers should review this PR to ensure adequate coverage.

Description

  • Add comprehensive test coverage for the PipelinePolicy extension (57 tests across 14 test files)
  • Add PipelinePolicy client implementation for creating and managing PipelinePolicy CRDs
  • Add pipeline_policy_extension_service image configuration for the ThreatAssessmentService gRPC backend
  • Mark known failing tests with xfail linked to upstream issues for policy isolation, deletion reconciliation, top-level fail action, deny with CEL body, and auth+deny timeout

Changes

New: PipelinePolicy Client

  • testsuite/kuadrant/extensions/pipeline_policy.pyPipelinePolicy class with methods for deny, fail, response headers, gRPC method actions, and action method definitions

New: Test Infrastructure

  • conftest.py — shared fixtures: pipeline_policy, threat_assessment_service, CRD existence check (check_pipeline_policy_crd)
  • interactions/conftest.py — fixtures for AuthPolicy/RateLimitPolicy interaction tests: authorization, auth, rate_limit
  • config/settings.yaml / config/settings.local.yaml.tpl — added pipeline_policy_extension_service.image setting
  • testsuite/utils/constants.py — added THREAT_ASSESSMENT_THRESHOLD and EXTENSION_POLICY_PROPAGATION_WAIT

Known Issues (xfail) - xfails has been removed; these issues no longer exist

Verification steps

Run all PipelinePolicy tests sequentially:

poetry run pytest -vv -n4 testsuite/tests/singlecluster/extensions/pipeline_policy/

Closes #974, #975, #976

Summary by CodeRabbit

  • New Features

    • Added pipeline_policy_extension_service configuration with a default image, plus an optional commented local override.
    • Expanded PipelinePolicy capabilities for request/response actions, including conditional gRPC threat assessment and response/header enforcement.
  • Tests

    • Added extensive integration and end-to-end tests covering composition rules, lifecycle (create/update/delete), isolation, deny/header behaviour, authentication and rate limiting interactions, and gRPC success/error scenarios.
  • Chores

    • Updated cleanup targets to include PipelinePolicy extension resources.

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a PipelinePolicy SDK model, configuration, fixtures, a threat-assessment gRPC service fixture, and comprehensive integration coverage for actions, composition, targeting, lifecycle, validation, and interactions with authentication and rate limiting.

Changes

PipelinePolicy testing infrastructure

Layer / File(s) Summary
Configuration and constants
Makefile, config/settings*.yaml*, testsuite/utils/constants.py
Adds PipelinePolicy resource cleanup, threat-assessment service configuration, and shared test timing and threshold constants.
PipelinePolicy model and base fixtures
testsuite/kuadrant/extensions/pipeline_policy.py, testsuite/tests/singlecluster/extensions/pipeline_policy/conftest.py
Adds action-section builders, PipelinePolicy creation and action-method helpers, CRD checks, and policy lifecycle fixtures.
Basic and composition semantics
testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_basic.py, test_pipeline_policy_composition.py
Tests basic deny and response-header behaviour, action ordering, empty pipelines, and request-only and response-only configurations.
Deny, response headers, targeting, and isolation
testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_{deny,response,targeting,isolation}.py
Covers customised denies, conditional headers, gateway targeting, and policy isolation across routes and gateways.
gRPC actions and fail behaviour
testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/*
Deploys a threat-assessment service and tests conditional gRPC calls, threat-based decisions, fail and deny composition, dynamic response bodies, and gRPC errors.
Auth and rate-limit interaction coverage
testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/*
Adds OIDC and rate-limit fixtures and verifies PipelinePolicy behaviour with AuthPolicy, RateLimitPolicy, and their combination.
Lifecycle update/delete and validation failures
testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_{lifecycle,validation}.py
Verifies policy updates and deletion propagation, plus rejection of invalid targets, CEL, fail actions, variable references, and duplicate variables.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant PipelinePolicy
  participant ThreatAssessmentService
  Client->>PipelinePolicy: Send HTTP request
  PipelinePolicy->>ThreatAssessmentService: Invoke configured gRPC action
  ThreatAssessmentService-->>PipelinePolicy: Return threat assessment
  PipelinePolicy-->>Client: Apply deny, fail, or response headers
Loading

Possibly related issues

  • Issue 976 — Adds the comprehensive PipelinePolicy action, validation, lifecycle, targeting, gRPC, and interaction coverage.
  • Issue 975 — Adds the PipelinePolicy model, fixtures, and basic happy-path tests.

Suggested labels: Test case, Authorino

Suggested reviewers: crstrn13, emmaaroche

Poem

I’m a rabbit with policies neatly in line,
Denying bad paths with a header to shine.
gRPC hops through the test-service air,
Auth and limits now dance everywhere.
Pipelines update, vanish, and grow—
Carrots applaud as the green tests glow!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title uses a valid conventional commit type and clearly states the main change: adding PipelinePolicy tests.
Description check ✅ Passed The description includes the required Description, Changes, and Verification sections and is specific enough.
Linked Issues check ✅ Passed The PR covers the PipelinePolicy model, fixtures, happy path, edge cases, targeting, gRPC actions, and policy interactions requested by #974/#975/#976.
Out of Scope Changes check ✅ Passed No clearly unrelated changes are present; the config, cleanup, and test-support updates all serve the PipelinePolicy test suite.
Docstring Coverage ✅ Passed Docstring coverage is 98.17% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@silvi-t silvi-t self-assigned this Jun 1, 2026
@silvi-t silvi-t added this to Kuadrant Jun 1, 2026
@silvi-t silvi-t moved this to In Progress in Kuadrant Jun 1, 2026
@silvi-t silvi-t changed the title Extension pipeline policy tests test: add extension PipelinePolicy tests Jun 1, 2026
@silvi-t
silvi-t force-pushed the extension-PipelinePolicy-tests branch from 3556628 to ce39290 Compare June 2, 2026 08:41
@silvi-t
silvi-t marked this pull request as ready for review June 2, 2026 10:04

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py (1)

69-73: TODO: assert the expected validation message once available.

Until the message is asserted, this test only checks Accepted=False, which could pass for an unrelated rejection reason. Tighten it when the upstream validation lands.

Would you like me to open a tracking issue for adding the message assertion here?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py`
around lines 69 - 73, Test currently only asserts
policy.wait_until(has_condition("Accepted", "False"), ...) which can match
unrelated rejections; update the test to also assert the expected validation
message once the upstream validation is implemented by replacing the TODO with
an assertion that the policy's condition contains the exact expected message
(use policy.refresh().model.status.conditions to read conditions) — for example,
extend has_condition usage or add an assert that any(c.type == "Accepted" and
c.status == "False" and expected_message in c.message for c in
policy.refresh().model.status.conditions) where expected_message is the upstream
validation text; leave a TODO/issue reference if the message is not yet
available.
testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py (1)

8-22: ⚖️ Poor tradeoff

Optional: consider centralising the duplicated pipeline_policy/commit fixtures.

The header-only pipeline_policy fixture and the commit autouse fixture are repeated (with small variations in committed components) across the three interaction modules. A shared parametrised helper in interactions/conftest.py could reduce drift, though the per-module differences make this a low priority.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py`
around lines 8 - 22, Extract the duplicated pipeline_policy fixture and the
autouse commit fixture into a shared interactions/conftest.py: move the
header-only pipeline_policy setup (the pipeline_policy fixture that calls
pipeline_policy.on_http_response.add_headers([...]) and returns pipeline_policy)
and the commit fixture (which iterates components, calls
request.addfinalizer(component.delete), component.commit(), and
component.wait_for_ready()) into conftest.py, and make commit parametrisable so
callers can pass which components to commit (e.g., via pytest param or an
injected fixture) while preserving autouse behavior where needed; update the
three interaction test modules to import/rely on the shared pipeline_policy and
to call the commit fixture with the appropriate component list to avoid
duplication.
testsuite/kuadrant/extensions/pipeline_policy.py (1)

77-83: 💤 Low value

Inconsistent header serialisation between add_headers and add_deny.

add_headers serialises the headers list via str(headers), producing a Python repr with single quotes (e.g. [['x-single', 'one']]), whereas add_deny(with_headers=...) expects a caller-supplied double-quoted string (e.g. '[["x-deny-reason", "blocked"]]'). This only works if the extension parses headersToAdd/withHeaders as CEL (single quotes valid) rather than strict JSON. Consider accepting a List[List[str]] in both methods and serialising consistently to avoid surprises for future callers.

Please confirm the extension accepts a Python-style single-quoted list for headersToAdd (i.e. CEL evaluation rather than JSON parsing); the response-header tests should fail if it does not.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testsuite/kuadrant/extensions/pipeline_policy.py` around lines 77 - 83, The
add_headers method currently serialises headers with Python's str(headers)
producing single quotes, causing inconsistency with add_deny's expected
double-quoted JSON; change add_headers (and ensure add_deny) to accept headers
as a List[List[str]] and serialise them with json.dumps before placing into the
action dict (keys "headersToAdd" and "withHeaders"), preserving predicate
handling and the `@modify` decorator so both methods produce consistent JSON-style
double-quoted strings for downstream parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@testsuite/kuadrant/extensions/pipeline_policy.py`:
- Around line 77-83: The add_headers method currently serialises headers with
Python's str(headers) producing single quotes, causing inconsistency with
add_deny's expected double-quoted JSON; change add_headers (and ensure add_deny)
to accept headers as a List[List[str]] and serialise them with json.dumps before
placing into the action dict (keys "headersToAdd" and "withHeaders"), preserving
predicate handling and the `@modify` decorator so both methods produce consistent
JSON-style double-quoted strings for downstream parsing.

In
`@testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py`:
- Around line 8-22: Extract the duplicated pipeline_policy fixture and the
autouse commit fixture into a shared interactions/conftest.py: move the
header-only pipeline_policy setup (the pipeline_policy fixture that calls
pipeline_policy.on_http_response.add_headers([...]) and returns pipeline_policy)
and the commit fixture (which iterates components, calls
request.addfinalizer(component.delete), component.commit(), and
component.wait_for_ready()) into conftest.py, and make commit parametrisable so
callers can pass which components to commit (e.g., via pytest param or an
injected fixture) while preserving autouse behavior where needed; update the
three interaction test modules to import/rely on the shared pipeline_policy and
to call the commit fixture with the appropriate component list to avoid
duplication.

In
`@testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py`:
- Around line 69-73: Test currently only asserts
policy.wait_until(has_condition("Accepted", "False"), ...) which can match
unrelated rejections; update the test to also assert the expected validation
message once the upstream validation is implemented by replacing the TODO with
an assertion that the policy's condition contains the exact expected message
(use policy.refresh().model.status.conditions to read conditions) — for example,
extend has_condition usage or add an assert that any(c.type == "Accepted" and
c.status == "False" and expected_message in c.message for c in
policy.refresh().model.status.conditions) where expected_message is the upstream
validation text; leave a TODO/issue reference if the message is not yet
available.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d003d1c4-2f1a-4a81-ac35-7eac9f6ba6e9

📥 Commits

Reviewing files that changed from the base of the PR and between 4796963 and 25a4747.

📒 Files selected for processing (23)
  • Makefile
  • config/settings.local.yaml.tpl
  • config/settings.yaml
  • testsuite/kuadrant/extensions/pipeline_policy.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/__init__.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/conftest.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/__init__.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/conftest.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_auth.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_auth_ratelimit.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_basic.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_composition.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_deny.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_fail.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_grpc.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_grpc_errors.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_isolation.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_lifecycle.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_response.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_targeting.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py
  • testsuite/utils/constants.py

@silvi-t
silvi-t requested a review from a team June 2, 2026 10:18
@silvi-t silvi-t moved this from In Progress to Ready For Review in Kuadrant Jun 2, 2026
@silvi-t
silvi-t force-pushed the extension-PipelinePolicy-tests branch 2 times, most recently from 89a0f0f to edb84c8 Compare June 4, 2026 08:50

@crstrn13 crstrn13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Extract gRPC-dependent tests into a grpc/ submodule

The gRPC service URL construction + add_action_method boilerplate is repeated across 4 files (~5 times total):

svc_url = (
    f"grpc://{threat_assessment_service.name()}.{threat_assessment_service.namespace()}.svc.cluster.local:8080"
)
policy.add_action_method(
    name="assess",
    url=svc_url,
    service="threat.v1.ThreatAssessmentService",
    method="AssessRequest",
    message_template="threat.v1.ThreatRequest{uri: request.path}",
)

The majority of tests (basic, deny, response, isolation, lifecycle, targeting, validation, interactions) don't need the threat assessment service at all. Moving the gRPC-dependent tests into a grpc/ subdirectory avoids deploying the service (Deployment + Service) for those modules and centralizes the shared setup:

pipeline_policy/
  ...existing non-gRPC modules...
  grpc/
    __init__.py
    conftest.py
    test_grpc_basic.py         # ← from test_pipeline_policy_grpc.py
    test_grpc_errors.py        # ← from test_pipeline_policy_grpc_errors.py
    test_grpc_fail.py          # ← from test_pipeline_policy_fail.py
    test_grpc_composition.py   # ← gRPC-dependent tests from test_pipeline_policy_composition.py

The grpc/conftest.py would provide:

@pytest.fixture(scope="module")
def threat_service_url(threat_assessment_service):
    """gRPC URL for the threat assessment service."""
    svc = threat_assessment_service
    return f"grpc://{svc.name()}.{svc.namespace()}.svc.cluster.local:8080"


@pytest.fixture(scope="module")
def pipeline_policy(pipeline_policy, threat_service_url):
    """PipelinePolicy with the threat assessment gRPC action method pre-registered."""
    pipeline_policy.add_action_method(
        name="assess",
        url=threat_service_url,
        service="threat.v1.ThreatAssessmentService",
        method="AssessRequest",
        message_template="threat.v1.ThreatRequest{uri: request.path}",
    )
    return pipeline_policy

Individual test modules then only configure their deny/fail/headers actions on top — no URL construction or add_action_method calls.

Note: test_grpc_errors.py creates its own policies with intentionally bad configurations (wrong URL, wrong service name, wrong method name), so it wouldn't use the shared pipeline_policy fixture. It would still benefit from threat_service_url for the 2 tests that point at the real service, and it overrides commit() to empty as it already does.

The 4 non-gRPC composition tests (test_first_deny_wins, test_response_action_ordering, test_empty_pipeline, test_request_only_pipeline, test_response_only_pipeline) stay in test_pipeline_policy_composition.py.

crstrn13 and others added 12 commits July 14, 2026 12:58
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
This reverts commit 468dddc.

Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
crstrn13 and others added 6 commits July 14, 2026 12:58
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
…tests to subdirectory

Signed-off-by: Silvia Tarabova <starabov@redhat.com>
…e test isolation

Signed-off-by: Silvia Tarabova <starabov@redhat.com>
…top-level-fail validation tests

Signed-off-by: Silvia Tarabova <starabov@redhat.com>
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
@silvi-t
silvi-t force-pushed the extension-PipelinePolicy-tests branch from edb84c8 to 30664e5 Compare July 14, 2026 11:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
testsuite/kuadrant/extensions/pipeline_policy.py (1)

40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use implicit Optional typing for parameters that default to None.

Please use implicit optional typing (e.g., predicate: str = None) instead of Optional[str] to align with the rest of the codebase.

  • testsuite/kuadrant/extensions/pipeline_policy.py#L40-L40: Change var: Optional[str] = None and predicate: Optional[str] = None to var: str = None and predicate: str = None.
  • testsuite/kuadrant/extensions/pipeline_policy.py#L50-L56: Update predicate, with_status, with_headers, and with_body to use implicit optional typing.
  • testsuite/kuadrant/extensions/pipeline_policy.py#L70-L70: Change predicate: Optional[str] = None to predicate: str = None.
  • testsuite/kuadrant/extensions/pipeline_policy.py#L78-L78: Change predicate: Optional[str] = None to predicate: str = None.
  • testsuite/kuadrant/extensions/pipeline_policy.py#L6-L6: Remove Optional from the typing import if it is no longer required.

Based on learnings, follow the project-wide convention to use implicit/unspecified Optional typing for parameters/variables that default to None (e.g., namespace: str = None) rather than Optional[str].

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testsuite/kuadrant/extensions/pipeline_policy.py` at line 40, Update
testsuite/kuadrant/extensions/pipeline_policy.py at lines 40-40, 50-56, 70-70,
and 78-78 to replace Optional[str] annotations for parameters defaulting to
None—including add_grpc_method’s var and predicate, plus predicate, with_status,
with_headers, and with_body—with implicit str = None typing; remove Optional
from the typing import at lines 6-6 if unused.

Source: Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/conftest.py`:
- Around line 11-13: Update the threat_assessment_service fixture to validate
its required test configuration section before using testconfig or deploying the
service, by calling testconfig.validators.validate with only set to the
fixture’s required section name. Keep the existing deployment flow unchanged
after validation.

In
`@testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_composition.py`:
- Line 27: Update the deny predicate in the policy setup to target the `/admin`
request path used by the test, while leaving the fail action and request flow
unchanged so both predicates match and action precedence is exercised.

---

Nitpick comments:
In `@testsuite/kuadrant/extensions/pipeline_policy.py`:
- Line 40: Update testsuite/kuadrant/extensions/pipeline_policy.py at lines
40-40, 50-56, 70-70, and 78-78 to replace Optional[str] annotations for
parameters defaulting to None—including add_grpc_method’s var and predicate,
plus predicate, with_status, with_headers, and with_body—with implicit str =
None typing; remove Optional from the typing import at lines 6-6 if unused.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33a2acc4-36ab-42e5-974e-9eb3d7574b10

📥 Commits

Reviewing files that changed from the base of the PR and between f60dcf7 and 30664e5.

📒 Files selected for processing (26)
  • Makefile
  • config/settings.local.yaml.tpl
  • config/settings.yaml
  • testsuite/kuadrant/extensions/pipeline_policy.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/__init__.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/conftest.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/__init__.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/conftest.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_basic.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_composition.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_errors.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_fail.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/__init__.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/conftest.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_auth.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_auth_ratelimit.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_basic.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_composition.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_deny.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_isolation.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_lifecycle.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_response.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_targeting.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py
  • testsuite/utils/constants.py
🚧 Files skipped from review as they are similar to previous changes (15)
  • config/settings.yaml
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_basic.py
  • testsuite/utils/constants.py
  • config/settings.local.yaml.tpl
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_auth_ratelimit.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_auth.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/conftest.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_targeting.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_lifecycle.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_deny.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_response.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py
  • testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_isolation.py
  • Makefile

@silvi-t
silvi-t force-pushed the extension-PipelinePolicy-tests branch from 30664e5 to c891624 Compare July 14, 2026 12:50
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
@silvi-t
silvi-t force-pushed the extension-PipelinePolicy-tests branch from c891624 to 581dd53 Compare July 14, 2026 13:06
@silvi-t
silvi-t requested a review from crstrn13 July 14, 2026 13:15

@crstrn13 crstrn13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All 18 PipelinePolicy.create_instance() calls are missing labels={"testRun": module_label}. Without this label, make clean cannot find and delete orphaned PipelinePolicies from interrupted test runs. Every other policy type in the testsuite passes this label — PipelinePolicy should too.

See inline comments for each location with suggested fixes.

Comment thread testsuite/tests/singlecluster/extensions/pipeline_policy/conftest.py Outdated
Comment thread testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_errors.py Outdated
Comment thread testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_errors.py Outdated
Comment thread testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/test_grpc_errors.py Outdated

@crstrn13 crstrn13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded gRPC port 8080 in three places in grpc/conftest.py — should use HTTP_API_PORT from testsuite.utils.constants for consistency with the magic-number cleanup done across the rest of this PR.

Comment thread testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/conftest.py Outdated
Comment thread testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/conftest.py Outdated
Comment thread testsuite/tests/singlecluster/extensions/pipeline_policy/grpc/conftest.py Outdated
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
@silvi-t
silvi-t force-pushed the extension-PipelinePolicy-tests branch from f39c8b9 to 87c7722 Compare July 15, 2026 13:21
@silvi-t
silvi-t requested a review from crstrn13 July 15, 2026 13:24

@crstrn13 crstrn13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a small comment about the creation of the pipeline policy. Other than that, looks good.

Comment thread testsuite/kuadrant/extensions/pipeline_policy.py
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
@silvi-t
silvi-t requested a review from crstrn13 July 16, 2026 11:04

@crstrn13 crstrn13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉 :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready For Review

Development

Successfully merging this pull request may close these issues.

Extensions SDK testing: PipelinePolicy (out-of-tree extension)

2 participants