Skip to content

Commit 7c2041a

Browse files
blarghmateyclaude
andcommitted
fix(preview-gated): chain-constrain dependencies to the previous stage's deploy
Chain-wide `dependencies` (a built Docker image, an AMI) previously carried only whatever `passed` the caller set on them directly -- typically their own build job. Nothing tied them to the PREVIOUS STAGE's deploy the way `pulumi_code` already is via `passed_from`, so an image built after a stage's own build step, but never actually deployed to the environment before it, was still eligible to trigger and be approved there. Confirmed against ol-infrastructure's generated pipelines (PR mitodl/ol-infrastructure#5600): QA's preview job for the `concourse` AMI accepted any AMI that passed the packer build, not specifically the one that deployed to CI. Append the previous stage's deploy job to each chain-wide dependency's `passed`, same guarantee `pulumi_code` gets, stacked on top of (not replacing) whatever `passed` the caller already set. Copied, not mutated in place, so this doesn't reintroduce the deploy-chained mutation trap this topology exists to avoid. `custom_dependencies` is untouched -- those are index-scoped and the caller already sets `passed` there explicitly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019mvaFgGzBYBL2aatAZ2mJG
1 parent 2fe26bb commit 7c2041a

2 files changed

Lines changed: 87 additions & 7 deletions

File tree

pipeline_lib/src/ol_concourse/lib/jobs/infrastructure.py

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,14 @@ def pulumi_jobs_chain( # noqa: PLR0913, PLR0912, PLR0915
194194
:param project_source_path: The path within the `pulumi_code` resource where the
195195
code being executed is located
196196
:param dependencies: A list of `Get` step definitions that are used as inputs or
197-
triggers for the jobs in the chain
197+
triggers for the jobs in the chain. Under ``topology="preview-gated"``, each
198+
of these is also `passed`-constrained to the previous stage's deploy job (in
199+
addition to whatever `passed` it already carries, typically a build job) --
200+
same promotion guarantee as the Pulumi code get, applied to whatever artifact
201+
the Pulumi run also consumes (a built image, an AMI, ...).
198202
:param custom_dependencies: A dict of indices and `Get` step definitions that are
199-
used as inputs or triggers for the jobs in the chain.
203+
used as inputs or triggers for the jobs in the chain. Index-scoped and NOT
204+
stage-chained -- the caller sets `passed` explicitly per stage here.
200205
:param github_issue_assignees: A list of GitHub usernames that should be assigned
201206
:param github_issue_labels: A list of GitHub labels that should be applied
202207
:param record_deployments: Only used with ``topology="preview-gated"``. When
@@ -738,6 +743,13 @@ def _preview_gated_chain( # noqa: PLR0913, PLR0915
738743
it back, since promotion to the next stage is `passed` on the deploy job
739744
itself and the gate issue already authorised the deploy. Set ``False`` to
740745
drop it.
746+
747+
*dependencies* (the chain-wide ones, not *custom_dependencies*) are
748+
`passed`-chained to the previous stage's deploy job here too, on top of
749+
whatever `passed` the caller already set (typically a build job) --
750+
without that, a Docker image or AMI built after this stage's own build
751+
step, but never actually deployed to the PREVIOUS stage, would still be
752+
eligible to trigger and be approved here.
741753
"""
742754
exempt = {s.lower() for s in (auto_deploy_stages or [])}
743755

@@ -824,17 +836,32 @@ def _alerts(job: Job, stack: str, kind: str) -> None:
824836

825837
previous_deploy: Job | None = None
826838
for index, stack_name in enumerate(stack_names):
827-
# Chain-wide dependencies plus this stage's index-keyed custom ones.
839+
passed_from = [previous_deploy.name] if previous_deploy else None
840+
841+
# Chain-wide dependencies promote stage-to-stage exactly like
842+
# pulumi_code: an artifact must have reached the PREVIOUS stage's
843+
# deploy to be eligible here, same guarantee `passed_from` gives the
844+
# code get. Copied (never mutating the caller's step -- see
845+
# _stage_inputs), so this stacks with whatever `passed` the caller
846+
# already set (typically the build job) rather than replacing it.
847+
# custom_dependencies stays index-scoped and unchained: those already
848+
# encode explicit per-stage semantics the caller controls directly.
849+
chained_dependencies = []
850+
for dep in dependencies or []:
851+
dep_copy = dep.model_copy(deep=True)
852+
if passed_from and hasattr(dep_copy, "passed"):
853+
dep_copy.passed = [*(dep_copy.passed or []), *passed_from]
854+
chained_dependencies.append(dep_copy)
855+
828856
stage_inputs, stage_effects = _split_stage_steps(
829857
[
830-
*(dependencies or []),
858+
*chained_dependencies,
831859
*((custom_dependencies or {}).get(index) or []),
832860
]
833861
)
834862
post_steps = list((additional_post_steps or {}).get(index) or [])
835863
slug = stack_name.lower().replace(".", "-")
836864
serial_group = _stack_serial_group(project_name, stack_name)
837-
passed_from = [previous_deploy.name] if previous_deploy else None
838865

839866
record_issue = None
840867
if record_deployments:

pipeline_lib/tests/test_infrastructure.py

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,55 @@ def test_callers_step_objects_are_not_mutated(self):
727727
_gated_chain(dependencies=[dep])
728728
assert dep.trigger is True
729729

730+
def test_chain_dependencies_are_passed_constrained_to_the_previous_deploy(self):
731+
"""Without this, an image that only passed its OWN build job -- never
732+
actually deployed to the previous stage -- could still trigger and be
733+
approved here.
734+
"""
735+
fragment = _gated_chain(dependencies=[self._dep()])
736+
qa_dep = self._gets(
737+
_job(fragment, "preview-ol-substructure-keycloak-qa"), "some-image"
738+
)[0]
739+
assert qa_dep.passed == ["deploy-ol-substructure-keycloak-ci"]
740+
741+
production_dep = self._gets(
742+
_job(fragment, "preview-ol-substructure-keycloak-production"),
743+
"some-image",
744+
)[0]
745+
assert production_dep.passed == ["deploy-ol-substructure-keycloak-qa"]
746+
747+
def test_chain_dependencies_keep_their_own_passed_constraint_too(self):
748+
"""The chain appends the previous deploy; it doesn't replace an
749+
existing constraint such as the artifact's own build job.
750+
"""
751+
fragment = _gated_chain(dependencies=[self._dep(passed=["build-job"])])
752+
qa_dep = self._gets(
753+
_job(fragment, "preview-ol-substructure-keycloak-qa"), "some-image"
754+
)[0]
755+
assert qa_dep.passed == [
756+
"build-job",
757+
"deploy-ol-substructure-keycloak-ci",
758+
]
759+
760+
def test_chain_dependencies_unconstrained_on_the_first_stage(self):
761+
fragment = _gated_chain(dependencies=[self._dep()])
762+
ci_dep = self._gets(
763+
_job(fragment, "deploy-ol-substructure-keycloak-ci"), "some-image"
764+
)[0]
765+
assert not ci_dep.passed
766+
767+
def test_custom_dependencies_stay_unchained(self):
768+
"""Index-scoped custom_dependencies already encode explicit per-stage
769+
`passed` semantics the caller controls -- chaining must not touch them.
770+
"""
771+
fragment = _gated_chain(
772+
custom_dependencies={1: [self._dep("qa-only-artifact")]}
773+
)
774+
dep = self._gets(
775+
_job(fragment, "preview-ol-substructure-keycloak-qa"), "qa-only-artifact"
776+
)[0]
777+
assert not dep.passed
778+
730779

731780
class TestPreviewGatedSideEffects:
732781
"""Nothing that writes to the outside world may fire from a preview.
@@ -902,10 +951,14 @@ def test_existing_passed_constraints_are_kept(self):
902951
assert "build-image" in (dep.passed or [])
903952

904953
def test_preview_input_is_not_self_correlated(self):
905-
"""The preview cannot require having passed itself."""
954+
"""The preview cannot require having passed itself.
955+
956+
It IS chain-constrained to the previous stage's deploy, same as any
957+
other chain-wide dependency -- just not to its own not-yet-run preview.
958+
"""
906959
preview = _job(self._chain(), "preview-ol-substructure-keycloak-qa")
907960
dep = next(s for s in preview.plan if str(getattr(s, "get", "")) == "app-image")
908-
assert dep.passed == ["build-image"]
961+
assert dep.passed == ["build-image", "deploy-ol-substructure-keycloak-ci"]
909962

910963
def test_exempt_stage_input_is_untouched(self):
911964
deploy = _job(self._chain(), "deploy-ol-substructure-keycloak-ci")

0 commit comments

Comments
 (0)