feat: allow a PodAccessRequest to override the container image - #460
Open
dnguy078 wants to merge 1 commit into
Open
feat: allow a PodAccessRequest to override the container image#460dnguy078 wants to merge 1 commit into
dnguy078 wants to merge 1 commit into
Conversation
By default the Pod that Oz launches runs the same image as the workload its PodAccessTemplate points at. That image is often the wrong tool for the job - a distroless production image has no shell, and a schema migration may need tooling deliberately kept out of the production build. Today the only way around it is for an administrator to author a dedicated PodAccessTemplate (or hand-write a patchSpecOperations JSON patch against /spec/containers/N/image), which puts a human in the loop for what is otherwise a self-service operation. This adds an optional `spec.image` to PodAccessRequest, plus an `--image` flag to `ozctl create PodAccessRequest`, so a developer can ask for a different image against an existing template. The image is only honored if it matches an allow-list configured on the controller at deploy time (`--allowed-image-patterns`, exposed as the `controllerManager.manager.allowedImagePatterns` Helm value). The allow-list lives on the controller rather than on a PodAccessTemplate on purpose: the set of registries an organization will run code from is a cluster-wide security boundary, and letting template authors widen it would defeat the point of having one. With no patterns configured the feature is off and any `spec.image` is rejected, so this is inert until an administrator opts in. Pattern matching confines `*` to a single path segment and offers `**` for matching across segments. This is what stops a leading wildcard from swallowing an untrusted registry host: under naive glob semantics `*.dkr.ecr.*.amazonaws.com/team/*` would also match `evil.example.com/x.dkr.ecr.us-west-2.amazonaws.com/team/backdoor`. Enforcement happens in two places. The validating webhook rejects the request at admission time so the developer gets immediate feedback, and the PodAccessBuilder re-validates before building the Pod. The second check is not redundant: the ValidatingWebhookConfiguration is optional (the chart's `webhook.create` value can disable it), so the builder is the only check guaranteed to run. Notes on the implementation: - `spec.image` is immutable. The Pod is built once, on first reconcile, so a later edit would silently do nothing while leaving the resource claiming to run an image it does not. - The override resolves the "default" container against the *unmutated* PodSpec, matching what PatchPodTemplateSpec() already does. A template's patchSpecOperations may rename containers, so resolving afterwards can fail to find a container that existed when the template author named it. getDefaultContainerID() is extracted into an exported GetDefaultContainerID() to share that logic with callers that have no PodTemplateSpecMutationConfig at all. - Only the image is replaced. The Pod still inherits the target workload's service account, secrets, environment and network identity, so any allow-listed image can run code with that workload's privileges - documented prominently in the chart values and README. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage Report for CI Build 30587074353Coverage increased (+2.2%) to 38.092%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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.
Problem
The Pod that Oz launches runs the same image as the workload its
PodAccessTemplatepoints at. This allows developers to test various test images with ozctlToday the only ways around it are for an administrator to author a dedicated
PodAccessTemplate, or to hand-write apatchSpecOperationsJSON patch against/spec/containers/N/image. Either way a human is in the loop for what is otherwise a self-service operation.What this does
Adds an optional
spec.imagetoPodAccessRequestand an--imageflag toozctl, so a developer can request a different image against an existing template:$ ozctl create PodAccessRequest deployment-example \ --image registry.example.com/team/debug:v1The template is still required — its
accessConfigis the authorization model (who may request what, for how long). Only the image becomes self-service.Opt-in, controller-scoped allow-list
The image is honored only if it matches an allow-list configured at deploy time via the new
controllerManager.manager.allowedImagePatternsHelm value:The allow-list lives on the controller, not on a
PodAccessTemplate, deliberately: the set of registries an organization will run code from is a cluster-wide security boundary, and letting template authors widen it would defeat the point of having one.The default is
[], which disables the feature entirely — this change is inert until an administrator opts in.Pattern semantics
*matches within a single path segment;**matches across segments. This is a security property, not a stylistic one — under naive "*matches everything" globbing,*.dkr.ecr.*.amazonaws.com/team/*would also match:because the leading wildcard would swallow an untrusted registry host. There's a regression test for exactly this.
Enforcement in two places
kubectl apply/ozctltimePodAccessBuilderValidatingWebhookConfigurationis optional (webhook.createcan disable it)The second check is not redundant. Without it, a cluster running with webhooks disabled would apply an unvalidated image.
Implementation notes
spec.imageis immutable. The Pod is built once, on first reconcile, so a later edit would silently do nothing while leaving the resource claiming to run an image it does not.PatchPodTemplateSpec()already does. A template'spatchSpecOperationsmay rename containers, so resolving afterwards can fail to find a container that existed when the template author named it. (A test in this PR caught this — the existing fixture renames its container tooz.)getDefaultContainerID()is extracted into an exportedGetDefaultContainerID()to share the logic with callers that have noPodTemplateSpecMutationConfig.imagePullSecretsare inherited too, so an image from a registry the target workload cannot pull from will simply fail to start — documented rather than worked around.Testing
internal/imagepolicypackage with table-driven tests covering the prefix-smuggling bypass,*vs**semantics, digests, tag-less refs, over-long input and control characters.go test -raceacross all non-e2e packages,reviveandgolangci-lintall clean;API.md, chart README and CRDs regenerated.Note for reviewers
config/crd/bases/crds.wizardofoz.co_podaccesstemplates.yamlhas pre-existing drift from the k8s.io v0.35.4 bump (#457) thatmake manifestswants to rewrite. I reverted it to keep this diff focused — it's worth a separate regeneration commit.🤖 Generated with Claude Code