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.

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

fix: oras push should return error when artifacttype is empty for spec 1.1 #1317

Merged
merged 3 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
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
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
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
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
case option.ImageSpecV1_1:
return &oerrors.Error{
Err: errors.New(`artifact type missing for OCI image-spec v1.1 artifacts`),
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
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("artifact type missing 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,
}))
qweeah marked this conversation as resolved.
Show resolved Hide resolved
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
Loading