Skip to content

AAP-79952 - feat: add Konflux contract validation pipeline#454

Open
dmzoneill wants to merge 1 commit into
ansible:develfrom
Redhat-forks:AAP-79952-contract-validation-pipeline
Open

AAP-79952 - feat: add Konflux contract validation pipeline#454
dmzoneill wants to merge 1 commit into
ansible:develfrom
Redhat-forks:AAP-79952-contract-validation-pipeline

Conversation

@dmzoneill

Copy link
Copy Markdown
Member

Summary

  • Add PipelinesAsCode configuration (.tekton/contract-validation-pull-request.yaml) to trigger billing contract validation on PRs to devel
  • Add Dockerfile.contract-validation that builds a test image containing automation-analytics-backend with the PR's version of metrics-utility installed

How it works

When a PR is opened against devel:

  1. PipelinesAsCode webhook fires and submits the PipelineRun to the aap-aa-contract-tenant Konflux namespace
  2. The Dockerfile clones automation-analytics-backend, installs all dependencies, and installs metrics-utility from the PR branch
  3. The IntegrationTestScenario deploys the image to an ephemeral namespace and runs pytest -m billing
  4. Contract compliance tests validate real production bundles against the PR's schemas
  5. Results are reported back as a GitHub check on the PR

Prerequisites

  • aap-aa-contract-tenant must be created in konflux-release-data (separate MR)
  • PipelinesAsCode GitHub App must be configured for this repo
  • automation-analytics-backend Dockerfile must have METRICS_UTILITY_REF build arg (separate MR)

Related

Generated with Claude Code

Add PipelinesAsCode configuration and Dockerfile for automated billing
contract validation. When a PR is opened against devel, Konflux builds
an image containing automation-analytics-backend with the PR's schemas
installed, deploys to an ephemeral namespace, and runs the billing test
suite to validate contract compliance.

New files:
- .tekton/contract-validation-pull-request.yaml: PipelineRun triggered
  on PRs to devel via PipelinesAsCode
- Dockerfile.contract-validation: Builds test image with backend code
  and the PR branch of metrics-utility installed
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Added Tekton pipeline configuration for automated contract validation triggered on pull requests targeting the development branch, with configurable run retention and automatic cancellation of concurrent runs.
    • Added container image build definition with system dependencies and supporting utilities to enable contract validation execution in isolated environments.

Walkthrough

Two new files are added: Dockerfile.contract-validation, which defines a UBI9 Python 3.12 container that clones automation-analytics-backend, installs dependencies via pipenv, and installs metrics-utility from GitHub at a parameterized ref; and .tekton/contract-validation-pull-request.yaml, a Tekton PipelineRun manifest that triggers a multi-platform OCI build of that Dockerfile on pull requests to devel.

Changes

Contract-validation CI build pipeline

Layer / File(s) Summary
Contract-validation Dockerfile
Dockerfile.contract-validation
New UBI9 Python 3.12 Dockerfile that installs postgresql and git, configures corporate CA trust and REQUESTS_CA_BUNDLE, clones automation-analytics-backend, installs pipenv dependencies (prod + dev), installs metrics-utility from GitHub at METRICS_UTILITY_REF (default: devel), and sets ./launcher as the entrypoint on port 8080.
Tekton PipelineRun trigger and spec
.tekton/contract-validation-pull-request.yaml
New PipelineRun in namespace aap-aa-contract-tenant with Pipelines-as-Code annotations to fire on pull_request events targeting devel, limit retained runs, cancel in-progress runs, pass git URL/revision, set image output and expiration, reference Dockerfile.contract-validation, pass METRICS_UTILITY_REF mapped to the current revision, and resolve the docker-build-multi-platform-oci-ta pipeline from the Tekton catalog bundle.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding a Konflux contract validation pipeline, which directly aligns with the new PipelineRun manifest and Dockerfile introduced.
Description check ✅ Passed The PR description is largely complete with a clear summary, detailed workflow explanation, and prerequisites listed. However, it lacks structured sections matching the template (Description, Testing, Required Actions, Self-Review Checklist) and includes minimal testing details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
.tekton/contract-validation-pull-request.yaml (1)

43-44: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider pinning the pipeline bundle to a specific version.

Using :latest means pipeline behavior can change unexpectedly when the upstream catalog is updated, potentially breaking builds. Pinning to a digest or release tag improves reproducibility.

🤖 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 @.tekton/contract-validation-pull-request.yaml around lines 43 - 44, The
bundle parameter in the pipeline configuration is using the :latest tag which
causes unpredictable behavior since it will pull whatever version is currently
latest from the upstream catalog. Replace the :latest tag in the bundle value
with a specific version, release tag, or digest to ensure reproducible and
stable builds. This change should be made to the bundle value assignment to pin
it to a known stable version instead of dynamically resolving to whatever is
latest.
🤖 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 @.tekton/contract-validation-pull-request.yaml:
- Around line 43-44: The bundle parameter in the pipeline configuration is using
the :latest tag which causes unpredictable behavior since it will pull whatever
version is currently latest from the upstream catalog. Replace the :latest tag
in the bundle value with a specific version, release tag, or digest to ensure
reproducible and stable builds. This change should be made to the bundle value
assignment to pin it to a known stable version instead of dynamically resolving
to whatever is latest.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: aab864ef-16cb-479a-a555-dec69378348f

📥 Commits

Reviewing files that changed from the base of the PR and between 1c53c4b and 05215f0.

📒 Files selected for processing (2)
  • .tekton/contract-validation-pull-request.yaml
  • Dockerfile.contract-validation

@sonarqubecloud

Copy link
Copy Markdown

ARG METRICS_UTILITY_REF=devel

WORKDIR /opt/app-root/src
RUN git clone --depth 1 https://gitlab.cee.redhat.com/automation-analytics/automation-analytics-backend.git .

@himdel himdel Jun 23, 2026

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.

this seems to make upstream tests dependent on the state of a private gitlab repo .. that doesn't sounds right

iff this were to be used for failing PR checks, it would be a wrong thing to depend on a private repo
iff this is used to notify us or analytics of incompatible changes, it shouldn't be a problem

PR description seems to indicate the former, discussions the latter 🤷

RUN pip install --no-deps git+https://github.com/ansible/metrics-utility.git@${METRICS_UTILITY_REF}

EXPOSE 8080
CMD ["./launcher"]

@himdel himdel Jun 24, 2026

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.

assuming this is inteded to run launcher with APP="billing-validator", how?

Nothing sets APP in this PR, and nothing uses the billing-validator string that launcher checks for?

(or is contract-validation different from billing-validator and just not there yet?)
(or is this always overriden by pytest -m billing?)


Claude seems to think there's a whole bonfire clowdapp setup around it that eventually ends up running the equivalent of pytest -m billing somewhere, which matches the PR description, so probably this is fine.

@himdel himdel 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.

in konflux-release-data:

  • c-contract-validation.yaml configure-pac means this reports as a github check
  • its-contract-billing.yaml has a test.appstudio.openshift.io/optional: "false" label, making this a required check

upstream should not have required checks depending on private repos

@himdel

himdel commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

TLDR on what it does, from automation-analytics-backend, ~100 test functions across 15 files

test/billing_validator/ (17 tests)

  • test_validator.py — reconciliation (count/total mismatch detection), orphan tarball/swatch event detection, accuracy calculations
  • test_worker.py — metrics initialization, sync lag gauges, retry metrics

test/processor/aap_billing_controller/real_bundles/ (contract tests — ✨ most relevant to the PR ✨ )

  • test_aap_billing_v1_0.py — v1 bundle processing, display name, sender enable/disable
  • test_aap_billing_v2_0.py — v2 bundle processing, manifest validation, display name fallback, strategy selection
  • test_aap_billing_config_v2.py — config v2 bundle processing
  • test_aap_billing_infra_usage.py — infra usage bundles, negative charge prevention, timestamp boundaries
  • test_tenant_id_override.py — tenant ID override with allowed/disallowed service accounts

test/processor/aap_billing_controller/file_handlers/ (unit tests)

  • test_vcpu_usage_handler.py — vCPU data validation (negative, zero, missing fields, large values, SQL injection protection)
  • test_vcpu_usage_integration.py — end-to-end vCPU flow, DB write flag, duplicate detection
  • test_config_handler.py — handler registration, strategy selection, capability detection
  • test_data_collection_status_handler.py — billing status active/inactive, DB write enable/disable, strategy selection
  • test_job_host_summary_handler.py — message structure, vCPU data, pre-process sync entries, display name resolution

test/processor/aap_billing_controller/ (integration + sender tests)

  • test_aap_billing_controller_integration.py — full integration flow, error handling, performance
  • test_subscription_watch_infra_vcpu_sender.py — swatch sender with subswatch write flags, timestamp parsing (epoch, ISO, datetime)
  • test_vcpu_sender_display_name.py — pre-process records with staging data, display name from config

The real_bundles/ tests are the ones that would catch an incompatible contract change in metrics-utility — they process actual production tarball fixtures through the pipeline.


I wonder if this should only run a subset?

Cc @hunterkepley , the scope of what's being tested is probably relevant to the contract too :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants