Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

enhance skopeo inspect #1608

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

enhance skopeo inspect #1608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
109 changes: 62 additions & 47 deletions internal/image/docker_schema1_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,56 +273,71 @@ func TestManifestSchema1Inspect(t *testing.T) {
manifestSchema1FromFixture(t, "schema1.json"),
manifestSchema1FromComponentsLikeFixture(t),
} {

labels := map[string]string{
"Kolla-SHA": "5.0.0-39-g6f1b947b",
"architecture": "x86_64",
"authoritative-source-url": "registry.access.redhat.com",
"build-date": "2018-01-25T00:32:27.807261",
"com.redhat.build-host": "ip-10-29-120-186.ec2.internal",
"com.redhat.component": "openstack-nova-api-docker",
"description": "Red Hat OpenStack Platform 12.0 nova-api",
"distribution-scope": "public",
"io.k8s.description": "Red Hat OpenStack Platform 12.0 nova-api",
"io.k8s.display-name": "Red Hat OpenStack Platform 12.0 nova-api",
"io.openshift.tags": "rhosp osp openstack osp-12.0",
"kolla_version": "stable/pike",
"name": "rhosp12/openstack-nova-api",
"release": "20180124.1",
"summary": "Red Hat OpenStack Platform 12.0 nova-api",
"tripleo-common_version": "7.6.3-23-g4891cfe",
"url": "https://access.redhat.com/containers/#/registry.access.redhat.com/rhosp12/openstack-nova-api/images/12.0-20180124.1",
"vcs-ref": "9b31243b7b448eb2fc3b6e2c96935b948f806e98",
"vcs-type": "git",
"vendor": "Red Hat, Inc.",
"version": "12.0",
"version-release": "12.0-20180124.1",
}

Layers := []string{
"sha256:9cadd93b16ff2a0c51ac967ea2abfadfac50cfa3af8b5bf983d89b8f8647f3e4",
"sha256:4aa565ad8b7a87248163ce7dba1dd3894821aac97e846b932ff6b8ef9a8a508a",
"sha256:f576d102e09b9eef0e305aaef705d2d43a11bebc3fd5810a761624bd5e11997e",
"sha256:9e92df2aea7dc0baf5f1f8d509678d6a6306de27ad06513f8e218371938c07a6",
"sha256:62e48e39dc5b30b75a97f05bccc66efbae6058b860ee20a5c9a184b9d5e25788",
"sha256:e623934bca8d1a74f51014256445937714481e49343a31bda2bc5f534748184d",
}

Env := []string{
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"container=oci",
"KOLLA_BASE_DISTRO=rhel",
"KOLLA_INSTALL_TYPE=binary",
"KOLLA_INSTALL_METATYPE=rhos",
"PS1=$(tput bold)($(printenv KOLLA_SERVICE_NAME))$(tput sgr0)[$(id -un)@$(hostname -s) $(pwd)]$ ",
}

ii, err := m.Inspect(context.Background())
require.NoError(t, err)
created := time.Date(2018, 1, 25, 0, 37, 48, 268558000, time.UTC)
assert.Equal(t, types.ImageInspectInfo{
Tag: "latest",
Created: &created,
DockerVersion: "1.12.6",
Labels: map[string]string{
"Kolla-SHA": "5.0.0-39-g6f1b947b",
"architecture": "x86_64",
"authoritative-source-url": "registry.access.redhat.com",
"build-date": "2018-01-25T00:32:27.807261",
"com.redhat.build-host": "ip-10-29-120-186.ec2.internal",
"com.redhat.component": "openstack-nova-api-docker",
"description": "Red Hat OpenStack Platform 12.0 nova-api",
"distribution-scope": "public",
"io.k8s.description": "Red Hat OpenStack Platform 12.0 nova-api",
"io.k8s.display-name": "Red Hat OpenStack Platform 12.0 nova-api",
"io.openshift.tags": "rhosp osp openstack osp-12.0",
"kolla_version": "stable/pike",
"name": "rhosp12/openstack-nova-api",
"release": "20180124.1",
"summary": "Red Hat OpenStack Platform 12.0 nova-api",
"tripleo-common_version": "7.6.3-23-g4891cfe",
"url": "https://access.redhat.com/containers/#/registry.access.redhat.com/rhosp12/openstack-nova-api/images/12.0-20180124.1",
"vcs-ref": "9b31243b7b448eb2fc3b6e2c96935b948f806e98",
"vcs-type": "git",
"vendor": "Red Hat, Inc.",
"version": "12.0",
"version-release": "12.0-20180124.1",
},
Architecture: "amd64",
Os: "linux",
Layers: []string{
"sha256:9cadd93b16ff2a0c51ac967ea2abfadfac50cfa3af8b5bf983d89b8f8647f3e4",
"sha256:4aa565ad8b7a87248163ce7dba1dd3894821aac97e846b932ff6b8ef9a8a508a",
"sha256:f576d102e09b9eef0e305aaef705d2d43a11bebc3fd5810a761624bd5e11997e",
"sha256:9e92df2aea7dc0baf5f1f8d509678d6a6306de27ad06513f8e218371938c07a6",
"sha256:62e48e39dc5b30b75a97f05bccc66efbae6058b860ee20a5c9a184b9d5e25788",
"sha256:e623934bca8d1a74f51014256445937714481e49343a31bda2bc5f534748184d",
},
Env: []string{
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"container=oci",
"KOLLA_BASE_DISTRO=rhel",
"KOLLA_INSTALL_TYPE=binary",
"KOLLA_INSTALL_METATYPE=rhos",
"PS1=$(tput bold)($(printenv KOLLA_SERVICE_NAME))$(tput sgr0)[$(id -un)@$(hostname -s) $(pwd)]$ ",
},
}, *ii)
assert.Equal(t, "latest", ii.Tag)
assert.Equal(t, &created, ii.Created)
assert.Equal(t, "1.12.6", ii.DockerVersion)
assert.Equal(t, labels, ii.Labels)
assert.Equal(t, "amd64", ii.Architecture)
assert.Equal(t, "linux", ii.Os)
assert.Equal(t, Layers, ii.Layers)
assert.Equal(t, Env, ii.Env)
assert.NotEqual(t, nil, ii.Config)
assert.Equal(t, int64(0), ii.Size)
assert.Equal(t, "", ii.Author)
Copy link
Collaborator

@mtrmac mtrmac Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Please maintain the original whole-structure comparison.

A single Equal over the whole structure is more useful because it ensures that nothing was left out in the tests. Doing this item by item requires much more manual inspection and maintenance.

(In all three formats.)

for _, layer := range ii.LayersDetail {
assert.NotEqual(t, nil, layer)
Copy link
Collaborator

@mtrmac mtrmac Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

AFAICS that’s always true (members of the slice are not pointers at all), and does not test the created contents at all. That’s not useful.

Same for the other loop.

}
assert.Equal(t, "", ii.Author)
for _, history := range ii.History {
assert.NotEqual(t, nil, history)
}
}
}

Expand Down
57 changes: 34 additions & 23 deletions internal/image/docker_schema2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,30 +256,41 @@ func TestManifestSchema2Inspect(t *testing.T) {
ii, err := m.Inspect(context.Background())
require.NoError(t, err)
created := time.Date(2016, 9, 23, 23, 20, 45, 789764590, time.UTC)
assert.Equal(t, types.ImageInspectInfo{
Tag: "",
Created: &created,
DockerVersion: "1.12.1",
Labels: map[string]string{},
Architecture: "amd64",
Os: "linux",
Layers: []string{
"sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb",
"sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c",
"sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9",
"sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
"sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
},
Env: []string{
"PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"HTTPD_PREFIX=/usr/local/apache2",
"HTTPD_VERSION=2.4.23",
"HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f",
"HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download&filename=httpd/httpd-2.4.23.tar.bz2",
"HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc",
},
}, *ii)
layers := []string{
"sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb",
"sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c",
"sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9",
"sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
"sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
}

env := []string{
"PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"HTTPD_PREFIX=/usr/local/apache2",
"HTTPD_VERSION=2.4.23",
"HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f",
"HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download&filename=httpd/httpd-2.4.23.tar.bz2",
"HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc",
}
assert.Equal(t, "", ii.Tag)
assert.Equal(t, &created, ii.Created)
assert.Equal(t, layers, ii.Layers)

assert.Equal(t, "1.12.1", ii.DockerVersion)
assert.Equal(t, map[string]string{}, ii.Labels)
assert.Equal(t, "amd64", ii.Architecture)
assert.Equal(t, "linux", ii.Os)
assert.Equal(t, layers, ii.Layers)
assert.Equal(t, env, ii.Env)
assert.NotEqual(t, nil, ii.Config)
assert.Equal(t, int64(0), ii.Size)
assert.Equal(t, "", ii.Author)
for _, layer := range ii.LayersDetail {
assert.NotEqual(t, nil, layer)
}
for _, history := range ii.History {
assert.NotEqual(t, nil, history)
}
// nil configBlob will trigger an error in m.ConfigBlob()
m = manifestSchema2FromComponentsLikeFixture(nil)
_, err = m.Inspect(context.Background())
Expand Down
60 changes: 37 additions & 23 deletions internal/image/oci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,30 +281,44 @@ func TestManifestOCI1Inspect(t *testing.T) {
ii, err := m.Inspect(context.Background())
require.NoError(t, err)
created := time.Date(2016, 9, 23, 23, 20, 45, 789764590, time.UTC)
assert.Equal(t, types.ImageInspectInfo{
Tag: "",
Created: &created,
DockerVersion: "1.12.1",
Labels: map[string]string{},
Architecture: "amd64",
Os: "linux",
Layers: []string{
"sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb",
"sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c",
"sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9",
"sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
"sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
},
Env: []string{
"PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"HTTPD_PREFIX=/usr/local/apache2",
"HTTPD_VERSION=2.4.23",
"HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f",
"HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download&filename=httpd/httpd-2.4.23.tar.bz2",
"HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc",
},
}, *ii)

layers := []string{
"sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb",
"sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c",
"sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9",
"sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
"sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
}

env := []string{
"PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"HTTPD_PREFIX=/usr/local/apache2",
"HTTPD_VERSION=2.4.23",
"HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f",
"HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download&filename=httpd/httpd-2.4.23.tar.bz2",
"HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc",
}

assert.Equal(t, "", ii.Tag)
assert.Equal(t, &created, ii.Created)
assert.Equal(t, layers, ii.Layers)

assert.Equal(t, "1.12.1", ii.DockerVersion)
assert.Equal(t, map[string]string{}, ii.Labels)
assert.Equal(t, "amd64", ii.Architecture)
assert.Equal(t, "linux", ii.Os)
assert.Equal(t, layers, ii.Layers)
assert.Equal(t, env, ii.Env)
assert.NotEqual(t, nil, ii.Config)
assert.Equal(t, int64(-1), ii.Size)
assert.Equal(t, "", ii.Author)

for _, layer := range ii.LayersDetail {
assert.NotEqual(t, nil, layer)
}
for _, history := range ii.History {
assert.NotEqual(t, nil, history)
}
// nil configBlob will trigger an error in m.ConfigBlob()
m = manifestOCI1FromComponentsLikeFixture(nil)
_, err = m.Inspect(context.Background())
Expand Down
26 changes: 26 additions & 0 deletions manifest/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

compressiontypes "github.com/containers/image/v5/pkg/compression/types"
"github.com/containers/image/v5/types"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -228,3 +229,28 @@ func compressionVariantsRecognizeMIMEType(variantTable []compressionMIMETypeSet,
variants := findCompressionMIMETypeSet(variantTable, mimeType)
return variants != nil // Alternatively, this could be len(variants) > 1, but really the caller should ask about a specific algorithm.
}

// imgInspectLayersFromLayerInfos converts a list of layer infos, presumably obtained from a Manifest.LayerInfos()
// method call, into a format suitable for inclusion in a types.ImageInspectInfo structure.
func imgInspectLayersFromLayerInfos(infos []LayerInfo) []types.LayerDetail {
layers := make([]types.LayerDetail, len(infos))
for i, info := range infos {
layers[i].MIMEType = info.MediaType
layers[i].Digest = info.Digest
layers[i].Size = info.Size
layers[i].Annotations = info.Annotations
}
return layers
}

func schema2HistoryToV1History(history []Schema2History) []v1.History {
v1History := make([]v1.History, len(history))
for index, value := range history {
v1History[index].Created = &value.Created
Copy link
Collaborator

Choose a reason for hiding this comment

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

created := value.Created
v1History[index].Created = &created

would be more obviously safe, because the pointer points at an individually-allocated value that will never be modified by whatever owns the input history value.

v1History[index].Author = value.Author
v1History[index].CreatedBy = value.CreatedBy
v1History[index].Comment = value.Comment
v1History[index].EmptyLayer = value.EmptyLayer
}
return v1History
}
38 changes: 37 additions & 1 deletion manifest/docker_schema1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,23 +215,59 @@ func validateV1ID(id string) error {
return nil
}

type schema2Image struct {
Schema2V1Image
Parent digest.Digest `json:"parent,omitempty"`
RootFS *Schema2RootFS `json:"rootfs,omitempty"`
History []Schema2History `json:"history,omitempty"`
OSVersion string `json:"os.version,omitempty"`
OSFeatures []string `json:"os.features,omitempty"`
}

// Inspect returns various information for (skopeo inspect) parsed from the manifest and configuration.
func (m *Schema1) Inspect(_ func(types.BlobInfo) ([]byte, error)) (*types.ImageInspectInfo, error) {
s1 := &Schema2V1Image{}
if err := json.Unmarshal([]byte(m.History[0].V1Compatibility), s1); err != nil {
return nil, err
}
layerInfos := m.LayerInfos()
i := &types.ImageInspectInfo{
Tag: m.Tag,
Created: &s1.Created,
DockerVersion: s1.DockerVersion,
Architecture: s1.Architecture,
Os: s1.OS,
Layers: layerInfosToStrings(m.LayerInfos()),
Layers: layerInfosToStrings(layerInfos),
LayersDetail: imgInspectLayersFromLayerInfos(layerInfos),
Author: s1.Author,
Size: s1.Size,
}

diffIDs := []digest.Digest{digest.FromString(s1.ID)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

s1.ID is not a diffID, and the single-value array would frequently not even have the right number of elements.

Passing an empty value would at least be consistent with the existing OCIConfig call.

(This is another reason not to include all of OCIv1 config in the inspect output: the value is not fully valid, and the OCIConfig API is focused enough on this single thing that it can document that at least a bit more effectively.)

Actually the best thing to do is not to involve the schema2 JSON format at all: refactor the convertedHistory computation out from ToSchema2Config into a separate function, and call it here. Then

  • We don’t need to invent fake DiffID values
  • ToSchema2Config avoids a whole lot of JSON manipulation
  • This code can just consume convenient Go values instead of having to unmarshal again.

schema2Config, err := m.ToSchema2Config(diffIDs)
if err != nil {
return nil, err
}
imageConfig := &schema2Image{}
Copy link
Collaborator

Choose a reason for hiding this comment

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

(Why is this using a custom type and not the existing schema2 manifest.Schema2Image? But see elsewhere, we shouldn’t need to involve JSON for this.)

if err := json.Unmarshal(schema2Config, imageConfig); err != nil {
return nil, err
}
i.History = schema2HistoryToV1History(imageConfig.History)
if s1.Config != nil {
i.Labels = s1.Config.Labels
i.Env = s1.Config.Env
i.Config.Env = s1.Config.Env
i.Config.Labels = s1.Config.Labels
i.Config.User = s1.Config.User
i.Config.Volumes = s1.Config.Volumes
i.Config.Entrypoint = s1.Config.Entrypoint
for key, value := range s1.Config.ExposedPorts {
exposedPorts := make(map[string]struct{})
exposedPorts[string(key)] = value
i.Config.ExposedPorts = exposedPorts
}
i.Config.StopSignal = s1.Config.StopSignal
i.Config.WorkingDir = s1.Config.WorkingDir
}
return i, nil
}
Expand Down
19 changes: 18 additions & 1 deletion manifest/docker_schema2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,18 +271,35 @@ func (m *Schema2) Inspect(configGetter func(types.BlobInfo) ([]byte, error)) (*t
if err := json.Unmarshal(config, s2); err != nil {
return nil, err
}
layerInfos := m.LayerInfos()
i := &types.ImageInspectInfo{
Tag: "",
Created: &s2.Created,
DockerVersion: s2.DockerVersion,
Architecture: s2.Architecture,
Variant: s2.Variant,
Os: s2.OS,
Layers: layerInfosToStrings(m.LayerInfos()),
Layers: layerInfosToStrings(layerInfos),
LayersDetail: imgInspectLayersFromLayerInfos(layerInfos),
History: schema2HistoryToV1History(s2.History),
Author: s2.Author,
Size: s2.Size,
}
if s2.Config != nil {
i.Labels = s2.Config.Labels
i.Env = s2.Config.Env
i.Config.Env = s2.Config.Env
i.Config.Labels = s2.Config.Labels
i.Config.User = s2.Config.User
i.Config.Volumes = s2.Config.Volumes
i.Config.Entrypoint = s2.Config.Entrypoint
for key, value := range s2.Config.ExposedPorts {
exposedPorts := make(map[string]struct{})
exposedPorts[string(key)] = value
i.Config.ExposedPorts = exposedPorts
}
i.Config.StopSignal = s2.Config.StopSignal
i.Config.WorkingDir = s2.Config.WorkingDir
Comment on lines +291 to +302
Copy link
Collaborator

Choose a reason for hiding this comment

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

See the OCIConfig comment… this feels like a code that shouldn’t need to exist.

}
return i, nil
}
Expand Down
Loading