fix(iac): one bucket policy per bucket, fallback LF ARNs name the account, generate iac calls no AWS API - #684
Merged
Conversation
…ount, generate iac calls no AWS API Three defects found on main while fixing the examples for #674. 1. On the {account}-fluid-data fallback bucket (an unresolved {{ env.DATA_LAKE_BUCKET }}), the Lake Formation location and the bucket-policy ARNs were plain strings, so the renderer escaped the account token to $${...}, a literal: tofu plan registered arn:aws:s3:::${data.aws_caller_identity...}-fluid-data/... as written. The bucket policy also referenced an aws_s3_bucket the module never declares, so a plan with an ARN grantee failed "Reference to undeclared resource". The fallback ARNs are now TofuExpr with the contract-derived suffix escaped (the way _emit_glue builds the table location), decided on the raw input, and the policy addresses the fallback bucket by name. 2. Every expose emitted the bucket's aws_s3_bucket_policy under the same key, so a second expose on the bucket replaced the first: a grantee in another account lost s3:GetObject on the first prefix. The exposes on a bucket are now merged into one policy, grantees numbered across it so Sids and data.aws_arn keys stay unique. A one-expose bucket gets the same policy as before, byte for byte. Two exposes asking one bucket for different bucketPolicy modes are refused. 3. fluid generate iac built the native AWS provider, which resolves a missing account with sts:GetCallerIdentity over whatever credentials the machine has. generate iac now gives the planner a placeholder account when AWS_ACCOUNT_ID is unset, so no boto3 client is built and the sovereignty check still runs, and refuses a module that would carry the placeholder (generate_iac_aws_account_required, naming AWS_ACCOUNT_ID). fluid apply keeps resolving the account as before. Proved by tofu plan against moto (tests/iac/test_iac_lakeformation_bucket_policy_plan.py) and by a guard on botocore's Session.create_client (tests/iac/test_iac_generate_offline.py).
📄 Documentation ReminderThis PR appears to be missing a documentation reference. Our docs live in a separate repo. Please update the PR description with one of:
See the Contributing Guide for details. |
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
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.
Three defects on
main(736494c), found on 28 Sept 2026 while fixing the examples for #674. This PR is kept tofluid_build/iac/providers/aws.py,fluid_build/cli/generate_iac.py, one error-catalog entry and tests, so it merges beside #674: a trialgit mergeof #674 onto this branch is conflict-free, and on the merged tree 421 tests across the examples e2e, LF, column-restriction, cross-account, packaging and apply-engine files pass (41 skipped). No example, workflow or CHANGELOG change.1. Lake Formation ARNs on the fallback bucket carried an unresolved account
Evidence (main,
tofu planagainst moto). Withlocation.bucket: "{{ env.DATA_LAKE_BUCKET }}"unset, the bucket falls back to<account>-fluid-data. The Lake Formation location and bucket-policy ARNs were plain strings, so the renderer escaped the account token to$${...}, a literal:That is the planned value, i.e.
registerLocationwould register the ARN as written. With anarn:grantee the plan did not get that far, because the bucket policy referenced anaws_s3_bucketkeyed off the token that the module never declares:Fix.
_lf_s3_arnbuilds the LF location ARN and the policy's bucket and object ARNs. On the fallback it returns aTofuExprwith the contract-derived suffix escaped first, the same way_emit_gluealready builds the table'slocation. The fallback is decided on the raw input (bucket_uses_fallback), so a contract that writes the token itself still gets a literal (tested). On the fallback the policy'sbucketis the fallback bucket's name, interpolated, not a reference to an undeclared resource. Inall-granteesmode the static document becomes aTofuExprthere, with principals and prefixes escaped before encoding. Any other bucket emits exactly what it did before.I did not fail closed here.
tests/iac/test_iac_packaging_env_bucket.pypins that an owned or LEGACY bucket falls back to<account>-fluid-data("the fallback is the point there"). Only a pool bucket fails closed (shared-bucket-unresolved), and that stays as it is.Proof (
tofu planvs moto, account 123456789012). Plannedaws_lakeformation_resource.arn = arn:aws:s3:::123456789012-fluid-data/orders/. The plannedaws_s3_bucket_policyhasbucket = 123456789012-fluid-dataand its statements' resources arearn:aws:s3:::123456789012-fluid-dataand.../orders/*. Bothcross-accountandall-granteesare covered (TestTheFallbackBucketAtPlanTime).2. Two exposes on one bucket: the second bucket policy replaced the first
Evidence. Every expose emitted the bucket's
aws_s3_bucket_policy, itsaws_iam_policy_documentand itsaws_arngrantees under the same keys, so the last expose on a bucket won. I planned #674'sexamples/aws-medallion-lakeagainst moto, with the grantees' account (AWS_ACCOUNT_ID=222222222222) different from moto's so they count as cross-account.GetObjectstatements in the planned policy:curated/iot/sensor_readings/*only (Sids 0 and 1).raw/is gone.raw/iot/sensor_readings/*(Sids 0, 1) andcurated/iot/sensor_readings/*(Sids 2, 3).On main, the new synthetic plan test shows the same loss: only
FluidLfBucketGet0onlf-plan-lake/curated/*is planned.Fix.
_lf_bucket_policiesmerges each bucket's exposes, in contract order, into one policy, andemit()writes oneaws_s3_bucket_policyper bucket. S3 allows one policy per bucket, and the hashicorp/aws docs warn that a secondaws_s3_bucket_policyon the same bucket silently overrides the first. Grantees are numbered across the whole policy, so Sids anddata.aws_arnkeys stay unique. The provider'saws_iam_policy_documentrejects a duplicate Sid. Incross-accountmode each expose gets its own pair ofdynamicstatements, scoped to its own prefix and filtered over its own grantees (tostring(i + N)). The resource'scount, and the product KMS key's reader statement, use amerge()of those maps. A bucket with one expose gets the same bytes as before: the existing byte pins intest_iac_lakeformation_bucket_policy.pyandtest_iac_packaging_default_pin.pypass unchanged. If two exposes ask one bucket for differentbucketPolicymodes (cross-accountandall-grantees), the emit refuses (lakeformation-bucket-policy). An expose withnoneadds no statements and does not remove the others'.Proof (
tofu planvs moto). InTestOneBucketTwoExposesAtPlanTime, one planned instance,aws_s3_bucket_policy.lf_plan_lf_bucket_policy_lf_plan_lake[0], carriesFluidLfBucketList1/Get1forraw/*andFluidLfBucketList2/Get2forcurated/*, with the same-account grantee filtered out.all-granteesplans all four statements. Two same-account zones plan no policy.3.
fluid generate iaccalledsts:GetCallerIdentityEvidence. I ran
fluid generate iac examples/aws-medallion-lake/contract.fluid.yamlwithAWS_ACCOUNT_IDunset and a spy on client construction. It built onestsclient:The account it finds, from whatever credentials the machine has, then goes into the ARNs of the planned orchestration resources. The examples' READMEs say
generate iacneeds no AWS account or credentials.Fix.
native_actions(contract, logger, *, offline=False).generate iac(plain and--shadow) passesoffline=True. With AWS as the provider and noAWS_ACCOUNT_ID, the planner then gets the placeholder accountfluid-aws-account-id-unset, so no boto3 client is built. The planner still runs, including the sovereignty check that only it enforces for AWS. The emitter's own account references were alreadydata.aws_caller_identitylookups made at plan time. Only planner-built ARNs (Lambda, EventBridge, Step Functions) name the account. If one of those would reach the module,run()refuses before writing anything:generate_iac_aws_account_required, whose message and catalog suggestions nameAWS_ACCOUNT_ID.fluid applycallsnative_actions(contract, logger)and resolves the account exactly as before (pinned by a test).One behaviour changes. Take a contract whose orchestration plans Lambda or EventBridge resources, run with
AWS_ACCOUNT_IDunset. Before, a machine with credentials got those credentials' account written into the module. A machine without credentials got a module silently missing those resources: the planner failed its own check on the bucket nameNone-fluid-stagingand the failure was logged at DEBUG. Now both are refused with the message above. WithAWS_ACCOUNT_IDset, the output is unchanged. The shipped examples plan no such resources, so they emit as before.Proof.
tests/iac/test_iac_generate_offline.pyreplaces botocore'sSession.create_client, which every boto3 client and resource goes through, with a recorder. It runsfluid generate iacon all eightexamples/aws-*contracts, plain and--shadow, and on a Lake Formation contract on the fallback bucket, and asserts that no client is built and the placeholder never reaches a module. A scheduled contract is refused with nothing written. WithAWS_ACCOUNT_IDset, the same contract emitsrole = arn:aws:iam::123456789012:role/fluid-workflow-execution.Tests that fail without the fix (revert checks)
generate_iac.pyreverted to main: 19 failed, 3 passed intest_iac_generate_offline.py. The failures areassert ['sts'] == [], or['sts', 'sts']for--shadow. The three that pass hold on main too: the examples-exist check, theAWS_ACCOUNT_ID-set case, and the apply-path pin. Seeding only the removal of the placeholder assignment gives the same 19 failures.aws.pyreverted to main: 18 failed, 29 passed across the LF unit and plan files. The plan failures are the undeclared-resource error above, and a two-zone policy left with only thecurated/*statements. The new tests that pass on main are guards that hold either way: spoofed token,noneexpose, two buckets, the candidate-set equality, two same-account zones._lf_s3_arn(bug 1): 7 failed, including both fallback plan tests.tests/iac/test_iac_packaging_default_pin.py: its threenative_actionsstubs now accept keyword arguments (lambda contract, logger, **_: []), becauserun()passesoffline=True. Those lines are outside #674's hunk in that file.Gates (local)
ruff check fluid_build/ tests/: pass. The dead-code gate (--select F401,F841,F811,B007): pass.black --check fluid_build/ tests/with black 24.10.0: 1767 files unchanged.python scripts/mypy_strict.py: no issues in 7 source files.lint-imports: 6 contracts kept, 0 broken.python scripts/check_license_headers.py: pass.detect-secrets-hook1.5.0 with.secrets.baselineon the 8 changed files: exit 0.pytest tests/iac tests/cli tests/test_aws_examples_e2e.py tests/providers/aws -p randomly --randomly-seed=1 -n 8, withtofu1.12.0, moto 5.2.3,AWS_CONFIG_FILE=/dev/nullandAWS_SHARED_CREDENTIALS_FILE=/dev/null: 3109 passed, 201 skipped, 1 xpassed, 0 failed.The new plan tests are in
test_iac_lakeformation_bucket_policy_plan.py, whichiac-tests.ymlStage 1 already runs against moto and asserts did not only skip. The offline test isunit-marked, and.[dev,local]pulls boto3 in, so the main matrix runs it.Prior art (borrow-before-build receipts)
aws_s3_bucket_policydocs (website/docs/r/s3_bucket_policy.html.markdown): define oneaws_s3_bucket_policyper bucket, because the policy applied last silently overrides the previous one. Hence one merged policy per bucket, and no second resource.aws_iam_policy_documentsource (internal/service/iam/policy_document_data_source.go):principals.identifiersis aTypeSet, and a repeated Sid is rejected ("duplicate Sid"). Hence grantees numbered across the policy, not per expose.$${produces a literal${. That escaping is what turned the fallback ARN into a literal. The fix follows the repo's own_emit_gluepattern: escape the contract-derived pieces, then wrap the emitter-built value inTofuExpr.aws_caller_identitydata source docs:account_idis the account the provider connection runs as, which is why the emitter already defers the account to plan time rather than asking STS at generate time.boto3/session.py:Session.clientreturnsself._session.create_client(...), andSession.resourcecallsclient. Hence the guard on botocore'sSession.create_client, which catches every boto3 client and resource.Found, not fixed (separate change)
On the fallback,
_emit_s3still creates anaws_s3_bucketnamed literally{{ env.DATA_LAKE_BUCKET }}, from the raw contract value. That plans, and S3 would refuse it at apply. Meanwhile the Glue table, the LF location and now the bucket policy all target<account>-fluid-data.discover_importsalso imports that bucket by the raw name.