From add651e361d5be5c0c289f07662209becf887c90 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Wed, 10 Jun 2026 13:17:31 +0400 Subject: [PATCH 1/4] fix(security+k8s): retry cosign sign on Rekor 409 conflict; honor cloudExtras startupProbe + periodSeconds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two production-surfaced fixes: 1) Image signing: parallel CI deploys of stacks sharing one image can fail with Rekor "[POST /api/v1/log/entries][409] createLogEntryConflict" — an identical entry already exists, typically because cosign re-uploaded an entry whose first attempt timed out client-side but succeeded server-side. The signature is valid and present, yet the deploy aborts. Retry the full cosign invocation (fresh ephemeral / randomized-ECDSA signature cannot conflict with itself) up to 3 attempts; any non-conflict error still fails fast. Applied to both keyless and key-based signers via a shared helper. 2) Probes: cloudExtras.startupProbe was silently dropped — CloudExtras and DeploymentConfig had no StartupProbe field, so slow-booting services were stuck with the readiness probe (or its 60s default window) as their cold- start budget and got SIGTERM-killed mid-boot on fresh nodes. Add the field and thread it through gke_autopilot -> DeploymentConfig -> DeploymentArgs -> container spec with the same ingress-container fallback semantics as readiness/liveness. Also add the k8s-native periodSeconds spelling to CloudRunProbe (precedence over the legacy duration-typed interval, which does not survive YAML conversion of plain integers). Tests: Rekor-conflict detection + retry/no-retry/give-up behavior via an exec seam; ConvertDescriptor regression tests proving startupProbe/periodSeconds now survive the yaml mapping; toProbeArgs periodSeconds precedence table. Signed-off-by: Dmitrii Creed --- pkg/clouds/gcloud/gke_autopilot.go | 1 + pkg/clouds/k8s/kube_run.go | 1 + pkg/clouds/k8s/kube_run_probe_test.go | 73 +++++++++++ pkg/clouds/k8s/types.go | 2 + pkg/clouds/pulumi/kubernetes/deployment.go | 22 +++- .../pulumi/kubernetes/deployment_test.go | 62 ++++++++++ pkg/clouds/pulumi/kubernetes/kube_run.go | 1 + pkg/security/signing/keybased.go | 5 +- pkg/security/signing/keyless.go | 40 +++++- pkg/security/signing/keyless_test.go | 117 ++++++++++++++++++ 10 files changed, 315 insertions(+), 9 deletions(-) create mode 100644 pkg/clouds/k8s/kube_run_probe_test.go diff --git a/pkg/clouds/gcloud/gke_autopilot.go b/pkg/clouds/gcloud/gke_autopilot.go index dbf83e85..82211d03 100644 --- a/pkg/clouds/gcloud/gke_autopilot.go +++ b/pkg/clouds/gcloud/gke_autopilot.go @@ -107,6 +107,7 @@ func ToGkeAutopilotConfig(tpl any, composeCfg compose.Config, stackCfg *api.Stac deployCfg.VPA = k8sCloudExtras.VPA // Extract VPA configuration from CloudExtras deployCfg.ReadinessProbe = k8sCloudExtras.ReadinessProbe // Extract global readiness probe configuration deployCfg.LivenessProbe = k8sCloudExtras.LivenessProbe // Extract global liveness probe configuration + deployCfg.StartupProbe = k8sCloudExtras.StartupProbe // Extract global startup probe configuration deployCfg.PriorityClassName = k8sCloudExtras.PriorityClassName // Extract PriorityClass for pod scheduling and preemption // Process affinity rules and merge with existing NodeSelector if needed diff --git a/pkg/clouds/k8s/kube_run.go b/pkg/clouds/k8s/kube_run.go index 84eb3b4c..e0fc94b6 100644 --- a/pkg/clouds/k8s/kube_run.go +++ b/pkg/clouds/k8s/kube_run.go @@ -25,6 +25,7 @@ type CloudExtras struct { VPA *VPAConfig `json:"vpa" yaml:"vpa"` ReadinessProbe *CloudRunProbe `json:"readinessProbe" yaml:"readinessProbe"` LivenessProbe *CloudRunProbe `json:"livenessProbe" yaml:"livenessProbe"` + StartupProbe *CloudRunProbe `json:"startupProbe" yaml:"startupProbe"` EphemeralVolumes []GenericEphemeralVolume `json:"ephemeralVolumes" yaml:"ephemeralVolumes"` // Generic ephemeral volumes for large temp storage PriorityClassName *string `json:"priorityClassName" yaml:"priorityClassName"` // Kubernetes PriorityClass for pod scheduling and preemption } diff --git a/pkg/clouds/k8s/kube_run_probe_test.go b/pkg/clouds/k8s/kube_run_probe_test.go new file mode 100644 index 00000000..2cf89f6e --- /dev/null +++ b/pkg/clouds/k8s/kube_run_probe_test.go @@ -0,0 +1,73 @@ +package k8s + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/simple-container-com/api/pkg/api" +) + +// TestStartupProbeExtraction is the regression test for cloudExtras.startupProbe +// being silently dropped: CloudExtras had no StartupProbe field, so user-supplied +// startup budgets never reached the cluster and pods fell back to the readiness +// probe's (short) window during cold starts. +func TestStartupProbeExtraction(t *testing.T) { + cloudExtras := map[string]any{ + "startupProbe": map[string]any{ + "httpGet": map[string]any{ + "path": "/health/", + "port": 8000, + }, + "initialDelaySeconds": 10, + "timeoutSeconds": 5, + "periodSeconds": 15, + "failureThreshold": 16, + "successThreshold": 1, + }, + } + + result, err := api.ConvertDescriptor(cloudExtras, &CloudExtras{}) + + require.NoError(t, err) + require.NotNil(t, result.StartupProbe) + assert.Equal(t, "/health/", result.StartupProbe.HttpGet.Path) + assert.Equal(t, 8000, result.StartupProbe.HttpGet.Port) + assert.Equal(t, 10, *result.StartupProbe.InitialDelaySeconds) + assert.Equal(t, 5, *result.StartupProbe.TimeoutSeconds) + assert.Equal(t, 15, *result.StartupProbe.PeriodSeconds) + assert.Equal(t, 16, *result.StartupProbe.FailureThreshold) + assert.Equal(t, 1, *result.StartupProbe.SuccessThreshold) +} + +// TestProbePeriodSecondsExtraction covers the k8s-native periodSeconds spelling on +// the readiness probe. Previously only the duration-typed `interval` key existed, +// so configs written with periodSeconds silently fell back to the kubelet default. +func TestProbePeriodSecondsExtraction(t *testing.T) { + cloudExtras := map[string]any{ + "readinessProbe": map[string]any{ + "httpGet": map[string]any{ + "path": "/ready/", + "port": 8080, + }, + "periodSeconds": 20, + "failureThreshold": 3, + }, + } + + result, err := api.ConvertDescriptor(cloudExtras, &CloudExtras{}) + + require.NoError(t, err) + require.NotNil(t, result.ReadinessProbe) + assert.Equal(t, 20, *result.ReadinessProbe.PeriodSeconds) + assert.Equal(t, 3, *result.ReadinessProbe.FailureThreshold) +} + +// TestStartupProbeAbsent ensures the zero-value path stays nil. +func TestStartupProbeAbsent(t *testing.T) { + result, err := api.ConvertDescriptor(map[string]any{}, &CloudExtras{}) + + require.NoError(t, err) + assert.Nil(t, result.StartupProbe) +} diff --git a/pkg/clouds/k8s/types.go b/pkg/clouds/k8s/types.go index 055feab0..8ded8258 100644 --- a/pkg/clouds/k8s/types.go +++ b/pkg/clouds/k8s/types.go @@ -30,6 +30,7 @@ type DeploymentConfig struct { VPA *VPAConfig `json:"vpa" yaml:"vpa"` // Vertical Pod Autoscaler configuration ReadinessProbe *CloudRunProbe `json:"readinessProbe" yaml:"readinessProbe"` // Global readiness probe configuration LivenessProbe *CloudRunProbe `json:"livenessProbe" yaml:"livenessProbe"` // Global liveness probe configuration + StartupProbe *CloudRunProbe `json:"startupProbe" yaml:"startupProbe"` // Global startup probe configuration EphemeralVolumes []GenericEphemeralVolume `json:"ephemeralVolumes" yaml:"ephemeralVolumes"` // Generic ephemeral volumes for large temp storage PriorityClassName *string `json:"priorityClassName" yaml:"priorityClassName"` // Kubernetes PriorityClass for pod scheduling and preemption } @@ -134,6 +135,7 @@ type CloudRunProbe struct { Interval *time.Duration `json:"interval" yaml:"interval"` InitialDelaySeconds *int `json:"initialDelaySeconds" yaml:"initialDelaySeconds"` IntervaSeconds *int `json:"intervaSeconds" yaml:"intervaSeconds"` + PeriodSeconds *int `json:"periodSeconds" yaml:"periodSeconds"` // k8s-native spelling; takes precedence over Interval FailureThreshold *int `json:"failureThreshold" yaml:"failureThreshold"` SuccessThreshold *int `json:"successThreshold" yaml:"successThreshold"` TimeoutSeconds *int `json:"timeoutSeconds" yaml:"timeoutSeconds"` diff --git a/pkg/clouds/pulumi/kubernetes/deployment.go b/pkg/clouds/pulumi/kubernetes/deployment.go index 6d1d0487..62f6b50f 100644 --- a/pkg/clouds/pulumi/kubernetes/deployment.go +++ b/pkg/clouds/pulumi/kubernetes/deployment.go @@ -46,6 +46,7 @@ type Args struct { VPA *k8s.VPAConfig // Vertical Pod Autoscaler configuration ReadinessProbe *k8s.CloudRunProbe // Global readiness probe configuration LivenessProbe *k8s.CloudRunProbe // Global liveness probe configuration + StartupProbe *k8s.CloudRunProbe // Global startup probe configuration EphemeralSize string // TerminationGracePeriodSeconds overrides pod-level terminationGracePeriodSeconds. TerminationGracePeriodSeconds *int @@ -177,10 +178,17 @@ func DeploySimpleContainer(ctx *sdk.Context, args Args, opts ...sdk.ResourceOpti } var startupProbe *corev1.ProbeArgs - if c.Container.StartupProbe == nil && (len(c.Container.Ports) == 1 || c.Container.MainPort != nil) { + cStartupProbe := c.Container.StartupProbe + // Use global startup probe if container doesn't have one AND it's the ingress container, + // mirroring the readiness/liveness fallback above. Without this, a slow-booting service + // is stuck with the readiness probe's (short) window as its startup budget. + if cStartupProbe == nil && args.StartupProbe != nil && isIngressContainer { + cStartupProbe = args.StartupProbe + } + if cStartupProbe == nil && (len(c.Container.Ports) == 1 || c.Container.MainPort != nil) { startupProbe = readinessProbe - } else if c.Container.StartupProbe != nil && (len(c.Container.Ports) == 1 || c.Container.MainPort != nil) { - startupProbe = toProbeArgs(c, c.Container.StartupProbe) + } else if cStartupProbe != nil && (len(c.Container.Ports) == 1 || c.Container.MainPort != nil) { + startupProbe = toProbeArgs(c, cStartupProbe) } var resources corev1.ResourceRequirementsArgs @@ -325,8 +333,14 @@ func toProbeArgs(c *ContainerImage, probe *k8s.CloudRunProbe) *corev1.ProbeArgs probePort = c.Container.Ports[0] } + // periodSeconds (k8s-native spelling) wins over the legacy duration-typed interval; + // duration values only survive YAML conversion when given as Go duration strings. + periodSeconds := probe.PeriodSeconds + if periodSeconds == nil && probe.Interval != nil { + periodSeconds = lo.ToPtr(int(lo.FromPtr(probe.Interval).Seconds())) + } probeArgs := &corev1.ProbeArgs{ - PeriodSeconds: sdk.IntPtrFromPtr(lo.If(probe.Interval != nil, lo.ToPtr(int(lo.FromPtr(probe.Interval).Seconds()))).Else(nil)), + PeriodSeconds: sdk.IntPtrFromPtr(periodSeconds), InitialDelaySeconds: sdk.IntPtrFromPtr(probe.InitialDelaySeconds), FailureThreshold: sdk.IntPtrFromPtr(probe.FailureThreshold), SuccessThreshold: sdk.IntPtrFromPtr(probe.SuccessThreshold), diff --git a/pkg/clouds/pulumi/kubernetes/deployment_test.go b/pkg/clouds/pulumi/kubernetes/deployment_test.go index 4238fdef..a7146464 100644 --- a/pkg/clouds/pulumi/kubernetes/deployment_test.go +++ b/pkg/clouds/pulumi/kubernetes/deployment_test.go @@ -5,6 +5,7 @@ import ( "time" . "github.com/onsi/gomega" + sdk "github.com/pulumi/pulumi/sdk/v3/go/pulumi" "github.com/samber/lo" "github.com/simple-container-com/api/pkg/clouds/k8s" @@ -326,3 +327,64 @@ func TestToProbeArgs_HeaderPreservation(t *testing.T) { Expect(result).ToNot(BeNil(), "ProbeArgs should not be nil") Expect(result.HttpGet).ToNot(BeNil(), "Should have HTTP GET configured") } + +// TestToProbeArgs_PeriodSeconds verifies the precedence between the k8s-native +// periodSeconds field and the legacy duration-typed interval field. +func TestToProbeArgs_PeriodSeconds(t *testing.T) { + container := &ContainerImage{ + Container: k8s.CloudRunContainer{ + Name: "test-container", + Ports: []int{8080}, + MainPort: lo.ToPtr(8080), + }, + } + + testCases := []struct { + name string + probe *k8s.CloudRunProbe + expected sdk.IntPtrInput + }{ + { + name: "periodSeconds used directly", + probe: &k8s.CloudRunProbe{ + PeriodSeconds: lo.ToPtr(15), + }, + expected: sdk.IntPtr(15), + }, + { + name: "interval converted to seconds when periodSeconds absent", + probe: &k8s.CloudRunProbe{ + Interval: lo.ToPtr(20 * time.Second), + }, + expected: sdk.IntPtr(20), + }, + { + name: "periodSeconds wins over interval", + probe: &k8s.CloudRunProbe{ + PeriodSeconds: lo.ToPtr(15), + Interval: lo.ToPtr(99 * time.Second), + }, + expected: sdk.IntPtr(15), + }, + { + name: "neither set leaves kubelet default", + probe: &k8s.CloudRunProbe{}, + expected: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + + result := toProbeArgs(container, tc.probe) + + Expect(result).ToNot(BeNil()) + if tc.expected == nil { + Expect(result.PeriodSeconds).To(BeNil()) + } else { + Expect(result.PeriodSeconds).To(Equal(tc.expected)) + } + }) + } +} diff --git a/pkg/clouds/pulumi/kubernetes/kube_run.go b/pkg/clouds/pulumi/kubernetes/kube_run.go index 81e00ffc..bf7faca8 100644 --- a/pkg/clouds/pulumi/kubernetes/kube_run.go +++ b/pkg/clouds/pulumi/kubernetes/kube_run.go @@ -145,6 +145,7 @@ func KubeRun(ctx *sdk.Context, stack api.Stack, input api.ResourceInput, params VPA: kubeRunInput.Deployment.VPA, // Pass VPA configuration from DeploymentConfig ReadinessProbe: kubeRunInput.Deployment.ReadinessProbe, // Pass global readiness probe configuration LivenessProbe: kubeRunInput.Deployment.LivenessProbe, // Pass global liveness probe configuration + StartupProbe: kubeRunInput.Deployment.StartupProbe, // Pass global startup probe configuration EphemeralSize: lo.FromPtr(kubeRunInput.Deployment.StackConfig).Size.Ephemeral, } diff --git a/pkg/security/signing/keybased.go b/pkg/security/signing/keybased.go index 9f125494..631e795a 100644 --- a/pkg/security/signing/keybased.go +++ b/pkg/security/signing/keybased.go @@ -73,9 +73,8 @@ func (s *KeyBasedSigner) Sign(ctx context.Context, imageRef string) (*SignResult // Execute cosign sign command args := []string{"sign", "--key", keyPath, imageRef} - stdout, stderr, err := tools.ExecCommand(ctx, "cosign", args, env, s.Timeout) - if err != nil { - return nil, fmt.Errorf("cosign sign failed: %w\nStderr: %s\nStdout: %s", err, stderr, stdout) + if _, err := runCosignSign(ctx, args, env, s.Timeout); err != nil { + return nil, err } result := &SignResult{ diff --git a/pkg/security/signing/keyless.go b/pkg/security/signing/keyless.go index 474dc6fa..10363e84 100644 --- a/pkg/security/signing/keyless.go +++ b/pkg/security/signing/keyless.go @@ -10,6 +10,42 @@ import ( "github.com/simple-container-com/api/pkg/security/tools" ) +// execCommand is a seam for tests; production code always uses tools.ExecCommand. +var execCommand = tools.ExecCommand + +// maxSignAttempts bounds the Rekor-conflict retry loop in runCosignSign. +const maxSignAttempts = 3 + +// isRekorConflict reports whether a cosign failure was caused by Rekor's +// createLogEntryConflict (HTTP 409 on POST /api/v1/log/entries). This happens +// when an identical entry is already in the transparency log — typically +// cosign re-uploading an entry whose first attempt timed out client-side but +// succeeded server-side (seen under parallel CI runners signing concurrently). +func isRekorConflict(output string) bool { + return strings.Contains(output, "createLogEntryConflict") || + (strings.Contains(output, "409") && strings.Contains(output, "/api/v1/log/entries")) +} + +// runCosignSign executes `cosign sign`, retrying the full invocation when the +// only failure is a Rekor entry conflict. A fresh invocation mints a fresh +// (ephemeral or randomized-ECDSA) signature, so a retry cannot conflict with +// itself; any other error is returned immediately. +func runCosignSign(ctx context.Context, args, env []string, timeout time.Duration) (string, error) { + var lastErr error + for attempt := 1; attempt <= maxSignAttempts; attempt++ { + stdout, stderr, err := execCommand(ctx, "cosign", args, env, timeout) + if err == nil { + return stdout, nil + } + lastErr = fmt.Errorf("cosign sign failed: %w\nStderr: %s\nStdout: %s", err, stderr, stdout) + if !isRekorConflict(stderr) && !isRekorConflict(stdout) { + return "", lastErr + } + fmt.Printf("Warning: Rekor transparency-log conflict on sign attempt %d/%d, retrying with a fresh signature\n", attempt, maxSignAttempts) + } + return "", lastErr +} + // KeylessSigner implements keyless signing using OIDC tokens type KeylessSigner struct { OIDCToken string @@ -41,9 +77,9 @@ func (s *KeylessSigner) Sign(ctx context.Context, imageRef string) (*SignResult, // Execute cosign sign command args := []string{"sign", "--yes", imageRef} - stdout, stderr, err := tools.ExecCommand(ctx, "cosign", args, env, s.Timeout) + stdout, err := runCosignSign(ctx, args, env, s.Timeout) if err != nil { - return nil, fmt.Errorf("cosign sign failed: %w\nStderr: %s\nStdout: %s", err, stderr, stdout) + return nil, err } // Parse output for Rekor entry URL diff --git a/pkg/security/signing/keyless_test.go b/pkg/security/signing/keyless_test.go index 68f208a2..7610ee0e 100644 --- a/pkg/security/signing/keyless_test.go +++ b/pkg/security/signing/keyless_test.go @@ -2,6 +2,7 @@ package signing import ( "context" + "fmt" "testing" "time" @@ -144,3 +145,119 @@ func TestGetRekorEntryFromOutput(t *testing.T) { Expect(GetRekorEntryFromOutput(output)).To(Equal(expected)) } + +func TestIsRekorConflict(t *testing.T) { + RegisterTestingT(t) + + tests := []struct { + name string + output string + want bool + }{ + { + name: "cosign bundle 409 createLogEntryConflict", + output: `signing registry.example.com/app@sha256:a7c43eb1700be291e4ea2bc146c8d5d23118f1d9: signing bundle: error signing bundle: ` + + `[POST /api/v1/log/entries][409] createLogEntryConflict {"code":409,"message":"an equivalent entry already exists in the transparency log with UUID 108e9186e8c5677a"}`, + want: true, + }, + { + name: "bare conflict marker", + output: "createLogEntryConflict", + want: true, + }, + { + name: "409 against the rekor entries endpoint", + output: "[POST /api/v1/log/entries][409] something else", + want: true, + }, + { + name: "unrelated 409 from a registry", + output: "GET https://registry.example.com/v2/: unexpected status 409", + want: false, + }, + { + name: "fulcio auth failure", + output: "error signing: getting signer: getting key from Fulcio: retrieving cert: oidc: token expired", + want: false, + }, + { + name: "empty", + output: "", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RegisterTestingT(t) + Expect(isRekorConflict(tt.output)).To(Equal(tt.want)) + }) + } +} + +func TestKeylessSigner_Sign_RetriesOnRekorConflict(t *testing.T) { + RegisterTestingT(t) + + origExec := execCommand + defer func() { execCommand = origExec }() + + conflictStderr := `signing bundle: error signing bundle: [POST /api/v1/log/entries][409] createLogEntryConflict {"code":409,"message":"an equivalent entry already exists in the transparency log"}` + + calls := 0 + execCommand = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + calls++ + Expect(name).To(Equal("cosign")) + Expect(args[0]).To(Equal("sign")) + if calls == 1 { + return "", conflictStderr, fmt.Errorf("exit status 1") + } + return "tlog entry created with index: 123456", "", nil + } + + signer := NewKeylessSigner("a.b.c", time.Second) + result, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") + + Expect(err).ToNot(HaveOccurred()) + Expect(calls).To(Equal(2), "conflict must trigger exactly one retry") + Expect(result.RekorEntry).To(Equal("https://rekor.sigstore.dev/api/v1/log/entries?logIndex=123456")) +} + +func TestKeylessSigner_Sign_NoRetryOnOtherErrors(t *testing.T) { + RegisterTestingT(t) + + origExec := execCommand + defer func() { execCommand = origExec }() + + calls := 0 + execCommand = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + calls++ + return "", "error signing: getting signer: oidc: token expired", fmt.Errorf("exit status 1") + } + + signer := NewKeylessSigner("a.b.c", time.Second) + _, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("token expired")) + Expect(calls).To(Equal(1), "non-conflict errors must not be retried") +} + +func TestKeylessSigner_Sign_GivesUpAfterMaxConflictAttempts(t *testing.T) { + RegisterTestingT(t) + + origExec := execCommand + defer func() { execCommand = origExec }() + + calls := 0 + execCommand = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + calls++ + return "", "[POST /api/v1/log/entries][409] createLogEntryConflict", fmt.Errorf("exit status 1") + } + + signer := NewKeylessSigner("a.b.c", time.Second) + _, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("createLogEntryConflict")) + Expect(calls).To(Equal(maxSignAttempts)) +} From 283c39c70feaa71dcd0cbd64f158f7d0f958ab6f Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Wed, 10 Jun 2026 13:50:12 +0400 Subject: [PATCH 2/4] test+docs: gomega-align probe tests; document startupProbe, periodSeconds and Rekor-409 self-heal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - kube_run_probe_test.go converted from testify to gomega per docs/TESTING.md (gomega is the mandated framework; the neighboring testify file predates the policy). - GKE Autopilot guide: startupProbe added to the Complete CloudExtras Reference yaml + field table. The reference already promised periodSeconds on probes before the code supported it — now both are true. - Troubleshooting: createLogEntryConflict (Rekor 409) entry documenting the cause and the automatic fresh-signature retry. Signed-off-by: Dmitrii Creed --- docs/docs/guides/parent-gcp-gke-autopilot.md | 17 ++++++++ .../troubleshooting/container-security.md | 24 ++++++++++++ pkg/clouds/k8s/kube_run_probe_test.go | 39 +++++++++++-------- 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/docs/docs/guides/parent-gcp-gke-autopilot.md b/docs/docs/guides/parent-gcp-gke-autopilot.md index 36d48176..005dd360 100644 --- a/docs/docs/guides/parent-gcp-gke-autopilot.md +++ b/docs/docs/guides/parent-gcp-gke-autopilot.md @@ -683,6 +683,22 @@ stacks: timeoutSeconds: 10 periodSeconds: 30 failureThreshold: 3 + + # Global startup probe configuration — the cold-start budget. + # Readiness/liveness checks are suspended until this probe succeeds, + # so a slow first boot (image pull, migrations, JIT warmup) is not + # killed mid-start. Total budget here: 10s + 16 × 15s = 250s. + # Without it, the startup window defaults to the readiness probe, + # whose short thresholds are tuned for steady-state, not first boot. + startupProbe: + httpGet: + path: "/health" + port: 8080 + initialDelaySeconds: 10 + timeoutSeconds: 5 + periodSeconds: 15 + failureThreshold: 16 + successThreshold: 1 ``` ## **CloudExtras Field Reference** @@ -697,6 +713,7 @@ stacks: | `vpa` | `object` | Vertical Pod Autoscaler | Native GKE support | | `readinessProbe` | `object` | Global readiness probe | Full support | | `livenessProbe` | `object` | Global liveness probe | Full support | +| `startupProbe` | `object` | Global startup probe (cold-start budget) | Full support | | `priorityClassName` | `string` | Kubernetes PriorityClass for pod scheduling | Full support | | `ephemeralVolumes` | `[]object` | Generic ephemeral volumes (>10GB storage) | Full support | diff --git a/docs/docs/troubleshooting/container-security.md b/docs/docs/troubleshooting/container-security.md index 5ff24fde..76acd99e 100644 --- a/docs/docs/troubleshooting/container-security.md +++ b/docs/docs/troubleshooting/container-security.md @@ -113,6 +113,30 @@ signing: ``` 3. Re-sign image if signature corrupted +### Error: "createLogEntryConflict" (Rekor HTTP 409) + +**Problem:** `sc image sign` fails with: + +``` +signing bundle: error signing bundle: [POST /api/v1/log/entries][409] createLogEntryConflict +{"code":409,"message":"an equivalent entry already exists in the transparency log with UUID ..."} +``` + +An identical entry is already in the transparency log — typically cosign +re-uploading an entry whose first attempt timed out client-side but succeeded +server-side. Common under parallel CI deploys of stacks that share one image. +The image *is* effectively signed; only the duplicate upload was rejected. + +**Solution:** + +- `sc` retries the sign automatically with a fresh signature (a fresh + invocation cannot conflict with itself), so transient conflicts self-heal. +- If the error persists after the automatic retries, verify the existing + signature instead of re-signing: `cosign tree IMAGE` and `sc image verify`. +- Persistent conflicts with key-based signing can indicate a deterministic + re-sign of an unchanged digest — skip signing when the digest is already + signed by the same identity. + ### Warning: "Rekor entry not found" **Problem:** Keyless signature not in transparency log. diff --git a/pkg/clouds/k8s/kube_run_probe_test.go b/pkg/clouds/k8s/kube_run_probe_test.go index 2cf89f6e..49a59c9b 100644 --- a/pkg/clouds/k8s/kube_run_probe_test.go +++ b/pkg/clouds/k8s/kube_run_probe_test.go @@ -3,8 +3,7 @@ package k8s import ( "testing" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + . "github.com/onsi/gomega" "github.com/simple-container-com/api/pkg/api" ) @@ -14,6 +13,8 @@ import ( // startup budgets never reached the cluster and pods fell back to the readiness // probe's (short) window during cold starts. func TestStartupProbeExtraction(t *testing.T) { + RegisterTestingT(t) + cloudExtras := map[string]any{ "startupProbe": map[string]any{ "httpGet": map[string]any{ @@ -30,21 +31,23 @@ func TestStartupProbeExtraction(t *testing.T) { result, err := api.ConvertDescriptor(cloudExtras, &CloudExtras{}) - require.NoError(t, err) - require.NotNil(t, result.StartupProbe) - assert.Equal(t, "/health/", result.StartupProbe.HttpGet.Path) - assert.Equal(t, 8000, result.StartupProbe.HttpGet.Port) - assert.Equal(t, 10, *result.StartupProbe.InitialDelaySeconds) - assert.Equal(t, 5, *result.StartupProbe.TimeoutSeconds) - assert.Equal(t, 15, *result.StartupProbe.PeriodSeconds) - assert.Equal(t, 16, *result.StartupProbe.FailureThreshold) - assert.Equal(t, 1, *result.StartupProbe.SuccessThreshold) + Expect(err).ToNot(HaveOccurred()) + Expect(result.StartupProbe).ToNot(BeNil()) + Expect(result.StartupProbe.HttpGet.Path).To(Equal("/health/")) + Expect(result.StartupProbe.HttpGet.Port).To(Equal(8000)) + Expect(*result.StartupProbe.InitialDelaySeconds).To(Equal(10)) + Expect(*result.StartupProbe.TimeoutSeconds).To(Equal(5)) + Expect(*result.StartupProbe.PeriodSeconds).To(Equal(15)) + Expect(*result.StartupProbe.FailureThreshold).To(Equal(16)) + Expect(*result.StartupProbe.SuccessThreshold).To(Equal(1)) } // TestProbePeriodSecondsExtraction covers the k8s-native periodSeconds spelling on // the readiness probe. Previously only the duration-typed `interval` key existed, // so configs written with periodSeconds silently fell back to the kubelet default. func TestProbePeriodSecondsExtraction(t *testing.T) { + RegisterTestingT(t) + cloudExtras := map[string]any{ "readinessProbe": map[string]any{ "httpGet": map[string]any{ @@ -58,16 +61,18 @@ func TestProbePeriodSecondsExtraction(t *testing.T) { result, err := api.ConvertDescriptor(cloudExtras, &CloudExtras{}) - require.NoError(t, err) - require.NotNil(t, result.ReadinessProbe) - assert.Equal(t, 20, *result.ReadinessProbe.PeriodSeconds) - assert.Equal(t, 3, *result.ReadinessProbe.FailureThreshold) + Expect(err).ToNot(HaveOccurred()) + Expect(result.ReadinessProbe).ToNot(BeNil()) + Expect(*result.ReadinessProbe.PeriodSeconds).To(Equal(20)) + Expect(*result.ReadinessProbe.FailureThreshold).To(Equal(3)) } // TestStartupProbeAbsent ensures the zero-value path stays nil. func TestStartupProbeAbsent(t *testing.T) { + RegisterTestingT(t) + result, err := api.ConvertDescriptor(map[string]any{}, &CloudExtras{}) - require.NoError(t, err) - assert.Nil(t, result.StartupProbe) + Expect(err).ToNot(HaveOccurred()) + Expect(result.StartupProbe).To(BeNil()) } From a59ed2e9b10546a3ca495aa565175f56b9b7af76 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Wed, 10 Jun 2026 16:45:55 +0400 Subject: [PATCH 3/4] fix(review): wire startupProbe through the GKE Autopilot stack path; honor explicit probes on multi-port containers; DI exec seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex + Gemini review findings: - gke_autopilot_stack.go forwards readiness/liveness into DeploymentArgs but not the new StartupProbe — the GKE Autopilot path (the primary consumer) silently dropped it, reintroducing the original bug one layer up. Wired. - deployment.go required a single port / mainPort even for an EXPLICIT startup probe, dropping it on multi-port containers; readiness applies explicit probes unconditionally and toProbeArgs resolves the port from httpGet.port / mainPort / first port. Mirrored that behavior. - Replaced the package-global execCommand test seam with an exec field on both signers (data race if tests ever parallelize; proper DI). - Retry warning now goes to stderr (stdout can be piped/parsed) and no longer claims a "fresh signature" — key-based signing with deterministic keys (ed25519) reproduces the same signature; documented why exhausting the loop and failing is the correct outcome there (a tlog entry existing does not prove the signature reached the registry). - IntervaSeconds typo field marked Deprecated. - Tests: key-based 409 persistent-conflict (exhausts loop, surfaces error) and transient-conflict (one retry then success) cases added. Declined (with rationale): applying the global startupProbe to non-ingress containers — global probes are deliberately ingress-only, mirroring the readiness/liveness semantics; HTTP probes on worker/sidecar containers is the failure mode that guard exists to prevent. Signed-off-by: Dmitrii Creed --- pkg/clouds/k8s/types.go | 4 +- pkg/clouds/pulumi/gcp/gke_autopilot_stack.go | 1 + pkg/clouds/pulumi/kubernetes/deployment.go | 5 ++- pkg/security/signing/keybased.go | 9 ++++- pkg/security/signing/keybased_test.go | 42 ++++++++++++++++++++ pkg/security/signing/keyless.go | 31 ++++++++++----- pkg/security/signing/keyless_test.go | 24 +++-------- 7 files changed, 85 insertions(+), 31 deletions(-) diff --git a/pkg/clouds/k8s/types.go b/pkg/clouds/k8s/types.go index 8ded8258..04fdf675 100644 --- a/pkg/clouds/k8s/types.go +++ b/pkg/clouds/k8s/types.go @@ -134,8 +134,8 @@ type CloudRunProbe struct { HttpGet ProbeHttpGet `json:"httpGet" yaml:"httpGet"` Interval *time.Duration `json:"interval" yaml:"interval"` InitialDelaySeconds *int `json:"initialDelaySeconds" yaml:"initialDelaySeconds"` - IntervaSeconds *int `json:"intervaSeconds" yaml:"intervaSeconds"` - PeriodSeconds *int `json:"periodSeconds" yaml:"periodSeconds"` // k8s-native spelling; takes precedence over Interval + IntervaSeconds *int `json:"intervaSeconds" yaml:"intervaSeconds"` // Deprecated: misspelled and never consumed; use periodSeconds + PeriodSeconds *int `json:"periodSeconds" yaml:"periodSeconds"` // k8s-native spelling; takes precedence over Interval FailureThreshold *int `json:"failureThreshold" yaml:"failureThreshold"` SuccessThreshold *int `json:"successThreshold" yaml:"successThreshold"` TimeoutSeconds *int `json:"timeoutSeconds" yaml:"timeoutSeconds"` diff --git a/pkg/clouds/pulumi/gcp/gke_autopilot_stack.go b/pkg/clouds/pulumi/gcp/gke_autopilot_stack.go index 350bb448..61e1f9af 100644 --- a/pkg/clouds/pulumi/gcp/gke_autopilot_stack.go +++ b/pkg/clouds/pulumi/gcp/gke_autopilot_stack.go @@ -189,6 +189,7 @@ func GkeAutopilotStack(ctx *sdk.Context, stack api.Stack, input api.ResourceInpu VPA: gkeAutopilotInput.Deployment.VPA, // Pass VPA configuration to Kubernetes deployment ReadinessProbe: gkeAutopilotInput.Deployment.ReadinessProbe, // Pass global readiness probe configuration LivenessProbe: gkeAutopilotInput.Deployment.LivenessProbe, // Pass global liveness probe configuration + StartupProbe: gkeAutopilotInput.Deployment.StartupProbe, // Pass global startup probe configuration EphemeralSize: ephemeralSize, } diff --git a/pkg/clouds/pulumi/kubernetes/deployment.go b/pkg/clouds/pulumi/kubernetes/deployment.go index 62f6b50f..5b8aaff8 100644 --- a/pkg/clouds/pulumi/kubernetes/deployment.go +++ b/pkg/clouds/pulumi/kubernetes/deployment.go @@ -187,7 +187,10 @@ func DeploySimpleContainer(ctx *sdk.Context, args Args, opts ...sdk.ResourceOpti } if cStartupProbe == nil && (len(c.Container.Ports) == 1 || c.Container.MainPort != nil) { startupProbe = readinessProbe - } else if cStartupProbe != nil && (len(c.Container.Ports) == 1 || c.Container.MainPort != nil) { + } else if cStartupProbe != nil { + // An explicit probe carries its own port resolution (httpGet.port / + // mainPort / first port) — mirror the readiness behavior instead of + // dropping it on multi-port containers. startupProbe = toProbeArgs(c, cStartupProbe) } diff --git a/pkg/security/signing/keybased.go b/pkg/security/signing/keybased.go index 631e795a..165f08b4 100644 --- a/pkg/security/signing/keybased.go +++ b/pkg/security/signing/keybased.go @@ -15,6 +15,9 @@ type KeyBasedSigner struct { PrivateKey string // Path to private key file or key content Password string // Optional password for encrypted keys Timeout time.Duration + + // exec overrides command execution in tests; nil means tools.ExecCommand. + exec execFn } // NewKeyBasedSigner creates a new key-based signer @@ -73,7 +76,11 @@ func (s *KeyBasedSigner) Sign(ctx context.Context, imageRef string) (*SignResult // Execute cosign sign command args := []string{"sign", "--key", keyPath, imageRef} - if _, err := runCosignSign(ctx, args, env, s.Timeout); err != nil { + exec := s.exec + if exec == nil { + exec = tools.ExecCommand + } + if _, err := runCosignSign(ctx, exec, args, env, s.Timeout); err != nil { return nil, err } diff --git a/pkg/security/signing/keybased_test.go b/pkg/security/signing/keybased_test.go index 07cde858..e553daec 100644 --- a/pkg/security/signing/keybased_test.go +++ b/pkg/security/signing/keybased_test.go @@ -2,6 +2,7 @@ package signing import ( "context" + "fmt" "os" "path/filepath" "strings" @@ -100,3 +101,44 @@ func TestKeyBasedSignerPasswordHandling(t *testing.T) { withPassword := NewKeyBasedSigner("/tmp/cosign.key", "secret123", 5*time.Minute) Expect(strings.Contains("COSIGN_PASSWORD="+withPassword.Password, "COSIGN_PASSWORD=secret123")).To(BeTrue()) } + +func TestKeyBasedSigner_Sign_GivesUpOnPersistentRekorConflict(t *testing.T) { + RegisterTestingT(t) + + // A deterministic key (e.g. ed25519) reproduces the same signature on + // retry, so a persistent conflict must exhaust the bounded loop and + // surface the error — never be treated as success: a tlog entry existing + // does not prove the signature was attached to the registry. + calls := 0 + signer := NewKeyBasedSigner("test-key-content", "", time.Second) + signer.exec = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + calls++ + return "", "[POST /api/v1/log/entries][409] createLogEntryConflict", fmt.Errorf("exit status 1") + } + + _, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("createLogEntryConflict")) + Expect(calls).To(Equal(maxSignAttempts)) +} + +func TestKeyBasedSigner_Sign_RetriesOnceOnTransientConflict(t *testing.T) { + RegisterTestingT(t) + + calls := 0 + signer := NewKeyBasedSigner("test-key-content", "", time.Second) + signer.exec = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + calls++ + if calls == 1 { + return "", "createLogEntryConflict", fmt.Errorf("exit status 1") + } + return "", "", nil + } + + result, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") + + Expect(err).ToNot(HaveOccurred()) + Expect(result).ToNot(BeNil()) + Expect(calls).To(Equal(2)) +} diff --git a/pkg/security/signing/keyless.go b/pkg/security/signing/keyless.go index 10363e84..55e28774 100644 --- a/pkg/security/signing/keyless.go +++ b/pkg/security/signing/keyless.go @@ -3,6 +3,7 @@ package signing import ( "context" "fmt" + "os" "regexp" "strings" "time" @@ -10,8 +11,9 @@ import ( "github.com/simple-container-com/api/pkg/security/tools" ) -// execCommand is a seam for tests; production code always uses tools.ExecCommand. -var execCommand = tools.ExecCommand +// execFn matches tools.ExecCommand; signers carry it as a field so tests can +// inject a fake without racing on package state. +type execFn func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) // maxSignAttempts bounds the Rekor-conflict retry loop in runCosignSign. const maxSignAttempts = 3 @@ -27,13 +29,17 @@ func isRekorConflict(output string) bool { } // runCosignSign executes `cosign sign`, retrying the full invocation when the -// only failure is a Rekor entry conflict. A fresh invocation mints a fresh -// (ephemeral or randomized-ECDSA) signature, so a retry cannot conflict with -// itself; any other error is returned immediately. -func runCosignSign(ctx context.Context, args, env []string, timeout time.Duration) (string, error) { +// only failure is a Rekor entry conflict. Keyless (ephemeral key) and +// randomized-ECDSA key-based invocations mint a fresh signature each run, so +// their retries cannot conflict with themselves. Deterministic keys (e.g. +// ed25519) reproduce the same signature — for them the bounded loop exhausts +// and surfaces the conflict, which is correct: the tlog entry existing does +// NOT prove the signature was attached to the registry, so success must not +// be assumed. Any non-conflict error is returned immediately. +func runCosignSign(ctx context.Context, exec execFn, args, env []string, timeout time.Duration) (string, error) { var lastErr error for attempt := 1; attempt <= maxSignAttempts; attempt++ { - stdout, stderr, err := execCommand(ctx, "cosign", args, env, timeout) + stdout, stderr, err := exec(ctx, "cosign", args, env, timeout) if err == nil { return stdout, nil } @@ -41,7 +47,7 @@ func runCosignSign(ctx context.Context, args, env []string, timeout time.Duratio if !isRekorConflict(stderr) && !isRekorConflict(stdout) { return "", lastErr } - fmt.Printf("Warning: Rekor transparency-log conflict on sign attempt %d/%d, retrying with a fresh signature\n", attempt, maxSignAttempts) + fmt.Fprintf(os.Stderr, "Warning: Rekor transparency-log conflict on sign attempt %d/%d, retrying\n", attempt, maxSignAttempts) } return "", lastErr } @@ -50,6 +56,9 @@ func runCosignSign(ctx context.Context, args, env []string, timeout time.Duratio type KeylessSigner struct { OIDCToken string Timeout time.Duration + + // exec overrides command execution in tests; nil means tools.ExecCommand. + exec execFn } // NewKeylessSigner creates a new keyless signer @@ -77,7 +86,11 @@ func (s *KeylessSigner) Sign(ctx context.Context, imageRef string) (*SignResult, // Execute cosign sign command args := []string{"sign", "--yes", imageRef} - stdout, err := runCosignSign(ctx, args, env, s.Timeout) + exec := s.exec + if exec == nil { + exec = tools.ExecCommand + } + stdout, err := runCosignSign(ctx, exec, args, env, s.Timeout) if err != nil { return nil, err } diff --git a/pkg/security/signing/keyless_test.go b/pkg/security/signing/keyless_test.go index 7610ee0e..b20a6099 100644 --- a/pkg/security/signing/keyless_test.go +++ b/pkg/security/signing/keyless_test.go @@ -198,13 +198,11 @@ func TestIsRekorConflict(t *testing.T) { func TestKeylessSigner_Sign_RetriesOnRekorConflict(t *testing.T) { RegisterTestingT(t) - origExec := execCommand - defer func() { execCommand = origExec }() - conflictStderr := `signing bundle: error signing bundle: [POST /api/v1/log/entries][409] createLogEntryConflict {"code":409,"message":"an equivalent entry already exists in the transparency log"}` calls := 0 - execCommand = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + signer := NewKeylessSigner("a.b.c", time.Second) + signer.exec = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { calls++ Expect(name).To(Equal("cosign")) Expect(args[0]).To(Equal("sign")) @@ -213,8 +211,6 @@ func TestKeylessSigner_Sign_RetriesOnRekorConflict(t *testing.T) { } return "tlog entry created with index: 123456", "", nil } - - signer := NewKeylessSigner("a.b.c", time.Second) result, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") Expect(err).ToNot(HaveOccurred()) @@ -225,16 +221,12 @@ func TestKeylessSigner_Sign_RetriesOnRekorConflict(t *testing.T) { func TestKeylessSigner_Sign_NoRetryOnOtherErrors(t *testing.T) { RegisterTestingT(t) - origExec := execCommand - defer func() { execCommand = origExec }() - calls := 0 - execCommand = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + signer := NewKeylessSigner("a.b.c", time.Second) + signer.exec = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { calls++ return "", "error signing: getting signer: oidc: token expired", fmt.Errorf("exit status 1") } - - signer := NewKeylessSigner("a.b.c", time.Second) _, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") Expect(err).To(HaveOccurred()) @@ -245,16 +237,12 @@ func TestKeylessSigner_Sign_NoRetryOnOtherErrors(t *testing.T) { func TestKeylessSigner_Sign_GivesUpAfterMaxConflictAttempts(t *testing.T) { RegisterTestingT(t) - origExec := execCommand - defer func() { execCommand = origExec }() - calls := 0 - execCommand = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { + signer := NewKeylessSigner("a.b.c", time.Second) + signer.exec = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { calls++ return "", "[POST /api/v1/log/entries][409] createLogEntryConflict", fmt.Errorf("exit status 1") } - - signer := NewKeylessSigner("a.b.c", time.Second) _, err := signer.Sign(context.Background(), "registry.example.com/app:1.0.0") Expect(err).To(HaveOccurred()) From 49fb7db1dbbabfd1da6b781b1dc58c26b7fb84ca Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Wed, 10 Jun 2026 17:56:39 +0400 Subject: [PATCH 4/4] fix(ci): bump go directive to 1.26.4 for stdlib CVE fixes; trim comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit govulncheck fails on GO-2026-5039 (net/textproto) + GO-2026-5037 (crypto/x509), both fixed in go1.26.4 — pre-existing on main (red since 2026-06-04), surfaced here because the gate blocks this PR. setup-go reads go-version-file: go.mod, so the directive bump moves CI to the patched toolchain. No toolchain line, no dependency changes. Also trims multi-line comment blocks added by this PR down to one-liners. Signed-off-by: Dmitrii Creed --- go.mod | 2 +- pkg/clouds/k8s/kube_run_probe_test.go | 10 ++-------- pkg/clouds/pulumi/kubernetes/deployment.go | 11 +++-------- pkg/security/signing/keybased_test.go | 6 ++---- pkg/security/signing/keyless.go | 23 ++++++++-------------- 5 files changed, 16 insertions(+), 36 deletions(-) diff --git a/go.mod b/go.mod index 7e4047eb..a4d9aedc 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/simple-container-com/api -go 1.26.3 +go 1.26.4 require ( cloud.google.com/go/storage v1.62.2 diff --git a/pkg/clouds/k8s/kube_run_probe_test.go b/pkg/clouds/k8s/kube_run_probe_test.go index 49a59c9b..d79f1a5a 100644 --- a/pkg/clouds/k8s/kube_run_probe_test.go +++ b/pkg/clouds/k8s/kube_run_probe_test.go @@ -8,10 +8,7 @@ import ( "github.com/simple-container-com/api/pkg/api" ) -// TestStartupProbeExtraction is the regression test for cloudExtras.startupProbe -// being silently dropped: CloudExtras had no StartupProbe field, so user-supplied -// startup budgets never reached the cluster and pods fell back to the readiness -// probe's (short) window during cold starts. +// Regression: cloudExtras.startupProbe was silently dropped (no struct field). func TestStartupProbeExtraction(t *testing.T) { RegisterTestingT(t) @@ -42,9 +39,7 @@ func TestStartupProbeExtraction(t *testing.T) { Expect(*result.StartupProbe.SuccessThreshold).To(Equal(1)) } -// TestProbePeriodSecondsExtraction covers the k8s-native periodSeconds spelling on -// the readiness probe. Previously only the duration-typed `interval` key existed, -// so configs written with periodSeconds silently fell back to the kubelet default. +// Regression: periodSeconds was an unknown key and fell back to kubelet defaults. func TestProbePeriodSecondsExtraction(t *testing.T) { RegisterTestingT(t) @@ -67,7 +62,6 @@ func TestProbePeriodSecondsExtraction(t *testing.T) { Expect(*result.ReadinessProbe.FailureThreshold).To(Equal(3)) } -// TestStartupProbeAbsent ensures the zero-value path stays nil. func TestStartupProbeAbsent(t *testing.T) { RegisterTestingT(t) diff --git a/pkg/clouds/pulumi/kubernetes/deployment.go b/pkg/clouds/pulumi/kubernetes/deployment.go index 5b8aaff8..bd7a239d 100644 --- a/pkg/clouds/pulumi/kubernetes/deployment.go +++ b/pkg/clouds/pulumi/kubernetes/deployment.go @@ -179,18 +179,14 @@ func DeploySimpleContainer(ctx *sdk.Context, args Args, opts ...sdk.ResourceOpti var startupProbe *corev1.ProbeArgs cStartupProbe := c.Container.StartupProbe - // Use global startup probe if container doesn't have one AND it's the ingress container, - // mirroring the readiness/liveness fallback above. Without this, a slow-booting service - // is stuck with the readiness probe's (short) window as its startup budget. + // Global fallback for the ingress container only, mirroring readiness/liveness above if cStartupProbe == nil && args.StartupProbe != nil && isIngressContainer { cStartupProbe = args.StartupProbe } if cStartupProbe == nil && (len(c.Container.Ports) == 1 || c.Container.MainPort != nil) { startupProbe = readinessProbe } else if cStartupProbe != nil { - // An explicit probe carries its own port resolution (httpGet.port / - // mainPort / first port) — mirror the readiness behavior instead of - // dropping it on multi-port containers. + // Explicit probes resolve their own port — apply even on multi-port containers startupProbe = toProbeArgs(c, cStartupProbe) } @@ -336,8 +332,7 @@ func toProbeArgs(c *ContainerImage, probe *k8s.CloudRunProbe) *corev1.ProbeArgs probePort = c.Container.Ports[0] } - // periodSeconds (k8s-native spelling) wins over the legacy duration-typed interval; - // duration values only survive YAML conversion when given as Go duration strings. + // periodSeconds (k8s-native) wins over the legacy duration-typed interval periodSeconds := probe.PeriodSeconds if periodSeconds == nil && probe.Interval != nil { periodSeconds = lo.ToPtr(int(lo.FromPtr(probe.Interval).Seconds())) diff --git a/pkg/security/signing/keybased_test.go b/pkg/security/signing/keybased_test.go index e553daec..836b73ab 100644 --- a/pkg/security/signing/keybased_test.go +++ b/pkg/security/signing/keybased_test.go @@ -105,10 +105,8 @@ func TestKeyBasedSignerPasswordHandling(t *testing.T) { func TestKeyBasedSigner_Sign_GivesUpOnPersistentRekorConflict(t *testing.T) { RegisterTestingT(t) - // A deterministic key (e.g. ed25519) reproduces the same signature on - // retry, so a persistent conflict must exhaust the bounded loop and - // surface the error — never be treated as success: a tlog entry existing - // does not prove the signature was attached to the registry. + // Deterministic keys reproduce the same signature: a persistent conflict + // must exhaust the loop and fail, never be treated as success. calls := 0 signer := NewKeyBasedSigner("test-key-content", "", time.Second) signer.exec = func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) { diff --git a/pkg/security/signing/keyless.go b/pkg/security/signing/keyless.go index 55e28774..c1d6e6f8 100644 --- a/pkg/security/signing/keyless.go +++ b/pkg/security/signing/keyless.go @@ -11,31 +11,24 @@ import ( "github.com/simple-container-com/api/pkg/security/tools" ) -// execFn matches tools.ExecCommand; signers carry it as a field so tests can -// inject a fake without racing on package state. +// execFn matches tools.ExecCommand; injectable for tests. type execFn func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) // maxSignAttempts bounds the Rekor-conflict retry loop in runCosignSign. const maxSignAttempts = 3 -// isRekorConflict reports whether a cosign failure was caused by Rekor's -// createLogEntryConflict (HTTP 409 on POST /api/v1/log/entries). This happens -// when an identical entry is already in the transparency log — typically -// cosign re-uploading an entry whose first attempt timed out client-side but -// succeeded server-side (seen under parallel CI runners signing concurrently). +// isRekorConflict reports a Rekor createLogEntryConflict (HTTP 409) — an +// identical entry already in the tlog, typically a cosign upload retry after +// a client-side timeout whose first attempt succeeded server-side. func isRekorConflict(output string) bool { return strings.Contains(output, "createLogEntryConflict") || (strings.Contains(output, "409") && strings.Contains(output, "/api/v1/log/entries")) } -// runCosignSign executes `cosign sign`, retrying the full invocation when the -// only failure is a Rekor entry conflict. Keyless (ephemeral key) and -// randomized-ECDSA key-based invocations mint a fresh signature each run, so -// their retries cannot conflict with themselves. Deterministic keys (e.g. -// ed25519) reproduce the same signature — for them the bounded loop exhausts -// and surfaces the conflict, which is correct: the tlog entry existing does -// NOT prove the signature was attached to the registry, so success must not -// be assumed. Any non-conflict error is returned immediately. +// runCosignSign retries the full `cosign sign` on Rekor entry conflicts (a +// fresh invocation can't conflict with itself). Deterministic keys reproduce +// the same signature and exhaust the loop — correct, since a tlog entry does +// not prove the signature reached the registry. Other errors fail fast. func runCosignSign(ctx context.Context, exec execFn, args, env []string, timeout time.Duration) (string, error) { var lastErr error for attempt := 1; attempt <= maxSignAttempts; attempt++ {