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

Move logic out of cmd/lifecycle/analyzer.go #805

Merged
merged 35 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5a4832b
Acceptance tests pass with creator commented out
natalieparellano Feb 22, 2022
ed1b3a7
Fix creator
natalieparellano Feb 22, 2022
fc4a7e3
Add unit tests for analyze inputs
natalieparellano Feb 22, 2022
2b505c6
Add scaffold for analyzer builder unit tests
natalieparellano Feb 22, 2022
6fef994
Add many unit tests, still have some TODOs
natalieparellano Feb 23, 2022
043974b
Address most TODOs
natalieparellano Feb 23, 2022
02a5c74
Acceptance tests pass
natalieparellano Feb 23, 2022
785d81a
A few more TODOs
natalieparellano Feb 23, 2022
e58d007
Bring back go 1.16
natalieparellano Feb 23, 2022
125f01f
Add unit test
natalieparellano Feb 23, 2022
4b4e816
Fix acceptance tests
natalieparellano Feb 24, 2022
8294ea5
Fix units
natalieparellano Feb 24, 2022
fd6beb3
Remove nolint
natalieparellano Feb 24, 2022
8335a56
Remove unneeded things
natalieparellano Feb 24, 2022
9ee1dfd
Use operations pattern
natalieparellano Mar 3, 2022
d21352f
Refactor tests to take advantage of operations pattern
natalieparellano Mar 3, 2022
d81cb6c
Add missing tests
natalieparellano Mar 4, 2022
d815707
Fix lint
natalieparellano Mar 4, 2022
46a24ae
Analyzer factory assigns "nop" services by default
natalieparellano Mar 4, 2022
fe83502
Try to fix registry handler test
natalieparellano Mar 4, 2022
f72848d
Move new package from cmd/lifecycle/platform to platform/inputs
natalieparellano Mar 8, 2022
a457244
Merge branch 'main' into cmd-package
natalieparellano Mar 30, 2022
c828d03
Updates per PR review
natalieparellano Apr 26, 2022
e0e8d8a
Test the platform instead of the exiter
natalieparellano Apr 26, 2022
0af8ab2
Combine cmd/lifecycle/platform and platform
natalieparellano Apr 27, 2022
fcac2e3
Clean up constructors
natalieparellano Apr 27, 2022
59097ca
Remove ForAnalyzer struct
natalieparellano Apr 27, 2022
76811ce
Combine cmd/launcher/platform and platform
natalieparellano Apr 27, 2022
2d4a9bc
Remove comment
natalieparellano Apr 27, 2022
e06cb9c
Remove spec alias
natalieparellano Apr 27, 2022
c982fb1
Move cache metadata back to platform
natalieparellano Apr 28, 2022
4aa3115
Add comment and remove unused vars
natalieparellano Apr 28, 2022
2078007
Merge branch 'main' into cmd-package
natalieparellano Apr 28, 2022
352b6ab
Add test for buildpack incompatibility error
natalieparellano May 3, 2022
20a0110
Fix typo
natalieparellano May 3, 2022
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
10 changes: 5 additions & 5 deletions acceptance/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ func testAnalyzerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
output, err := cmd.CombinedOutput()
h.AssertNotNil(t, err)

h.AssertStringContains(t, string(output), "failed to : ensure registry read access to some-run-image") // TODO: update some-run-image to have explicit permissions when https://github.com/buildpacks/lifecycle/pull/685 is merged
h.AssertStringContains(t, string(output), "validating registry read access: ensure registry read access to some-run-image") // TODO: update some-run-image to have explicit permissions when https://github.com/buildpacks/lifecycle/pull/685 is merged
})

when("stack.toml not present", func() {
Expand Down Expand Up @@ -385,7 +385,7 @@ func testAnalyzerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
})
})

when("the provided destination tags are on different registries", func() {
when.Pend("the provided destination tags are on different registries", func() {
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved
it("errors", func() {
h.SkipIf(t, api.MustParse(platformAPI).LessThan("0.7"), "Platform API < 0.7 does not accept destination tags")

Expand Down Expand Up @@ -933,7 +933,7 @@ func testAnalyzerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
output, err := cmd.CombinedOutput()

h.AssertNotNil(t, err)
expected := "failed to : ensure registry read access to " + analyzeRegFixtures.InaccessibleImage
expected := "validating registry read access: ensure registry read access to " + analyzeRegFixtures.InaccessibleImage
h.AssertStringContains(t, string(output), expected)
})
})
Expand Down Expand Up @@ -1033,7 +1033,7 @@ func testAnalyzerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
output, err := cmd.CombinedOutput()

h.AssertNotNil(t, err)
expected := "failed to : ensure registry read/write access to " + analyzeRegFixtures.ReadOnlyCacheImage
expected := "validating registry write access: ensure registry read/write access to " + analyzeRegFixtures.ReadOnlyCacheImage
h.AssertStringContains(t, string(output), expected)
})
})
Expand Down Expand Up @@ -1106,7 +1106,7 @@ func testAnalyzerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
output, err := cmd.CombinedOutput()

h.AssertNotNil(t, err)
expected := "failed to : ensure registry read/write access to " + analyzeRegFixtures.InaccessibleImage
expected := "validating registry write access: ensure registry read/write access to " + analyzeRegFixtures.InaccessibleImage
h.AssertStringContains(t, string(output), expected)
})
})
Expand Down
164 changes: 153 additions & 11 deletions analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/buildpacks/lifecycle/api"
"github.com/buildpacks/lifecycle/buildpack"
"github.com/buildpacks/lifecycle/cache"
"github.com/buildpacks/lifecycle/image"
"github.com/buildpacks/lifecycle/internal/layer"
"github.com/buildpacks/lifecycle/platform"
Expand All @@ -15,41 +16,186 @@ type Platform interface {
API() *api.Version
}

type AnalyzerFactory struct {
platformAPI *api.Version
cacheHandler CacheHandler
configHandler ConfigHandler
imageHandler ImageHandler
registryHandler RegistryHandler
}

func NewAnalyzerFactory(platformAPI *api.Version, cacheHandler CacheHandler, configHandler ConfigHandler, imageHandler ImageHandler, registryHandler RegistryHandler) *AnalyzerFactory {
Copy link
Member Author

Choose a reason for hiding this comment

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

It seemed like the constructor for a lifecycle.Analyzer belonged in the lifecycle package.

return &AnalyzerFactory{
platformAPI: platformAPI,
cacheHandler: cacheHandler,
configHandler: configHandler,
imageHandler: imageHandler,
registryHandler: registryHandler,
}
}

type Analyzer struct {
PreviousImage imgutil.Image
RunImage imgutil.Image
Logger Logger
Platform Platform
SBOMRestorer layer.SBOMRestorer

// Platform API < 0.7
Buildpacks []buildpack.GroupBuildpack
Cache Cache
LayerMetadataRestorer layer.MetadataRestorer
RestoresLayerMetadata bool
}

func (f *AnalyzerFactory) NewAnalyzer(
Copy link
Member Author

Choose a reason for hiding this comment

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

In the past, adding new inputs to the analyzer/restorer/etc. was a source of bugs because we would forget to update the creator. Accepting the inputs as a list, while ugly, can avoid this problem.

additionalTags []string,
cacheImageRef string,
launchCacheDir string,
layersDir string,
legacyCacheDir string,
legacyGroup buildpack.Group,
legacyGroupPath string,
outputImageRef string,
previousImageRef string,
runImageRef string,
skipLayers bool,
logger Logger,
) (*Analyzer, error) {
analyzer := &Analyzer{
LayerMetadataRestorer: &layer.NopMetadataRestorer{},
Logger: logger,
SBOMRestorer: &layer.NopSBOMRestorer{},
}

if f.platformAPI.AtLeast("0.7") {
if err := f.ensureRegistryAccess(additionalTags, cacheImageRef, outputImageRef, runImageRef, previousImageRef); err != nil {
return nil, err
}
} else {
if err := f.setBuildpacks(analyzer, legacyGroup, legacyGroupPath); err != nil {
return nil, err
}

if err := f.setCache(analyzer, cacheImageRef, legacyCacheDir); err != nil {
return nil, err
}

analyzer.LayerMetadataRestorer = &layer.DefaultMetadataRestorer{
LayersDir: layersDir,
Logger: logger,
SkipLayers: skipLayers,
}
analyzer.RestoresLayerMetadata = true
}
if f.platformAPI.AtLeast("0.8") && !skipLayers {
analyzer.SBOMRestorer = &layer.DefaultSBOMRestorer{
LayersDir: layersDir,
Logger: logger,
}
}
if err := f.setPrevious(analyzer, previousImageRef, launchCacheDir); err != nil {
return nil, err
}
if err := f.setRun(analyzer, runImageRef); err != nil {
return nil, err
}
return analyzer, nil
}

func (f *AnalyzerFactory) ensureRegistryAccess(
Copy link
Member Author

Choose a reason for hiding this comment

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

I got rid of the "ops" pattern - it seemed to make things more complicated.

additionalTags []string,
cacheImageRef string,
outputImageRef string,
runImageRef string,
previousImageRef string,
) error {
var readImages, writeImages []string
writeImages = append(writeImages, cacheImageRef)
if !f.imageHandler.Docker() {
readImages = append(readImages, previousImageRef, runImageRef)
writeImages = append(writeImages, outputImageRef)
writeImages = append(writeImages, additionalTags...)
}

if err := f.registryHandler.EnsureReadAccess(readImages...); err != nil {
return errors.Wrap(err, "validating registry read access")
}
if err := f.registryHandler.EnsureWriteAccess(writeImages...); err != nil {
return errors.Wrap(err, "validating registry write access")
}
return nil
}

func (f *AnalyzerFactory) setBuildpacks(analyzer *Analyzer, group buildpack.Group, path string) error {
if len(group.Group) > 0 {
analyzer.Buildpacks = group.Group
return nil
}
var err error
analyzer.Buildpacks, err = f.configHandler.ReadGroup(path)
return err
}

func (f *AnalyzerFactory) setCache(analyzer *Analyzer, imageRef string, dir string) error {
var err error
analyzer.Cache, err = f.cacheHandler.InitCache(imageRef, dir)
return err
}

func (f *AnalyzerFactory) setPrevious(analyzer *Analyzer, imageRef string, launchCacheDir string) error {
if imageRef == "" {
return nil
}
var err error
analyzer.PreviousImage, err = f.imageHandler.InitImage(imageRef)
if err != nil {
return errors.Wrap(err, "getting previous image")
}
if launchCacheDir == "" || !f.imageHandler.Docker() {
return nil
}

volumeCache, err := cache.NewVolumeCache(launchCacheDir)
if err != nil {
return errors.Wrap(err, "creating launch cache")
}
analyzer.PreviousImage = cache.NewCachingImage(analyzer.PreviousImage, volumeCache)
return nil
}

func (f *AnalyzerFactory) setRun(analyzer *Analyzer, imageRef string) error {
if imageRef == "" {
return nil
}
var err error
analyzer.RunImage, err = f.imageHandler.InitImage(imageRef)
if err != nil {
return errors.Wrap(err, "getting run image")
}
return nil
}

// Analyze fetches the layers metadata from the previous image and writes analyzed.toml.
func (a *Analyzer) Analyze() (platform.AnalyzedMetadata, error) {
var (
err error
appMeta platform.LayersMetadata
cacheMeta platform.CacheMetadata
previousImageID *platform.ImageIdentifier
runImageID *platform.ImageIdentifier
err error
)

if a.PreviousImage != nil { // Previous image is optional in Platform API >= 0.7
previousImageID, err = a.getImageIdentifier(a.PreviousImage)
if err != nil {
if previousImageID, err = a.getImageIdentifier(a.PreviousImage); err != nil {
return platform.AnalyzedMetadata{}, errors.Wrap(err, "retrieving image identifier")
}

// continue even if the label cannot be decoded
if err := image.DecodeLabel(a.PreviousImage, platform.LayerMetadataLabel, &appMeta); err != nil {
if err = image.DecodeLabel(a.PreviousImage, platform.LayerMetadataLabel, &appMeta); err != nil {
appMeta = platform.LayersMetadata{}
}

if err := a.SBOMRestorer.RestoreFromPrevious(a.PreviousImage, bomSHA(appMeta)); err != nil {
if err = a.SBOMRestorer.RestoreFromPrevious(a.PreviousImage, bomSHA(appMeta)); err != nil {
return platform.AnalyzedMetadata{}, errors.Wrap(err, "retrieving launch SBOM layer")
}
} else {
Expand All @@ -63,7 +209,7 @@ func (a *Analyzer) Analyze() (platform.AnalyzedMetadata, error) {
}
}

if a.restoresLayerMetadata() {
if a.RestoresLayerMetadata {
cacheMeta, err = retrieveCacheMetadata(a.Cache, a.Logger)
if err != nil {
return platform.AnalyzedMetadata{}, err
Expand All @@ -82,10 +228,6 @@ func (a *Analyzer) Analyze() (platform.AnalyzedMetadata, error) {
}, nil
}

func (a *Analyzer) restoresLayerMetadata() bool {
return a.Platform.API().LessThan("0.7")
}

func (a *Analyzer) getImageIdentifier(image imgutil.Image) (*platform.ImageIdentifier, error) {
if !image.Found() {
a.Logger.Infof("Previous image with name %q not found", image.Name())
Expand Down
Loading