-
Notifications
You must be signed in to change notification settings - Fork 237
Layered analysis for single version packages #248
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,10 +17,12 @@ limitations under the License. | |
package differs | ||
|
||
import ( | ||
"errors" | ||
"strings" | ||
|
||
pkgutil "github.com/GoogleContainerTools/container-diff/pkg/util" | ||
"github.com/GoogleContainerTools/container-diff/util" | ||
"github.com/sirupsen/logrus" | ||
) | ||
|
||
type MultiVersionPackageAnalyzer interface { | ||
|
@@ -33,6 +35,11 @@ type SingleVersionPackageAnalyzer interface { | |
Name() string | ||
} | ||
|
||
type SingleVersionPackageLayerAnalyzer interface { | ||
getPackages(image pkgutil.Image) ([]map[string]util.PackageInfo, error) | ||
Name() string | ||
} | ||
|
||
func multiVersionDiff(image1, image2 pkgutil.Image, differ MultiVersionPackageAnalyzer) (*util.MultiVersionPackageDiffResult, error) { | ||
pack1, err := differ.getPackages(image1) | ||
if err != nil { | ||
|
@@ -71,6 +78,13 @@ func singleVersionDiff(image1, image2 pkgutil.Image, differ SingleVersionPackage | |
}, nil | ||
} | ||
|
||
// singleVersionLayerDiff returns an error as this diff is not supported as | ||
// it is far from obvious to define it in meaningful way | ||
func singleVersionLayerDiff(image1, image2 pkgutil.Image, differ SingleVersionPackageLayerAnalyzer) (*util.SingleVersionPackageLayerDiffResult, error) { | ||
logrus.Warning("'diff' command for packages on layers is not supported, consider using 'analyze' on each image instead") | ||
return &util.SingleVersionPackageLayerDiffResult{}, errors.New("Diff for packages on layers is not supported, only analysis is supported") | ||
} | ||
|
||
func multiVersionAnalysis(image pkgutil.Image, analyzer MultiVersionPackageAnalyzer) (*util.MultiVersionPackageAnalyzeResult, error) { | ||
pack, err := analyzer.getPackages(image) | ||
if err != nil { | ||
|
@@ -98,3 +112,39 @@ func singleVersionAnalysis(image pkgutil.Image, analyzer SingleVersionPackageAna | |
} | ||
return &analysis, nil | ||
} | ||
|
||
// singleVersionLayerAnalysis returns the packages included, deleted or | ||
// updated in each layer | ||
func singleVersionLayerAnalysis(image pkgutil.Image, analyzer SingleVersionPackageLayerAnalyzer) (*util.SingleVersionPackageLayerAnalyzeResult, error) { | ||
pack, err := analyzer.getPackages(image) | ||
if err != nil { | ||
return &util.SingleVersionPackageLayerAnalyzeResult{}, err | ||
} | ||
var pkgDiffs []util.PackageDiff | ||
|
||
// Each layer with modified packages includes a complete list of packages | ||
// in its package database. Thus we diff the current layer with the | ||
// previous one that contains a package database. Layers that do not | ||
// include a package database are omitted. | ||
preInd := -1 | ||
for i := range pack { | ||
var pkgDiff util.PackageDiff | ||
if preInd < 0 && len(pack[i]) > 0 { | ||
pkgDiff = util.GetMapDiff(make(map[string]util.PackageInfo), pack[i]) | ||
preInd = i | ||
} else if preInd >= 0 && len(pack[i]) > 0 { | ||
pkgDiff = util.GetMapDiff(pack[preInd], pack[i]) | ||
preInd = i | ||
} | ||
|
||
pkgDiffs = append(pkgDiffs, pkgDiff) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Could you explain this a bit more? Are you saying that each layer with modified packages contains all packages installed in the previous layer, so diffing those two gives you only the packages modified between layer I think this block would also be a little more clear if it looked something like for i := range pack {
if len(pack[i] == 0) {
continue
}
if i == 0 {
pkgDiffs = append(pkgDiffs, util.GetMapDiff(make(map[string]util.PackageInfo), pack[i]))
} else {
pkgDiffs = append(pkgDiffs, util.GetMapDiff(pack[i-1], pack[i]))
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I haven't coded it as you suggest because we can't expect to have different packages in each layer. Image there are some layers that do not modify the package database, because they configure files or run some scripts or whatever; for those layers the reuslt of the getPackages is an empty map. I don't want to diff those empty maps with any other layer that actually modifies the package database, because the result will be the same as treating any package (regardless the layer in which they were appended) as a new addition. So what I do here to compare only layers which actually include a modified package database. That means the diff is not with the immediate previous layer, but with the previous layer that actually includes some package database. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh ok, that makes sense now. Could you just update your comment slightly to explain that it's not the previous layer, but the previous layer that contains a package db? |
||
|
||
return &util.SingleVersionPackageLayerAnalyzeResult{ | ||
Image: image.Source, | ||
AnalyzeType: strings.TrimSuffix(analyzer.Name(), "Analyzer"), | ||
Analysis: util.PackageLayerDiff{ | ||
PackageDiffs: pkgDiffs, | ||
}, | ||
}, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this shares a lot of code with
AptAnalyzer.getPackages()
inapt_diff.go
. Could you pull this out into a shared method between the two implementations?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure you are right.