Validate selected setup templates and review-app health

.github/workflows/cpflow-deploy-review-app.yml

@@ -28,6 +28,12 @@ concurrency:
 env:
   PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}
   PRIMARY_WORKLOAD: ${{ vars.PRIMARY_WORKLOAD }}
+  # Review apps use their own health knobs so teams can keep production
+  # promotion stricter or slower without changing PR feedback behavior.
+  REVIEW_APP_HEALTH_CHECK_RETRIES: ${{ vars.REVIEW_APP_HEALTH_CHECK_RETRIES || '24' }}
+  REVIEW_APP_HEALTH_CHECK_INTERVAL: ${{ vars.REVIEW_APP_HEALTH_CHECK_INTERVAL || '15' }}
+  REVIEW_APP_HEALTH_CHECK_ACCEPTED_STATUSES: ${{ vars.REVIEW_APP_HEALTH_CHECK_ACCEPTED_STATUSES || '200 301 302' }}
+  REVIEW_APP_HEALTH_CHECK_CURL_MAX_TIME: ${{ vars.REVIEW_APP_HEALTH_CHECK_CURL_MAX_TIME || '10' }}
 
 jobs:
   deploy:
@@ -439,6 +445,19 @@ jobs:
 
           cpflow deploy-image "${deploy_args[@]}"
 
+      - name: Wait for deployment health
+        if: steps.config.outputs.ready == 'true' && steps.source.outputs.allowed == 'true' && (steps.check-app.outputs.exists == 'true' || steps.setup-review-app.outcome == 'success')
+        id: health-check
+        uses: ./.cpflow/.github/actions/cpflow-wait-for-health
+        with:
+          workload_name: ${{ env.PRIMARY_WORKLOAD || 'rails' }}
+          app_name: ${{ steps.review-config.outputs.app_name }}
+          org: ${{ steps.review-config.outputs.cpln_org }}
+          max_retries: ${{ env.REVIEW_APP_HEALTH_CHECK_RETRIES }}
+          interval_seconds: ${{ env.REVIEW_APP_HEALTH_CHECK_INTERVAL }}
+          accepted_statuses: ${{ env.REVIEW_APP_HEALTH_CHECK_ACCEPTED_STATUSES }}
+          curl_max_time: ${{ env.REVIEW_APP_HEALTH_CHECK_CURL_MAX_TIME }}
+
       - name: Retrieve app URL
         if: steps.config.outputs.ready == 'true' && steps.source.outputs.allowed == 'true' && (steps.check-app.outputs.exists == 'true' || steps.setup-review-app.outcome == 'success')
         id: workload

docs/ci-automation.md

@@ -173,7 +173,11 @@ disambiguate generated review-app config:
 
 - `CPLN_ORG_STAGING`: override the staging/review org inferred from `cpln_org`, for example `company-staging`
 - `REVIEW_APP_PREFIX`: override the inferred review-app prefix; required only when multiple review app prefixes exist in `controlplane.yml`
-- `PRIMARY_WORKLOAD`: override the public workload used to discover the public endpoint and do production health checks; defaults to `rails`
+- `PRIMARY_WORKLOAD`: override the public workload used to discover the public endpoint and do review/production health checks; defaults to `rails`
+- `REVIEW_APP_HEALTH_CHECK_RETRIES`: override review-app health polling attempts; defaults to `24`
+- `REVIEW_APP_HEALTH_CHECK_INTERVAL`: override seconds between review-app health attempts; defaults to `15`
+- `REVIEW_APP_HEALTH_CHECK_ACCEPTED_STATUSES`: override space-separated healthy HTTP statuses for review apps; defaults to `200 301 302`
+- `REVIEW_APP_HEALTH_CHECK_CURL_MAX_TIME`: override per-request review-app curl timeout in seconds; defaults to `10`
 
 If `controlplane.yml` defines more than one app with
 `match_if_app_name_starts_with: true`, inference intentionally fails. Set
@@ -461,6 +465,12 @@ The action will start an SSH agent, add the key, write `known_hosts`, and pass `
 - Attempts a rollback of every configured application workload if the new production image does not come up healthy.
 - Creates a GitHub release after a successful promotion.
 
+`cpflow-deploy-review-app.yml`
+
+- Builds and deploys the pull request image after the review app exists or is created.
+- Waits for `PRIMARY_WORKLOAD` to report `readyLatest=true` and return an accepted HTTP status before marking the GitHub deployment successful.
+- Uses `REVIEW_APP_HEALTH_CHECK_*` repository variables to tune review-app health checks independently from production promotion.
+
 `cpflow-cleanup-stale-review-apps.yml`
 
 - Runs nightly and on demand.

docs/commands.md

@@ -554,7 +554,7 @@ cpflow terraform import
 Regenerates the generated cpflow GitHub Actions wrappers and helper files
 from the currently installed cpflow gem. Use this after updating the
 cpflow gem so checked-in workflow wrappers move to the matching upstream
-release tag, for example `v5.1.0`.
+release tag, for example `v5.1.1`.
 
 If the existing generated staging workflow uses a custom single staging
 branch, the command preserves it. Pass `--staging-branch BRANCH` to set or

lib/core/doctor_service.rb

@@ -40,8 +40,7 @@ def validate_config
 
   def validate_templates
     @template_parser = TemplateParser.new(@command)
-    filenames = Dir.glob("#{@template_parser.template_dir}/*.yml")
-    templates = @template_parser.parse(filenames)
+    templates = @template_parser.parse(template_filenames)
 
     check_for_duplicate_templates(templates)
     warn_deprecated_template_variables
@@ -88,6 +87,35 @@ def check_for_duplicate_templates(templates)
     raise ValidationError, "#{Shell.color("ERROR: #{message}", :red)}\n#{list}"
   end
 
+  def template_filenames
+    return existing_arg_template_filenames if config.args.any?
+
+    template_names = config.current[:setup_app_templates]
+    return Dir.glob("#{@template_parser.template_dir}/*.yml") if template_names.nil? || template_names.empty?
+
+    filenames = template_names.map { |name| @template_parser.template_filename(name) }
+    ensure_templates_exist!(template_names, filenames)
+    filenames
+  end
+
+  def existing_arg_template_filenames
+    filenames = config.args.map { |name| @template_parser.template_filename(name) }
+    return [] unless filenames.all? { |filename| File.exist?(filename) }
+
+    filenames
+  end
+
+  def ensure_templates_exist!(template_names, filenames)
+    missing_templates = template_names.zip(filenames).reject { |_, filename| File.exist?(filename) }
+    return if missing_templates.empty?
+
+    missing_templates_str = missing_templates.map do |name, filename|
+      "  - #{name} (#{filename})"
+    end.join("\n")
+    message = "#{Shell.color('Missing templates:', :red)}\n#{missing_templates_str}"
+    raise ValidationError, message
+  end
+
   def warn_deprecated_template_variables
     deprecated_variables = @template_parser.deprecated_variables
     return if deprecated_variables.empty?

lib/core/template_parser.rb

@@ -42,12 +42,11 @@ def replace_variables(yaml_file) # rubocop:disable Metrics/MethodLength
                 .gsub("{{APP_NAME}}", config.app)
                 .gsub("{{APP_LOCATION}}", config.location)
                 .gsub("{{APP_LOCATION_LINK}}", config.location_link)
-                .gsub("{{APP_IMAGE}}", cp.latest_image)
-                .gsub("{{APP_IMAGE_LINK}}", config.image_link(cp.latest_image))
                 .gsub("{{APP_IDENTITY}}", config.identity)
                 .gsub("{{APP_IDENTITY_LINK}}", config.identity_link)
                 .gsub("{{APP_SECRETS}}", config.secrets)
                 .gsub("{{APP_SECRETS_POLICY}}", config.secrets_policy)
+    yaml_file = replace_image_variables(yaml_file)
 
     config.shared_secret_placeholders.each do |placeholder, secret_name|
       yaml_file = yaml_file.gsub(placeholder, secret_name)
@@ -60,7 +59,22 @@ def replace_variables(yaml_file) # rubocop:disable Metrics/MethodLength
       .gsub("APP_ORG", config.org)
       .gsub("APP_GVC", config.app)
       .gsub("APP_LOCATION", config.location)
-      .gsub("APP_IMAGE", cp.latest_image)
+      .then { |updated_yaml| replace_legacy_image_variable(updated_yaml) }
+  end
+
+  def replace_image_variables(yaml_file)
+    return yaml_file unless yaml_file.include?("{{APP_IMAGE}}") || yaml_file.include?("{{APP_IMAGE_LINK}}")
+
+    latest_image = cp.latest_image
+    yaml_file
+      .gsub("{{APP_IMAGE}}", latest_image)
+      .gsub("{{APP_IMAGE_LINK}}", config.image_link(latest_image))
+  end
+
+  def replace_legacy_image_variable(yaml_file)
+    return yaml_file unless yaml_file.include?("APP_IMAGE")
+
+    yaml_file.gsub("APP_IMAGE", cp.latest_image)
   end
 
   def find_deprecated_variables(yaml_file)

spec/command/doctor_spec.rb

@@ -33,7 +33,7 @@
   end
 
   context "when validating templates" do
-    let!(:app) { dummy_test_app }
+    let!(:app) { dummy_test_app("default") }
 
     it "raises error if app is not provided" do
       result = run_cpflow_command("doctor", "--validations", "templates")
@@ -42,8 +42,8 @@
       expect(result[:stderr]).to include("App is required for templates validation")
     end
 
-    it "fails if there are duplicate templates" do
-      stub_template_filenames("app", "app-without-identity", "rails")
+    it "fails if selected templates contain duplicate rendered kind/names" do
+      app = dummy_test_app("duplicate-templates")
 
       result = run_cpflow_command("doctor", "--validations", "templates", "-a", app)
 
@@ -52,9 +52,39 @@
       expect(result[:stderr]).to include("- kind: gvc, name: #{app}")
     end
 
-    it "passes if there are no issues" do
-      stub_template_filenames("app", "rails")
+    it "fails if a selected setup template is missing" do
+      app = dummy_test_app("missing-template")
+
+      result = run_cpflow_command("doctor", "--validations", "templates", "-a", app)
+
+      expect(result[:status]).not_to eq(0)
+      expect(result[:stderr]).to include("[FAIL] templates")
+      expect(result[:stderr]).to include("Missing templates")
+      expect(result[:stderr]).to include("- nonexistent")
+    end
+
+    it "validates all templates if no setup templates are selected" do
+      app = dummy_test_app("nothing")
+      stub_template_filenames("app", "app-without-identity")
+
+      result = run_cpflow_command("doctor", "--validations", "templates", "-a", app)
+
+      expect(result[:status]).not_to eq(0)
+      expect(result[:stderr]).to include("[FAIL] templates")
+      expect(result[:stderr]).to include("- kind: gvc, name: #{app}")
+    end
+
+    it "passes if unselected templates render duplicate kind/names" do
+      app = dummy_test_app("alternate-app-template")
+
+      result = run_cpflow_command("doctor", "--validations", "templates", "-a", app)
+
+      expect(result[:status]).to eq(0)
+      expect(result[:stderr]).to include("[PASS] templates")
+      expect(result[:stderr]).not_to include("DEPRECATED")
+    end
 
+    it "passes if selected templates have no issues" do
       result = run_cpflow_command("doctor", "--validations", "templates", "-a", app)
 
       expect(result[:status]).to eq(0)
@@ -63,7 +93,7 @@
     end
 
     it "warns about deprecated variables" do
-      stub_template_filenames("app-with-deprecated-variables")
+      app = dummy_test_app("deprecated-template")
 
       result = run_cpflow_command("doctor", "--validations", "templates", "-a", app)
 
@@ -73,7 +103,7 @@
       expect(result[:stderr]).to include("APP_ORG -> {{APP_ORG}}")
       expect(result[:stderr]).to include("APP_GVC -> {{APP_NAME}}")
       expect(result[:stderr]).to include("APP_LOCATION -> {{APP_LOCATION}}")
-      expect(result[:stderr]).to include("APP_IMAGE -> {{APP_IMAGE}}")
+      expect(result[:stderr]).not_to include("APP_IMAGE -> {{APP_IMAGE}}")
     end
   end
 

spec/command/generate_github_actions_spec.rb

@@ -636,6 +636,27 @@ def shared_workflow_path(name)
       expect(contents).to include("📋 [View Completed Action Build and Deploy Logs]")
     end
 
+    it "waits for review-app workload health before finalizing deployment success" do
+      contents = reusable_review_app_workflow_path.read
+
+      expect(contents).to include("- name: Wait for deployment health")
+      expect(contents).to include("id: health-check")
+      expect(contents).to include("uses: ./.cpflow/.github/actions/cpflow-wait-for-health")
+      expect(contents).to include("workload_name: ${{ env.PRIMARY_WORKLOAD || 'rails' }}")
+      expect(contents).to include("app_name: ${{ steps.review-config.outputs.app_name }}")
+      expect(contents).to include("org: ${{ steps.review-config.outputs.cpln_org }}")
+      expect(contents).to include(
+        "REVIEW_APP_HEALTH_CHECK_ACCEPTED_STATUSES: " \
+        "${{ vars.REVIEW_APP_HEALTH_CHECK_ACCEPTED_STATUSES || '200 301 302' }}"
+      )
+      expect(contents).to include("max_retries: ${{ env.REVIEW_APP_HEALTH_CHECK_RETRIES }}")
+      expect(contents).to include("interval_seconds: ${{ env.REVIEW_APP_HEALTH_CHECK_INTERVAL }}")
+      expect(contents).to include(
+        "accepted_statuses: ${{ env.REVIEW_APP_HEALTH_CHECK_ACCEPTED_STATUSES }}"
+      )
+      expect(contents).to include("curl_max_time: ${{ env.REVIEW_APP_HEALTH_CHECK_CURL_MAX_TIME }}")
+    end
+
     it "supports an animated deploy status icon with a repository override" do
       contents = reusable_review_app_workflow_path.read
 

spec/core/template_parser_spec.rb

@@ -57,5 +57,13 @@
         }
       )
     end
+
+    it "does not fetch the latest image when templates do not use image placeholders" do
+      allow(cp).to receive(:latest_image)
+
+      parser.parse([template_file.path])
+
+      expect(cp).not_to have_received(:latest_image)
+    end
   end
 end

spec/dummy/.controlplane/controlplane.yml

@@ -48,6 +48,35 @@ apps:
       - app
       - rails
 
+  dummy-test-duplicate-templates-{GLOBAL_IDENTIFIER}:
+    <<: *common
+
+    match_if_app_name_starts_with: true
+    setup_app_templates:
+      - app
+      - app-without-identity
+
+  dummy-test-alternate-app-template-{GLOBAL_IDENTIFIER}:
+    <<: *common
+
+    match_if_app_name_starts_with: true
+    setup_app_templates:
+      - app-review
+
+  dummy-test-deprecated-template-{GLOBAL_IDENTIFIER}:
+    <<: *common
+
+    match_if_app_name_starts_with: true
+    setup_app_templates:
+      - app-with-deprecated-variables-no-image
+
+  dummy-test-missing-template-{GLOBAL_IDENTIFIER}:
+    <<: *common
+
+    match_if_app_name_starts_with: true
+    setup_app_templates:
+      - nonexistent
+
   dummy-test-full-{GLOBAL_IDENTIFIER}:
     <<: *common
 

spec/dummy/.controlplane/templates/app-review.yml

@@ -0,0 +1,11 @@
+kind: gvc
+name: {{APP_NAME}}
+spec:
+  env:
+    - name: DATABASE_URL
+      value: postgres://postgres:password@shared-postgres.cpln.local:5432/{{APP_NAME}}
+    - name: SECRET_KEY_BASE
+      value: "123"
+  staticPlacement:
+    locationLinks:
+      - {{APP_LOCATION_LINK}}

spec/dummy/.controlplane/templates/app-with-deprecated-variables-no-image.yml

@@ -0,0 +1,16 @@
+kind: gvc
+name: {{APP_NAME}}
+spec:
+  env:
+    - name: ORG
+      value: APP_ORG
+    - name: NAME
+      value: APP_GVC
+    - name: LOCATION
+      value: APP_LOCATION
+  staticPlacement:
+    locationLinks:
+      - {{APP_LOCATION_LINK}}
+---
+kind: identity
+name: {{APP_IDENTITY}}