Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Redesign OCI image status: Display the image's exact repository digest #307

Merged
merged 6 commits into from
Aug 9, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions pkg/apis/ignite/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,10 @@ type OCIImageClaim struct {
// OCIImageSource specifies how the OCI image was imported.
// It is the status variant of OCIImageClaim
type OCIImageSource struct {
// ID defines the source's ID (e.g. the Docker image ID)
ID string `json:"id"`
// ID defines the source's content ID (e.g. the canonical OCI path or Docker image ID)
ID *meta.OCIContentID `json:"id"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add omitempty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this pretty much required? I can't think of a situation where only the image/kernel size would be reported.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, true 👍

// Size defines the size of the source in bytes
Size meta.Size `json:"size"`
// RepoDigests defines the image name as it was when pulled
// from a repository, and the digest of the image
// The format is $registry/$user/$image@sha256:$digest
// This field is unpopulated if the image used as the source
// has never been pushed to or pulled from a registry
RepoDigests []string `json:"repoDigests,omitempty"`
}

// ImageStatus defines the status of the image
Expand Down
10 changes: 2 additions & 8 deletions pkg/apis/ignite/v1alpha2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,10 @@ type OCIImageClaim struct {
// OCIImageSource specifies how the OCI image was imported.
// It is the status variant of OCIImageClaim
type OCIImageSource struct {
// ID defines the source's ID (e.g. the Docker image ID)
ID string `json:"id"`
// ID defines the source's content ID (e.g. the canonical OCI path or Docker image ID)
ID *meta.OCIContentID `json:"id"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add omitempty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, IMO this field is required.

// Size defines the size of the source in bytes
Size meta.Size `json:"size"`
// RepoDigests defines the image name as it was when pulled
// from a repository, and the digest of the image
// The format is $registry/$user/$image@sha256:$digest
// This field is unpopulated if the image used as the source
// has never been pushed to or pulled from a registry
RepoDigests []string `json:"repoDigests,omitempty"`
}

// ImageStatus defines the status of the image
Expand Down
96 changes: 90 additions & 6 deletions pkg/apis/meta/v1alpha1/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,21 @@ package v1alpha1
import (
"encoding/json"
"fmt"
"net/url"

"github.com/containers/image/docker/reference"
"github.com/opencontainers/go-digest"
)

// NewOCIImageRef parses and normalizes a reference to an OCI (docker) image.
func NewOCIImageRef(imageStr string) (OCIImageRef, error) {
named, err := reference.ParseDockerRef(imageStr)
if err != nil {
return OCIImageRef(""), err
return "", err
}
namedTagged, ok := named.(reference.NamedTagged)
if !ok {
return OCIImageRef(""), fmt.Errorf("could not parse image name with a tag %s", imageStr)
return "", fmt.Errorf("could not parse image name with a tag %s", imageStr)
}
return OCIImageRef(reference.FamiliarString(namedTagged)), nil
}
Expand All @@ -32,10 +34,6 @@ func (i OCIImageRef) IsUnset() bool {
return len(i) == 0
}

func (i OCIImageRef) MarshalJSON() ([]byte, error) {
return json.Marshal(string(i))
}

func (i *OCIImageRef) UnmarshalJSON(b []byte) error {
var str string
var err error
Expand All @@ -47,3 +45,89 @@ func (i *OCIImageRef) UnmarshalJSON(b []byte) error {
*i, err = NewOCIImageRef(str)
return err
}

func ParseOCIContentID(str string) (*OCIContentID, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a short godoc showing sample valid/invalid strings you can give it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in d4495d0.

named, err := reference.ParseDockerRef(str)
if err != nil {
return nil, err
}

if canonical, ok := named.(reference.Canonical); ok {
return &OCIContentID{
repoName: named.Name(),
digest: canonical.Digest().String(),
}, nil
}

d, err := digest.Parse(str)
if err != nil {
return nil, err
}

return &OCIContentID{
digest: d.String(),
}, nil
}

type OCIContentID struct {
repoName string // Fully qualified image name, e.g. "docker.io/library/node" or blank if the image is local
digest string // Repo digest of the image, or sha256sum provided by the source if the image is local
}

var _ json.Marshaler = &OCIContentID{}
var _ json.Unmarshaler = &OCIContentID{}

func (o *OCIContentID) String() string {
if len(o.repoName) > 0 {
return fmt.Sprintf("oci://%s", o.RepoDigest())
}

return fmt.Sprintf("docker://%s", o.Digest())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe make local constants for these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in d4495d0.

}

func parseOCIString(s string) (*OCIContentID, error) {
u, err := url.Parse(s)
if err != nil {
return nil, err
}

// Remove the "docker://" or "oci://" scheme by only caring about the host and path
return ParseOCIContentID(u.Host + u.Path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the input is oci://docker.io/library/node@sha256:abc, is the stuff after @ really preserved? have you tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's preserved. I tested it will multiple combinations: https://play.golang.org/p/KMFTEOUBVh_W

}

// Local returns true if the image has no repoName, i.e. it's not available from a registry
func (o *OCIContentID) Local() bool {
return len(o.repoName) == 0
}

// Digest is a getter for the digest field
func (o *OCIContentID) Digest() string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return digest.Digest? I think that would make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in d4495d0 and also for RepoDigest()

return o.digest
}

// RepoDigest returns a repo digest based on the OCIContentID if it is not local
func (o *OCIContentID) RepoDigest() (s string) {
if !o.Local() {
s = fmt.Sprintf("%s@%s", o.repoName, o.digest)
}

return
}

func (o *OCIContentID) MarshalJSON() ([]byte, error) {
return json.Marshal(o.String())
}

func (o *OCIContentID) UnmarshalJSON(b []byte) (err error) {
var s string
if err = json.Unmarshal(b, &s); err != nil {
return err
}

var id *OCIContentID
if id, err = parseOCIString(s); err == nil {
*o = *id
}

return
}
22 changes: 19 additions & 3 deletions pkg/source/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,26 @@ func (ds *DockerSource) Parse(ociRef meta.OCIImageRef) (*api.OCIImageSource, err
}

ds.imageID = res.ID

// By default parse the OCI content ID from the Docker image ID
contentRef := res.ID
if len(res.RepoDigests) > 0 {
// If the image has Repo digests, use the first one of them to parse
// the fully qualified OCI image name and digest. All of the digests
// point to the same contents, so it doesn't matter which one we use
// here. It will be translated to the right content by the runtime.
contentRef = res.RepoDigests[0]
}

// Parse the OCI content ID based on the available reference
ci, err := meta.ParseOCIContentID(contentRef)
if err != nil {
return nil, err
}

return &api.OCIImageSource{
ID: res.ID,
RepoDigests: res.RepoDigests,
Size: meta.NewSizeFromBytes(uint64(res.Size)),
ID: ci,
Size: meta.NewSizeFromBytes(uint64(res.Size)),
}, nil
}

Expand Down