Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Do not fail on non RFC3339 labels but log warning
Browse files Browse the repository at this point in the history
Some image vendors have not read the spec properly and pass along
a timestamp in a different format than RFC3339. Before this change
the unmarshal of the JSON would fail for those labels and the affected
images would be excluded from our cache.

We now include those images in our cache and collect all labels that
failed parsing in a custom error, which we detect and log (instead
of bailing out).
  • Loading branch information
hiddeco committed May 23, 2019
1 parent a6b5661 commit 3622a69
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 9 deletions.
32 changes: 27 additions & 5 deletions image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,17 @@ func (i Ref) WithNewTag(t string) Ref {
return img
}

type LabelTimestampFormatError struct {
Labels []string
}

func (e *LabelTimestampFormatError) Error() string {
return fmt.Sprintf(
"failed to parse %d timestamp label(s) as RFC3339 (%s); ask the repository administrator to correct this as it conflicts with the spec",
len(e.Labels),
strings.Join(e.Labels, ","))
}

// Labels has all the image labels we are interested in for an image
// ref, the JSON struct tag keys should be equal to the label.
type Labels struct {
Expand Down Expand Up @@ -261,12 +272,23 @@ func (l *Labels) UnmarshalJSON(b []byte) error {
Created string `json:"org.opencontainers.image.created,omitempty"`
}{}
json.Unmarshal(b, &unencode)

var err error
if err = decodeTime(unencode.BuildDate, &l.BuildDate); err == nil {
err = decodeTime(unencode.Created, &l.Created)
labelErr := LabelTimestampFormatError{}
if err := decodeTime(unencode.BuildDate, &l.BuildDate); err != nil {
if _, ok := err.(*time.ParseError); !ok {
return err
}
labelErr.Labels = append(labelErr.Labels, "org.label-schema.build-date")
}
return err
if err := decodeTime(unencode.Created, &l.Created); err != nil {
if _, ok := err.(*time.ParseError); !ok {
return err
}
labelErr.Labels = append(labelErr.Labels, "org.opencontainers.image.created")
}
if len(labelErr.Labels) >= 1 {
return &labelErr
}
return nil
}

// Info has the metadata we are able to determine about an image ref,
Expand Down
17 changes: 17 additions & 0 deletions image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,23 @@ func TestImageLabelsSerialisation(t *testing.T) {
assert.Equal(t, labels, labels1)
}

func TestNonRFC3339ImageLabelsUnmarshal(t *testing.T) {
str := `{
"org.label-schema.build-date": "20190523",
"org.opencontainers.image.created": "20190523"
}`

var labels Labels
err := json.Unmarshal([]byte(str), &labels)
lpe, ok := err.(*LabelTimestampFormatError)
if !ok {
t.Fatalf("Got %v, but expected LabelTimestampFormatError", err)
}
if lc := len(lpe.Labels); lc != 2 {
t.Errorf("Got error for %v labels, expected 2", lc)
}
}

func TestImageLabelsZeroTime(t *testing.T) {
labels := Labels{}
bytes, err := json.Marshal(labels)
Expand Down
5 changes: 4 additions & 1 deletion registry/cache/repocachemanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,10 @@ func (c *repoCacheManager) updateImage(ctx context.Context, update imageToUpdate
if ctx.Err() == context.DeadlineExceeded {
return registry.ImageEntry{}, c.clientTimeoutError()
}
return registry.ImageEntry{}, err
if _, ok := err.(*image.LabelTimestampFormatError); !ok {
return registry.ImageEntry{}, err
}
c.logger.Log("err", err, "ref", imageID)
}

refresh := update.previousRefresh
Expand Down
13 changes: 10 additions & 3 deletions registry/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ interpret:
return ImageEntry{}, fetchErr
}

var labelErr error
info := image.Info{ID: a.repo.ToRef(ref), Digest: manifestDigest.String()}

// TODO(michael): can we type switch? Not sure how dependable the
Expand All @@ -139,7 +140,10 @@ interpret:
}

if err = json.Unmarshal([]byte(man.History[0].V1Compatibility), &v1); err != nil {
return ImageEntry{}, err
if _, ok := err.(*image.LabelTimestampFormatError); !ok {
return ImageEntry{}, err
}
labelErr = err
}
// This is not the ImageID that Docker uses, but assumed to
// identify the image as it's the topmost layer.
Expand All @@ -162,7 +166,10 @@ interpret:
} `json:"container_config"`
}
if err = json.Unmarshal(configBytes, &config); err != nil {
return ImageEntry{}, err
if _, ok := err.(*image.LabelTimestampFormatError); !ok {
return ImageEntry{}, err
}
labelErr = err
}
// This _is_ what Docker uses as its Image ID.
info.ImageID = man.Config.Digest.String()
Expand All @@ -184,5 +191,5 @@ interpret:
t := reflect.TypeOf(manifest)
return ImageEntry{}, errors.New("unknown manifest type: " + t.String())
}
return ImageEntry{Info: info}, nil
return ImageEntry{Info: info}, labelErr
}

0 comments on commit 3622a69

Please sign in to comment.