worker/database: Move upgrade detection logic out of database to worker

config.example.yaml

@@ -40,7 +40,7 @@ clair:
     # 32-bit URL-safe base64 key used to encrypt pagination tokens
     # If one is not provided, it will be generated.
     # Multiple clair instances in the same cluster need the same value.
-    paginationKey:
+    paginationkey:
 
     # Optional PKI configuration
     # If you want to easily generate client certificates and CAs, try the following projects:
@@ -61,7 +61,7 @@ clair:
     attempts: 3
 
     # Duration before a failed notification is retried
-    renotifyInterval: 2h
+    renotifyinterval: 2h
 
     http:
       # Optional endpoint that will receive notifications via POST requests

config/config.go

@@ -118,7 +118,6 @@ func Load(path string) (config *Config, err error) {
 	config = &cfgFile.Clair
 
 	// Generate a pagination key if none is provided.
-	// TODO(Quentin-M): Move to the API code.
 	if config.API.PaginationKey == "" {
 		var key fernet.Key
 		if err = key.Generate(); err != nil {

database/pgsql/layer.go

@@ -333,7 +333,7 @@ func (pgSQL *pgSQL) updateDiffFeatureVersions(tx *sql.Tx, layer, existingLayer *
 		addNV := utils.CompareStringLists(layerFeaturesNV, parentLayerFeaturesNV)
 		delNV := utils.CompareStringLists(parentLayerFeaturesNV, layerFeaturesNV)
 
-		// Fill the structures containing the added and deleted FeatureVersions
+		// Fill the structures containing the added and deleted FeatureVersions.
 		for _, nv := range addNV {
 			add = append(add, *layerFeaturesMapNV[nv])
 		}
@@ -375,7 +375,7 @@ func createNV(features []database.FeatureVersion) (map[string]*database.FeatureV
 
 	for i := 0; i < len(features); i++ {
 		featureVersion := &features[i]
-		nv := featureVersion.Feature.Name + ":" + featureVersion.Version.String()
+		nv := featureVersion.Feature.Namespace.Name + ":" + featureVersion.Feature.Name + ":" + featureVersion.Version.String()
 		mapNV[nv] = featureVersion
 		sliceNV = append(sliceNV, nv)
 	}

database/pgsql/layer_test.go

@@ -170,7 +170,7 @@ func testInsertLayerTree(t *testing.T, datastore database.Datastore) {
 			Namespace: database.Namespace{Name: "TestInsertLayerNamespace3"},
 			Name:      "TestInsertLayerFeature3",
 		},
-		Version: types.NewVersionUnsafe("0.57"),
+		Version: types.NewVersionUnsafe("0.56"),
 	}
 	f6 := database.FeatureVersion{
 		Feature: database.Feature{

worker/worker.go

@@ -113,7 +113,7 @@ func Process(datastore database.Datastore, imageFormat, name, parentName, path s
 }
 
 // detectContent downloads a layer's archive and extracts its Namespace and Features.
-func detectContent(imageFormat, name, path string, headers map[string]string, parent *database.Layer) (namespace *database.Namespace, features []database.FeatureVersion, err error) {
+func detectContent(imageFormat, name, path string, headers map[string]string, parent *database.Layer) (namespace *database.Namespace, featureVersions []database.FeatureVersion, err error) {
 	data, err := detectors.DetectData(imageFormat, path, headers, append(detectors.GetRequiredFilesFeatures(), detectors.GetRequiredFilesNamespace()...), maxFileSize)
 	if err != nil {
 		log.Errorf("layer %s: failed to extract data from %s: %s", name, utils.CleanURL(path), err)
@@ -121,41 +121,33 @@ func detectContent(imageFormat, name, path string, headers map[string]string, pa
 	}
 
 	// Detect namespace.
-	namespace, err = detectNamespace(data, parent)
+	namespace = detectNamespace(name, data, parent)
-	if err != nil {
-		return
-	}
-	if namespace != nil {
-		log.Debugf("layer %s: Namespace is %s.", name, namespace.Name)
-	} else {
-		log.Debugf("layer %s: OS is unknown.", name)
-	}
 
 	// Detect features.
-	features, err = detectFeatures(name, data, namespace)
+	featureVersions, err = detectFeatureVersions(name, data, namespace, parent)
 	if err != nil {
 		return
 	}
-
+	if len(featureVersions) > 0 {
-	// If there are no feature detected, use parent's features if possible.
+		log.Debugf("layer %s: detected %d features", name, len(featureVersions))
-	// TODO(Quentin-M): We eventually want to give the choice to each detectors to use none/some
-	// parent's Features. It would be useful for detectors that can't find their entire result using
-	// one Layer.
-	if len(features) == 0 && parent != nil {
-		features = parent.Features
 	}
 
-	log.Debugf("layer %s: detected %d features", name, len(features))
 	return
 }
 
-func detectNamespace(data map[string][]byte, parent *database.Layer) (namespace *database.Namespace, err error) {
+func detectNamespace(name string, data map[string][]byte, parent *database.Layer) (namespace *database.Namespace) {
+	// Use registered detectors to get the Namespace.
 	namespace = detectors.DetectNamespace(data)
+	if namespace != nil {
+		log.Debugf("layer %s: detected namespace %q", name, namespace.Name)
+		return
+	}
 
-	// Attempt to detect the OS from the parent layer.
+	// Use the parent's Namespace.
-	if namespace == nil && parent != nil {
+	if parent != nil {
 		namespace = parent.Namespace
-		if err != nil {
+		if namespace != nil {
+			log.Debugf("layer %s: detected namespace %q (from parent)", name, namespace.Name)
 			return
 		}
 	}
@@ -163,8 +155,8 @@ func detectNamespace(data map[string][]byte, parent *database.Layer) (namespace
 	return
 }
 
-func detectFeatures(name string, data map[string][]byte, namespace *database.Namespace) (features []database.FeatureVersion, err error) {
+func detectFeatureVersions(name string, data map[string][]byte, namespace *database.Namespace, parent *database.Layer) (features []database.FeatureVersion, err error) {
-	// TODO(Quentin-M): We need to pass the parent image DetectFeatures because it's possible that
+	// TODO(Quentin-M): We need to pass the parent image to DetectFeatures because it's possible that
 	// some detectors would need it in order to produce the entire feature list (if they can only
 	// detect a diff). Also, we should probably pass the detected namespace so detectors could
 	// make their own decision.
@@ -173,18 +165,45 @@ func detectFeatures(name string, data map[string][]byte, namespace *database.Nam
 		return
 	}
 
-	// Ensure that every feature has a Namespace associated, otherwise associate the detected
+	// If there are no FeatureVersions, use parent's FeatureVersions if possible.
-	// namespace. If there is no detected namespace, we'll throw an error.
+	// TODO(Quentin-M): We eventually want to give the choice to each detectors to use none/some of
-	for i := 0; i < len(features); i++ {
+	// their parent's FeatureVersions. It would be useful for detectors that can't find their entire
-		if features[i].Feature.Namespace.Name == "" {
+	// result using one Layer.
-			if namespace != nil {
+	if len(features) == 0 && parent != nil {
-				features[i].Feature.Namespace = *namespace
+		features = parent.Features
-			} else {
+		return
-				log.Warningf("layer %s: Layer's namespace is unknown but non-namespaced features have been detected", name)
+	}
-				err = ErrUnsupported
+
-				return
+	// Build a map of the namespaces for each FeatureVersion in our parent layer.
-			}
+	parentFeatureNamespaces := make(map[string]database.Namespace)
+	if parent != nil {
+		for _, parentFeature := range parent.Features {
+			parentFeatureNamespaces[parentFeature.Feature.Name+":"+parentFeature.Version.String()] = parentFeature.Feature.Namespace
+		}
+	}
+
+	// Ensure that each FeatureVersion has an associated Namespace.
+	for i, feature := range features {
+		if feature.Feature.Namespace.Name != "" {
+			// There is a Namespace associated.
+			continue
 		}
+
+		if parentFeatureNamespace, ok := parentFeatureNamespaces[feature.Feature.Name+":"+feature.Version.String()]; ok {
+			// The FeatureVersion is present in the parent layer; associate with their Namespace.
+			features[i].Feature.Namespace = parentFeatureNamespace
+			continue
+		}
+
+		if namespace != nil {
+			// The Namespace has been detected in this layer; associate it.
+			features[i].Feature.Namespace = *namespace
+			continue
+		}
+
+		log.Warningf("layer %s: Layer's namespace is unknown but non-namespaced features have been detected", name)
+		err = ErrUnsupported
+		return
 	}
 
 	return