Skip to content

Support allowed users in GitHub trigger - #136

Draft
maxknv wants to merge 28 commits into
mainfrom
support-lambda-gh-trigger-allowed-users
Draft

Support allowed users in GitHub trigger#136
maxknv wants to merge 28 commits into
mainfrom
support-lambda-gh-trigger-allowed-users

Conversation

@maxknv

@maxknv maxknv commented Jul 11, 2026

Copy link
Copy Markdown
Member

Summary

  • add ALLOWED_USERS_JSON parsing to lambda_gh_trigger
  • skip pull_request events from senders outside ext.allowed_users
  • export OrchestratorPool.ext["allowed_users"] to the trigger Lambda environment

Tests

  • python3 -m pytest ci/tests/test_lambda_gh_trigger.py ci/tests/test_infra_projects.py
  • python3 -m compileall -q praktika/infrastructure/native/lambda_gh_trigger.py praktika/infrastructure/native/orchestrator_pool.py ci/tests/test_lambda_gh_trigger.py ci/tests/test_infra_projects.py
  • git diff --check

@praktika-gh

praktika-gh Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Workflow [Praktika CI Advanced], commit [c13d39b]

Summary:

@maxknv
maxknv force-pushed the support-lambda-gh-trigger-allowed-users branch from ba37f80 to c766230 Compare July 11, 2026 08:29
maxknv and others added 2 commits July 11, 2026 14:32
Orchestrator CI dispatch failed at the GitHub-token step: the
praktika_project_slug instance tag was set from the raw project name
(e.g. "ClickHouse") rather than the normalized slug, so the controller
invoked a wrongly-cased "{name}-gh-token" lambda that neither exists nor
is covered by the GitHubTokenMinterInvoke IAM grant. Set the tag from
the normalized project prefix instead.

Use "_" (not "-") as the intra-slug separator everywhere a slug is built
or validated (project_init, cloud._project_prefix, validator), and
reject a PROJECT_SLUG containing "-" in iam_scope. The slug is the
"{slug}-" resource-name prefix, so a "-" inside it lets one project's
scoped IAM wildcard (clickhouse-*) also match another project's
resources (clickhouse-private-*).

Also resolve the AWS account id from credentials (aws_account_id) instead
of a hardcoded Settings.AWS_ACCOUNT_ID, making that setting optional, and
bump the default controller wheel to 0.1.4.

Update tests accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The controller cloned the repo before the orchestrator subprocess posted
any GitHub check, so a slow or interrupted clone (e.g. the large
ClickHouse repo, or an instance replaced mid-clone) left the PR with no
CI signal at all.

Now the controller opens an in_progress check run (provisional name "CI",
with the Cancel action) right after auth and before clone_repo, then
threads its id to the orchestrate subprocess via
PRAKTIKA_BOOTSTRAP_CHECK_RUN_ID. The orchestrator adopts that check-run
id for the first matched workflow (CheckRun.retitle renames it to the
real workflow name and attaches the report URL) instead of opening a new
one; further matched workflows still open their own. If the controller
fails before the orchestrator takes over (clone, runtime resolution),
it finalizes the early check as failed so the PR shows the failure
rather than a check stuck in_progress. Posting the early check is
best-effort and never blocks the workflow (e.g. a fork head_sha absent
from the base repo).

Bump praktika 0.1.8 -> 0.1.9 and praktika-controller 0.1.4 -> 0.1.5.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@maxknv
maxknv marked this pull request as draft July 11, 2026 12:53
maxknv and others added 9 commits July 11, 2026 19:16
On a workflow infra failure (or task cleanup failure) the controller
released the SQS message (visibility=0) and requested ASG self-
termination, then returned from poll(). Termination is async, and the
controller unit is Restart=always, so systemd relaunched it within
seconds and it re-received the just-released message, starting a second
attempt (clone + orchestrate) on an instance already being torn down --
doing both "terminate" and "retry" at once.

Add _await_termination(): after requesting termination the controller
stops polling and blocks, waiting to be killed. As a safety net it forces
a local shutdown after a grace period in case the ASG termination never
lands. The message stays released so a fresh replacement instance retries.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename the report page file and update every reference (settings
HTML_PAGE_FILE, report_page_config path, docs, PROTOCOL, and report URL
examples/tests). No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…PATH

Surface runner-side failures instead of losing them:
- job_runner.run_job now records a crash into the job Result (status
  ERROR + traceback) so the orchestrator check-run and report show the
  cause instead of a bare failure.
- runner._post_run guards the JOB_OUTPUT_STREAM writes: that field is
  GitHub Actions' $GITHUB_OUTPUT and is empty in native mode, so
  open("") no longer raises FileNotFoundError and fails the job.
- state.py finalizes terminal check runs with an explicit output
  (DISPATCH FAILED / CANCELLED) so a cancelled or undispatchable job's
  check no longer keeps its stale "QUEUED" summary.

Scope runner resources correctly:
- Drop "./ci" from the job PYTHONPATH (both docker and non-docker); it
  let a bare `import praktika` resolve to the repo's vendored ci/praktika
  instead of the installed package. Matches the GH Actions path (".").
- Add RunnerPool.allowed_s3_prefixes_readonly for read-only S3 grants
  (GetObject/ListBucket, no writes), project-namespaced like the
  read/write list.

Disable the deploy-time infrastructure-config check in the pipeline
validator: it required ./ci/infrastructure/projects.py to exist, which
runners don't ship, so pipeline validation failed on them.

Tests updated/added accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Config validation and dispatch now accept job configs written for the
GitHub Actions engine without failing or misrouting:

- Ignore the "self-hosted" runs_on label for non-GHActions engines. The
  validator's "exactly one runs_on label" check filters it out, and the
  orchestrator's queue resolver skips it so
  ["self-hosted", "style-checker-aarch64"] routes to
  <slug>-style-checker-aarch64 (not a bogus <slug>-self-hosted queue).
- Drop the validation error for job.enable_commit_status on Praktika
  engine workflows: the flag is harmless (the Checks API is used
  regardless), so a job carrying it no longer fails config validation.
  The workflow-level enable_commit_status_on_failure check is unchanged.
- CIDB insert: when enable_cidb is set but SECRET_CI_DB_CONNECTION is
  empty, log a clear "skipping CIDB insert" note instead of crashing
  _post_run with a cryptic "Failed to find secret []".

Tests added/updated accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
enable_commit_status (job) and enable_commit_status_on_failure
(workflow) are harmless no-ops on the Praktika engine, which always
publishes status via the GitHub Checks API. Stop rejecting workflows
that carry the failure flag and reword both docstrings as
GH-Actions-only. Also sharpen the docker-registry secret assertions to
say what to set and why.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JobState.skip(post_check=True) reused _create_check(), which can only
POST a check as status=queued (pending), then PATCHed it to
completed/skipped -- two API calls per skipped job and a brief pending
flash in the PR UI. Add JobCheckRun.create_completed() to POST the
terminal state in one call and route skip() through a new
_create_completed_check() helper. The run/kick path is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prebuilt-venv validation step asserted
`import boto3, jwt, cryptography, requests, pytest`, but pytest is an
optional, project-chosen extra — not a Praktika runtime dependency. A
project that bakes a venv without pytest failed the image build with
ModuleNotFoundError. Assert only Praktika's own runtime deps (the
`infrastructure` extra); pytest and friends are the project's choice.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@maxknv
maxknv force-pushed the support-lambda-gh-trigger-allowed-users branch from 2ebeb69 to 8066d6a Compare July 14, 2026 10:35
maxknv and others added 10 commits July 14, 2026 13:42
- _build_check_output / sweep_liveness now include the runner pool
  (runs_on) alongside the runner instance id in the check summary and
  text, so a job's check shows both which instance and which pool ran it.
- praktika run: shlex-quote each --test value individually so a
  parametrized integration-test node id containing spaces, parentheses
  or quotes survives as a single, unmangled shell argument.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pre/post hooks and docker jobs need the repo's own `ci.*` modules
importable, while a bare `import praktika` must resolve to the installed
package (not the repo's vendored copy).

- runner._run docker command: PYTHONPATH is now
  "{staged_praktika_dir}:{checkout_root}" — staged dir first so
  `import praktika` uses the installed package, plus the checkout root so
  `ci.*` imports work (previously only the staged dir was set, breaking
  `import ci`).
- Pre/post hooks now run via Result.from_commands_run(env=_job_python_env())
  so they inherit PYTHONPATH with the checkout root ("."); they otherwise
  ran with an env lacking it and failed with
  ModuleNotFoundError: No module named 'ci'.
- Result.from_commands_run gains an `env` param, forwarded to Shell.run.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@maxknv
maxknv force-pushed the support-lambda-gh-trigger-allowed-users branch 2 times, most recently from df5e2e9 to d1fd3c6 Compare July 19, 2026 20:02
@maxknv
maxknv force-pushed the support-lambda-gh-trigger-allowed-users branch from d1fd3c6 to c13d39b Compare July 19, 2026 20:19
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.

1 participant