test(e2e): drive the oidc-credential chain from formae connect - #696
Draft
JeroenSoeters wants to merge 7 commits into
Draft
test(e2e): drive the oidc-credential chain from formae connect#696JeroenSoeters wants to merge 7 commits into
formae connect#696JeroenSoeters wants to merge 7 commits into
Conversation
The oidc-credential e2e proved the exchange but not how the trust it
exchanges against came to exist. The IAM identity provider and the role
were hand-provisioned standing resources and the test asserted on a role
name it could only have been told, so what `formae connect` actually
provisions was never exercised end to end.
The role is now made by the run that uses it. A stub control plane serves
the cloud-connection setup and receives the registration, a stub auth
plugin stands in for the hosted one the gate demands, and `formae connect
aws --profile-aws` provisions the identity provider and the role for real
before the agent starts. The echo plugin then assumes that role, so a
trust policy connect writes wrongly fails the test instead of passing
beside a correct one somebody made by hand.
Three things the fixtures had pinned now travel from the run:
- the role ARN, because it does not exist until connect makes it;
- the subject, which moves to the grammar the control plane really
produces, `fai:<tenant>/<installation>`, rather than an invented shape
production never emits and the GCP path cannot even parse;
- the audience, since each cloud spells it differently.
The exchange retries while IAM propagates. A role created seconds earlier
is not immediately assumable and STS answers AccessDenied until it is,
which failed the first real run against a trust policy that was correct.
The window sits inside the agent's plugin call deadline, so a genuinely
wrong policy still fails, at the end of it rather than the start.
The standing forma keeps the issuer, its published keys and the identity
provider, and drops the role. The provider stays because it is
account-global and shared, and because connect validates and reuses an
existing one rather than needing to create it. The per-run role carries
the suite's own name prefix so the pre-cleanup purge reclaims one a dead
run left behind.
The same property against a different federation mechanism: connect provisions the workload identity pool, the provider trusting the broker's issuer, and the project bindings; the broker mints for the provider's own resource name; Google's STS exchanges that for a federated access token; and the token reads the project. The read is not decoration. An exchange proves the provider trusts the issuer, subject and audience, and only spending its result proves connect granted the federated principal anything, which is the half a successful exchange alone leaves unchecked. Nothing torn down afterwards, deliberately: everything connect creates here converges rather than accumulates. The pool and provider are fixed per project and patched in place, and the bindings are set for one principal that does not change between runs. It skips until a project is named, and nothing names one yet. provx/gcp resolves its issuer from a compile-time constant instead of the one its caller passes, so connect provisions trust for the production issuer no matter which issuer the control plane named and the exchange could never succeed. That fix is in flight upstream; wiring the project in belongs with the dependency bump that carries it.
`formae connect --profile-aws` failed in CI with InvalidClientTokenId. The profile the workflow writes carries an access key and a secret key but no session token, and the credentials it is built from are temporary ones from an assumed role, which AWS refuses without it. Nothing caught it because nothing had read the profile as a profile. The credentials action also exports the three values as job environment variables, the SDK's default chain prefers those over shared config, and the access-test step passes no --profile — so every other e2e test authenticates through the environment and the half-written profile is never consulted. `--profile-aws` resolves through config.WithSharedConfigProfile, which forces the file and bypasses the environment, making it the first thing here to depend on those three lines being complete.
…idated `connect gcp` read an issuer, held it against the one the control plane reports, and then dropped it: the call to provx built a workload identity provider trusting whatever issuer that library had compiled in. The two agree in production, so the effect was latent there and total everywhere else — provisioning reported success while establishing trust that could never accept a token. provx now takes the issuer as a parameter, as its AWS half always has, so passing the validated one through is the whole fix. The dependency moves with it. This also switches the GCP half of the oidc-credential e2e on. It federates into its own project rather than the one the GCP plugin's suites use: the pool and provider ids are fixed per project and provx refuses to converge a provider trusting a different issuer, so a project already carrying the production-issuer connection cannot carry a test issuer as well. Absent the project, the test skips.
…fterwards The oidc-credential suite provisioned its trust under fixed names: one subject, one role, one set of GCP bindings, reused by every run. That made a green result mean less than it appeared to. From the second run onward the exchange succeeded against trust the first run had left standing, so a connect that had quietly stopped provisioning, or that failed to repair state something else had broken, would keep passing. On AWS the same fixed name also meant one run's teardown deleted a role another run might still be using, and that a role surviving a crashed run could be ridden to a pass by a later broken one. The subject and the role name are now derived from a fingerprint of FORMAE_TEST_RUN_ID, which the workflow already sets per run and attempt, falling back to the clock so a developer machine gets the same property. An exchange that succeeds is therefore attributable to the invocation under test: the trust it spends did not exist before that invocation created it. Per-run subjects make the GCP teardown mandatory rather than optional. The provisioner grants its principal roles/editor and roles/resourcemanager.projectIamAdmin, so a fixed subject left one standing grant behind while a per-run one would deposit a new privileged principal on every run, permanently, on a project whose issuer signs with a standing key. The bindings are now revoked in cleanup through the project IAM policy. The pool and provider are deliberately left alone: both are shared between installations, and provx's own Delete leaves them for that reason. The stub control plane also stops accepting requests the real API would reject. It now asserts the installation the run addressed rather than answering for any, refuses unknown fields and trailing content, and reads the trust coordinates through pointers so an absent field is told apart from an empty one. That last one retires a false claim in a comment here: the GCP test said it proved roleArn never crossed the wire, when both an absent field and "" had been decoding to the same empty string.
The exchange retried every failure for a fixed window measured between attempts. Both halves were wrong in the same direction: they spent time that bought nothing and buried the reason. A wall-clock check between attempts bounds when the last request may start, not when it may finish, so an attempt begun just inside the window could run past the agent's plugin call deadline. The failure then arrives as an operation timeout instead of the recorded exchange error, which reads as the harness breaking rather than as the exchange refusing. The window is now a child context, so nothing the loop starts can outlive it. Retrying every error also meant a settled answer — a malformed ARN, a rejected token, an expired credential — cost the whole window before being reported. Only AccessDenied is waited out now, because that is what STS returns for a role whose trust policy has not propagated yet. It is also what a genuinely wrong trust policy returns, and the two are not distinguishable from outside, which is why this is a bounded wait rather than a poll for readiness.
The revocation treated an empty result as success, which made a principal string that does not match what Google stores indistinguishable from a clean teardown. Both paths returned without complaint, so privileged bindings could survive under a green run — the precise failure the cleanup exists to prevent, reintroduced inside it. It runs only once connect has reported the provider it provisioned, so by then the binding it granted must exist. Finding none is therefore a defect worth reporting, not a state worth tolerating.
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
1 participant
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.
What
The oidc-credential e2e proved the token exchange but not how the trust it exchanges against came to exist: the IAM identity provider and the role were hand-provisioned standing resources, and the test asserted on a role name it could only have been told. What
formae connectactually provisions was never exercised end to end.The role is now made by the run that uses it:
oidc, stands in for the hosted one the connect gate demands;formae connect aws --profile-awsprovisions the identity provider and the role for real;A trust policy connect writes wrongly now fails the test, rather than passing beside a correct one somebody made by hand.
What moved out of the fixtures
The role ARN (it does not exist until connect makes it), the subject, and the audience all travel from the run. The subject in particular moves to the grammar the control plane really produces,
fai:<tenant>/<installation>, instead of an invented shape production never emits — and which the GCP path cannot parse at all.The standing forma keeps the issuer, its published keys and the identity provider, and drops the role. The provider stays because it is account-global and shared, and because connect validates and reuses an existing one rather than needing to create it.
A real bug the first live run found
A role created seconds earlier is not immediately assumable: IAM propagates asynchronously and STS answers
AccessDenieduntil it has. The first run against the real account failed on a trust policy that was correct. The exchange now retries inside a window well short of the agent's 60s plugin call deadline, so a genuinely wrong policy still fails — at the end of the window rather than the start.Testing
Run against the real e2e account from a local worktree:
The matrix entry needs the
e2e-testshared-config profile the workflow already writes;formae connectprovisions with it.Draft, because of the second commit
The GCP sibling test lands here but is not wired on.
provx/gcpresolves its issuer from a compile-time constant rather than the one its caller passes, soconnect gcpprovisions trust for the production issuer regardless of what the control plane named, and the exchange could never succeed. Fix upstream: platform-engineering-labs/oox#5. Once that merges, this branch takes the dependency bump, the one-line wiring ingcpprovision.go, and the workflow'sE2E_GCP_PROJECTline together, and comes out of draft.