Validate selected setup templates and review-app health

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,28 @@
       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 "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 +82,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 +92,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
 
@@ -85,12 +104,4 @@
       expect(result[:stderr]).to include("Invalid value provided for option --validations")
     end
   end
-
-  def stub_template_filenames(*names)
-    allow(Dir).to receive(:glob).and_wrap_original do |method, *args|
-      method.call(*args).select do |filename|
-        names.any? { |name| filename.end_with?("#{name}.yml") }
-      end
-    end
-  end
 end

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}}