From c6497dda0a95a3309dc649761243250634a31d40 Mon Sep 17 00:00:00 2001 From: Sida Chen Date: Wed, 27 Feb 2019 17:02:36 -0500 Subject: [PATCH] clair: Fix namespace update logic Only when a namespace's version is updated, we understand and replace the namespace instead of using detector as the indicator. --- ancestry.go | 108 +++++++++++++----- ancestry_test.go | 286 +++++++++++++++++++++++++---------------------- 2 files changed, 230 insertions(+), 164 deletions(-) diff --git a/ancestry.go b/ancestry.go index 0e6ce4e9..9f47e784 100644 --- a/ancestry.go +++ b/ancestry.go @@ -17,6 +17,7 @@ package clair import ( "crypto/sha256" "encoding/hex" + "encoding/json" "strings" log "github.com/sirupsen/logrus" @@ -25,14 +26,14 @@ import ( ) type layerIndexedFeature struct { - feature *database.LayerFeature - namespace *layerIndexedNamespace - introducedIn int + Feature *database.LayerFeature + Namespace *layerIndexedNamespace + IntroducedIn int } type layerIndexedNamespace struct { - namespace database.LayerNamespace - introducedIn int + Namespace database.LayerNamespace `json:"namespace"` + IntroducedIn int `json:"introducedIn"` } // AncestryBuilder builds an Ancestry, which contains an ordered list of layers @@ -41,7 +42,7 @@ type AncestryBuilder struct { layerIndex int layerNames []string detectors []database.Detector - namespaces map[database.Detector]*layerIndexedNamespace + namespaces []layerIndexedNamespace // unique namespaces features map[database.Detector][]layerIndexedFeature } @@ -53,7 +54,7 @@ func NewAncestryBuilder(detectors []database.Detector) *AncestryBuilder { return &AncestryBuilder{ layerIndex: 0, detectors: detectors, - namespaces: make(map[database.Detector]*layerIndexedNamespace), + namespaces: make([]layerIndexedNamespace, 0), features: make(map[database.Detector][]layerIndexedFeature), } } @@ -105,7 +106,7 @@ func (b *AncestryBuilder) addLayerFeatures(detector database.Detector, features for i := range existingFeatures { feature := existingFeatures[i] for j := range features { - if features[j] == *feature.feature { + if features[j] == *feature.Feature { currentFeatures = append(currentFeatures, feature) break } @@ -116,7 +117,7 @@ func (b *AncestryBuilder) addLayerFeatures(detector database.Detector, features for i := range features { found := false for j := range existingFeatures { - if *existingFeatures[j].feature == features[i] { + if *existingFeatures[j].Feature == features[i] { found = true break } @@ -125,7 +126,6 @@ func (b *AncestryBuilder) addLayerFeatures(detector database.Detector, features if !found { namespace, found := b.lookupNamespace(&features[i]) if !found { - log.WithField("Layer Hashes", b.layerNames).Error("skip, could not find the proper namespace for feature") continue } @@ -143,29 +143,59 @@ func (b *AncestryBuilder) addLayerFeatures(detector database.Detector, features // the new namespace. func (b *AncestryBuilder) updateNamespace(layerNamespace *database.LayerNamespace) { var ( - previous *layerIndexedNamespace - ok bool + previous *layerIndexedNamespace + foundUpgrade bool ) - if previous, ok = b.namespaces[layerNamespace.By]; !ok { - b.namespaces[layerNamespace.By] = &layerIndexedNamespace{ - namespace: *layerNamespace, - introducedIn: b.layerIndex, + newNSNames := strings.Split(layerNamespace.Name, ":") + if len(newNSNames) != 2 { + log.Error("invalid namespace name") + } + + newNSName := newNSNames[0] + newNSVersion := newNSNames[1] + for i, ns := range b.namespaces { + nsNames := strings.Split(ns.Namespace.Name, ":") + if len(nsNames) != 2 { + log.Error("invalid namespace name") + continue } + nsName := nsNames[0] + nsVersion := nsNames[1] + if ns.Namespace.VersionFormat == layerNamespace.VersionFormat && nsName == newNSName { + if nsVersion != newNSVersion { + previous = &b.namespaces[i] + foundUpgrade = true + break + } else { + // not changed + return + } + } + } + + // we didn't found the namespace is a upgrade from another namespace, so we + // simply add it. + if !foundUpgrade { + b.namespaces = append(b.namespaces, layerIndexedNamespace{ + Namespace: *layerNamespace, + IntroducedIn: b.layerIndex, + }) + return } // All features referencing to this namespace are now pointing to the new namespace. // Also those features are now treated as introduced in the same layer as // when this new namespace is introduced. - previous.namespace = *layerNamespace - previous.introducedIn = b.layerIndex + previous.Namespace = *layerNamespace + previous.IntroducedIn = b.layerIndex for _, features := range b.features { for i, feature := range features { - if feature.namespace == previous { - features[i].introducedIn = previous.introducedIn + if feature.Namespace == previous { + features[i].IntroducedIn = previous.IntroducedIn } } } @@ -173,19 +203,37 @@ func (b *AncestryBuilder) updateNamespace(layerNamespace *database.LayerNamespac func (b *AncestryBuilder) createLayerIndexedFeature(namespace *layerIndexedNamespace, feature *database.LayerFeature) layerIndexedFeature { return layerIndexedFeature{ - feature: feature, - namespace: namespace, - introducedIn: b.layerIndex, + Feature: feature, + Namespace: namespace, + IntroducedIn: b.layerIndex, } } func (b *AncestryBuilder) lookupNamespace(feature *database.LayerFeature) (*layerIndexedNamespace, bool) { - for _, namespace := range b.namespaces { - if namespace.namespace.VersionFormat == feature.VersionFormat { - return namespace, true + matchedNamespaces := []*layerIndexedNamespace{} + for i, namespace := range b.namespaces { + if namespace.Namespace.VersionFormat == feature.VersionFormat { + matchedNamespaces = append(matchedNamespaces, &b.namespaces[i]) } } + if len(matchedNamespaces) == 1 { + return matchedNamespaces[0], true + } + + serialized, _ := json.Marshal(matchedNamespaces) + fields := log.Fields{ + "feature.Name": feature.Name, + "feature.VersionFormat": feature.VersionFormat, + "ancestryBuilder.namespaces": string(serialized), + } + + if len(matchedNamespaces) > 1 { + log.WithFields(fields).Warn("skip features with ambiguous namespaces") + } else { + log.WithFields(fields).Warn("skip features with no matching namespace") + } + return nil, false } @@ -193,14 +241,14 @@ func (b *AncestryBuilder) ancestryFeatures(index int) []database.AncestryFeature ancestryFeatures := []database.AncestryFeature{} for detector, features := range b.features { for _, feature := range features { - if feature.introducedIn == index { + if feature.IntroducedIn == index { ancestryFeatures = append(ancestryFeatures, database.AncestryFeature{ NamespacedFeature: database.NamespacedFeature{ - Feature: feature.feature.Feature, - Namespace: feature.namespace.namespace.Namespace, + Feature: feature.Feature.Feature, + Namespace: feature.Namespace.Namespace.Namespace, }, FeatureBy: detector, - NamespaceBy: feature.namespace.namespace.By, + NamespaceBy: feature.Namespace.Namespace.By, }) } } diff --git a/ancestry_test.go b/ancestry_test.go index 2884d6e1..437cf703 100644 --- a/ancestry_test.go +++ b/ancestry_test.go @@ -23,23 +23,54 @@ import ( ) var ( - dpkg = database.NewFeatureDetector("dpkg", "1.0") - rpm = database.NewFeatureDetector("rpm", "1.0") - pip = database.NewFeatureDetector("pip", "1.0") - python = database.NewNamespaceDetector("python", "1.0") - osrelease = database.NewNamespaceDetector("os-release", "1.0") - ubuntu = *database.NewNamespace("ubuntu:14.04", "dpkg") - ubuntu16 = *database.NewNamespace("ubuntu:16.04", "dpkg") - python2 = *database.NewNamespace("python:2", "pip") - sed = *database.NewSourcePackage("sed", "4.4-2", "dpkg") - sedBin = *database.NewBinaryPackage("sed", "4.4-2", "dpkg") - tar = *database.NewBinaryPackage("tar", "1.29b-2", "dpkg") - scipy = *database.NewSourcePackage("scipy", "3.0.0", "pip") + dpkg = database.NewFeatureDetector("dpkg", "1.0") + rpm = database.NewFeatureDetector("rpm", "1.0") + pip = database.NewFeatureDetector("pip", "1.0") + python = database.NewNamespaceDetector("python", "1.0") + osrelease = database.NewNamespaceDetector("os-release", "1.0") + aptsources = database.NewNamespaceDetector("apt-sources", "1.0") + ubuntu = *database.NewNamespace("ubuntu:14.04", "dpkg") + ubuntu16 = *database.NewNamespace("ubuntu:16.04", "dpkg") + debian = *database.NewNamespace("debian:7", "dpkg") + python2 = *database.NewNamespace("python:2", "pip") + sed = *database.NewSourcePackage("sed", "4.4-2", "dpkg") + sedByRPM = *database.NewBinaryPackage("sed", "4.4-2", "rpm") + sedBin = *database.NewBinaryPackage("sed", "4.4-2", "dpkg") + tar = *database.NewBinaryPackage("tar", "1.29b-2", "dpkg") + scipy = *database.NewSourcePackage("scipy", "3.0.0", "pip") detectors = []database.Detector{dpkg, osrelease, rpm} multinamespaceDetectors = []database.Detector{dpkg, osrelease, pip} ) +type ancestryBuilder struct { + ancestry *database.Ancestry +} + +func newAncestryBuilder(name string) *ancestryBuilder { + return &ancestryBuilder{&database.Ancestry{Name: name}} +} + +func (b *ancestryBuilder) addDetectors(d ...database.Detector) *ancestryBuilder { + b.ancestry.By = append(b.ancestry.By, d...) + return b +} + +func (b *ancestryBuilder) addLayer(hash string, f ...database.AncestryFeature) *ancestryBuilder { + l := database.AncestryLayer{Hash: hash} + l.Features = append(l.Features, f...) + b.ancestry.Layers = append(b.ancestry.Layers, l) + return b +} + +func ancestryFeature(namespace database.Namespace, feature database.Feature, nsBy database.Detector, fBy database.Detector) database.AncestryFeature { + return database.AncestryFeature{ + NamespacedFeature: database.NamespacedFeature{feature, namespace}, + FeatureBy: fBy, + NamespaceBy: nsBy, + } +} + // layerBuilder is for helping constructing the layer test artifacts. type layerBuilder struct { layer *database.Layer @@ -49,6 +80,15 @@ func newLayerBuilder(hash string) *layerBuilder { return &layerBuilder{&database.Layer{Hash: hash, By: detectors}} } +func newLayerBuilderWithoutDetector(hash string) *layerBuilder { + return &layerBuilder{&database.Layer{Hash: hash}} +} + +func (b *layerBuilder) addDetectors(d ...database.Detector) *layerBuilder { + b.layer.By = append(b.layer.By, d...) + return b +} + func (b *layerBuilder) addNamespace(detector database.Detector, ns database.Namespace) *layerBuilder { b.layer.Namespaces = append(b.layer.Namespaces, database.LayerNamespace{ Namespace: ns, @@ -85,177 +125,155 @@ var testImage = []*database.Layer{ newLayerBuilder("7").addFeature(dpkg, sed).layer, } -var clairLimit = []*database.Layer{ - // TODO(sidac): how about install rpm package under ubuntu? - newLayerBuilder("1").addNamespace(osrelease, ubuntu).layer, - newLayerBuilder("2").addFeature(rpm, sed).layer, -} - -var multipleNamespace = []*database.Layer{ - // TODO(sidac): support for multiple namespaces -} - var invalidNamespace = []*database.Layer{ // add package without namespace, this indicates that the namespace detector // could not detect the namespace. newLayerBuilder("0").addFeature(dpkg, sed).layer, } +var noMatchingNamespace = []*database.Layer{ + newLayerBuilder("0").addFeature(rpm, sedByRPM).addFeature(dpkg, sed).addNamespace(osrelease, ubuntu).layer, +} + var multiplePackagesOnFirstLayer = []*database.Layer{ newLayerBuilder("0").addFeature(dpkg, sed).addFeature(dpkg, tar).addFeature(dpkg, sedBin).addNamespace(osrelease, ubuntu16).layer, } +var twoNamespaceDetectorsWithSameResult = []*database.Layer{ + newLayerBuilderWithoutDetector("0").addDetectors(dpkg, aptsources, osrelease).addFeature(dpkg, sed).addNamespace(aptsources, ubuntu).addNamespace(osrelease, ubuntu).layer, +} + +var sameVersionFormatDiffName = []*database.Layer{ + newLayerBuilder("0").addFeature(dpkg, sed).addNamespace(aptsources, ubuntu).addNamespace(osrelease, debian).layer, +} + func TestAddLayer(t *testing.T) { cases := []struct { - title string - image []*database.Layer - - expectedAncestry database.Ancestry + title string + image []*database.Layer + nonDefaultDetectors []database.Detector + expectedAncestry database.Ancestry }{ { title: "empty image", - expectedAncestry: database.Ancestry{Name: ancestryName([]string{}), By: detectors}, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{})).addDetectors(detectors...).ancestry, }, { - title: "empty layer", - image: testImage[:1], - expectedAncestry: database.Ancestry{Name: ancestryName([]string{"0"}), By: detectors, Layers: []database.AncestryLayer{{Hash: "0"}}}, + title: "empty layer", + image: testImage[:1], + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0"})).addDetectors(detectors...). + addLayer("0").ancestry, }, { title: "ubuntu", image: testImage[:2], - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0", "1"}), - By: detectors, - Layers: []database.AncestryLayer{{Hash: "0"}, {Hash: "1"}}, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0", "1"})).addDetectors(detectors...). + addLayer("0"). + addLayer("1").ancestry, }, { title: "ubuntu install sed", image: testImage[:3], - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0", "1", "2"}), - By: detectors, - Layers: []database.AncestryLayer{{Hash: "0"}, {Hash: "1"}, {Hash: "2", Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: sed, Namespace: ubuntu}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }, - }}}, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0", "1", "2"})).addDetectors(detectors...). + addLayer("0"). + addLayer("1"). + addLayer("2", ancestryFeature(ubuntu, sed, osrelease, dpkg)).ancestry, }, { title: "ubuntu install tar", image: testImage[:4], - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0", "1", "2", "3"}), - By: detectors, - Layers: []database.AncestryLayer{{Hash: "0"}, {Hash: "1"}, {Hash: "2", Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: sed, Namespace: ubuntu}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }, - }}, { - Hash: "3", Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: tar, Namespace: ubuntu}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }, - }, - }}, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0", "1", "2", "3"})).addDetectors(detectors...). + addLayer("0"). + addLayer("1"). + addLayer("2", ancestryFeature(ubuntu, sed, osrelease, dpkg)). + addLayer("3", ancestryFeature(ubuntu, tar, osrelease, dpkg)).ancestry, }, { title: "ubuntu uninstall tar", image: testImage[:5], - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0", "1", "2", "3", "4"}), - By: detectors, - Layers: []database.AncestryLayer{{Hash: "0"}, {Hash: "1"}, {Hash: "2", Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: sed, Namespace: ubuntu}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }, - }}, {Hash: "3"}, {Hash: "4"}}, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0", "1", "2", "3", "4"})).addDetectors(detectors...). + addLayer("0"). + addLayer("1"). + addLayer("2", ancestryFeature(ubuntu, sed, osrelease, dpkg)). + addLayer("3"). + addLayer("4").ancestry, }, { title: "ubuntu upgrade", image: testImage[:6], - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0", "1", "2", "3", "4", "5"}), - By: detectors, - Layers: []database.AncestryLayer{{Hash: "0"}, {Hash: "1"}, {Hash: "2"}, {Hash: "3"}, {Hash: "4"}, {Hash: "5", Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: sed, Namespace: ubuntu16}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }}}, - }, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0", "1", "2", "3", "4", "5"})).addDetectors(detectors...). + addLayer("0"). + addLayer("1"). + addLayer("2"). + addLayer("3"). + addLayer("4"). + addLayer("5", ancestryFeature(ubuntu16, sed, osrelease, dpkg)).ancestry, }, { title: "no change to the detectable files", image: testImage[:7], - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0", "1", "2", "3", "4", "5", "6"}), - By: detectors, - Layers: []database.AncestryLayer{{Hash: "0"}, {Hash: "1"}, {Hash: "2"}, {Hash: "3"}, {Hash: "4"}, {Hash: "5", Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: sed, Namespace: ubuntu16}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }}}, {Hash: "6"}}, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0", "1", "2", "3", "4", "5", "6"})).addDetectors(detectors...). + addLayer("0"). + addLayer("1"). + addLayer("2"). + addLayer("3"). + addLayer("4"). + addLayer("5", ancestryFeature(ubuntu16, sed, osrelease, dpkg)). + addLayer("6").ancestry, }, { title: "change to the package installer database but no features are affected.", image: testImage[:8], - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0", "1", "2", "3", "4", "5", "6", "7"}), - By: detectors, - Layers: []database.AncestryLayer{{Hash: "0"}, {Hash: "1"}, {Hash: "2"}, {Hash: "3"}, {Hash: "4"}, {Hash: "5", Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: sed, Namespace: ubuntu16}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }}}, {Hash: "6"}, {Hash: "7"}}, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0", "1", "2", "3", "4", "5", "6", "7"})).addDetectors(detectors...). + addLayer("0"). + addLayer("1"). + addLayer("2"). + addLayer("3"). + addLayer("4"). + addLayer("5", ancestryFeature(ubuntu16, sed, osrelease, dpkg)). + addLayer("6"). + addLayer("7").ancestry, }, { title: "layers with features and namespace.", image: multiplePackagesOnFirstLayer, - expectedAncestry: database.Ancestry{ - Name: ancestryName([]string{"0"}), - By: detectors, - Layers: []database.AncestryLayer{ - { - Hash: "0", - Features: []database.AncestryFeature{ - { - NamespacedFeature: database.NamespacedFeature{Feature: sed, Namespace: ubuntu16}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }, - { - NamespacedFeature: database.NamespacedFeature{Feature: sedBin, Namespace: ubuntu16}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }, - { - NamespacedFeature: database.NamespacedFeature{Feature: tar, Namespace: ubuntu16}, - FeatureBy: dpkg, - NamespaceBy: osrelease, - }, - }, - }, - }, - }, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0"})).addDetectors(detectors...). + addLayer("0", + ancestryFeature(ubuntu16, sed, osrelease, dpkg), + ancestryFeature(ubuntu16, sedBin, osrelease, dpkg), + ancestryFeature(ubuntu16, tar, osrelease, dpkg)). + ancestry, + }, { + title: "two namespace detectors giving same namespace.", + image: twoNamespaceDetectorsWithSameResult, + nonDefaultDetectors: []database.Detector{osrelease, aptsources, dpkg}, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0"})).addDetectors(osrelease, aptsources, dpkg). + addLayer("0", ancestryFeature(ubuntu, sed, aptsources, dpkg)). + ancestry, + }, { + title: "feature without namespace", + image: invalidNamespace, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0"})).addDetectors(detectors...). + addLayer("0"). + ancestry, + }, { + title: "two namespaces with the same version format but different names", + image: sameVersionFormatDiffName, + // failure of matching a namespace will result in the package not being added. + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0"})).addDetectors(detectors...). + addLayer("0"). + ancestry, + }, { + title: "noMatchingNamespace", + image: noMatchingNamespace, + expectedAncestry: *newAncestryBuilder(ancestryName([]string{"0"})).addDetectors(detectors...).addLayer("0", ancestryFeature(ubuntu, sed, osrelease, dpkg)).ancestry, }, } for _, test := range cases { t.Run(test.title, func(t *testing.T) { - builder := NewAncestryBuilder(detectors) + var builder *AncestryBuilder + if len(test.nonDefaultDetectors) != 0 { + builder = NewAncestryBuilder(test.nonDefaultDetectors) + } else { + builder = NewAncestryBuilder(detectors) + } + for _, layer := range test.image { builder.AddLeafLayer(layer) }