Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes error interpolation case where err is nil #289

Merged
merged 2 commits into from
Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion postal/service.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package postal

import (
"errors"
"fmt"
"io"
"path/filepath"
Expand Down Expand Up @@ -179,7 +180,7 @@ func (s Service) Deliver(dependency Dependency, cnbPath, layerPath, platformPath
}

if !ok {
return fmt.Errorf("checksum does not match: %s", err)
return errors.New("failed to validate dependency: checksum does not match")
}

return nil
Expand Down
78 changes: 69 additions & 9 deletions postal/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,17 @@ version = "this is super not semver"
})

context("failure cases", func() {
context("when dependency mapping resolver fails", func() {
it.Before(func() {
mappingResolver.FindDependencyMappingCall.Returns.Error = fmt.Errorf("some dependency mapping error")
})
it("fails to find dependency mappings", func() {
err := deliver()

Expect(err).To(MatchError(ContainSubstring("some dependency mapping error")))
})
})

context("when the transport cannot fetch a dependency", func() {
it.Before(func() {
transport.DropCall.Returns.Error = errors.New("there was an error")
Expand Down Expand Up @@ -616,7 +627,7 @@ version = "this is super not semver"
"",
)

Expect(err).To(MatchError(ContainSubstring("checksum does not match")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is testing

packit/postal/service.go

Lines 172 to 175 in 86f0343

err = vacation.NewArchive(validatedReader).WithName(name).StripComponents(dependency.StripComponents).Decompress(layerPath)
if err != nil {
return err
}
I think. Is that intended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I missed that we had an earlier test that asserts we handle the error coming back from the decompression. Unfortunately, I've been unable to create a test case that covers what I really want to test, which is the validatedReader.Valid() conditional. I can't seem to create a tarball that triggers the situation I am able to see in a real dependency in a real buildpack.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What buildpacks are you seeing this error in?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have constructed a buffer that can get to the second failure state. https://gist.github.com/ForestEckhardt/84eb2305c4626e3b93a5457baed20764

Expect(err).To(MatchError("validation error: checksum does not match"))
})
})

Expand Down Expand Up @@ -684,15 +695,64 @@ version = "this is super not semver"
Expect(err).To(MatchError(ContainSubstring("failed to extract symlink")))
})
})
})
context("when dependency mapping resolver fails", func() {
it.Before(func() {
mappingResolver.FindDependencyMappingCall.Returns.Error = fmt.Errorf("some dependency mapping error")
})
it("fails to find dependency mappings", func() {
err := deliver()

Expect(err).To(MatchError(ContainSubstring("some dependency mapping error")))
context("when the has additional data in the byte stream", func() {
it.Before(func() {
var err error
layerPath, err = os.MkdirTemp("", "path")
Expect(err).NotTo(HaveOccurred())

buffer := bytes.NewBuffer(nil)
tw := tar.NewWriter(buffer)

file := "some-file"
Expect(tw.WriteHeader(&tar.Header{Name: file, Mode: 0755, Size: int64(len(file))})).To(Succeed())
_, err = tw.Write([]byte(file))
Expect(err).NotTo(HaveOccurred())

Expect(tw.Close()).To(Succeed())

sum := sha256.Sum256(buffer.Bytes())
dependencySHA = hex.EncodeToString(sum[:])

// Empty block is tricking tar reader into think that we have reached
// EOF becuase we have surpassed the maximum block header size
var block [1024]byte
_, err = buffer.Write(block[:])
Expect(err).NotTo(HaveOccurred())

_, err = buffer.WriteString("additional data")
Expect(err).NotTo(HaveOccurred())

transport.DropCall.Returns.ReadCloser = io.NopCloser(buffer)

deliver = func() error {
return service.Deliver(
postal.Dependency{
ID: "some-entry",
Stacks: []string{"some-stack"},
URI: "https://dependencies.example.com/dependencies/some-file-name.txt",
SHA256: dependencySHA,
Version: "1.2.3",
},
"some-cnb-path",
layerPath,
"some-platform-dir",
)
}
})

it.After(func() {
Expect(os.RemoveAll(layerPath)).To(Succeed())
})

it("returns an error", func() {
err := deliver()
Expect(err).To(MatchError("failed to validate dependency: checksum does not match"))

Expect(transport.DropCall.Receives.Root).To(Equal("some-cnb-path"))
Expect(transport.DropCall.Receives.Uri).To(Equal("https://dependencies.example.com/dependencies/some-file-name.txt"))
})
})
})
})
Expand Down