Merge pull request #2161 from buildpacks/fix/buildpack-toml-validation

When validating buildpack.toml, don't warn for keys that are actually defined
buildpacks · May 20, 2024 · e1d6106 · e1d6106
2 parents 302963e + 47ae7a3
commit e1d6106
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 4 deletions.
diff --git a/pkg/buildpack/buildpack.go b/pkg/buildpack/buildpack.go
@@ -79,7 +79,7 @@ func FromBuildpackRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactor
 		return nil, err
 	}
 	if len(undecodedKeys) > 0 {
-		logger.Warnf("Ignoring unexpected key(s) in descriptor for buildpack %s: %s", descriptor.EscapedID(), strings.Join(undecodedKeys, ","))
+		logger.Warnf("Ignoring unexpected key(s) in descriptor for buildpack %s: %s", descriptor.EscapedID(), strings.Join(undecodedKeys, ", "))
 	}
 	if err := detectPlatformSpecificValues(&descriptor, blob); err != nil {
 		return nil, err
@@ -101,7 +101,7 @@ func FromExtensionRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactor
 		return nil, err
 	}
 	if len(undecodedKeys) > 0 {
-		logger.Warnf("Ignoring unexpected key(s) in descriptor for extension %s: %s", descriptor.EscapedID(), strings.Join(undecodedKeys, ","))
+		logger.Warnf("Ignoring unexpected key(s) in descriptor for extension %s: %s", descriptor.EscapedID(), strings.Join(undecodedKeys, ", "))
 	}
 	if err := validateExtensionDescriptor(descriptor); err != nil {
 		return nil, err
@@ -130,6 +130,13 @@ func readDescriptor(kind string, descriptor interface{}, blob Blob) (undecodedKe
 
 	undecoded := md.Undecoded()
 	for _, k := range undecoded {
+		// FIXME: we should ideally update dist.ModuleInfo to expect sbom-formats, but this breaks other tests;
+		// it isn't possible to make [metadata] a decoded key because its type is undefined in the buildpack spec.
+		if k.String() == "metadata" || strings.HasPrefix(k.String(), "metadata.") ||
+			k.String() == "buildpack.sbom-formats" {
+			// buildpack.toml & extension.toml can contain [metadata] which is arbitrary
+			continue
+		}
 		undecodedKeys = append(undecodedKeys, k.String())
 	}
 

diff --git a/pkg/buildpack/buildpack_test.go b/pkg/buildpack/buildpack_test.go
@@ -513,6 +513,7 @@ api = "0.3"
 id = "bp.one"
 version = "1.2.3"
 homepage = "http://geocities.com/cool-bp"
+sbom-formats = ["this should not warn"]
 
 [[targets]]
 os = "some-os"
@@ -524,12 +525,15 @@ version = "some-distro-version"
 [[targets.distros]]
 name = "some-distro-name"
 versions = ["some-distro-version"]
+
+[metadata]
+this-key = "is totally allowed and should not warn"
 `))
 						return tarBuilder.Reader(archive.DefaultTarWriterFactory())
 					},
 				}, archive.DefaultTarWriterFactory(), logger)
 				h.AssertNil(t, err)
-				h.AssertContains(t, outBuf.String(), "Warning: Ignoring unexpected key(s) in descriptor for buildpack bp.one: targets.distributions,targets.distributions.name,targets.distributions.version,targets.distros.versions")
+				h.AssertContains(t, outBuf.String(), "Warning: Ignoring unexpected key(s) in descriptor for buildpack bp.one: targets.distributions, targets.distributions.name, targets.distributions.version, targets.distros.versions")
 			})
 		})
 	})

diff --git a/pkg/client/example_fetcher_test.go b/pkg/client/example_fetcher_test.go
@@ -54,6 +54,6 @@ func (f *fetcher) Fetch(_ context.Context, imageName string, _ image.FetchOption
 	return nil, errors.New("not implemented")
 }
 
-func (f *fetcher) CheckReadAccess(_ string, _ FetchOptions) bool {
+func (f *fetcher) CheckReadAccess(_ string, _ image.FetchOptions) bool {
 	return true
 }