Skip to content

Commit

Permalink
Exporter does not re-use layer from volume cache if layer contents ar…
Browse files Browse the repository at this point in the history
…e corrupted

Signed-off-by: Natalie Arellano <[email protected]>
  • Loading branch information
natalieparellano committed Oct 22, 2024
1 parent 559e0d7 commit fe17ab4
Show file tree
Hide file tree
Showing 15 changed files with 432 additions and 330 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ install-mockgen:

install-golangci-lint:
@echo "> Installing golangci-lint..."
$(GOCMD) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57.2
$(GOCMD) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.61.0

lint: install-golangci-lint
@echo "> Linting code..."
Expand Down
1 change: 0 additions & 1 deletion acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const (
var (
latestPlatformAPI = api.Platform.Latest().String()
buildDir string
cacheFixtureDir string
)

func TestVersion(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion acceptance/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ func TestAnalyzer(t *testing.T) {

analyzeImage = analyzeTest.testImageRef
analyzerPath = analyzeTest.containerBinaryPath
cacheFixtureDir = filepath.Join("testdata", "cache-dir")
analyzeRegAuthConfig = analyzeTest.targetRegistry.authConfig
analyzeRegNetwork = analyzeTest.targetRegistry.network
analyzeDaemonFixtures = analyzeTest.targetDaemon.fixtures
Expand Down
1 change: 0 additions & 1 deletion acceptance/creator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ func TestCreator(t *testing.T) {

createImage = createTest.testImageRef
creatorPath = createTest.containerBinaryPath
cacheFixtureDir = filepath.Join("testdata", "creator", "cache-dir")
createRegAuthConfig = createTest.targetRegistry.authConfig
createRegNetwork = createTest.targetRegistry.network
createDaemonFixtures = createTest.targetDaemon.fixtures
Expand Down
691 changes: 380 additions & 311 deletions acceptance/exporter_test.go

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"buildpacks": [
{
"key": "corrupted_buildpack",
"version": "corrupted_v1",
"layers": {
"corrupted-layer": {
"sha": "sha256:258dfa0cc987efebc17559694866ebc91139e7c0e574f60d1d4092f53d7dff59",
"data": null,
"build": false,
"launch": true,
"cache": true
}
}
}
]
}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[types]
cache = true
launch = true
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
digest-not-match-data
5 changes: 5 additions & 0 deletions acceptance/testdata/exporter/container/layers/group.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,8 @@
id = "cacher_buildpack"
version = "cacher_v1"
api = "0.8"

[[group]]
id = "corrupted_buildpack"
version = "corrupted_v1"
api = "0.8"
1 change: 1 addition & 0 deletions cache/image_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ func (c *ImageCache) Commit() error {
return nil
}

// VerifyLayer returns an error if the layer contents do not match the provided sha.
func (c *ImageCache) VerifyLayer(_ string) error {
// we assume the registry is verifying digests for us
return nil
Expand Down
1 change: 1 addition & 0 deletions cache/volume_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func (c *VolumeCache) setupStagingDir() error {
return os.MkdirAll(c.stagingDir, 0777)
}

// VerifyLayer returns an error if the layer contents do not match the provided sha.
func (c *VolumeCache) VerifyLayer(diffID string) error {
layerRC, err := c.RetrieveLayer(diffID)
if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions cmd/lifecycle/restorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ func (r *restoreCmd) Exec() error {
return cmd.FailErr(err, "get digest reference for builder image")
}
analyzedMD.BuildImage = &files.ImageIdentifier{Reference: digestRef.String()}
cmd.DefaultLogger.Debugf("Adding build image info to analyzed metadata: ")
cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.BuildImage))
cmd.DefaultLogger.Debug("Adding build image info to analyzed metadata: ")
cmd.DefaultLogger.Debug(encoding.ToJSONMaybe(analyzedMD.BuildImage))
}
var (
runImage imgutil.Image
Expand Down Expand Up @@ -187,12 +187,12 @@ func (r *restoreCmd) updateAnalyzedMD(analyzedMD *files.Analyzed, runImage imgut
return cmd.FailErr(err, "read target data from run image")
}
}
cmd.DefaultLogger.Debugf("Run image info in analyzed metadata was: ")
cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.RunImage))
cmd.DefaultLogger.Debug("Run image info in analyzed metadata was: ")
cmd.DefaultLogger.Debug(encoding.ToJSONMaybe(analyzedMD.RunImage))
analyzedMD.RunImage.Reference = digestRef.String()
analyzedMD.RunImage.TargetMetadata = targetData
cmd.DefaultLogger.Debugf("Run image info in analyzed metadata is: ")
cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.RunImage))
cmd.DefaultLogger.Debug("Run image info in analyzed metadata is: ")
cmd.DefaultLogger.Debug(encoding.ToJSONMaybe(analyzedMD.RunImage))
return nil
}

Expand Down
24 changes: 16 additions & 8 deletions phase/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,23 @@ func (e *Exporter) addOrReuseCacheLayer(cache Cache, layerDir LayerDir, previous
return "", errors.Wrapf(err, "creating layer '%s'", layerDir.Identifier())
}
if layer.Digest == previousSHA {
e.Logger.Infof("Reusing cache layer '%s'\n", layer.ID)
e.Logger.Debugf("Layer '%s' SHA: %s\n", layer.ID, layer.Digest)
err = cache.ReuseLayer(previousSHA)
if err != nil {
isReadErr, readErr := c.IsReadErr(err)
if !isReadErr {
return "", errors.Wrapf(err, "reusing layer %s", layer.ID)
if err = cache.VerifyLayer(previousSHA); err == nil {
e.Logger.Infof("Reusing cache layer '%s'\n", layer.ID)
e.Logger.Debugf("Layer '%s' SHA: %s\n", layer.ID, layer.Digest)
if err = cache.ReuseLayer(previousSHA); err != nil {
if isReadErr, readErr := c.IsReadErr(err); isReadErr {
// we shouldn't get here, as VerifyLayer would've returned an error
e.Logger.Warnf("Skipping re-use for layer %s: %s", layer.ID, readErr.Error())
} else {
return "", errors.Wrapf(err, "reusing layer %s", layer.ID)
}
}
} else {
if isReadErr, readErr := c.IsReadErr(err); isReadErr {
e.Logger.Warnf("Skipping reuse for layer %s: %s", layer.ID, readErr.Error())
} else {
return "", errors.Wrapf(err, "verifying layer '%s'", layerDir.Identifier())
}
e.Logger.Warnf("Skipping re-use for layer %s: %s", layer.ID, readErr.Error())
}
}
e.Logger.Infof("Adding cache layer '%s'\n", layer.ID)
Expand Down
2 changes: 1 addition & 1 deletion phase/rebaser.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func (r *Rebaser) validateTarget(appImg imgutil.Image, newBaseImg imgutil.Image)
}
if rebasable == "false" {
if !r.Force {
return fmt.Errorf(msgAppImageNotMarkedRebasable + "; " + msgProvideForceToOverride)
return errors.New(msgAppImageNotMarkedRebasable + "; " + msgProvideForceToOverride)
}
r.Logger.Warn(msgAppImageNotMarkedRebasable)
}
Expand Down

0 comments on commit fe17ab4

Please sign in to comment.