Skip to content

Commit e907192

Browse files
authored
fix: variable file default discovery for nested tiles (#538)
There's a bug where Kiln `re-bake` wasn't accounting for the relative tile / Kilnfile path when automatically discovering variable files. Since `re-bake` relies heavily on convention in typical use, we'd like to fix this. ### Reproducing the bug I forked `hello-tile` and modified it to have a nested tile directory with a single variable `$( variable "hellothere" )` to be interpolated in its `base.yml`. I created a variables file under `nest/variables/hellothere.yml` defining its namesake. It's expected at this point that we need to pass the `--variables-file` explicitly because `kiln` doesn't know about the nesting yet: **Creating the record** ```sh kiln bake --final --version 6 \ --vr artifactory_host=<redacted> \ --vr artifactory_repo=<redacted> \ --vr artifactory_username=<redacted> \ --vr artifactory_password=<redacted> \ --metadata nest/base.yml \ --icon=nest/icon.png \ --kilnfile nest/Kilnfile \ --variables-file nest/variables/hellothere.yml Setting default credentials from ~/.kiln/credentials.yml. (hint: --variable-file overrides this default. --variable overrides both.) Warning: The "allow-only-publishable-releases" flag was not set. Some fetched releases may be intended for development/testing only. EXERCISE CAUTION WHEN PUBLISHING A TILE WITH THESE RELEASES! Gathering releases... All releases already downloaded Reading release manifests... Reading stemcell criteria from Kilnfile.lock Encoding icon... Building tile-6.pivotal... Adding metadata/metadata.yml to tile-6.pivotal... Creating empty migrations folder in tile-6.pivotal... Adding releases/hello-release-1.0.5-ubuntu-jammy-1.737.tgz to tile-6.pivotal… ``` This is expected behavior. Then we find the bug: **Re-baking the record** ```sh mv nest/bake_records/6.json /tmp/6.json mv tile-6.pivotal ~/workspace/tilediff/tile-6-new.zip kiln re-bake --output-file /tmp/tile-6-rebake.pivotal /tmp/6.json Setting default credentials from ~/.kiln/credentials.yml. (hint: --variable-file overrides this default. --variable overrides both.) Warning: The "allow-only-publishable-releases" flag was not set. Some fetched releases may be intended for development/testing only. EXERCISE CAUTION WHEN PUBLISHING A TILE WITH THESE RELEASES! Gathering releases... All releases already downloaded Reading release manifests... Reading stemcell criteria from Kilnfile.lock Encoding icon... 2025/03/13 13:45:30 could not execute "re-bake": failed when rendering a template: nest/base.yml:3:10: executing "nest/base.yml" at <variable "hellothere">: error calling variable: could not find variable with key 'hellothere' ``` ### The fix Essentially we're including the nested directory when available during our variable files search. We define ```go func getVariablesDir(fs flags.FileSystem, kilnfilePath string) string ``` and then use it during the search: ```go func getVariablesFilePaths(fs flags.FileSystem, kilnfilePath string) ([]string, error) { variablesDirPath := getVariablesDir(fs, kilnfilePath) files, err := fs.ReadDir(variablesDirPath) ... ``` and ```go func variablesDirPresent(fs flags.FileSystem, kilnfilePath string) bool { variablesDirPath := getVariablesDir(fs, kilnfilePath) file, err := fs.Stat(variablesDirPath) ... ``` ### Testing the fix Let's build this branch as `kiln-dev` and then re-run the above commands in an equivalent way: **Creating the record** ```sh kiln-dev bake --final --version 6 \ --vr artifactory_host=<redacted> \ --vr artifactory_repo=<redacted> \ --vr artifactory_username=<redacted> \ --vr artifactory_password=<redacted> \ --metadata nest/base.yml \ --icon=nest/icon.png \ --kilnfile nest/Kilnfile Setting default credentials from ~/.kiln/credentials.yml. (hint: --variable-file overrides this default. --variable overrides both.) Warning: The "allow-only-publishable-releases" flag was not set. Some fetched releases may be intended for development/testing only. EXERCISE CAUTION WHEN PUBLISHING A TILE WITH THESE RELEASES! Gathering releases... All releases already downloaded Reading release manifests... Reading stemcell criteria from Kilnfile.lock Encoding icon... Building tile-6.pivotal... Adding metadata/metadata.yml to tile-6.pivotal... Creating empty migrations folder in tile-6.pivotal... Adding releases/hello-release-1.0.5-ubuntu-jammy-1.737.tgz to tile-6.pivotal... ``` **Re-baking the record** ```sh kiln-dev re-bake --output-file /tmp/tile-6-rebake.pivotal /tmp/6.json Setting default credentials from ~/.kiln/credentials.yml. (hint: --variable-file overrides this default. --variable overrides both.) Warning: The "allow-only-publishable-releases" flag was not set. Some fetched releases may be intended for development/testing only. EXERCISE CAUTION WHEN PUBLISHING A TILE WITH THESE RELEASES! Gathering releases... All releases already downloaded Reading release manifests... Reading stemcell criteria from Kilnfile.lock Encoding icon... Building /tmp/tile-6-rebake.pivotal... Adding metadata/metadata.yml to /tmp/tile-6-rebake.pivotal... Creating empty migrations folder in /tmp/tile-6-rebake.pivotal... Adding releases/hello-release-1.0.5-ubuntu-jammy-1.737.tgz to /tmp/tile-6-rebake.pivotal… ``` fixing the issue.
2 parents 4553049 + 4701812 commit e907192

2 files changed

Lines changed: 76 additions & 9 deletions

File tree

internal/commands/bake.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -299,25 +299,39 @@ func shouldNotUseDefaultKilnfileFlag(args []string) bool {
299299
!flags.IsSet("kf", "kilnfile", args)
300300
}
301301

302-
func variablesDirPresent(fs flags.FileSystem) bool {
303-
file, err := fs.Stat("variables")
302+
func variablesDirPresent(fs flags.FileSystem, kilnfilePath string) bool {
303+
variablesDirPath := getVariablesDir(fs, kilnfilePath)
304+
file, err := fs.Stat(variablesDirPath)
304305
return err == nil && file != nil
305306
}
306307

307-
func getVariablesFilePaths(fs flags.FileSystem) ([]string, error) {
308-
files, err := fs.ReadDir("variables")
308+
func getVariablesFilePaths(fs flags.FileSystem, kilnfilePath string) ([]string, error) {
309+
variablesDirPath := getVariablesDir(fs, kilnfilePath)
310+
files, err := fs.ReadDir(variablesDirPath)
309311
if err != nil {
310312
return nil, err
311313
}
312314
var varFiles []string
313315
for _, file := range files {
314316
if strings.HasSuffix(file.Name(), ".yml") {
315-
varFiles = append(varFiles, "variables/"+file.Name())
317+
varFiles = append(varFiles, filepath.Join(variablesDirPath, file.Name()))
316318
}
317319
}
318320
return varFiles, nil
319321
}
320322

323+
func getVariablesDir(fs flags.FileSystem, kilnfilePath string) string {
324+
variablesDirPath := "variables"
325+
if kilnfilePath != "" {
326+
_, err := fs.Stat(kilnfilePath)
327+
if err == nil {
328+
kilnfileParentDir := filepath.Dir(kilnfilePath)
329+
variablesDirPath = filepath.Join(kilnfileParentDir, variablesDirPath)
330+
}
331+
}
332+
return variablesDirPath
333+
}
334+
321335
func (b *Bake) loadFlags(args []string) error {
322336
_, err := flags.LoadWithDefaultFilePaths(&b.Options, args, b.fs.Stat)
323337
if err != nil {
@@ -338,8 +352,9 @@ func (b *Bake) loadFlags(args []string) error {
338352
}
339353

340354
// setup default tile variables
341-
if variablesDirPresent(b.fs) {
342-
variablesFilePaths, err := getVariablesFilePaths(b.fs)
355+
kilnfilePath := b.Options.Kilnfile
356+
if variablesDirPresent(b.fs, kilnfilePath) {
357+
variablesFilePaths, err := getVariablesFilePaths(b.fs, kilnfilePath)
343358
if err == nil {
344359
if noTileVariablesFileAlreadySet(b, variablesFilePaths) {
345360
setADefaultTileVariablesFile(b, variablesFilePaths)

internal/commands/bake_test.go

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,59 @@ var _ = Describe("Bake", func() {
583583
})
584584

585585
Context("when Kilnfile is specified", func() {
586+
Context("when variable files can be auto discovered", func() {
587+
It("renders with the discovered variables file", func() {
588+
f1 := &fakes.FileInfo{}
589+
f1.NameReturns("variables-file-1.yml")
590+
f1.SizeReturns(100)
591+
f1.ModeReturns(os.ModePerm)
592+
f1.ModTimeReturns(time.Now())
593+
f1.IsDirReturns(false)
594+
595+
f2 := &fakes.FileInfo{}
596+
f2.NameReturns("variables-file-2.yml")
597+
f2.SizeReturns(100)
598+
f2.ModeReturns(os.ModePerm)
599+
f2.ModTimeReturns(time.Now())
600+
f2.IsDirReturns(false)
601+
602+
fakeFilesystem.ReadDirReturns([]os.FileInfo{f1, f2}, nil)
603+
604+
outputFile := "some-output-dir/some-product-file-1.2.3-build.4"
605+
err := bake.Execute([]string{
606+
"--forms-directory", "some-forms-directory",
607+
"--instance-groups-directory", "some-instance-groups-directory",
608+
"--jobs-directory", "some-jobs-directory",
609+
"--metadata", "some-metadata",
610+
"--output-file", outputFile,
611+
"--properties-directory", "some-properties-directory",
612+
"--releases-directory", someReleasesDirectory,
613+
"--runtime-configs-directory", "some-other-runtime-configs-directory",
614+
"--kilnfile", "Kilnfile",
615+
"--bosh-variables-directory", "some-variables-directory",
616+
"--version", "1.2.3", "--migrations-directory", "some-migrations-directory",
617+
"--migrations-directory", "some-other-migrations-directory",
618+
"--variables-file", "variables/variables-file-1.yml",
619+
"--variables-file", "variables/variables-file-2.yml",
620+
})
621+
Expect(err).NotTo(HaveOccurred())
622+
Expect(fakeStemcellService.FromKilnfileCallCount()).To(Equal(1))
623+
Expect(fakeStemcellService.FromKilnfileArgsForCall(0)).To(Equal("Kilnfile"))
624+
Expect(fakeFetcher.ExecuteCallCount()).To(Equal(1))
625+
executeArgsForFetch := fakeFetcher.ExecuteArgsForCall(0)
626+
Expect(executeArgsForFetch).To(Equal([]string{
627+
"--kilnfile", "Kilnfile",
628+
"--variables-file", "variables/variables-file-1.yml",
629+
"--variables-file", "variables/variables-file-2.yml",
630+
"--variables-file", "/home/.kiln/credentials.yml",
631+
"--download-threads", "0",
632+
"--no-confirm",
633+
"--releases-directory",
634+
someReleasesDirectory,
635+
}))
636+
})
637+
})
638+
586639
It("renders the stemcell criteria in tile metadata from that specified the Kilnfile.lock", func() {
587640
outputFile := "some-output-dir/some-product-file-1.2.3-build.4"
588641
err := bake.Execute([]string{
@@ -606,8 +659,7 @@ var _ = Describe("Bake", func() {
606659
executeArgsForFetch := fakeFetcher.ExecuteArgsForCall(0)
607660
Expect(executeArgsForFetch).To(Equal([]string{
608661
"--kilnfile", "Kilnfile",
609-
"--variables-file",
610-
"/home/.kiln/credentials.yml",
662+
"--variables-file", "/home/.kiln/credentials.yml",
611663
"--download-threads", "0",
612664
"--no-confirm",
613665
"--releases-directory",

0 commit comments

Comments
 (0)