Skip to content

Commit

Permalink
fix: oras push should return error when artifacttype is empty for spe…
Browse files Browse the repository at this point in the history
…c 1.1 (#1317)

Signed-off-by: Xiaoxuan Wang <[email protected]>
  • Loading branch information
wangxiaoxuan273 authored Apr 8, 2024
1 parent 122f01b commit 2ef6dc0
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 7 deletions.
16 changes: 9 additions & 7 deletions cmd/oras/internal/option/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
const (
ImageSpecV1_1 = "v1.1"
ImageSpecV1_0 = "v1.0"
ImageSpecAuto = "auto"
)

const (
Expand All @@ -36,15 +37,15 @@ const (

// ImageSpec option struct which implements pflag.Value interface.
type ImageSpec struct {
flag string
Flag string
PackVersion oras.PackManifestVersion
}

// Set validates and sets the flag value from a string argument.
func (is *ImageSpec) Set(value string) error {
is.flag = value
is.Flag = value
switch value {
case ImageSpecV1_1:
case ImageSpecV1_1, ImageSpecAuto:
is.PackVersion = oras.PackManifestVersion1_1
case ImageSpecV1_0:
is.PackVersion = oras.PackManifestVersion1_0
Expand All @@ -67,20 +68,21 @@ func (is *ImageSpec) Options() string {
return strings.Join([]string{
ImageSpecV1_1,
ImageSpecV1_0,
ImageSpecAuto,
}, ", ")
}

// String returns the string representation of the flag.
func (is *ImageSpec) String() string {
return is.flag
return is.Flag
}

// ApplyFlags applies flags to a command flag set.
func (is *ImageSpec) ApplyFlags(fs *pflag.FlagSet) {
// default to v1.1-rc.4
// default to auto
is.PackVersion = oras.PackManifestVersion1_1
defaultFlag := ImageSpecV1_1
fs.Var(is, "image-spec", fmt.Sprintf(`[Experimental] specify manifest type for building artifact. Options: %s (default %q)`, is.Options(), defaultFlag))
is.Flag = ImageSpecAuto
fs.Var(is, "image-spec", fmt.Sprintf(`[Experimental] specify manifest type for building artifact. Options: %s (default %q)`, is.Options(), ImageSpecAuto))
}

// DistributionSpec option struct which implements pflag.Value interface.
Expand Down
16 changes: 16 additions & 0 deletions cmd/oras/root/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,22 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t
if err := option.Parse(cmd, &opts); err != nil {
return err
}

if opts.manifestConfigRef != "" && opts.artifactType == "" {
switch opts.Flag {
case option.ImageSpecAuto:
// switch to v1.0 manifest since artifact type is suggested by OCI v1.1
// artifact guidance but is not presented
// see https://github.com/opencontainers/image-spec/blob/e7f7c0ca69b21688c3cea7c87a04e4503e6099e2/manifest.md?plain=1#L170
opts.PackVersion = oras.PackManifestVersion1_0
case option.ImageSpecV1_1:
return &oerrors.Error{
Err: errors.New(`missing artifact type for OCI image-spec v1.1 artifacts`),
Recommendation: "set an artifact type via `--artifact-type` or consider image spec v1.0",
}
}
}

switch opts.PackVersion {
case oras.PackManifestVersion1_0:
if opts.manifestConfigRef != "" && opts.artifactType != "" {
Expand Down
33 changes: 33 additions & 0 deletions test/e2e/suite/command/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@ var _ = Describe("ORAS beginners:", func() {
MatchErrKeyWords("Error:", invalidFlag, "Available options: v1.1, v1.0").
Exec()
})

It("should fail if image spec v1.1 is used, with --config and without --artifactType", func() {
testRepo := pushTestRepo("v1-1/no-artifact-type")
subjectRef := RegistryRef(ZOTHost, testRepo, foobar.Tag)
imageSpecFlag := "v1.1"
ORAS("push", subjectRef, "--config", foobar.FileConfigName, Flags.ImageSpec, imageSpecFlag).
ExpectFailure().
MatchErrKeyWords("missing artifact type for OCI image-spec v1.1 artifacts").
Exec()
})
})
})

Expand Down Expand Up @@ -233,6 +243,29 @@ var _ = Describe("Remote registry users:", func() {
}))
})

It("should pack with image spec v1.0 when --config is used, --artifact-type is not used, and --image-spec set to auto", func() {
repo := pushTestRepo("config/without/artifact/type")
configType := "my/config/type"
tempDir := PrepareTempFiles()

ORAS("push", RegistryRef(ZOTHost, repo, tag), "--config", fmt.Sprintf("%s:%s", foobar.FileConfigName, configType), foobar.FileBarName, "-v").
MatchStatus([]match.StateKey{
{Digest: foobar.FileConfigStateKey.Digest, Name: configType},
foobar.FileBarStateKey,
}, true, 2).
WithWorkDir(tempDir).Exec()
// validate
fetched := ORAS("manifest", "fetch", RegistryRef(ZOTHost, repo, tag)).Exec().Out.Contents()
var manifest ocispec.Manifest
Expect(json.Unmarshal(fetched, &manifest)).ShouldNot(HaveOccurred())
Expect(manifest.Config).Should(Equal(ocispec.Descriptor{
MediaType: configType,
Size: int64(foobar.FileConfigSize),
Digest: foobar.FileConfigDigest,
}))
Expect(manifest.ArtifactType).Should(Equal(""))
})

It("should push files with customized config file and mediatype", func() {
repo := pushTestRepo("config/mediatype")
configType := "config/type"
Expand Down

0 comments on commit 2ef6dc0

Please sign in to comment.