Skip to content

Let a worker chain through an intermediate role - #6434

Open
koikonom wants to merge 1 commit into
mainfrom
streaming-intermediate-role
Open

koikonom wants to merge 1 commit into
mainfrom
streaming-intermediate-role

Conversation

@koikonom

Copy link
Copy Markdown
Contributor

Why

The wallets worker must reach the same Kinesis destination as the streaming
worker, and it must also read and write its own S3 bucket. EKS Pod Identity
binds one IAM role to one ServiceAccount, so the worker cannot hold two roles.

We tried to share one role between the two workers
(lago-infrastructure 771e1800) and reverted it (7e1a567b): two workers on one
role means each worker holds every grant of the other.

What this does

The producer reads a new variable, LAGO_STREAMING_INTERMEDIATE_ROLE_ARN.
When it is set, the STS client that assumes the destination role is itself
signed by that intermediate role. The destination therefore sees the
intermediate principal, and the trust policy of the stream owner needs no
change.

When the variable is empty, the code path is the same as before: one
AssumeRole from the ambient Pod Identity credentials. The streaming worker
leaves it unset and does not change.

Other changes:

  • Credentials for the intermediate role are cached per region, in a map that
    is separate from the per-destination cache. One process uses one
    intermediate role for every destination.
  • client_timeouts becomes the CLIENT_TIMEOUTS constant. The new class
    method needs the same values, and a class method cannot call an instance
    method.

Tests

bundle exec rspec spec/lib/lago/kinesis/producer_spec.rb — 29 examples, 0
failures. Four examples are new:

  • the intermediate role is assumed before the destination role
  • the destination assume is signed with the intermediate credentials
  • the intermediate role is assumed once per region, not once per destination
  • with no intermediate role, the destination is reached from the ambient
    identity in one hop

Notes for the reviewer

  • The variable is read at class load. A change to it needs a pod restart.
  • Chained sessions expire after one hour. AssumeRoleCredentials refreshes
    itself, so no code handles the expiry.
  • The IAM side is a separate PR in lago-infrastructure. Without it, and
    without the variable in the lago-deploy values file, this change is inert.

🤖 Generated with Claude Code

The wallets worker needs the Kinesis destination and its own S3 bucket.
Pod Identity binds one role to one ServiceAccount, so the worker cannot
hold two. Sharing `SynthesiaKinesis` with the streaming worker gives
both workers every grant of the other, which is not acceptable.

The producer now reads `LAGO_STREAMING_INTERMEDIATE_ROLE_ARN`. When it
is set, the STS client that assumes the destination role is itself
signed by that role. The destination sees the intermediate principal, so
the trust policy of the stream owner needs no change. When the variable
is empty, the code path is the same as before: one AssumeRole from the
ambient Pod Identity credentials.

The credentials for the intermediate role are cached per region, apart
from the per-destination cache, because one process uses one
intermediate role for every destination. `client_timeouts` becomes the
`CLIENT_TIMEOUTS` constant, because the new class method needs the same
values.

The variable is read at class load. A change to it needs a pod restart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green

HOLD — LAGO_STREAMING_INTERMEDIATE_ROLE_ARN is a new deployment setting but is absent from the required environment-variable documentation.

  • Add it to AGENTS.md with its purpose, an example ARN, and the fact that workers must restart after changing it because the value is read at process startup.

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