Add scoped containerImages support to Bicep recipes#12361
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a type-scoped execution hook to the Bicep recipe driver so Radius.Compute/containerImages recipes can run a packaged build script inside dynamic-rp after deployment and return the pushed imageReference.
Changes:
- Introduces a new container image build hook that extracts a reserved
imageBuildoutput, validates it, runs a statically embedded script, and mergesimageReferenceinto the recipe output. - Adds Kubernetes Secret-based registry auth materialization (via the shared cluster-access resolver) into a temporary
DOCKER_CONFIGfor authenticated builds, and explicitly blanksDOCKER_CONFIGfor unauthenticated builds. - Documents the new scoped hook behavior in the
dynamic-rparchitecture guide and adds unit tests covering parsing, execution, and result contract enforcement.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/recipes/driver/bicep/containerimages.go | Implements imageBuild parsing/validation, script execution, result reading, and docker auth config writing. |
| pkg/recipes/driver/bicep/containerimages_unix.go | Unix process-group/cancel handling so child processes are killed on timeout/cancel. |
| pkg/recipes/driver/bicep/containerimages_windows.go | Windows stub so the package cross-compiles. |
| pkg/recipes/driver/bicep/containerimages_test.go | Unit tests for spec/script extraction, deterministic args, execution contract, and docker config writing. |
| pkg/recipes/driver/bicep/bicep.go | Wires in the hook during Execute and initializes the cluster access resolver on the driver. |
| docs/architecture/dynamic-rp.md | Documents the scoped containerImages Bicep hook and its constraints. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12361 +/- ##
==========================================
+ Coverage 53.34% 53.45% +0.11%
==========================================
Files 756 758 +2
Lines 49030 49269 +239
==========================================
+ Hits 26153 26338 +185
- Misses 20450 20480 +30
- Partials 2427 2451 +24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // dynamic-rp. The build script is deliberately not part of this evaluated output: it is read | ||
| // from a static compiled-template variable instead, so developer-controlled parameters can | ||
| // only be data. | ||
| type imageBuildSpec struct { |
There was a problem hiding this comment.
this looks like it's tied to the schema for Radius.Compute/containerImages. if we change that type schema, then this will need to be updated too.
| if err != nil { | ||
| return fmt.Errorf("failed to create a target-cluster client for registry Secret '%s/%s': %w", runtime.Namespace, spec.RegistrySecretName, err) | ||
| } | ||
| secret, err := clientset.CoreV1().Secrets(runtime.Namespace).Get(ctx, spec.RegistrySecretName, metav1.GetOptions{}) |
There was a problem hiding this comment.
how does the terraform version load the secrets? ideally we shouldn't be pulling the secrets from kubernetes because we might be deploying to ACI or some other compute platform
There was a problem hiding this comment.
Terraform also reads username and password from a Kubernetes Secret in the runtime namespace. The original design runs the build inside the dynamic-rp Pod using its BuildKit sidecar, and the Bicep path keeps the same behavior
| // configureScriptProcessGroup runs the script in its own process group and kills the whole | ||
| // group on cancellation, so processes the script spawned (e.g. buildctl) do not outlive the | ||
| // shell when the operation times out or is canceled. | ||
| func configureScriptProcessGroup(cmd *exec.Cmd) { |
There was a problem hiding this comment.
is there anything we can do to remove these per-platform files?
There was a problem hiding this comment.
There may be a way, but I'm not sure. configureScriptProcessGroup uses Unix-only process APIs to put the shell and its child processes into one group. If the operation is canceled, Radius kills the entire group, including buildctl. Windows does not provide the same APIs, so this implementation cannot live in the shared Go file without breaking Windows compilation. The small Windows stub keeps cross-platform builds working, even though the hook runs only on Linux.
|
|
||
| func (d *bicepDriver) executeImageBuild(ctx context.Context, script string, spec *imageBuildSpec, opts driver.ExecuteOptions) (string, error) { | ||
| logger := logr.FromContextOrDiscard(ctx) | ||
| tempDir, err := os.MkdirTemp("", "radius-imagebuild-") |
There was a problem hiding this comment.
is this similar behavior to the terraform version? or specific to bicep?
There was a problem hiding this comment.
This function is Bicep-specific plumbing, but the build outcome matches Terraform. Terraform runs buildctl through local-exec in its module directory and uses Terraform state to skip unchanged builds. Bicep cannot run shell commands, so the driver creates a temporary directory, runs the embedded script, waits for buildctl to finish, reads the result, and cleans up. Bicep runs this on every recipe execution, while Terraform may skip unchanged builds.
|
|
||
| // writeDockerConfig materializes registry credentials from the recipe runtime namespace. The | ||
| // Secret values are already decoded by client-go and are never logged. | ||
| func (d *bicepDriver) writeDockerConfig(ctx context.Context, spec *imageBuildSpec, dir string, opts driver.ExecuteOptions) error { |
There was a problem hiding this comment.
how do we guarantee parity with terraform?
There was a problem hiding this comment.
Parity is not automatic because Terraform and Bicep are separate implementations. This function matches Terraform by reading the same Secret, requiring username and password, and writing the same Docker authentication config for buildctl. The tests verify the target cluster, Secret path, generated JSON, and file permissions.
willdavsmith
left a comment
There was a problem hiding this comment.
looking good, left some comments
20e47de to
9f2ec0e
Compare
| for _, value := range env { | ||
| name, _, _ := strings.Cut(value, "=") | ||
| if name == dockerConfigEnvName || name == execOutputEnvName { | ||
| continue | ||
| } |
| @@ -0,0 +1,367 @@ | |||
| /* | |||
Signed-off-by: Mike Azure <mikeazure+microsoft@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ebd094d9-0447-48e6-9ab2-07506f7755c2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aea4b23a-3bc7-4672-be88-6ed2103d8ea4 Signed-off-by: Mike Azure <mikeazure+microsoft@microsoft.com>
Signed-off-by: Mike Azure <mikeazure+microsoft@microsoft.com>
9f2ec0e to
88e99a9
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
| if err != nil { | ||
| message := fmt.Sprintf("recipe %q script failed: %s", imageBuildOutputName, err.Error()) | ||
| if stderrTail != "" { | ||
| message += "\nstderr (tail):\n" + stderrTail | ||
| } | ||
| return "", errors.New(message) | ||
| } |
| text, err := reader.ReadString('\n') | ||
| if text != "" { | ||
| logger.Info(logPrefix + strings.TrimRight(text, "\r\n")) | ||
| if tail != nil { | ||
| tail.appendText(text) |
| func Test_RunScript_LongOutputLineDoesNotHang(t *testing.T) { | ||
| ctx, cancel := context.WithTimeout(testcontext.New(t), 10*time.Second) | ||
| defer cancel() |
| func Test_RunScript_CancelKillsSpawnedProcesses(t *testing.T) { | ||
| ctx, cancel := context.WithTimeout(testcontext.New(t), 500*time.Millisecond) | ||
| defer cancel() | ||
| start := time.Now() |
Add scoped
containerImagessupport to Bicep recipesDescription
Adds a small hook to the Bicep recipe driver for
Radius.Compute/containerImages. After the Bicep deployment finishes,dynamic-rpruns the build script included in the published recipe, waits forbuildctlto push the image, and then returnsimageReference.The script must be included in the published recipe. Resource properties are passed to it as command arguments, so app developers can't supply shell code. For a private registry, Radius gets the username and password from a Kubernetes Secret in the cluster and namespace where the app runs.
The hook runs only for
Radius.Compute/containerImages. Other Bicep recipes keep their current behavior.Related to radius-project/resource-types-contrib#220
Paired with radius-project/resource-types-contrib#228
Why this needs a Radius change
Bicep can describe resources, but it can't call the BuildKit sidecar in the
dynamic-rpPod or return the result of that image push. Running the script indynamic-rpuses the existing BuildKit setup and lets Radius wait for success before savingimageReference.Testing
go test ./pkg/recipes/driver/bicep