Skip to content

Commit

Permalink
Merge pull request #2042 from colincasey/fix_1922_silently_ignoring_k…
Browse files Browse the repository at this point in the history 
…eys_in_project_toml

Warn if project.toml contains keys not supported by schema
  • Loading branch information
@jjbustamante
jjbustamante authored Feb 12, 2024
2 parents 5cbc93e + bfaafe9 commit ded78f3
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 23 deletions.
6 changes: 3 additions & 3 deletions internal/commands/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob

inputPreviousImage := client.ParseInputImageReference(flags.PreviousImage)

descriptor, actualDescriptorPath, err := parseProjectToml(flags.AppPath, flags.DescriptorPath)
descriptor, actualDescriptorPath, err := parseProjectToml(flags.AppPath, flags.DescriptorPath, logger)
if err != nil {
return err
}
Expand Down Expand Up @@ -372,7 +372,7 @@ func addEnvVar(env map[string]string, item string) map[string]string {
return env
}

func parseProjectToml(appPath, descriptorPath string) (projectTypes.Descriptor, string, error) {
func parseProjectToml(appPath, descriptorPath string, logger logging.Logger) (projectTypes.Descriptor, string, error) {
actualPath := descriptorPath
computePath := descriptorPath == ""

Expand All @@ -387,7 +387,7 @@ func parseProjectToml(appPath, descriptorPath string) (projectTypes.Descriptor,
return projectTypes.Descriptor{}, "", errors.Wrap(err, "stat project descriptor")
}

descriptor, err := project.ReadProjectDescriptor(actualPath)
descriptor, err := project.ReadProjectDescriptor(actualPath, logger)
return descriptor, actualPath, err
}

Expand Down
31 changes: 28 additions & 3 deletions pkg/project/project.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/BurntSushi/toml"
"github.com/pkg/errors"

"github.com/buildpacks/pack/pkg/logging"
"github.com/buildpacks/pack/pkg/project/types"
v01 "github.com/buildpacks/pack/pkg/project/v01"
v02 "github.com/buildpacks/pack/pkg/project/v02"
Expand All @@ -21,12 +23,12 @@ type VersionDescriptor struct {
Project Project `toml:"_"`
}

var parsers = map[string]func(string) (types.Descriptor, error){
var parsers = map[string]func(string) (types.Descriptor, toml.MetaData, error){
"0.1": v01.NewDescriptor,
"0.2": v02.NewDescriptor,
}

func ReadProjectDescriptor(pathToFile string) (types.Descriptor, error) {
func ReadProjectDescriptor(pathToFile string, logger logging.Logger) (types.Descriptor, error) {
projectTomlContents, err := os.ReadFile(filepath.Clean(pathToFile))
if err != nil {
return types.Descriptor{}, err
Expand All @@ -45,21 +47,44 @@ func ReadProjectDescriptor(pathToFile string) (types.Descriptor, error) {

version := versionDescriptor.Project.Version
if version == "" {
logger.Warn("No schema version declared in project.toml, defaulting to schema version 0.1")
version = "0.1"
}

if _, ok := parsers[version]; !ok {
return types.Descriptor{}, fmt.Errorf("unknown project descriptor schema version %s", version)
}

descriptor, err := parsers[version](string(projectTomlContents))
descriptor, tomlMetaData, err := parsers[version](string(projectTomlContents))
if err != nil {
return types.Descriptor{}, err
}

warnIfTomlContainsKeysNotSupportedBySchema(version, tomlMetaData, logger)

return descriptor, validate(descriptor)
}

func warnIfTomlContainsKeysNotSupportedBySchema(schemaVersion string, tomlMetaData toml.MetaData, logger logging.Logger) {
unsupportedKeys := []string{}

// filter out any keys from [_]
for _, undecodedKey := range tomlMetaData.Undecoded() {
keyName := undecodedKey.String()
if keyName != "_" && !strings.HasPrefix(keyName, "_.schema-version") {
unsupportedKeys = append(unsupportedKeys, keyName)
}
}

if len(unsupportedKeys) != 0 {
logger.Warnf("The following keys declared in project.toml are not supported in schema version %s:\n", schemaVersion)
for _, unsupportedKey := range unsupportedKeys {
logger.Warnf("- %s\n", unsupportedKey)
}
logger.Warn("The above keys will be ignored. If this is not intentional, maybe try updating your schema version.\n")
}
}

func validate(p types.Descriptor) error {
if p.Build.Exclude != nil && p.Build.Include != nil {
return errors.New("project.toml: cannot have both include and exclude defined")
Expand Down
82 changes: 73 additions & 9 deletions pkg/project/project_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/sclevine/spec"
"github.com/sclevine/spec/report"

"github.com/buildpacks/pack/pkg/logging"
h "github.com/buildpacks/pack/testhelpers"
)

Expand All @@ -23,6 +24,18 @@ func TestProject(t *testing.T) {
}

func testProject(t *testing.T, when spec.G, it spec.S) {
var (
logger *logging.LogWithWriters
readStdout func() string
)

it.Before(func() {
var stdout *color.Console
stdout, readStdout = h.MockWriterAndOutput()
stderr, _ := h.MockWriterAndOutput()
logger = logging.NewLogWithWriters(stdout, stderr)
})

when("#ReadProjectDescriptor", func() {
it("should parse a valid v0.2 project.toml file", func() {
projectToml := `
Expand Down Expand Up @@ -56,7 +69,7 @@ value = "this-should-get-overridden-because-its-deprecated"
t.Fatal(err)
}

projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name())
projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -143,7 +156,7 @@ value = "-Xmx300m"
t.Fatal(err)
}

projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name())
projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -188,7 +201,7 @@ pipeline = "Lucerne"
t.Fatal(err)
}

projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name())
projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -272,7 +285,7 @@ name = "gallant"
t.Fatal(err)
}

projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name())
projectDescriptor, err := ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err != nil {
t.Fatal(err)
}
Expand All @@ -290,7 +303,7 @@ name = "gallant"
})

it("should fail for an invalid project.toml path", func() {
_, err := ReadProjectDescriptor("/path/that/does/not/exist/project.toml")
_, err := ReadProjectDescriptor("/path/that/does/not/exist/project.toml", logger)

if !os.IsNotExist(err) {
t.Fatalf("Expected\n-----\n%#v\n-----\nbut got\n-----\n%#v\n",
Expand All @@ -311,7 +324,7 @@ include = [ "*.jpg" ]
if err != nil {
t.Fatal(err)
}
_, err = ReadProjectDescriptor(tmpProjectToml.Name())
_, err = ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err == nil {
t.Fatalf(
"Expected error for having both exclude and include defined")
Expand All @@ -331,7 +344,7 @@ version = "1.2.3"
t.Fatal(err)
}

_, err = ReadProjectDescriptor(tmpProjectToml.Name())
_, err = ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err == nil {
t.Fatalf("Expected error for NOT having id or uri defined for buildpacks")
}
Expand All @@ -351,7 +364,7 @@ version = "1.2.3"
t.Fatal(err)
}

_, err = ReadProjectDescriptor(tmpProjectToml.Name())
_, err = ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err == nil {
t.Fatal("Expected error for having both uri and version defined for a buildpack(s)")
}
Expand All @@ -369,11 +382,62 @@ name = "licenses should have either a type or uri defined"
t.Fatal(err)
}

_, err = ReadProjectDescriptor(tmpProjectToml.Name())
_, err = ReadProjectDescriptor(tmpProjectToml.Name(), logger)
if err == nil {
t.Fatal("Expected error for having neither type or uri defined for licenses")
}
})

it("should warn when no schema version is declared", func() {
projectToml := ``
tmpProjectToml, err := createTmpProjectTomlFile(projectToml)
if err != nil {
t.Fatal(err)
}

_, err = ReadProjectDescriptor(tmpProjectToml.Name(), logger)
h.AssertNil(t, err)

h.AssertContains(t, readStdout(), "Warning: No schema version declared in project.toml, defaulting to schema version 0.1\n")
})

it("should warn when unsupported keys are declared with schema v0.1", func() {
projectToml := `
[_]
schema-version = "0.1"
[unsupported-table]
unsupported-key = "some value"
`
tmpProjectToml, err := createTmpProjectTomlFile(projectToml)
if err != nil {
t.Fatal(err)
}

_, err = ReadProjectDescriptor(tmpProjectToml.Name(), logger)
h.AssertNil(t, err)

h.AssertContains(t, readStdout(), "Warning: The following keys declared in project.toml are not supported in schema version 0.1:\nWarning: - unsupported-table\nWarning: - unsupported-table.unsupported-key\nWarning: The above keys will be ignored. If this is not intentional, maybe try updating your schema version.\n")
})

it("should warn when unsupported keys are declared with schema v0.2", func() {
projectToml := `
[_]
schema-version = "0.2"
[unsupported-table]
unsupported-key = "some value"
`
tmpProjectToml, err := createTmpProjectTomlFile(projectToml)
if err != nil {
t.Fatal(err)
}

_, err = ReadProjectDescriptor(tmpProjectToml.Name(), logger)
h.AssertNil(t, err)

h.AssertContains(t, readStdout(), "Warning: The following keys declared in project.toml are not supported in schema version 0.2:\nWarning: - unsupported-table\nWarning: - unsupported-table.unsupported-key\nWarning: The above keys will be ignored. If this is not intentional, maybe try updating your schema version.\n")
})
})
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/project/v01/project.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ type Descriptor struct {
Metadata map[string]interface{} `toml:"metadata"`
}

func NewDescriptor(projectTomlContents string) (types.Descriptor, error) {
func NewDescriptor(projectTomlContents string) (types.Descriptor, toml.MetaData, error) {
versionedDescriptor := &Descriptor{}

_, err := toml.Decode(projectTomlContents, versionedDescriptor)
tomlMetaData, err := toml.Decode(projectTomlContents, versionedDescriptor)
if err != nil {
return types.Descriptor{}, err
return types.Descriptor{}, tomlMetaData, err
}

return types.Descriptor{
Project: versionedDescriptor.Project,
Build: versionedDescriptor.Build,
Metadata: versionedDescriptor.Metadata,
SchemaVersion: api.MustParse("0.1"),
}, nil
}, tomlMetaData, nil
}
8 changes: 4 additions & 4 deletions pkg/project/v02/project.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ type Descriptor struct {
IO IO `toml:"io"`
}

func NewDescriptor(projectTomlContents string) (types.Descriptor, error) {
func NewDescriptor(projectTomlContents string) (types.Descriptor, toml.MetaData, error) {
versionedDescriptor := &Descriptor{}
_, err := toml.Decode(projectTomlContents, &versionedDescriptor)
tomlMetaData, err := toml.Decode(projectTomlContents, &versionedDescriptor)
if err != nil {
return types.Descriptor{}, err
return types.Descriptor{}, tomlMetaData, err
}

// backward compatibility for incorrect key
Expand All @@ -72,5 +72,5 @@ func NewDescriptor(projectTomlContents string) (types.Descriptor, error) {
},
Metadata: versionedDescriptor.Project.Metadata,
SchemaVersion: api.MustParse("0.2"),
}, nil
}, tomlMetaData, nil
}

0 comments on commit ded78f3

Please sign in to comment.