feat: upload artifact through separate endpoint - #69
Open
nroi wants to merge 5 commits into
Open
Conversation
nroi
force-pushed
the
feature/CU-86cahupxr_Introduce-dedicated-artifact-upload-endpoint-for-BB-Run-artifacts
branch
2 times, most recently
from
July 23, 2026 09:40
bd7a57b to
510715d
Compare
nroi
marked this pull request as ready for review
July 23, 2026 14:55
nroi
force-pushed
the
feature/CU-86cahupxr_Introduce-dedicated-artifact-upload-endpoint-for-BB-Run-artifacts
branch
from
July 23, 2026 14:57
510715d to
9f67785
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates Terraform plan artifact handling so DETECT runs upload the raw plan bytes to a dedicated backend endpoint (via a new _links.planArtifactUpload.href) instead of embedding the plan as a base64 artifact in the final status update. This aligns the runner with a more scalable artifact-transfer mechanism and keeps APPLY runs consuming the plan via _links.planArtifact.href downloads.
Changes:
- Add a new
planArtifactUploadUrlfield (propagated from DTO → Run → TfCmdParams) and passRunApiintoPlanCmdso the plan command can upload artifacts. - Implement
UploadArtifactin the meshapi client and expose it viaRunApi.UploadPlanArtifact. - Remove plan artifact transport via status updates (
RunStatus.ArtifactandRunStatusUpdateDTO.Artifact) and update scenario tests to validate the new PUT upload behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tf-block-runner/tfrun/worker.go | Pass new upload URL into TF command params; route DETECT through PlanCmd(..., runApi). |
| tf-block-runner/tfrun/singlerunworker.go | Same as worker.go for single-run execution. |
| tf-block-runner/tfrun/tfcmd.go | Add planArtifactUploadUrl to TfCmdParams. |
| tf-block-runner/tfrun/tfplan.go | Upload plan bytes to planArtifactUploadUrl before terminal status update. |
| tf-block-runner/tfrun/runapi.go | Extend RunApi with UploadPlanArtifact and implement it in RunApiClient. |
| tf-block-runner/tfrun/run.go | Add PlanArtifactUploadUrl to internal Run model. |
| tf-block-runner/tfrun/dtos.go | Map _links.planArtifactUpload.href into the internal Run; remove status-update artifact encoding. |
| tf-block-runner/tfrun/runstatus.go | Remove RunStatus.Artifact (plan bytes no longer stored in status). |
| tf-block-runner/tfrun/worker_scenario_test.go | Extend mock HTTP routing to support PUT uploads; add run-details mocks returning the upload link. |
| tf-block-runner/tfrun/tfplan_scenario_test.go | Replace artifact-in-status assertions with upload-via-endpoint assertions; add upload-failure test. |
| go-meshapi-client/meshapi/dtos.go | Add planArtifactUpload link; remove artifact from status-update DTO. |
| go-meshapi-client/meshapi/client.go | Implement UploadArtifact (authenticated PUT of raw bytes). |
| go-meshapi-client/meshapi/client_test.go | Add tests verifying PUT behavior, headers, and non-2xx error surfacing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A missing planArtifactUpload link would previously let a DETECT run read the plan, discard it, and still report SUCCEEDED - leaving a follow-up APPLY with no plan to replay. Fail fast on the empty URL instead, before reading the plan file, consistent with treating a failed upload as a run failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
meshfed emits planArtifactUpload for every dry-run capable implementation and no longer checks the caller's rights, because we read the link from the checkout response (authenticated with the runner's own key) but upload with the run's ephemeral key. So a missing upload URL means version skew or a bug, never a missing permission, which is why a DETECT run fails instead of skipping the upload. CU-86cahupxr Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
meshfed renamed the newly introduced upload rel from planArtifactUpload to artifactUpload, so the DTO field and everything derived from it follow. The download rel stays planArtifact, since that one is released. Both changes must ship together: the runner matches the rel by name, so a DETECT run finds no upload URL and fails if only one side is deployed. CU-86cahupxr Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
CU-86cahupxr