feature: dynamodb service with connect and stream trigger links - #1
Merged
Merged
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+47
to
+121
| resource "aws_dynamodb_table" "main" { | ||
| name = var.table_name | ||
| billing_mode = var.billing_mode | ||
| hash_key = var.hash_key | ||
| range_key = local.has_range_key ? var.range_key : null | ||
|
|
||
| read_capacity = local.is_provisioned ? var.read_capacity : null | ||
| write_capacity = local.is_provisioned ? var.write_capacity : null | ||
|
|
||
| deletion_protection_enabled = var.deletion_protection | ||
|
|
||
| # ------------------------------------------------------------------------- | ||
| # Streams are always on, and the event always carries both images. | ||
| # | ||
| # These are deliberately not variables. Enabling a stream costs nothing — | ||
| # there is no charge for the stream itself nor for its 24h retention, and | ||
| # reads performed by a Lambda trigger are not billed — so leaving it off | ||
| # would only add a configuration step before every trigger. | ||
| # | ||
| # Keeping the view type fixed also keeps the stream ARN stable: changing it | ||
| # recreates the stream under a new ARN and every event source mapping | ||
| # pointing at the old one silently stops receiving records. | ||
| # ------------------------------------------------------------------------- | ||
| stream_enabled = true | ||
| stream_view_type = "NEW_AND_OLD_IMAGES" | ||
|
|
||
| dynamic "attribute" { | ||
| for_each = local.attributes | ||
| content { | ||
| name = attribute.value.name | ||
| type = attribute.value.type | ||
| } | ||
| } | ||
|
|
||
| dynamic "ttl" { | ||
| for_each = var.ttl_enabled ? [1] : [] | ||
| content { | ||
| attribute_name = var.ttl_attribute_name | ||
| enabled = true | ||
| } | ||
| } | ||
|
|
||
| dynamic "global_secondary_index" { | ||
| for_each = var.global_secondary_indexes | ||
| content { | ||
| name = global_secondary_index.value.name | ||
| hash_key = global_secondary_index.value.hash_key | ||
| range_key = trimspace(global_secondary_index.value.range_key) != "" ? global_secondary_index.value.range_key : null | ||
| projection_type = global_secondary_index.value.projection_type | ||
|
|
||
| non_key_attributes = ( | ||
| global_secondary_index.value.projection_type == "INCLUDE" | ||
| ? global_secondary_index.value.non_key_attributes | ||
| : null | ||
| ) | ||
|
|
||
| read_capacity = local.is_provisioned ? global_secondary_index.value.read_capacity : null | ||
| write_capacity = local.is_provisioned ? global_secondary_index.value.write_capacity : null | ||
| } | ||
| } | ||
|
|
||
| tags = local.common_tags | ||
|
|
||
| lifecycle { | ||
| precondition { | ||
| condition = !local.range_key_equals_hash_key | ||
| error_message = "The sort key and the partition key are both set to '${var.hash_key}'. A table cannot use the same attribute for both: pick a different attribute for the sort key, or leave it empty for a partition-key-only table." | ||
| } | ||
|
|
||
| precondition { | ||
| condition = !local.attribute_conflict | ||
| error_message = "The same attribute is declared with two different types across the table keys and the global secondary indexes. Every index that reuses an attribute must declare the same type for it." | ||
| } | ||
| } | ||
| } |
Comment on lines
+47
to
+121
| resource "aws_dynamodb_table" "main" { | ||
| name = var.table_name | ||
| billing_mode = var.billing_mode | ||
| hash_key = var.hash_key | ||
| range_key = local.has_range_key ? var.range_key : null | ||
|
|
||
| read_capacity = local.is_provisioned ? var.read_capacity : null | ||
| write_capacity = local.is_provisioned ? var.write_capacity : null | ||
|
|
||
| deletion_protection_enabled = var.deletion_protection | ||
|
|
||
| # ------------------------------------------------------------------------- | ||
| # Streams are always on, and the event always carries both images. | ||
| # | ||
| # These are deliberately not variables. Enabling a stream costs nothing — | ||
| # there is no charge for the stream itself nor for its 24h retention, and | ||
| # reads performed by a Lambda trigger are not billed — so leaving it off | ||
| # would only add a configuration step before every trigger. | ||
| # | ||
| # Keeping the view type fixed also keeps the stream ARN stable: changing it | ||
| # recreates the stream under a new ARN and every event source mapping | ||
| # pointing at the old one silently stops receiving records. | ||
| # ------------------------------------------------------------------------- | ||
| stream_enabled = true | ||
| stream_view_type = "NEW_AND_OLD_IMAGES" | ||
|
|
||
| dynamic "attribute" { | ||
| for_each = local.attributes | ||
| content { | ||
| name = attribute.value.name | ||
| type = attribute.value.type | ||
| } | ||
| } | ||
|
|
||
| dynamic "ttl" { | ||
| for_each = var.ttl_enabled ? [1] : [] | ||
| content { | ||
| attribute_name = var.ttl_attribute_name | ||
| enabled = true | ||
| } | ||
| } | ||
|
|
||
| dynamic "global_secondary_index" { | ||
| for_each = var.global_secondary_indexes | ||
| content { | ||
| name = global_secondary_index.value.name | ||
| hash_key = global_secondary_index.value.hash_key | ||
| range_key = trimspace(global_secondary_index.value.range_key) != "" ? global_secondary_index.value.range_key : null | ||
| projection_type = global_secondary_index.value.projection_type | ||
|
|
||
| non_key_attributes = ( | ||
| global_secondary_index.value.projection_type == "INCLUDE" | ||
| ? global_secondary_index.value.non_key_attributes | ||
| : null | ||
| ) | ||
|
|
||
| read_capacity = local.is_provisioned ? global_secondary_index.value.read_capacity : null | ||
| write_capacity = local.is_provisioned ? global_secondary_index.value.write_capacity : null | ||
| } | ||
| } | ||
|
|
||
| tags = local.common_tags | ||
|
|
||
| lifecycle { | ||
| precondition { | ||
| condition = !local.range_key_equals_hash_key | ||
| error_message = "The sort key and the partition key are both set to '${var.hash_key}'. A table cannot use the same attribute for both: pick a different attribute for the sort key, or leave it empty for a partition-key-only table." | ||
| } | ||
|
|
||
| precondition { | ||
| condition = !local.attribute_conflict | ||
| error_message = "The same attribute is declared with two different types across the table keys and the global secondary indexes. Every index that reuses an attribute must declare the same type for it." | ||
| } | ||
| } | ||
| } |
Comment on lines
+8
to
+17
| resource "aws_iam_user" "link" { | ||
| name = var.iam_user_name | ||
| path = "/nullplatform/dynamodb/" | ||
|
|
||
| tags = { | ||
| "managed-by" = "nullplatform" | ||
| "link-id" = var.link_id | ||
| "table" = var.table_name | ||
| } | ||
| } |
fedemaleh
approved these changes
Jul 31, 2026
Contributor
Author
Análisis de los checks en rojoAnotado, sin corregir todavía. No hay alertas de dependenciasNi dependabot ni code scanning reportan nada: Es esperable: el repositorio es Terraform y Bash, sin dependencias de código que auditar. Trivy — un hallazgo, y es legítimo
Sid = "ManageStateBuckets"
Action = ["s3:*"]
Resource = ["arn:aws:s3:::np-service-*", "arn:aws:s3:::np-service-*/*"]El recurso está acotado, pero la acción no. Los scripts usan seis operaciones concretas: más las que el backend de Terraform necesita para leer y escribir el state. El conventional-commit — el commit de GitHub, no uno nuestroEs Los dos Así que hay dos salidas:
|
The repository was created from the "Any Technology" application template, whose CI builds a Docker image and pushes it as a nullplatform asset on every push to main. That does not apply to a service repository. Swaps it for the service tooling used across nullplatform services: shellcheck, trivy, conventional commits, branch validation and release.
Developer-facing capabilities for the table (keys, billing mode, TTL, global secondary indexes, deletion protection) and the connect link with three access levels. stream_arn is exported so the trigger link can read it from the notification context instead of querying AWS for it.
Streams are enabled unconditionally with NEW_AND_OLD_IMAGES rather than being exposed as a capability. Enabling a stream is free — neither the stream nor its 24h retention is billed, and reads from a Lambda trigger are not charged — so leaving it off would only add a configuration step before every trigger. Keeping the view type fixed also keeps the stream ARN stable, since changing it recreates the stream under a new ARN and silently orphans existing triggers. Attribute definitions are the deduplicated union of the table keys and every index key, with a precondition that rejects the same attribute declared under two different types.
Action routing and the tofu runner, which copies the module to the working directory and downloads the binary when the agent image does not ship it.
build_context resolves the region from the account provider, ensures the per-instance tfstate bucket and assembles the tofu variables. Two things worth calling out: The table name is read back from .service.attributes.table_name and only computed on the first create. aws_dynamodb_table.name is a ForceNew attribute: recomputing it from the service name means that renaming the service in nullplatform destroys the table and everything in it. The reference implementation this service is modelled on lost that guard when it was forked, which is how the bug was found. Global secondary indexes travel through a tfvars.json file rather than a -var flag, because do_tofu word-splits TOFU_VARIABLES and a JSON list would break at the first space.
Every workflow resolves the IAM role for selector "dynamodb" from the account's identity-access-control provider and assumes it before touching AWS, falling back to the agent credentials when none is configured.
One IAM user and access key per link, under /nullplatform/dynamodb/, with an inline policy scoped to the table and its indexes. Index ARNs are included because queries against a global secondary index authorize on the index, not the table. Link state lives under links/<link-id>.tfstate in the service bucket, so linking and unlinking never touch the table state.
create, update, delete and read for the service; link, link-update and unlink for the connect link. All of them assume the role first and propagate the temporary credentials to the following steps.
The role the agent assumes, scoped to tables matching the managed prefix, to IAM users under /nullplatform/dynamodb/, and to the np-service-* state buckets.
Completes the scaffold swap: shellcheck, trivy, conventional commits, branch validation and release, plus the ignore files used across nullplatform services.
Adds a second link type that connects the table's stream to the Lambda function of the linked application, alongside the existing connect link. Routing had to come first: entrypoint/link picked the workflow from the action type alone, so creating either link type arrived as "create" and ran the same workflow. It now looks for <action>-<link slug>.yaml and falls back to <action>.yaml, which leaves connect untouched. The field carrying the link specification slug is undocumented, so the known candidates are tried in order and the link object is logged when none matches. The mapping points at the function alias rather than the function, so a blue/green deployment does not leave the trigger on a version that no longer serves traffic. Partial batch failure reporting and batch bisection are on: the first is inert for functions that do not implement it, the second keeps a poison record from blocking its shard for the full 24h retention. The target function is derived from the link context using the Lambda scope's naming convention rather than by querying AWS, matching how the connect link reads the table from the service attributes.
The create notification does not carry .service.name, so the table was always named np-svc-<uuid>. Confirmed against the aws-s3-bucket service: every bucket it has created since March follows the same shape, with the user-provided suffix hiding the uuid. Reading the name back from the API when the context field is empty gives np-dynamotest instead. Worth fixing rather than living with: the table name is a ForceNew attribute, so a service created under the wrong name keeps it for life — renaming it destroys the table.
Creating a connect link destroyed the DynamoDB table. build_permissions_context set its own backend key but kept the OUTPUT_DIR that build_context points at, /tmp/np-service-<service-id>, which already held a .terraform initialised against the service's state. `tofu init` failed with "Backend configuration changed", do_tofu carried on to apply anyway, and the apply ran the permissions module against the service state — where terraform read the table as removed from configuration and deleted it. Three fixes, each of which would have stopped it on its own: - build_permissions_context and build_trigger_context now override OUTPUT_DIR to /tmp/np-link-<link-id>, and the workflows declare it as a step output so the value reaches do_tofu. This is what aws-s3-bucket does and what was missed when the script was adapted. - do_tofu aborts when init fails instead of falling through to apply. A failed init means the state is not the one this module expects, and continuing turns an error into destruction. - outputs are read with `tofu output -json` instead of `-raw`, which returns nothing for values marked sensitive. That silently dropped the secret access key and wrote a null into the link attributes, leaving the application with credentials it could not authenticate with. Writing the link now fails loudly if the secret is missing.
Setting both keys to the same attribute made CreateTable fail with "Invalid KeySchema: Some index key attribute have no definition", which says nothing about the actual mistake and points at an index when no index is involved. A precondition now catches it during plan and names the attribute and the two ways out.
The terraform data source reported a missing function without saying why, and the most likely cause is a scope that was created but never deployed: the function exists and the main alias does not. Checking the alias before tofu runs separates the two cases. The function name now uses the same truncation expression as the Lambda scope so the two cannot drift apart.
The trigger built the function name from .link.scope, which does not exist: the .link object only carries the link's own id, slug, attributes and specification. Where the link lives arrives as tags — tags.scope_id, tags.scope and tags.application — and in entity_nrn. Every trigger link failed at build_trigger_context with all three values empty, before reaching tofu. Verified against the context of the link that failed: the tags resolve to 223816164-lambda-and-dynamo-db-lambda-scope-a, which exists in AWS with its main alias. build_context had the same two mistakes on SCOPE_ID and SCOPE_NRN. No script consumed them, so nothing broke, but they were exported empty to every link workflow.
Creating a trigger failed with AccessDeniedException on lambda:GetFunctionConcurrency. The data source does not settle for GetFunction: it also reads the code signing config and the reserved concurrency, and a denial on any of them fails the whole read. Adds those two plus the version and policy reads, all read-only.
Two problems surfaced while the trigger link ran to completion. The apply created the mapping and the stream policy and then failed reading the mapping's tags: the provider refreshes tags on every resource that supports them, whether or not the module declares any. And the mapping targeted data.aws_lambda_function.target.arn, which drops the qualifier — so it consumed $LATEST while the comment right above it said the opposite. qualified_arn is the attribute that keeps the alias.
…ports The link exported table_name and table_region, and so does the service. Creating a link failed with "Parameter DYNAMODB_TABLE_NAME: The parameter already exists" and aborted before creating its own two parameters — the credentials — so every connect link ended up failed and the application never received them. Both fields stay as read-only attributes for reference; the service is the one that exports them.
agustincelentano
force-pushed
the
feature/dynamodb-service
branch
from
July 31, 2026 18:44
f5f9d5b to
f7da500
Compare
Trivy flags s3:* on the per-service state buckets. The finding is real: the resource is scoped to np-service-*, but the action covers PutBucketPolicy and PutBucketPublicAccessBlock, and each link's access key lives in that state in clear text. Suppressed with an expiry rather than silently, so the check comes back if the actions are not enumerated by then. The scripts use six S3 operations, listed in the entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a DynamoDB service to the catalog: a table, and two links that connect it
to applications from opposite directions.
What it provides
The table — partition and sort key, on-demand or provisioned billing, TTL,
global secondary indexes and deletion protection. Streams are always on with
NEW_AND_OLD_IMAGES: they cost nothing while nobody reads them, and having themenabled is what makes the trigger link possible without recreating the table.
The
connectlink — a dedicated IAM user per link with an inline policy overthe table and its indexes, at one of three access levels (
read,read-write,full, the last one adding PartiQL). The credentials reach the application asenvironment variables.
The
triggerlink — an event source mapping from the table's stream to aLambda function, plus the stream-read policy on that function's execution role.
It targets the function's alias, not the bare function, so the mapping follows
the version actually serving traffic.
Verified end to end
In an AWS implementation, with a Node application deployed on two scopes of
the same repository — one Lambda, one Kubernetes:
POSTto the container scope writes to the table using the connect link'scredentials.
handler logs the
INSERTwith the exact item that was written.MODIFYcarrying both the old andthe new image.
LastProcessingResult: OK, which is what confirms theshard checkpoint advanced — the handler's
batchItemFailuresresponse wasaccepted.
Things worth a second look in review
The table name is frozen after creation.
aws_dynamodb_table.nameforcesreplacement, and replacing a table takes every item with it. The name is
computed once and then read back from the service attributes, so renaming the
service in nullplatform never reaches the table. This is the defect that the
existing fork of this service has.
Each link runs in its own working directory and state key. Omitting that had
a specific consequence: the link initialised against the service's backend, the
init failed, the apply continued, and the permissions module ran against the
service state — deleting the table because it was not in that configuration.
do_tofunow aborts when init fails, for the same reason.A record with no sequence number fails the whole batch. It cannot be named
in
batchItemFailures, and reporting success would drop it silently, so a retryis the lesser evil.
The connect link hands out static credentials. An IAM user with an access
key, readable by anyone who can call
GetFunctionConfigurationon the target.For Lambda targets, granting the execution role a policy — the way the trigger
link already does — would avoid that. Left as is for now, but it is a decision
worth making deliberately.
🤖 Generated with Claude Code