Skip to content

Commit cc6761d

Browse files
fix: reject config mounts that collide with the scratch volume (#44)
Rendering ConfigMounts verbatim trusted two things the render has no way to check. Both produced a manifest that passed core's static conformance in all three profiles and failed at apply: - A mount named "tmp", or mounted at /tmp, duplicates the volume name and mount path the template reserves for the scratch emptyDir. Core validates a mount against the other mounts, so it accepts both; only this agent knows what its own template reserves. Shadowing /tmp would also leave the app with no writable storage under readOnlyRootFilesystem, with HOME pointing at it. - An unnormalized mount carries VolumeName "" and ReadOnly nil, which render an unnamed volume and the string "<nil>". DeployKustomize normalizes, but the shared test helper normalizes only the service account (codefly-dev/core#614), so tests rendered a shape production never does. preparePodOverlay normalizes and checks the overlay at the agent boundary, on the path the overlay will arrive on once a producer populates it, and the test helper now goes through it instead of re-implementing normalization. Also drops two assertions that pinned other repos' behavior: the automount violation is matched rather than compared whole, with a message naming codefly-dev/core#602, and the pod labels no longer pin core's fixture version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f98c4ca commit cc6761d

2 files changed

Lines changed: 116 additions & 9 deletions

File tree

‎builder.go‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,49 @@ type Parameters struct {
247247
GRPCEnabled bool
248248
GRPCPort int
249249
// AutomountServiceAccountToken projects the ServiceAccount token into the
250-
// pod. It stays false while the platform's identity path is the metadata
251-
// server or a webhook-projected token and the app never calls the API
252-
// server; a workload that does call it sets this.
250+
// pod. It stays false because the identity path is the metadata server or a
251+
// webhook-projected token and the app does not call the API server. Setting
252+
// it true currently renders a manifest core's own manifest conformance
253+
// rejects (codefly-dev/core#602), so the field is the seam, not yet a
254+
// working switch.
253255
AutomountServiceAccountToken bool
254256
}
255257

258+
// The container runs with readOnlyRootFilesystem, so the template reserves one
259+
// scratch volume and mounts it at the path HOME also points at.
260+
const (
261+
scratchVolumeName = "tmp"
262+
scratchMountPath = "/tmp"
263+
)
264+
265+
// preparePodOverlay normalizes the overlay the way the render depends on and
266+
// rejects a ConfigMount that collides with the scratch mount.
267+
//
268+
// Core validates a mount against the other mounts, but only this agent knows
269+
// the volume name and container path its own template reserves. A colliding
270+
// mount renders a duplicate pod volume name (or a second volumeMount on the
271+
// same path) that core's static conformance accepts and the API server rejects
272+
// at apply — and shadowing the scratch mount would leave the app with no
273+
// writable storage at all.
274+
func preparePodOverlay(overlay *services.PodTemplateOverlay) error {
275+
if overlay == nil {
276+
return nil
277+
}
278+
// The template renders VolumeName and ReadOnly verbatim; unnormalized they
279+
// are empty and nil, which render an unnamed volume and a "<nil>" string
280+
// that survive YAML parsing and static conformance.
281+
overlay.DefaultConfigMounts()
282+
for _, mount := range overlay.ConfigMounts {
283+
if mount.VolumeName == scratchVolumeName {
284+
return fmt.Errorf("config mount %q collides with the reserved scratch volume %q", mount.ConfigMapName, scratchVolumeName)
285+
}
286+
if mount.MountPath == scratchMountPath {
287+
return fmt.Errorf("config mount %q would shadow the scratch mount at %s", mount.ConfigMapName, scratchMountPath)
288+
}
289+
}
290+
return overlay.Validate()
291+
}
292+
256293
// Deploy renders and applies k8s manifests.
257294
func (s *Builder) Deploy(ctx context.Context, req *builderv0.DeploymentRequest) (*builderv0.DeploymentResponse, error) {
258295
defer s.Wool.Catch()
@@ -272,9 +309,18 @@ func (s *Builder) Deploy(ctx context.Context, req *builderv0.DeploymentRequest)
272309
return nil, fmt.Errorf("declared health on additional endpoint %q requires a combined deployment probe; refusing to ignore it", endpoint.GetName())
273310
}
274311
}
312+
// No producer populates the overlay yet — the deployment request carries no
313+
// identity or config-mount payload (codefly-dev/core#594, codefly-dev/cli#757).
314+
// The check sits on the path the overlay will arrive on, so wiring the source
315+
// cannot skip it.
316+
var overlay *services.PodTemplateOverlay
317+
if err = preparePodOverlay(overlay); err != nil {
318+
return nil, err
319+
}
275320
return s.Base.Builder.DeployKustomize(ctx, req, services.KustomizeDeployment{
276321
EnvironmentVariables: s.EnvironmentVariables,
277322
Templates: deploymentFS,
323+
PodOverlay: overlay,
278324
Inputs: services.DeploymentInputs{
279325
OwnConfiguration: true,
280326
DependencyConfigurations: true,

‎deployment_test.go‎

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,13 @@ type serviceAccount struct {
7070
} `yaml:"metadata"`
7171
}
7272

73-
// renderDeployment renders the deployment templates with an overlay and returns
74-
// the workload's pod template, normalized the way DeployKustomize normalizes it
75-
// before rendering.
73+
// renderDeployment renders the deployment templates with an overlay, normalized
74+
// through the same agent entry point Deploy uses. The shared test helper calls
75+
// DefaultServiceAccountName but not DefaultConfigMounts, so a mount rendered
76+
// straight from it carries an empty volume name (codefly-dev/core#614).
7677
func renderDeployment(t *testing.T, parameters Parameters, overlay *services.PodTemplateOverlay) (podSpec, string) {
7778
t.Helper()
78-
overlay.DefaultConfigMounts()
79+
require.NoError(t, preparePodOverlay(overlay))
7980
destination := agenttesting.AssertKustomizeTemplatesWithOverlay(t, deploymentFS, deploymentTestParameters(t, parameters), overlay)
8081

8182
rendered, err := os.ReadFile(filepath.Join(destination, "base", "deployment.yaml"))
@@ -95,7 +96,10 @@ func TestDeploymentWithoutOverlayIsUnchanged(t *testing.T) {
9596
require.Empty(t, pod.Spec.ServiceAccountName)
9697
require.Equal(t, false, *pod.Spec.AutomountServiceAccountToken)
9798
require.Nil(t, pod.Metadata.Annotations)
98-
require.Equal(t, map[string]string{"app": "example-service", "sha": "1.2.3"}, pod.Metadata.Labels)
99+
// The selector label and the image sha, and nothing the overlay would add.
100+
require.Len(t, pod.Metadata.Labels, 2)
101+
require.Equal(t, "example-service", pod.Metadata.Labels["app"])
102+
require.NotEmpty(t, pod.Metadata.Labels["sha"])
99103
// Only the scratch mount: an absent overlay must add no volume.
100104
require.Len(t, pod.Spec.Volumes, 1)
101105
require.Equal(t, "tmp", pod.Spec.Volumes[0].Name)
@@ -214,5 +218,62 @@ func TestDeploymentAutomountServiceAccountTokenRejectsProjection(t *testing.T) {
214218
validation := services.ValidateKubernetesManifestTree(t.Context(), destination, "test", "codefly-test",
215219
builderv0.KubernetesOutputProfile_KUBERNETES_OUTPUT_PROFILE_EPHEMERAL_LOCAL_APPLY_V1, false, "", "")
216220

217-
require.Equal(t, []string{"Deployment/example-service must set automountServiceAccountToken: false"}, validation.GetViolations())
221+
require.Contains(t, validation.GetViolations(),
222+
"Deployment/example-service must set automountServiceAccountToken: false",
223+
"core no longer rejects a projected token: codefly-dev/core#602 is resolved, so "+
224+
"AutomountServiceAccountToken is now usable and this test should assert the render instead")
225+
}
226+
227+
// TestPreparePodOverlayRejectsScratchCollision covers the mounts core's own
228+
// validation accepts: it checks a mount against the other mounts, so a mount
229+
// landing on the volume name or path this template reserves renders a manifest
230+
// that passes static conformance and fails at apply.
231+
func TestPreparePodOverlayRejectsScratchCollision(t *testing.T) {
232+
for _, tc := range []struct {
233+
name string
234+
mount services.ConfigMount
235+
error string
236+
}{
237+
{
238+
name: "volume name",
239+
mount: services.ConfigMount{ConfigMapName: "tmp", MountPath: "/etc/trust"},
240+
error: `collides with the reserved scratch volume "tmp"`,
241+
},
242+
{
243+
name: "mount path",
244+
mount: services.ConfigMount{ConfigMapName: "trust-bundle", MountPath: "/tmp"},
245+
error: "would shadow the scratch mount at /tmp",
246+
},
247+
} {
248+
t.Run(tc.name, func(t *testing.T) {
249+
overlay := &services.PodTemplateOverlay{ConfigMounts: []services.ConfigMount{tc.mount}}
250+
// Core accepts both; only this agent knows what its template reserves.
251+
normalized := *overlay
252+
normalized.DefaultConfigMounts()
253+
require.NoError(t, normalized.Validate())
254+
255+
require.ErrorContains(t, preparePodOverlay(overlay), tc.error)
256+
})
257+
}
258+
}
259+
260+
// TestPreparePodOverlayNormalizesMounts pins the normalization the template
261+
// depends on: rendered verbatim, an unnormalized mount emits an unnamed volume
262+
// and a "<nil>" readOnly string, both of which survive static conformance.
263+
func TestPreparePodOverlayNormalizesMounts(t *testing.T) {
264+
overlay := &services.PodTemplateOverlay{
265+
ConfigMounts: []services.ConfigMount{{ConfigMapName: "trust.bundle", MountPath: "/etc/trust"}},
266+
}
267+
268+
require.NoError(t, preparePodOverlay(overlay))
269+
270+
mount := overlay.ConfigMounts[0]
271+
// Dots are legal in a ConfigMap name and illegal in a volume name.
272+
require.Equal(t, "trust-bundle", mount.VolumeName)
273+
require.NotNil(t, mount.ReadOnly)
274+
require.True(t, *mount.ReadOnly)
275+
}
276+
277+
func TestPreparePodOverlayAcceptsNoOverlay(t *testing.T) {
278+
require.NoError(t, preparePodOverlay(nil))
218279
}

0 commit comments

Comments
 (0)