From 9f957d6e75e6eb215643b78bda811ff16bd8d018 Mon Sep 17 00:00:00 2001 From: jordan Date: Mon, 9 Feb 2026 19:36:23 -0700 Subject: [PATCH] fix(templates): harden component CI steps and compile regexes - Add --connect-timeout 10 and --max-time 15 to all verify step curl calls to prevent hanging on registry health checks - Fix cli template: depends_on [deps] -> [preflight] for consistency - Add cross-reference comment to service template about verify logic being replicated across all 5 component templates - Document component CI step rules in composable-monorepo.md - Compile regexes at package level instead of per-call in component_updates.go - Add component_updates_test.go Co-Authored-By: Claude Opus 4.6 --- .../guides/services/composable-monorepo.md | 7 + .../app-astro/.woodpecker.step.yml.tmpl | 2 + .../app-nextjs/.woodpecker.step.yml.tmpl | 2 + .../app-react/.woodpecker.step.yml.tmpl | 2 + .../components/cli/.woodpecker.step.yml.tmpl | 2 +- .../service/.woodpecker.step.yml.tmpl | 4 + .../worker/.woodpecker.step.yml.tmpl | 2 + internal/service/component_updates.go | 11 +- internal/service/component_updates_test.go | 277 ++++++++++++++++++ 9 files changed, 304 insertions(+), 5 deletions(-) create mode 100644 internal/service/component_updates_test.go diff --git a/.claude/guides/services/composable-monorepo.md b/.claude/guides/services/composable-monorepo.md index e000cd2..7ddd0cf 100644 --- a/.claude/guides/services/composable-monorepo.md +++ b/.claude/guides/services/composable-monorepo.md @@ -118,6 +118,13 @@ internal/adapter/templates/templates/components/ Each component includes `.woodpecker.step.yml.tmpl` that defines its Kaniko build step. When components are added, the service renders this template and inserts it into the main pipeline. +**Component CI Step Rules:** +- ALL component build steps MUST use `depends_on: [preflight]` (not `[deps]`) — this ensures registry health is verified before any image build +- Components with Docker images (service, app-*, worker) MUST have a 3-step chain: `build` → `verify` (registry check) → `deploy` with explicit `depends_on` between each +- Build steps MUST NOT use `failure: ignore` — build failures should be visible, not swallowed +- The `verify` step uses `failure: ignore` so deploy is skipped (not failed) when the image doesn't exist +- `updateWoodpeckerYml` automatically wires each new `deploy-{name}` step into `build-complete`'s `depends_on` via the `# BUILD_COMPLETE_DEPS` marker + **Component Variables:** ```go {{.ProjectName}} // "acme" diff --git a/internal/adapter/templates/templates/components/app-astro/.woodpecker.step.yml.tmpl b/internal/adapter/templates/templates/components/app-astro/.woodpecker.step.yml.tmpl index 6675932..4e7bdd2 100644 --- a/internal/adapter/templates/templates/components/app-astro/.woodpecker.step.yml.tmpl +++ b/internal/adapter/templates/templates/components/app-astro/.woodpecker.step.yml.tmpl @@ -30,6 +30,8 @@ verify-{{COMPONENT_NAME}}: echo "==> Verifying image $REGISTRY/$REPO:$TAG exists in registry" HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ --insecure \ + --connect-timeout 10 \ + --max-time 15 \ "https://$REGISTRY/v2/$REPO/manifests/$TAG" \ -H "Accept: application/vnd.docker.distribution.manifest.v2+json") if [ "$HTTP_CODE" = "200" ]; then diff --git a/internal/adapter/templates/templates/components/app-nextjs/.woodpecker.step.yml.tmpl b/internal/adapter/templates/templates/components/app-nextjs/.woodpecker.step.yml.tmpl index 9613af7..9c3b44c 100644 --- a/internal/adapter/templates/templates/components/app-nextjs/.woodpecker.step.yml.tmpl +++ b/internal/adapter/templates/templates/components/app-nextjs/.woodpecker.step.yml.tmpl @@ -30,6 +30,8 @@ verify-{{COMPONENT_NAME}}: echo "==> Verifying image $REGISTRY/$REPO:$TAG exists in registry" HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ --insecure \ + --connect-timeout 10 \ + --max-time 15 \ "https://$REGISTRY/v2/$REPO/manifests/$TAG" \ -H "Accept: application/vnd.docker.distribution.manifest.v2+json") if [ "$HTTP_CODE" = "200" ]; then diff --git a/internal/adapter/templates/templates/components/app-react/.woodpecker.step.yml.tmpl b/internal/adapter/templates/templates/components/app-react/.woodpecker.step.yml.tmpl index 285c9a8..5ae6355 100644 --- a/internal/adapter/templates/templates/components/app-react/.woodpecker.step.yml.tmpl +++ b/internal/adapter/templates/templates/components/app-react/.woodpecker.step.yml.tmpl @@ -30,6 +30,8 @@ verify-{{COMPONENT_NAME}}: echo "==> Verifying image $REGISTRY/$REPO:$TAG exists in registry" HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ --insecure \ + --connect-timeout 10 \ + --max-time 15 \ "https://$REGISTRY/v2/$REPO/manifests/$TAG" \ -H "Accept: application/vnd.docker.distribution.manifest.v2+json") if [ "$HTTP_CODE" = "200" ]; then diff --git a/internal/adapter/templates/templates/components/cli/.woodpecker.step.yml.tmpl b/internal/adapter/templates/templates/components/cli/.woodpecker.step.yml.tmpl index 5e73d84..b06c917 100644 --- a/internal/adapter/templates/templates/components/cli/.woodpecker.step.yml.tmpl +++ b/internal/adapter/templates/templates/components/cli/.woodpecker.step.yml.tmpl @@ -5,7 +5,7 @@ # This step builds and tests the CLI. build-{{COMPONENT_NAME}}: - depends_on: [deps] + depends_on: [preflight] image: golang:1.25-alpine commands: - cd cli/{{COMPONENT_NAME}} diff --git a/internal/adapter/templates/templates/components/service/.woodpecker.step.yml.tmpl b/internal/adapter/templates/templates/components/service/.woodpecker.step.yml.tmpl index 49ee954..d5afcd1 100644 --- a/internal/adapter/templates/templates/components/service/.woodpecker.step.yml.tmpl +++ b/internal/adapter/templates/templates/components/service/.woodpecker.step.yml.tmpl @@ -1,5 +1,7 @@ # Woodpecker CI step for {{COMPONENT_NAME}} service # Add this step to your .woodpecker.yml +# NOTE: verify step is replicated in all component templates (service, app-react, +# app-astro, app-nextjs, worker). Update all 5 if changing the verify logic. build-{{COMPONENT_NAME}}: depends_on: [preflight] @@ -30,6 +32,8 @@ verify-{{COMPONENT_NAME}}: echo "==> Verifying image $REGISTRY/$REPO:$TAG exists in registry" HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ --insecure \ + --connect-timeout 10 \ + --max-time 15 \ "https://$REGISTRY/v2/$REPO/manifests/$TAG" \ -H "Accept: application/vnd.docker.distribution.manifest.v2+json") if [ "$HTTP_CODE" = "200" ]; then diff --git a/internal/adapter/templates/templates/components/worker/.woodpecker.step.yml.tmpl b/internal/adapter/templates/templates/components/worker/.woodpecker.step.yml.tmpl index cc00e2c..d4057ca 100644 --- a/internal/adapter/templates/templates/components/worker/.woodpecker.step.yml.tmpl +++ b/internal/adapter/templates/templates/components/worker/.woodpecker.step.yml.tmpl @@ -30,6 +30,8 @@ verify-{{COMPONENT_NAME}}: echo "==> Verifying image $REGISTRY/$REPO:$TAG exists in registry" HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ --insecure \ + --connect-timeout 10 \ + --max-time 15 \ "https://$REGISTRY/v2/$REPO/manifests/$TAG" \ -H "Accept: application/vnd.docker.distribution.manifest.v2+json") if [ "$HTTP_CODE" = "200" ]; then diff --git a/internal/service/component_updates.go b/internal/service/component_updates.go index ea79ff8..ce38d1a 100644 --- a/internal/service/component_updates.go +++ b/internal/service/component_updates.go @@ -9,6 +9,11 @@ import ( "github.com/orchard9/rdev/internal/logging" ) +var ( + reDeployStep = regexp.MustCompile(`(?m)^deploy-([a-zA-Z0-9_-]+):`) + reDependsOn = regexp.MustCompile(`depends_on:\s*\[([^\]]*)\]`) +) + // updateProcfile adds an entry for the component. func (s *ComponentService) updateProcfile(existing string, componentType domain.ComponentType, componentName, componentPath string, _ int) string { var entry string @@ -99,8 +104,7 @@ func (s *ComponentService) updateWoodpeckerYml(existing, stepYaml string) string // extractDeployStepName finds the deploy-{name}: key in a step YAML block. func extractDeployStepName(stepYaml string) string { - re := regexp.MustCompile(`(?m)^deploy-([a-zA-Z0-9_-]+):`) - match := re.FindStringSubmatch(stepYaml) + match := reDeployStep.FindStringSubmatch(stepYaml) if len(match) >= 2 { return "deploy-" + match[1] } @@ -120,8 +124,7 @@ func addBuildCompleteDep(yml, stepName string) string { // Parse existing depends_on array from the line // Format: " depends_on: [preflight, deploy-foo] # BUILD_COMPLETE_DEPS" - re := regexp.MustCompile(`depends_on:\s*\[([^\]]*)\]`) - match := re.FindStringSubmatch(line) + match := reDependsOn.FindStringSubmatch(line) if len(match) < 2 { break } diff --git a/internal/service/component_updates_test.go b/internal/service/component_updates_test.go new file mode 100644 index 0000000..82389f5 --- /dev/null +++ b/internal/service/component_updates_test.go @@ -0,0 +1,277 @@ +package service + +import ( + "strings" + "testing" +) + +func TestExtractDeployStepName(t *testing.T) { + tests := []struct { + name string + stepYaml string + want string + }{ + { + name: "service template with deploy step", + stepYaml: `build-studio-api: + depends_on: [preflight] + image: woodpeckerci/plugin-kaniko + +deploy-studio-api: + depends_on: [verify-studio-api] + image: bitnami/kubectl:latest`, + want: "deploy-studio-api", + }, + { + name: "worker template with deploy step", + stepYaml: `build-processor: + depends_on: [preflight] + +deploy-processor: + image: bitnami/kubectl:latest`, + want: "deploy-processor", + }, + { + name: "CLI template without deploy step", + stepYaml: `build-mytool: + depends_on: [deps] + image: golang:1.25-alpine + commands: + - go build ./cmd`, + want: "", + }, + { + name: "empty input", + stepYaml: "", + want: "", + }, + { + name: "hyphenated component name", + stepYaml: `deploy-my-cool-service: + image: bitnami/kubectl:latest`, + want: "deploy-my-cool-service", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := extractDeployStepName(tc.stepYaml) + if got != tc.want { + t.Errorf("extractDeployStepName() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestAddBuildCompleteDep(t *testing.T) { + baseYml := `steps: + preflight: + depends_on: [deps] + image: alpine/curl + + # COMPONENT_STEPS_BELOW + + build-complete: + depends_on: [preflight] # BUILD_COMPLETE_DEPS + image: alpine:3.19 + commands: + - echo "All component builds complete"` + + tests := []struct { + name string + yml string + stepName string + wantDeps string + }{ + { + name: "first component added", + yml: baseYml, + stepName: "deploy-studio-api", + wantDeps: "depends_on: [preflight, deploy-studio-api] # BUILD_COMPLETE_DEPS", + }, + { + name: "second component added", + yml: strings.Replace(baseYml, + "depends_on: [preflight] # BUILD_COMPLETE_DEPS", + "depends_on: [preflight, deploy-studio-api] # BUILD_COMPLETE_DEPS", 1), + stepName: "deploy-studio-ui", + wantDeps: "depends_on: [preflight, deploy-studio-api, deploy-studio-ui] # BUILD_COMPLETE_DEPS", + }, + { + name: "duplicate detection", + yml: strings.Replace(baseYml, "depends_on: [preflight] # BUILD_COMPLETE_DEPS", "depends_on: [preflight, deploy-studio-api] # BUILD_COMPLETE_DEPS", 1), + stepName: "deploy-studio-api", + wantDeps: "depends_on: [preflight, deploy-studio-api] # BUILD_COMPLETE_DEPS", + }, + { + name: "missing marker returns unchanged", + yml: "steps:\n build-complete:\n image: alpine:3.19", + stepName: "deploy-foo", + wantDeps: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := addBuildCompleteDep(tc.yml, tc.stepName) + + if tc.wantDeps == "" { + // Expect no change + if got != tc.yml { + t.Errorf("expected unchanged yml, but got modification") + } + return + } + + if !strings.Contains(got, tc.wantDeps) { + t.Errorf("result does not contain expected depends_on line\nwant: %s\ngot:\n%s", tc.wantDeps, got) + } + }) + } +} + +func TestAddBuildCompleteDep_PreservesIndentation(t *testing.T) { + yml := `steps: + build-complete: + depends_on: [preflight] # BUILD_COMPLETE_DEPS + image: alpine:3.19` + + got := addBuildCompleteDep(yml, "deploy-foo") + + // The indentation (8 spaces) should be preserved + want := " depends_on: [preflight, deploy-foo] # BUILD_COMPLETE_DEPS" + if !strings.Contains(got, want) { + t.Errorf("indentation not preserved\nwant line: %q\ngot:\n%s", want, got) + } +} + +func TestUpdateWoodpeckerYml_WiresBuildCompleteDeps(t *testing.T) { + skeleton := `steps: + preflight: + depends_on: [deps] + image: alpine/curl + + # COMPONENT_STEPS_BELOW + # Do not remove the marker above + + build-complete: + depends_on: [preflight] # BUILD_COMPLETE_DEPS + image: alpine:3.19 + commands: + - echo "All component builds complete"` + + stepYaml := `build-studio-api: + depends_on: [preflight] + image: woodpeckerci/plugin-kaniko + +verify-studio-api: + depends_on: [build-studio-api] + image: alpine/curl + +deploy-studio-api: + depends_on: [verify-studio-api] + image: bitnami/kubectl:latest` + + svc := &ComponentService{} + result := svc.updateWoodpeckerYml(skeleton, stepYaml) + + // Verify step YAML was inserted after marker + if !strings.Contains(result, " build-studio-api:") { + t.Error("component build step not inserted") + } + if !strings.Contains(result, " verify-studio-api:") { + t.Error("component verify step not inserted") + } + if !strings.Contains(result, " deploy-studio-api:") { + t.Error("component deploy step not inserted") + } + + // Verify build-complete depends_on was updated + if !strings.Contains(result, "depends_on: [preflight, deploy-studio-api] # BUILD_COMPLETE_DEPS") { + t.Errorf("build-complete depends_on not updated\ngot:\n%s", result) + } + + // Verify marker is preserved for future components + if !strings.Contains(result, "# COMPONENT_STEPS_BELOW") { + t.Error("COMPONENT_STEPS_BELOW marker was removed") + } +} + +func TestUpdateWoodpeckerYml_TwoComponents(t *testing.T) { + skeleton := `steps: + preflight: + depends_on: [deps] + + # COMPONENT_STEPS_BELOW + + build-complete: + depends_on: [preflight] # BUILD_COMPLETE_DEPS + image: alpine:3.19` + + step1 := `build-studio-api: + depends_on: [preflight] + +verify-studio-api: + depends_on: [build-studio-api] + +deploy-studio-api: + depends_on: [verify-studio-api]` + + step2 := `build-studio-ui: + depends_on: [preflight] + +verify-studio-ui: + depends_on: [build-studio-ui] + +deploy-studio-ui: + depends_on: [verify-studio-ui]` + + svc := &ComponentService{} + + // Add first component + result := svc.updateWoodpeckerYml(skeleton, step1) + + // Add second component + result = svc.updateWoodpeckerYml(result, step2) + + // Both deploy steps should be in build-complete depends_on + if !strings.Contains(result, "depends_on: [preflight, deploy-studio-api, deploy-studio-ui] # BUILD_COMPLETE_DEPS") { + t.Errorf("build-complete doesn't depend on both deploy steps\ngot:\n%s", result) + } +} + +func TestUpdateWoodpeckerYml_CLISkipsBuildCompleteDep(t *testing.T) { + skeleton := `steps: + # COMPONENT_STEPS_BELOW + + build-complete: + depends_on: [preflight] # BUILD_COMPLETE_DEPS + image: alpine:3.19` + + cliStep := `build-mytool: + depends_on: [deps] + image: golang:1.25-alpine + commands: + - go build ./cmd` + + svc := &ComponentService{} + result := svc.updateWoodpeckerYml(skeleton, cliStep) + + // CLI has no deploy step, so build-complete should stay unchanged + if !strings.Contains(result, "depends_on: [preflight] # BUILD_COMPLETE_DEPS") { + t.Errorf("build-complete was modified for CLI component (no deploy step)\ngot:\n%s", result) + } +} + +func TestUpdateWoodpeckerYml_MissingMarker(t *testing.T) { + noMarker := `steps: + build: + image: golang:1.25` + + svc := &ComponentService{} + result := svc.updateWoodpeckerYml(noMarker, "deploy-foo:\n image: test") + + if result != noMarker { + t.Error("expected unchanged yml when marker is missing") + } +}