New API by TomasKorbar · Pull Request #806 · packit/ai-workflows

TomasKorbar · 2026-09-08T08:53:49Z

Add HTTP API server for submitting consolidation jobs
Add Jira webhook command endpoint for bot-mention dispatching
Setup OIDC authentication support for our api server
Extend race prevention mechanism in consolidation job submission

nforro

It seems there are some missing pieces, here are some LLM findings:

P1: Public unauthenticated job submission

ymir/api/consolidation.py:80-97,146-148 exposes /api/consolidation without authentication through openshift/route-api.yml. Anyone reaching the route can enqueue costly arbitrary workflows. Additionally, webhook authentication fails open when JIRA_WEBHOOK_SECRET is unset (ymir/api/jira_webhook.py:65-69). Authentication should cover every mutating endpoint and fail closed.
P1: API image is never published

openshift/imagestream-api.yml:10 imports quay.io/jotnar/ymir-api:latest, but .github/workflows/build-and-push.yml has no job publishing Containerfile.api. After merge, deployment will receive no new API image.
P1: Deployment requires an undocumented secret

openshift/deployment-api.yml:50-51 references mandatory secret api-env, but openshift/README.md:11-46 neither documents nor creates it. Unless provisioned out-of-band, the pod remains in CreateContainerConfigError.
P2: None of the 30 API tests run in CI

Makefile.tests:29-52 enumerates existing package test directories but omits ymir/api/tests. Current green checks therefore provide no test coverage for the new server.
P2: source_issues accepts invalid cardinalities

ymir/api/consolidation.py:31,109 accepts zero, one, or more than two issues, although consolidation operates on a pair. One issue queues a successful no-op; more than two populates all branches while ymir/agents/prompts/mr_consolidation/prompt.j2:7-13 describes only the first two. Constrain this to exactly two entries when supplied.

TomasKorbar · 2026-09-09T08:04:53Z

It seems there are some missing pieces, here are some LLM findings:

1. **P1: Public unauthenticated job submission**
   `ymir/api/consolidation.py:80-97,146-148` exposes `/api/consolidation` without authentication through `openshift/route-api.yml`. Anyone reaching the route can enqueue costly arbitrary workflows. Additionally, webhook authentication fails open when `JIRA_WEBHOOK_SECRET` is unset (`ymir/api/jira_webhook.py:65-69`). Authentication should cover every mutating endpoint and fail closed.

Yeah there is no authentication, but the service is accessible only from internal network and ymir_todo can be placed by essentially anyone, so i am not sure whether implementing authentication is necessary/useful here.
The fail on missing JIRA_WEBHOOK_SECRET makes sense.

2. **P1: API image is never published**
   `openshift/imagestream-api.yml:10` imports `quay.io/jotnar/ymir-api:latest`, but `.github/workflows/build-and-push.yml` has no job publishing `Containerfile.api`. After merge, deployment will receive no new API image.

ah missed this. Did not know where to put it. Will fix.

3. **P1: Deployment requires an undocumented secret**
   `openshift/deployment-api.yml:50-51` references mandatory secret `api-env`, but `openshift/README.md:11-46` neither documents nor creates it. Unless provisioned out-of-band, the pod remains in `CreateContainerConfigError`.

Ah will add info to documentation.

4. **P2: None of the 30 API tests run in CI**
   `Makefile.tests:29-52` enumerates existing package test directories but omits `ymir/api/tests`. Current green checks therefore provide no test coverage for the new server.

Dammit.

5. **P2: `source_issues` accepts invalid cardinalities**
   `ymir/api/consolidation.py:31,109` accepts zero, one, or more than two issues, although consolidation operates on a pair. One issue queues a successful no-op; more than two populates all branches while `ymir/agents/prompts/mr_consolidation/prompt.j2:7-13` describes only the first two. Constrain this to exactly two entries when supplied.

Right. Makes sense.

nforro · 2026-09-09T08:27:22Z

Did not know where to put it. Will fix.

You also need to create the repo on quay.io.

lbarcziova · 2026-09-09T09:39:26Z

thanks a lot for kickstarting this!

As for the current code, I have few findings with the help of Codex, worth double-checking:

/api/consolidation is exposed through a public OpenShift Route without authentication - the point already brought up by Nikola, I think adding the authentication doesn't hurt and if we want this to be right now triggered only via Jira, it seems like a more controlled approach (I think we do check for the group of the user who currently triggers via ymir_todo, for this we should also do similar user/group validation)
Jira webhook authentication uses the wrong protocol. Native Jira Cloud webhooks send an HMAC in X-Hub-Signature, calculated over the raw request body. The PR instead compares a raw X-Webhook-Secret value. Standard Jira Cloud deliveries will therefore not authenticate. See ymir/api/jira_webhook.py:26,64-75 and Atlassian’s webhook documentation (https://developer.atlassian.com/cloud/jira/platform/webhooks/#validating-webhook-deliveries).
Queue submission is still vulnerable to a race. The API checks for existing pending/active jobs and then performs a separate Redis submission. Two concurrent requests can both pass the check, with the later HSET overwriting the first source-specific job. See ymir/api/consolidation.py:53-70 and ymir/common/merge_queue.py:47-66.
Redis failures in the pre-check path are handled inconsistently. The source-specific HGETs happen outside the try block, so Redis failures bypass the intended JSON error response and propagate through the webhook handler differently from failures during submit_merge_job.
Readiness does not reflect Redis availability. /healthz always returns 200, and the same endpoint is used for readiness. If Redis becomes unavailable after startup, Kubernetes will continue routing
requests to an API that cannot submit jobs.

I also wanted to bring up the Ymir triggering rearchitecture research here too (https://github.com/packit/research/blob/00a812858e100da0d7fb124d3b83220de990af19/research/ymir-triggering-rearchitecture/index.md) - the approach here uses webhooks, but I remember we were seriously considering also Automation + event router, with manual trigger forms. This choice affects authentication, payload format, authorization, deduplication, and UX. This would be worth re-descussing on arch. Considering that, we could split this PR into the API foundation and then the followup integration, wdyt? With that, the API foundation could still be tested standalone.

The initial research also showed the current OpenShift route may not be reachable from Jira directly, we would need to consider alternatives for that case: simple Ymir dashboard, polling, ..?

nforro

Two points from GPT-5.6 Luna:

P1: JWT issuer and audience are not validated

ymir/api/auth.py:89-95 constructs jwcrypto.jwt.JWT(key=keyset, jwt=token) and returns its claims, but never checks iss or aud against the configured OIDC provider/API. This accepts a correctly signed, unexpired token with arbitrary issuer and audience claims. I reproduced this with a token containing iss="wrong-issuer" and aud="other-client"; _validate_token() accepted it. Validate the expected issuer and API audience before authorizing the request, and add rejection tests.

P2: API tests are still absent from PR CI

Makefile.tests:18-25,92-93 now defines check-api-in-container, but .github/workflows/check-in-container.yml:31-39 has no check-api-in-container matrix entry. The current green PR checks therefore still do not execute ymir/api/tests; add the target to the workflow matrix.

lbarcziova

just few questions, otherwise LGTM

lbarcziova · 2026-09-22T09:01:53Z

+          registry_token: ${{ secrets.REGISTRY_TOKEN }}
+          dockerfile: "Containerfile.api"
+          docker_context: "."
+          image_name: "ymir-api"


please create the repo in quay for this, the auto building won't work without it

i did https://quay.io/repository/jotnar/ymir-api

lbarcziova · 2026-09-22T09:11:02Z

+        logger.warning("JIRA_BOT_ACCOUNT_ID not configured, ignoring webhook")
+        return web.json_response({"ignored": True, "reason": "bot account not configured"})
+
+    command_text = _extract_command(comment_body, bot_account_id)


didn't we want to have this restricted based on account role?

Right. Added

lbarcziova · 2026-09-22T09:14:20Z

+
+    @field_validator("source_issues")
+    @classmethod
+    def validate_source_issues_length(cls, v: list[str] | None) -> list[str] | None:


I thought we want to support by this also multiple issues to consolidate (>2), is that not the case?

Backend does not support that yet and i do not think that trying to mock that would be good solution. I will make submitting more issues possible with the consolidation rewrite.

Introduce a lightweight aiohttp-based API server (ymir/api/) that accepts POST requests to enqueue MR consolidation jobs into the existing Redis hash queue. - ymir/api/server.py: Generic API server with /healthz and pluggable route modules. Manages Redis lifecycle via redis_client(). - ymir/api/consolidation.py: POST /api/consolidation endpoint supporting auto mode (source_issues=null, dedup-safe) and label-triggered mode (source_issues set, returns 409 on conflict). - Containerfile.api: Minimal Fedora 44 container image. - compose.yaml: New 'api' service on port 8080 in agents profile. - OpenShift manifests: Deployment, Service, Route, ImageStream. - 13 unit tests covering both modes, validation, dedup, and errors. Co-authored-by: Cursor <cursoragent@cursor.com>

Add a generic POST /api/jira/webhook endpoint that parses Jira comment_created webhook payloads, detects bot mentions by account ID, and dispatches commands via an extensible command registry. - ymir/api/command_parser.py: Command registry with register() and dispatch(). New commands = one handler + one register() call. - ymir/api/jira_webhook.py: Webhook endpoint with X-Webhook-Secret validation, comment_created filtering, and [~accountId:] mention detection. - ymir/api/consolidation.py: Added handle_consolidate_command with argparse-based CLI parsing, registered as 'consolidate' command. Refactored shared _submit_consolidation_job() for reuse. - ymir/api/app_keys.py: Extracted REDIS_KEY to break circular imports. - Env vars: JIRA_WEBHOOK_SECRET, JIRA_BOT_ACCOUNT_ID added to compose.yaml and OpenShift deployment. - 17 new unit tests (command parser + webhook), 30 total. Co-authored-by: Cursor <cursoragent@cursor.com> Fix webhook auth to use HMAC signature per Jira Cloud protocol Jira Cloud signs webhook deliveries with an HMAC-SHA256 digest of the raw request body, sent in the X-Hub-Signature header as 'sha256=<hex-digest>'. The previous implementation compared a raw secret string from a non-standard X-Webhook-Secret header, which would never match real Jira Cloud deliveries. Replace the plain-text comparison with proper HMAC verification: read the raw body, compute HMAC-SHA256 keyed with JIRA_WEBHOOK_SECRET, and compare against the X-Hub-Signature header value. The _verify_signature helper is unit-tested against the known test vector from the Atlassian documentation. All 30 webhook tests updated to compute correct HMAC signatures. Co-authored-by: Cursor <cursoragent@cursor.com>

- Fail closed when JIRA_WEBHOOK_SECRET is unset: return HTTP 500 instead of skipping authentication entirely. - Add build-and-push-api job to the CI workflow so the ymir-api image is actually published to quay.io. - Document the api-env secret in openshift/README.md. - Add check-api and check-api-in-container targets to Makefile.tests so the API tests run in CI. - Constrain source_issues to exactly 2 entries via a Pydantic field_validator and tighten argparse nargs from '+' to 2. Co-authored-by: Cursor <cursoragent@cursor.com>

@Ymir

Introduce a new ymir-api service (aiohttp) with OIDC JWT authentication, a consolidation job submission endpoint, and a Jira webhook handler. API server (ymir/api/): - POST /api/consolidation: submit MR consolidation jobs into Redis. Pydantic validation enforces exactly 2 source_issues when set. - POST /api/jira/webhook: receive Jira Cloud webhooks, verify HMAC-SHA256 signatures (X-Hub-Signature), extract @Ymir bot-mention commands from ADF comment bodies, and dispatch via a command parser. Fails closed when JIRA_WEBHOOK_SECRET is unset. - /healthz (liveness) and /readyz (readiness, checks Redis connectivity). - Error comments posted back to Jira when webhook commands fail. OIDC authentication (ymir/api/auth.py): - aiohttp middleware validates Bearer JWTs against the OIDC provider's JWKS endpoint (jwcrypto). Checks signature, expiration, issuer (OIDC_ISSUER), and audience (OIDC_CLIENT_ID when configured). - OIDC_ISSUER defaults to OIDC_PROVIDER_URL but can be overridden when the JWKS fetch URL differs from the token issuer (e.g. local dev with containerized Keycloak). - CORS support for configured origins, including OPTIONS preflight. - Public paths (/healthz, /readyz, /api/jira/webhook) bypass auth. - Extracts preferred_username (fallback email, sub) as remote_user. - Disabled when OIDC_PROVIDER_URL is empty (local dev without auth). Trace server UI (trace_server/): - OIDC Authorization Code + PKCE login flow in the SPA (app.js). - New #/submit route with consolidation job submission form. - /oidc-config.json endpoint serves OIDC client config from env vars. OpenShift deployment: - Full manifests: Deployment, Service, Route, ImageStream, ConfigMaps. - api-oidc-env ConfigMap (configmap-api-oidc-env.yml) for OIDC provider URL and CORS origins. - trace-server-oidc-env ConfigMap for the trace server SPA. - CNAME route (route-trace-server-cname.yml) for ymir.redhat.com with automated TLS cert patching from ymir-cname-tls Secret in deploy.sh. - API image build job in GitHub Actions workflow. - check-api-in-container added to CI workflow matrix. Local development: - Keycloak service in compose.yaml with realm import including an audience protocol mapper for ymir-trace-ui. - OIDC_ISSUER override in compose.yaml for the container/host URL mismatch. Tests: 78 unit tests across test_auth, test_command_parser, test_consolidation_api, test_jira_reply, test_jira_webhook. Co-authored-by: Cursor <cursoragent@cursor.com>

…ript The API checked for existing pending/active jobs and then performed a separate Redis HSET. Two concurrent requests could both pass the check before either wrote, with the later HSET silently overwriting the first. Replace the check-then-set with _SUBMIT_JOB_LUA, a Lua script that runs HEXISTS + HSET atomically on the Redis server — the same pattern already used by _PICK_JOB_LUA for worker deduplication. The script supports two modes: - "strict" (label-triggered, source_issues set): rejects if either pending or active exists. - "auto" (no source_issues): rejects only if pending exists. submit_merge_job() now returns a SubmitResult enum (SUBMITTED, ALREADY_QUEUED, CONFLICT) instead of a plain bool, giving callers enough information to distinguish "already queued" from "conflict" without a separate pre-check. When the Jira issue fetcher encounters a CONFLICT for a label-triggered pair, it now posts a Jira comment explaining the conflict and asking the user to re-apply labels after the current job finishes, instead of silently cleaning the labels. All callers (API, Jira issue fetcher, agent tasks) updated accordingly. All FakeRedis test mocks updated to simulate the new Lua script. Co-authored-by: Cursor <cursoragent@cursor.com>

Verify that the Jira comment author belongs to the 'Red Hat Employee' group before dispatching any webhook bot-mention command. This matches the existing employee gate on the ymir_todo label path in the jira-issue-fetcher. The check calls GET /rest/api/3/user?expand=groups&accountId=<id> and fails closed: missing accountId, HTTP errors, and exceptions all result in a 403 rejection. - Add _is_rh_employee() async helper in jira_webhook.py - Insert the check after command extraction, before dispatch - Add 5 new tests covering accept, reject, missing ID, API failure, and non-command comment skip - Update THREAT_MODEL.md entry points table and T8 to document the new control Co-authored-by: Cursor <cursoragent@cursor.com>

lbarcziova

lgtm, thank you!

TomasKorbar force-pushed the consolidation_triggering branch 4 times, most recently from 347b96b to e0072e2 Compare September 8, 2026 13:18

nforro reviewed Sep 9, 2026

View reviewed changes

TomasKorbar force-pushed the consolidation_triggering branch from e0072e2 to 4f41782 Compare September 9, 2026 07:58

TomasKorbar force-pushed the consolidation_triggering branch 9 times, most recently from a072683 to fa6bd5c Compare September 18, 2026 09:29

nforro reviewed Sep 21, 2026

View reviewed changes

Comment thread openshift/README.md

TomasKorbar force-pushed the consolidation_triggering branch 4 times, most recently from 65a7eed to 75ac747 Compare September 22, 2026 07:19

lbarcziova reviewed Sep 22, 2026

View reviewed changes

TomasKorbar and others added 6 commits September 22, 2026 13:35

TomasKorbar force-pushed the consolidation_triggering branch from 75ac747 to eebf3a5 Compare September 22, 2026 11:35

lbarcziova approved these changes Sep 22, 2026

View reviewed changes

TomasKorbar merged commit 0ee74cd into packit:main Sep 22, 2026
15 checks passed

This was referenced Sep 23, 2026

Fix PACKIT-5421: Check if package already built with fixed dependency #833

Open

Recover interrupted MR consolidation jobs #847

Merged

This was referenced Oct 1, 2026

Explicitly enforce flexmock usage in AGENTS.md #857

Merged

Enable TLS encryption in transit for all Ymir services #864

Open

Conversation

TomasKorbar commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nforro left a comment

Choose a reason for hiding this comment

Uh oh!

TomasKorbar commented Sep 9, 2026

Uh oh!

nforro commented Sep 9, 2026

Uh oh!

lbarcziova commented Sep 9, 2026

Uh oh!

nforro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lbarcziova left a comment

Choose a reason for hiding this comment

Uh oh!

lbarcziova Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

TomasKorbar Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

lbarcziova Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

TomasKorbar Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

lbarcziova Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

TomasKorbar Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

lbarcziova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TomasKorbar commented Sep 8, 2026 •

edited

Loading