Handles deprecationDate in time.Time format in actual layer metadata

Prevents panic as described in issue paketo-buildpacks#269. This change makes the Equals method more robust, by not panicking when the deprecationDate is present in the layer metadata in an unexpected format.
jpastoor · Jan 14, 2024 · d134fc4 · d134fc4
1 parent f3767ec
commit d134fc4
Show file tree

Hide file tree

Showing 2 changed files with 129 additions and 21 deletions.
diff --git a/layer.go b/layer.go
@@ -119,37 +119,63 @@ func (l *LayerContributor) checkIfMetadataMatches(layer libcnb.Layer) (map[strin
 	l.Logger.Debugf("Expected metadata: %+v", expected)
 	l.Logger.Debugf("Actual metadata: %+v", layer.Metadata)
 
-	match, err := l.Equals(expected,layer.Metadata)
+	match, err := l.Equals(expected, layer.Metadata)
 	if err != nil {
 		return map[string]interface{}{}, false, fmt.Errorf("unable to compare metadata\n%w", err)
 	}
 	return expected, match, nil
 }
 
 func (l *LayerContributor) Equals(expectedM map[string]interface{}, layerM map[string]interface{}) (bool, error) {
-	if dep, ok := expectedM["dependency"].(map[string]interface{}); ok {
-		for k, v := range dep {
-			if k == "deprecation_date" {
-				deprecationDate := v.(time.Time).Truncate(time.Second).In(time.UTC)
-				dep["deprecation_date"] = deprecationDate
-				break
-			}
-		}
-    }
-	if dep, ok := layerM["dependency"].(map[string]interface{}); ok {
+	// TODO Do we want the Equals method to modify the underlying maps? Else we need to make a copy here.
+
+	if err := l.normalizeDependencyDeprecationDate(expectedM); err != nil {
+		return false, fmt.Errorf("%w (expected layer)", err)
+	}
+
+	if err := l.normalizeDependencyDeprecationDate(layerM); err != nil {
+		return false, fmt.Errorf("%w (actual layer)", err)
+	}
+
+	return reflect.DeepEqual(expectedM, layerM), nil
+}
+
+// normalizeDependencyDeprecationDate makes sure the dependency deprecation date is represented as a time.Time object
+// in the map whenever it exists.
+func (l *LayerContributor) normalizeDependencyDeprecationDate(input map[string]interface{}) error {
+	if dep, ok := input["dependency"].(map[string]interface{}); ok {
 		for k, v := range dep {
 			if k == "deprecation_date" {
-				deprecationDate, err := time.Parse(time.RFC3339, v.(string))
+				deprecationDate, err := l.parseDeprecationDate(v)
 				if err != nil {
-					return false, fmt.Errorf("unable to parse deprecation_date %s", v.(string))
+					return err
 				}
-				deprecationDate = deprecationDate.Truncate(time.Second).In(time.UTC)
 				dep["deprecation_date"] = deprecationDate
 				break
 			}
 		}
-	} 
-	return reflect.DeepEqual(expectedM, layerM), nil
+	}
+
+	return nil
+}
+
+// parseDeprecationDate accepts both string and time.Time as input, and returns
+// a truncated time.Time value.
+func (l *LayerContributor) parseDeprecationDate(v interface{}) (deprecationDate time.Time, err error) {
+	switch vDate := v.(type) {
+	case time.Time:
+		deprecationDate = vDate
+	case string:
+		deprecationDate, err = time.Parse(time.RFC3339, vDate)
+		if err != nil {
+			return time.Time{}, fmt.Errorf("unable to parse deprecation_date %s", vDate)
+		}
+	default:
+		return time.Time{}, fmt.Errorf("unexpected type %T for deprecation_date %v", v, v)
+	}
+
+	deprecationDate = deprecationDate.Truncate(time.Second).In(time.UTC)
+	return
 }
 
 func (l *LayerContributor) checkIfLayerRestored(layer libcnb.Layer) (bool, error) {

diff --git a/layer_test.go b/layer_test.go
@@ -521,9 +521,9 @@ func testLayer(t *testing.T, context spec.G, it spec.S) {
 				PURL:            "pkg:generic/some-java11@11.0.2?arch=amd64",
 				DeprecationDate: dependency.DeprecationDate, // parsed as '2021-04-01 00:00:00 +0000 UTC'
 			}
-			dlc.ExpectedMetadata = map[string]interface{}{ "dependency":dependency}
+			dlc.ExpectedMetadata = map[string]interface{}{"dependency": dependency}
 
-			layer.Metadata = map[string]interface{}{ "dependency": map[string]interface{}{
+			layer.Metadata = map[string]interface{}{"dependency": map[string]interface{}{
 				"id":      dependency.ID,
 				"name":    dependency.Name,
 				"version": dependency.Version,
@@ -554,7 +554,11 @@ func testLayer(t *testing.T, context spec.G, it spec.S) {
 			Expect(called).To(BeFalse())
 		})
 
-		it("does not call function with missing deprecation_date", func() {
+		it("gracefully handles a deprecationDate in time.Time format in actual layer metadata", func() {
+			// reusing It: does not call function with non-matching deprecation_date format
+			// but this time with a deprecationDate formatted as time.Time in the actual layer metadata
+			actualDeprecationDate, _ := time.Parse(time.RFC3339, "2021-04-01T00:00:00Z")
+
 			dependency = libpak.BuildpackDependency{
 				ID:      "test-id",
 				Name:    "test-name",
@@ -570,10 +574,88 @@ func testLayer(t *testing.T, context spec.G, it spec.S) {
 				},
 				CPEs:            []string{"cpe:2.3:a:some:jre:11.0.2:*:*:*:*:*:*:*"},
 				PURL:            "pkg:generic/some-java11@11.0.2?arch=amd64",
+				DeprecationDate: dependency.DeprecationDate, // parsed as '2021-04-01 00:00:00 +0000 UTC'
+			}
+			dlc.ExpectedMetadata = map[string]interface{}{"dependency": dependency}
+
+			layer.Metadata = map[string]interface{}{"dependency": map[string]interface{}{
+				"id":      dependency.ID,
+				"name":    dependency.Name,
+				"version": dependency.Version,
+				"uri":     dependency.URI,
+				"sha256":  dependency.SHA256,
+				"stacks":  []interface{}{dependency.Stacks[0]},
+				"licenses": []map[string]interface{}{
+					{
+						"type": dependency.Licenses[0].Type,
+						"uri":  dependency.Licenses[0].URI,
+					},
+				},
+				"cpes":             []interface{}{"cpe:2.3:a:some:jre:11.0.2:*:*:*:*:*:*:*"},
+				"purl":             "pkg:generic/some-java11@11.0.2?arch=amd64",
+				"deprecation_date": actualDeprecationDate, // does not match without truncation
+			}}
+
+			var called bool
+
+			_, err := dlc.Contribute(layer, func(artifact *os.File) (libcnb.Layer, error) {
+				defer artifact.Close()
+
+				called = true
+				return layer, nil
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(called).To(BeFalse())
+		})
+
+		it("does not panic on unsupported deprecationDate format in layer metadata", func() {
+			// Unexpected type (not string or time.Time)
+			actualDeprecationDate := 1234
+
+			dependency = libpak.BuildpackDependency{
+				ID:              "test-id",
+				DeprecationDate: dependency.DeprecationDate, // parsed as '2021-04-01 00:00:00 +0000 UTC'
+			}
+			dlc.ExpectedMetadata = map[string]interface{}{"dependency": dependency}
+
+			layer.Metadata = map[string]interface{}{"dependency": map[string]interface{}{
+				"id":               dependency.ID,
+				"deprecation_date": actualDeprecationDate, // does not match without truncation
+			}}
+
+			var called bool
+
+			_, err := dlc.Contribute(layer, func(artifact *os.File) (libcnb.Layer, error) {
+				defer artifact.Close()
+
+				called = true
+				return layer, nil
+			})
+			Expect(err).To(MatchError(ContainSubstring("unexpected type int for deprecation_date")))
+			Expect(called).To(BeFalse())
+		})
+
+		it("does not call function with missing deprecation_date", func() {
+			dependency = libpak.BuildpackDependency{
+				ID:      "test-id",
+				Name:    "test-name",
+				Version: "1.1.1",
+				URI:     fmt.Sprintf("%s/test-path", server.URL()),
+				SHA256:  "576dd8416de5619ea001d9662291d62444d1292a38e96956bc4651c01f14bca1",
+				Stacks:  []string{"test-stack"},
+				Licenses: []libpak.BuildpackDependencyLicense{
+					{
+						Type: "test-type",
+						URI:  "test-uri",
+					},
+				},
+				CPEs: []string{"cpe:2.3:a:some:jre:11.0.2:*:*:*:*:*:*:*"},
+				PURL: "pkg:generic/some-java11@11.0.2?arch=amd64",
 			}
-			dlc.ExpectedMetadata = map[string]interface{}{ "dependency":dependency}
+			dlc.ExpectedMetadata = map[string]interface{}{"dependency": dependency}
 
-			layer.Metadata = map[string]interface{}{ "dependency": map[string]interface{}{
+			layer.Metadata = map[string]interface{}{"dependency": map[string]interface{}{
 				"id":      dependency.ID,
 				"name":    dependency.Name,
 				"version": dependency.Version,