load tile_name variable from bake configuration

Co-authored-by: Ramkumar Vengadakrishnan <[email protected]>

internal/acceptance/workflows/baking_a_tile.feature

@@ -26,6 +26,13 @@ Feature: As a developer, I want to bake a tile
       | --stub-releases |
     Then a Tile is created
 
+  Scenario: it handles tiles with multiple tile names
+    Given I have a tile source directory "testdata/tiles/multiple-tile-names"
+    When I invoke kiln
+      | bake                |
+      | --tile-name=goodbye |
+    Then a Tile is created
+
   Scenario: it bakes a tile from a bake record
     Given I have a tile source directory "testdata/tiles/bake-record"
     When I invoke kiln

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/Kilnfile

@@ -0,0 +1,19 @@
+bake_configurations:
+  - tile_name: hello
+    metadata_filepath: product_template.yml
+    variables_filepaths:
+      - variables/hello.yml
+    icon_filepath: "gopher.png"
+    instance_groups_directories:
+      - job_types
+    properties_directories:
+      - configuration
+  - tile_name: goodbye
+    metadata_filepath: product_template.yml
+    variables_filepaths:
+      - variables/goodbye.yml
+    icon_filepath: "gopher.png"
+    instance_groups_directories:
+      - job_types
+    properties_directories:
+      - configuration

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/Kilnfile.lock

@@ -0,0 +1,3 @@
+stemcell_criteria:
+  os: ubuntu-jammy
+  version: "1.329"

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/configuration/networking.yml

@@ -0,0 +1,5 @@
+---
+- name: port
+  type: port
+  configurable: true
+  default: 8080

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/job_types/hello.yml

@@ -0,0 +1,55 @@
+name: hello-server
+label: Server
+resource_label: Server
+description: HTTP Server
+
+templates: []
+
+static_ip: 1
+dynamic_ip: 0
+
+max_in_flight: 1
+single_az_only: true
+
+instance_definition:
+  name: instances
+  type: integer
+  label: Instances
+  configurable: true
+  default: 1
+  constraints:
+    min: 0
+    max: 1
+
+resource_definitions:
+  - name: ram
+    type: integer
+    label: RAM
+    configurable: true
+    default: 1024
+    constraints:
+      min: 1024
+
+  - name: ephemeral_disk
+    type: integer
+    label: Ephemeral Disk
+    configurable: true
+    default: 4000
+    constraints:
+      min: 2000
+
+  - name: persistent_disk
+    type: integer
+    label: Persistent Disk
+    configurable: false
+    default: 4000
+    constraints:
+      min: 2000
+
+  - name: cpu
+    type: integer
+    label: CPU
+    configurable: true
+    default: 1
+    constraints:
+      min: 1

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/product_template.yml

@@ -0,0 +1,29 @@
+---
+name: $( variable "tile_name" )
+label: "some label"
+description: "some description"
+icon_image: $( icon )
+
+metadata_version: "2.7.0"
+minimum_version_for_upgrade: 0.1.0
+product_version: $( version )
+provides_product_versions:
+  - name: hello
+    version: $( version )
+
+rank: 90
+serial: false
+
+releases: []
+
+stemcell_criteria: $( stemcell )
+
+job_types:
+  - $( instance_group "hello-server" )
+
+runtime_configs: []
+
+property_blueprints:
+  - $( property "port" )
+
+form_types: []

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/variables/goodbye.yml

@@ -0,0 +1,2 @@
+label: "goodbye"
+description: "some description"

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/variables/hello.yml

@@ -0,0 +1,2 @@
+label: "hello"
+description: "some description"

internal/acceptance/workflows/testdata/tiles/multiple-tile-names/version

@@ -0,0 +1 @@
+1.2.3

internal/commands/bake.go

@@ -444,23 +444,22 @@ func (b Bake) Execute(args []string) error {
 		b.errLogger.Println("warning: --stemcell-tarball is being deprecated in favor of --stemcells-directory")
 	}
 
-	var templateVariables map[string]any
-	if b.Options.TileName != "" {
-		if templateVariables == nil {
-			templateVariables = make(map[string]any)
-		}
-		templateVariables[builder.TileNameVariable] = b.Options.TileName
-	}
-
-	if err := BakeArgumentsFromKilnfileConfiguration(&b.Options, templateVariables, b.loadKilnfile); err != nil {
+	if err := BakeArgumentsFromKilnfileConfiguration(&b.Options, b.loadKilnfile); err != nil {
 		return fmt.Errorf("failed to load bake configuration from Kilnfile: %w", err)
 	}
 
-	templateVariables, err = b.templateVariables.FromPathsAndPairs(b.Options.VariableFiles, b.Options.Variables)
+	templateVariables, err := b.templateVariables.FromPathsAndPairs(b.Options.VariableFiles, b.Options.Variables)
 	if err != nil {
 		return fmt.Errorf("failed to parse template variables: %s", err)
 	}
 
+	if b.Options.TileName != "" {
+		if tileNameVariable, ok := templateVariables[builder.TileNameVariable]; ok && tileNameVariable != b.Options.TileName {
+			return fmt.Errorf("tile-name flag value %q does not match tile_name variable %q", b.Options.TileName, tileNameVariable)
+		}
+		templateVariables[builder.TileNameVariable] = b.Options.TileName
+	}
+
 	releaseManifests, err := b.releases.FromDirectories(b.Options.ReleaseDirectories)
 	if err != nil {
 		return fmt.Errorf("failed to parse releases: %s", err)
@@ -613,7 +612,7 @@ func (b Bake) Usage() jhanda.Usage {
 	}
 }
 
-func BakeArgumentsFromKilnfileConfiguration(options *BakeOptions, variables map[string]any, loadKilnfile func(string) (cargo.Kilnfile, error)) error {
+func BakeArgumentsFromKilnfileConfiguration(options *BakeOptions, loadKilnfile func(string) (cargo.Kilnfile, error)) error {
 	if options.Kilnfile == "" || options.StemcellTarball != "" || len(options.StemcellsDirectories) > 0 {
 		return nil
 	}
@@ -622,14 +621,6 @@ func BakeArgumentsFromKilnfileConfiguration(options *BakeOptions, variables map[
 		return err
 	}
 
-	if variableValue, ok := variables[builder.TileNameVariable]; ok {
-		variableString, ok := variableValue.(string)
-		if !ok {
-			return fmt.Errorf("%s value must be a string got value %#[2]v with type %[2]T", builder.TileNameVariable, variableValue)
-		}
-		options.TileName = variableString
-	}
-
 	if len(kf.BakeConfigurations) == 0 {
 		return nil
 	} else if len(kf.BakeConfigurations) == 1 {
@@ -638,7 +629,8 @@ func BakeArgumentsFromKilnfileConfiguration(options *BakeOptions, variables map[
 			return fmt.Errorf("the provided tile_name %q does not match the configuration %q", options.TileName, configuration.TileName)
 		}
 		fromConfiguration(options, configuration)
-	} else if _, ok := variables[builder.TileNameVariable]; ok {
+		options.TileName = configuration.TileName
+	} else {
 		index := slices.IndexFunc(kf.BakeConfigurations, func(configuration cargo.BakeConfiguration) bool {
 			return configuration.TileName == options.TileName
 		})

internal/commands/bake_test.go

@@ -402,7 +402,7 @@ var _ = Describe("Bake", func() {
 				bake = bake.WithKilnfileFunc(func(s string) (cargo.Kilnfile, error) {
 					return cargo.Kilnfile{
 						BakeConfigurations: []cargo.BakeConfiguration{
-							{Metadata: "peach.yml"},
+							{TileName: "p-each", Metadata: "peach.yml"},
 						},
 					}, nil
 				})
@@ -414,6 +414,14 @@ var _ = Describe("Bake", func() {
 					Expect(fakeMetadataService.ReadArgsForCall(0)).To(Equal("peach.yml"))
 				})
 			})
+			When("generating metadata", func() {
+				It("it uses the value from the bake configuration", func() {
+					err := bake.Execute([]string{})
+					Expect(err).To(Not(HaveOccurred()))
+					input, _, _ := fakeInterpolator.InterpolateArgsForCall(0)
+					Expect(input.Variables).To(HaveKeyWithValue("tile_name", "p-each"))
+				})
+			})
 		})
 		Context("when bake configuration has multiple options", func() {
 			BeforeEach(func() {
@@ -995,13 +1003,10 @@ var _ = Describe("Bake", func() {
 
 var _ = Describe("BakeArgumentsFromKilnfileConfiguration", func() {
 	It("handles empty options and variables", func() {
-		opts := &commands.BakeOptions{}
-		variables := map[string]any{}
 		loadKilnfile := func(s string) (cargo.Kilnfile, error) {
 			return cargo.Kilnfile{}, nil
 		}
-
-		err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, loadKilnfile)
+		err := commands.BakeArgumentsFromKilnfileConfiguration(new(commands.BakeOptions), loadKilnfile)
 		Expect(err).NotTo(HaveOccurred())
 	})
 
@@ -1011,13 +1016,12 @@ var _ = Describe("BakeArgumentsFromKilnfileConfiguration", func() {
 				Kilnfile: "non-empty-path/Kilnfile",
 			},
 		}
-		variables := map[string]any{}
 
 		loadKilnfile := func(s string) (cargo.Kilnfile, error) {
 			return cargo.Kilnfile{}, os.ErrNotExist
 		}
 
-		err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, loadKilnfile)
+		err := commands.BakeArgumentsFromKilnfileConfiguration(opts, loadKilnfile)
 		Expect(err).To(HaveOccurred())
 	})
 
@@ -1034,16 +1038,14 @@ var _ = Describe("BakeArgumentsFromKilnfileConfiguration", func() {
 			}
 		})
 
-		It("handles empty variables", func() {
+		It("handles empty TileName", func() {
 			var kilnfilePathArg string
 			loadKilnfile := func(s string) (cargo.Kilnfile, error) {
 				kilnfilePathArg = s
 				return cargo.Kilnfile{}, nil
 			}
-
-			variables := map[string]any{}
-
-			err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, loadKilnfile)
+			opts.TileName = ""
+			err := commands.BakeArgumentsFromKilnfileConfiguration(opts, loadKilnfile)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(kilnfilePathArg).To(Equal(kilnfilePath))
 		})
@@ -1061,16 +1063,15 @@ var _ = Describe("BakeArgumentsFromKilnfileConfiguration", func() {
 			})
 			When("tile_name is unset", func() {
 				It("handles getting the first configuration", func() {
-					variables := make(map[string]any)
-					err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, loadKilnfile)
+					err := commands.BakeArgumentsFromKilnfileConfiguration(opts, loadKilnfile)
 					Expect(err).NotTo(HaveOccurred())
 					Expect(opts.Metadata).To(Equal("peach.yml"))
 				})
 			})
 			When("a tile_name is a variable and does not match the bake configuration", func() {
 				It("handles getting the first configuration", func() {
-					variables := map[string]any{builder.TileNameVariable: "banana"}
-					err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, loadKilnfile)
+					opts.TileName = "banana"
+					err := commands.BakeArgumentsFromKilnfileConfiguration(opts, loadKilnfile)
 					Expect(err).To(HaveOccurred())
 				})
 			})
@@ -1095,14 +1096,14 @@ var _ = Describe("BakeArgumentsFromKilnfileConfiguration", func() {
 				}
 			})
 			It("handles getting the first configuration by name", func() {
-				variables := map[string]any{"tile_name": "pair"}
-				err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, loadKilnfile)
+				opts.TileName = "pair"
+				err := commands.BakeArgumentsFromKilnfileConfiguration(opts, loadKilnfile)
 				Expect(err).NotTo(HaveOccurred())
 				Expect(opts.Metadata).To(Equal("pair.yml"))
 			})
 			It("handles getting the second configuration by name", func() {
-				variables := map[string]any{"tile_name": "peach"}
-				err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, loadKilnfile)
+				opts.TileName = "peach"
+				err := commands.BakeArgumentsFromKilnfileConfiguration(opts, loadKilnfile)
 				Expect(err).NotTo(HaveOccurred())
 				Expect(opts.Metadata).To(Equal("peach.yml"))
 			})
@@ -1113,28 +1114,6 @@ var _ = Describe("BakeArgumentsFromKilnfileConfiguration", func() {
 			//	Expect(opts.Metadata).To(Equal("peach.yml"))
 			//})
 		})
-
-		It("returns an error for unexpected tile_name type", func() {
-			variables := map[string]any{"tile_name": 8675309}
-
-			err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, func(s string) (cargo.Kilnfile, error) {
-				return cargo.Kilnfile{
-					BakeConfigurations: []cargo.BakeConfiguration{{TileName: "apple"}},
-				}, nil
-			})
-			Expect(err).To(MatchError("tile_name value must be a string got value 8675309 with type int"))
-		})
-
-		It("returns an error for unexpected tile_name type", func() {
-			variables := map[string]any{"tile_name": 8675309}
-
-			err := commands.BakeArgumentsFromKilnfileConfiguration(opts, variables, func(s string) (cargo.Kilnfile, error) {
-				return cargo.Kilnfile{
-					BakeConfigurations: []cargo.BakeConfiguration{{TileName: "apple"}},
-				}, nil
-			})
-			Expect(err).To(MatchError("tile_name value must be a string got value 8675309 with type int"))
-		})
 	})
 
 })