feat(k8s): per-container VPA policies (exclude sidecars from the app's minAllowed floor)

pkg/clouds/k8s/kube_run.go

@@ -133,8 +133,8 @@ type VPAConfig struct {
 	// MaxAllowed specifies maximum allowed resources
 	MaxAllowed *VPAResourceRequirements `json:"maxAllowed" yaml:"maxAllowed"`
 	// ControlledResources specifies which resources VPA should control.
-	// Per the VPA CRD this is a per-container field; SC places it inside each
-	// containerPolicy entry, not at resourcePolicy level.
+	// Per the VPA CRD this is a per-container field; SC places it inside the
+	// catch-all "*" containerPolicy entry, not at resourcePolicy level.
 	ControlledResources []string `json:"controlledResources" yaml:"controlledResources"`
 	// ControlledValues specifies which resource values VPA should control.
 	// One of "RequestsAndLimits" (default) or "RequestsOnly". Use "RequestsOnly"
@@ -143,6 +143,9 @@ type VPAConfig struct {
 	// proportionally with a lowered request — the proportional shrink causes
 	// CPU-throttle-induced startup probe failures.
 	ControlledValues *string `json:"controlledValues" yaml:"controlledValues"`
+	// ContainerPolicies are per-container overrides; the top-level fields render
+	// the catch-all "*". Common use: a sidecar with mode "Off" to exclude it.
+	ContainerPolicies []VPAContainerPolicy `json:"containerPolicies" yaml:"containerPolicies"`
 }
 
 // VPAResourceRequirements defines resource requirements for VPA
@@ -152,6 +155,17 @@ type VPAResourceRequirements struct {
 	EphemeralStorage *string `json:"ephemeral-storage" yaml:"ephemeral-storage"`
 }
 
+// VPAContainerPolicy mirrors a VPA CRD containerPolicies entry; mode "Off"
+// excludes the container from VPA.
+type VPAContainerPolicy struct {
+	ContainerName       string                   `json:"containerName" yaml:"containerName"`
+	Mode                *string                  `json:"mode" yaml:"mode"`
+	MinAllowed          *VPAResourceRequirements `json:"minAllowed" yaml:"minAllowed"`
+	MaxAllowed          *VPAResourceRequirements `json:"maxAllowed" yaml:"maxAllowed"`
+	ControlledResources []string                 `json:"controlledResources" yaml:"controlledResources"`
+	ControlledValues    *string                  `json:"controlledValues" yaml:"controlledValues"`
+}
+
 func (i *KubeRunInput) DependsOnResources() []api.StackConfigDependencyResource {
 	return i.Deployment.StackConfig.Dependencies
 }

pkg/clouds/pulumi/kubernetes/simple_container.go

@@ -1029,9 +1029,41 @@ ${proto}://${domain} {
 	return sc, nil
 }
 
+// ValidateVPAConfiguration rejects per-container policies the VPA CRD would
+// reject at apply time, so a bad config fails at plan time instead.
+func ValidateVPAConfiguration(vpa *k8s.VPAConfig) error {
+	if vpa == nil || !vpa.Enabled {
+		return nil
+	}
+	seen := make(map[string]bool, len(vpa.ContainerPolicies))
+	for i, cp := range vpa.ContainerPolicies {
+		switch {
+		case cp.ContainerName == "":
+			return errors.Errorf("containerPolicies[%d]: containerName must not be empty", i)
+		case cp.ContainerName == "*":
+			return errors.Errorf("containerPolicies[%d]: %q is reserved; set the catch-all via the top-level fields", i, "*")
+		case seen[cp.ContainerName]:
+			return errors.Errorf("containerPolicies: duplicate containerName %q", cp.ContainerName)
+		}
+		seen[cp.ContainerName] = true
+		if cp.Mode != nil && *cp.Mode != "Off" && *cp.Mode != "Auto" {
+			return errors.Errorf("containerPolicies[%q]: mode must be \"Off\" or \"Auto\", got %q", cp.ContainerName, *cp.Mode)
+		}
+	}
+	return nil
+}
+
 func createVPA(ctx *sdk.Context, args *SimpleContainerArgs, deploymentName string, namespace sdk.StringInput, labels, annotations map[string]string, opts ...sdk.ResourceOption) error {
 	vpaName := fmt.Sprintf("%s-vpa", deploymentName)
 
+	if err := ValidateVPAConfiguration(args.VPA); err != nil {
+		return errors.Wrapf(err, "invalid VPA configuration for %q", deploymentName)
+	}
+	if len(args.VPA.ContainerPolicies) > 0 && args.VPA.MinAllowed == nil && args.VPA.MaxAllowed == nil &&
+		len(args.VPA.ControlledResources) == 0 && args.VPA.ControlledValues == nil {
+		args.Log.Warn(ctx.Context(), "VPA for %q has containerPolicies but no top-level floor; unlisted containers autoscale without a minAllowed", deploymentName)
+	}
+
 	// Build VPA spec content
 	vpaSpec := map[string]interface{}{
 		"targetRef": map[string]interface{}{
@@ -1048,61 +1080,72 @@ func createVPA(ctx *sdk.Context, args *SimpleContainerArgs, deploymentName strin
 		}
 	}
 
-	// Add resource policy if specified
-	if args.VPA.MinAllowed != nil || args.VPA.MaxAllowed != nil || len(args.VPA.ControlledResources) > 0 || args.VPA.ControlledValues != nil {
-		resourcePolicy := map[string]interface{}{}
-
-		// Build the container policy. Per the VPA CRD, controlledResources and
-		// controlledValues are per-container fields and live inside the
-		// containerPolicy entry — not at the resourcePolicy level. Placing them
-		// at resourcePolicy level (the previous behavior) caused k8s to silently
-		// drop them.
-		containerPolicy := map[string]interface{}{
-			"containerName": "*",
-		}
-
-		if len(args.VPA.ControlledResources) > 0 {
-			containerPolicy["controlledResources"] = args.VPA.ControlledResources
+	// Top-level fields render the catch-all "*"; ContainerPolicies append
+	// per-container entries (an exact containerName wins over "*").
+	hasTopLevel := args.VPA.MinAllowed != nil || args.VPA.MaxAllowed != nil || len(args.VPA.ControlledResources) > 0 || args.VPA.ControlledValues != nil
+	if hasTopLevel || len(args.VPA.ContainerPolicies) > 0 {
+		resMap := func(r *k8s.VPAResourceRequirements) map[string]interface{} {
+			if r == nil {
+				return nil
+			}
+			m := map[string]interface{}{}
+			if r.CPU != nil {
+				m["cpu"] = lo.FromPtr(r.CPU)
+			}
+			if r.Memory != nil {
+				m["memory"] = lo.FromPtr(r.Memory)
+			}
+			if r.EphemeralStorage != nil {
+				m["ephemeral-storage"] = lo.FromPtr(r.EphemeralStorage)
+			}
+			if len(m) == 0 {
+				return nil
+			}
+			return m
 		}
 
-		if args.VPA.ControlledValues != nil {
-			containerPolicy["controlledValues"] = lo.FromPtr(args.VPA.ControlledValues)
-		}
+		containerPolicies := make([]interface{}, 0, 1+len(args.VPA.ContainerPolicies))
 
-		if args.VPA.MinAllowed != nil {
-			minAllowed := map[string]interface{}{}
-			if args.VPA.MinAllowed.CPU != nil {
-				minAllowed["cpu"] = lo.FromPtr(args.VPA.MinAllowed.CPU)
+		if hasTopLevel {
+			star := map[string]interface{}{"containerName": "*"}
+			if len(args.VPA.ControlledResources) > 0 {
+				star["controlledResources"] = args.VPA.ControlledResources
 			}
-			if args.VPA.MinAllowed.Memory != nil {
-				minAllowed["memory"] = lo.FromPtr(args.VPA.MinAllowed.Memory)
+			if args.VPA.ControlledValues != nil {
+				star["controlledValues"] = lo.FromPtr(args.VPA.ControlledValues)
 			}
-			if args.VPA.MinAllowed.EphemeralStorage != nil {
-				minAllowed["ephemeral-storage"] = lo.FromPtr(args.VPA.MinAllowed.EphemeralStorage)
+			if m := resMap(args.VPA.MinAllowed); m != nil {
+				star["minAllowed"] = m
 			}
-			if len(minAllowed) > 0 {
-				containerPolicy["minAllowed"] = minAllowed
+			if m := resMap(args.VPA.MaxAllowed); m != nil {
+				star["maxAllowed"] = m
 			}
+			containerPolicies = append(containerPolicies, star)
 		}
 
-		if args.VPA.MaxAllowed != nil {
-			maxAllowed := map[string]interface{}{}
-			if args.VPA.MaxAllowed.CPU != nil {
-				maxAllowed["cpu"] = lo.FromPtr(args.VPA.MaxAllowed.CPU)
+		for _, cp := range args.VPA.ContainerPolicies {
+			policy := map[string]interface{}{"containerName": cp.ContainerName}
+			if cp.Mode != nil {
+				policy["mode"] = lo.FromPtr(cp.Mode)
 			}
-			if args.VPA.MaxAllowed.Memory != nil {
-				maxAllowed["memory"] = lo.FromPtr(args.VPA.MaxAllowed.Memory)
+			if len(cp.ControlledResources) > 0 {
+				policy["controlledResources"] = cp.ControlledResources
 			}
-			if args.VPA.MaxAllowed.EphemeralStorage != nil {
-				maxAllowed["ephemeral-storage"] = lo.FromPtr(args.VPA.MaxAllowed.EphemeralStorage)
+			if cp.ControlledValues != nil {
+				policy["controlledValues"] = lo.FromPtr(cp.ControlledValues)
 			}
-			if len(maxAllowed) > 0 {
-				containerPolicy["maxAllowed"] = maxAllowed
+			if m := resMap(cp.MinAllowed); m != nil {
+				policy["minAllowed"] = m
 			}
+			if m := resMap(cp.MaxAllowed); m != nil {
+				policy["maxAllowed"] = m
+			}
+			containerPolicies = append(containerPolicies, policy)
 		}
 
-		resourcePolicy["containerPolicies"] = []interface{}{containerPolicy}
-		vpaSpec["resourcePolicy"] = resourcePolicy
+		vpaSpec["resourcePolicy"] = map[string]interface{}{
+			"containerPolicies": containerPolicies,
+		}
 	}
 
 	// Build the complete VPA resource with proper spec nesting

pkg/clouds/pulumi/kubernetes/vpa_container_policies_test.go

@@ -0,0 +1,177 @@
+package kubernetes
+
+import (
+	"testing"
+
+	"github.com/samber/lo"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	corev1 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/core/v1"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
+	sdk "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
+
+	"github.com/simple-container-com/api/pkg/api/logger"
+	"github.com/simple-container-com/api/pkg/clouds/k8s"
+)
+
+// renderVPAContainerPolicies runs NewSimpleContainer with the given VPA config
+// and returns the captured spec.resourcePolicy.containerPolicies array.
+func renderVPAContainerPolicies(t *testing.T, vpa *k8s.VPAConfig) []resource.PropertyValue {
+	t.Helper()
+	mocks := NewSimpleContainerMocks()
+	err := sdk.RunErr(func(ctx *sdk.Context) error {
+		args := &SimpleContainerArgs{
+			Namespace:  "myapp",
+			Service:    "myapp",
+			Deployment: "myapp",
+			ScEnv:      "production",
+			Domain:     "myapp.example.com",
+			Prefix:     "/",
+			Replicas:   2,
+			IngressContainer: &k8s.CloudRunContainer{
+				Name: "myapp", MainPort: lo.ToPtr(8080), Ports: []int{8080},
+			},
+			Log: logger.New(),
+			Containers: []corev1.ContainerArgs{
+				{Name: sdk.String("myapp"), Image: sdk.String("nginx:latest")},
+				{Name: sdk.String("cloudsql-proxy"), Image: sdk.String("cloud-sql-proxy:latest")},
+			},
+			VPA: vpa,
+		}
+		_, err := NewSimpleContainer(ctx, args)
+		return err
+	}, sdk.WithMocks("test", "test", mocks))
+	require.NoError(t, err)
+
+	mocks.mu.RLock()
+	defer mocks.mu.RUnlock()
+	for _, props := range mocks.createdResources {
+		kind, ok := props["kind"]
+		if !ok || !kind.IsString() || kind.StringValue() != "VerticalPodAutoscaler" {
+			continue
+		}
+		require.True(t, props["spec"].IsObject(), "VPA spec should be an object")
+		rp := props["spec"].ObjectValue()["resourcePolicy"]
+		require.True(t, rp.IsObject(), "VPA spec.resourcePolicy should be an object")
+		cps := rp.ObjectValue()["containerPolicies"]
+		require.True(t, cps.IsArray(), "resourcePolicy.containerPolicies should be an array")
+		return cps.ArrayValue()
+	}
+	t.Fatal("VPA should have been created and captured by the mock")
+	return nil
+}
+
+// VPA.ContainerPolicies must render per-container entries alongside the "*"
+// catch-all. The motivating case: an injected sidecar (cloudsql-proxy) set to
+// mode "Off" so it keeps its small template request instead of being floored at
+// the app container's minAllowed (the "*" policy).
+func TestCreateVPA_PerContainerPolicies(t *testing.T) {
+	arr := renderVPAContainerPolicies(t, &k8s.VPAConfig{
+		Enabled:             true,
+		UpdateMode:          lo.ToPtr("Auto"),
+		MinAllowed:          &k8s.VPAResourceRequirements{CPU: lo.ToPtr("250m"), Memory: lo.ToPtr("64Mi")},
+		ControlledResources: []string{"cpu", "memory"},
+		ControlledValues:    lo.ToPtr("RequestsOnly"),
+		ContainerPolicies: []k8s.VPAContainerPolicy{
+			{ContainerName: "cloudsql-proxy", Mode: lo.ToPtr("Off")},
+		},
+	})
+	require.Len(t, arr, 2, "expected exactly the '*' catch-all + the cloudsql-proxy override")
+
+	sawStar, sawProxy := false, false
+	for _, e := range arr {
+		require.True(t, e.IsObject())
+		o := e.ObjectValue()
+		require.True(t, o["containerName"].IsString())
+		switch o["containerName"].StringValue() {
+		case "*":
+			sawStar = true
+			assert.Equal(t, "RequestsOnly", o["controlledValues"].StringValue())
+			require.True(t, o["minAllowed"].IsObject())
+			assert.Equal(t, "250m", o["minAllowed"].ObjectValue()["cpu"].StringValue())
+		case "cloudsql-proxy":
+			sawProxy = true
+			require.True(t, o["mode"].IsString())
+			assert.Equal(t, "Off", o["mode"].StringValue(), "sidecar must be excluded from VPA via mode Off")
+			_, hasMin := o["minAllowed"]
+			assert.False(t, hasMin, "sidecar policy must not inherit the app minAllowed floor")
+		default:
+			t.Fatalf("unexpected containerPolicy entry: %s", o["containerName"].StringValue())
+		}
+	}
+	assert.True(t, sawStar, "expected a '*' catch-all containerPolicy")
+	assert.True(t, sawProxy, "expected a per-container policy for cloudsql-proxy")
+}
+
+// Backward compat: with no ContainerPolicies, the render is the historical
+// single "*" entry carrying the top-level floor/controlled* fields. Locks the
+// guarantee that existing stacks re-render byte-identical (no Pulumi diff).
+func TestCreateVPA_NoContainerPolicies_BackwardCompat(t *testing.T) {
+	arr := renderVPAContainerPolicies(t, &k8s.VPAConfig{
+		Enabled:             true,
+		UpdateMode:          lo.ToPtr("Auto"),
+		MinAllowed:          &k8s.VPAResourceRequirements{CPU: lo.ToPtr("250m"), Memory: lo.ToPtr("64Mi")},
+		MaxAllowed:          &k8s.VPAResourceRequirements{CPU: lo.ToPtr("2"), Memory: lo.ToPtr("2560Mi")},
+		ControlledResources: []string{"cpu", "memory"},
+		ControlledValues:    lo.ToPtr("RequestsOnly"),
+	})
+	require.Len(t, arr, 1, "no ContainerPolicies must render exactly one '*' entry")
+	o := arr[0].ObjectValue()
+	assert.Equal(t, "*", o["containerName"].StringValue())
+	assert.Equal(t, "RequestsOnly", o["controlledValues"].StringValue())
+	require.True(t, o["controlledResources"].IsArray())
+	assert.Len(t, o["controlledResources"].ArrayValue(), 2)
+	assert.Equal(t, "250m", o["minAllowed"].ObjectValue()["cpu"].StringValue())
+	assert.Equal(t, "2560Mi", o["maxAllowed"].ObjectValue()["memory"].StringValue())
+}
+
+// ContainerPolicies with no top-level fields renders ONLY the per-container
+// entries — no "*" catch-all (hasTopLevel == false branch). Also exercises a
+// per-container minAllowed override (not just mode Off).
+func TestCreateVPA_ContainerPoliciesOnly_NoStar(t *testing.T) {
+	arr := renderVPAContainerPolicies(t, &k8s.VPAConfig{
+		Enabled:    true,
+		UpdateMode: lo.ToPtr("Auto"),
+		ContainerPolicies: []k8s.VPAContainerPolicy{
+			{ContainerName: "cloudsql-proxy", Mode: lo.ToPtr("Off")},
+			{ContainerName: "myapp", MinAllowed: &k8s.VPAResourceRequirements{CPU: lo.ToPtr("100m")}},
+		},
+	})
+	require.Len(t, arr, 2, "containerPolicies-only must render exactly the listed entries, no '*'")
+	for _, e := range arr {
+		o := e.ObjectValue()
+		assert.NotEqual(t, "*", o["containerName"].StringValue(), "no '*' catch-all when no top-level fields are set")
+		if o["containerName"].StringValue() == "myapp" {
+			require.True(t, o["minAllowed"].IsObject())
+			assert.Equal(t, "100m", o["minAllowed"].ObjectValue()["cpu"].StringValue())
+		}
+	}
+}
+
+func TestValidateVPAConfiguration(t *testing.T) {
+	cases := []struct {
+		name    string
+		vpa     *k8s.VPAConfig
+		wantErr string
+	}{
+		{"nil", nil, ""},
+		{"disabled", &k8s.VPAConfig{Enabled: false, ContainerPolicies: []k8s.VPAContainerPolicy{{ContainerName: ""}}}, ""},
+		{"valid", &k8s.VPAConfig{Enabled: true, ContainerPolicies: []k8s.VPAContainerPolicy{{ContainerName: "cloudsql-proxy", Mode: lo.ToPtr("Off")}}}, ""},
+		{"empty name", &k8s.VPAConfig{Enabled: true, ContainerPolicies: []k8s.VPAContainerPolicy{{ContainerName: ""}}}, "must not be empty"},
+		{"star reserved", &k8s.VPAConfig{Enabled: true, ContainerPolicies: []k8s.VPAContainerPolicy{{ContainerName: "*"}}}, "reserved"},
+		{"duplicate", &k8s.VPAConfig{Enabled: true, ContainerPolicies: []k8s.VPAContainerPolicy{{ContainerName: "x"}, {ContainerName: "x"}}}, "duplicate"},
+		{"bad mode", &k8s.VPAConfig{Enabled: true, ContainerPolicies: []k8s.VPAContainerPolicy{{ContainerName: "x", Mode: lo.ToPtr("off")}}}, "mode must be"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := ValidateVPAConfiguration(tc.vpa)
+			if tc.wantErr == "" {
+				assert.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tc.wantErr)
+			}
+		})
+	}
+}