feat: add platform option to push command #1500

TerryHowe · 2024-09-16T17:21:24Z

What this PR does / why we need it:

Add the platform option to the push command for example:

oras push --plain-http localhost:15000/oras:darwin,arm64  --artifact-platform darwin/arm64 --artifact-type 'application/vnd.oci.image.config.v1+json'  bin/darwin/arm64/oras:application/x-elf

Create the manifest for example:

oras manifest index create localhost:15000/oras:v1 sha256:ed3691788db69623790c46256dd11d8d6edc31be6e4762b53c74dfc3bb6792cf

The results

% oras manifest fetch --pretty localhost:15000/oras@sha256:c9e54542075ad502900d11b9b5d5bdb4b974ad580842515684d3fbb4849e2bb1
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "artifactType": "application/vnd.oci.image.config.v1+json",
  "config": {
    "mediaType": "application/vnd.unknown.config.v1+json",
    "digest": "sha256:e9302bbb2fb8f6c2df866d3c4e41917849442f89a575f36f43366a7279804f70",
    "size": 38
  },
  "layers": [
    {
      "mediaType": "application/x-elf",
      "digest": "sha256:69c16b8386b7949fe37ff2356296b6331c1e6d5fbbc3e616192cc10ece55ea8d",
      "size": 11673840,
      "annotations": {
        "org.opencontainers.image.title": "bin/darwin/amd64/oras"
      }
    }
  ],
  "annotations": {
    "org.opencontainers.image.created": "2024-09-16T16:54:48Z"
  }
}
% oras manifest fetch-config --pretty localhost:15000/oras@sha256:c9e54542075ad502900d11b9b5d5bdb4b974ad580842515684d3fbb4849e2bb1
{
  "architecture": "amd64",
  "os": "darwin"
}

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Closes #1066
Partial #1053

Please check the following list:

Does the affected code have corresponding tests, e.g. unit test, E2E test?
Does this change require a documentation update?
Does this introduce breaking changes that would require an announcement or bumping the major version?
Do all new files have an appropriate license header?

codecov · 2024-09-16T17:23:49Z

Codecov Report

Attention: Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.08%. Comparing base (8dc05a7) to head (a8a6a36).

Files with missing lines	Patch %	Lines
cmd/oras/root/push.go	78.94%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1500      +/-   ##
==========================================
- Coverage   86.10%   86.08%   -0.02%     
==========================================
  Files         118      118              
  Lines        4224     4248      +24     
==========================================
+ Hits         3637     3657      +20     
- Misses        350      352       +2     
- Partials      237      239       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

qweeah · 2024-09-18T00:49:21Z

cmd/oras/root/push.go

@@ -97,6 +101,9 @@ Example - Push repository with manifest annotations:
 Example - Push repository with manifest annotation file:
  oras push --annotation-file annotation.json localhost:5000/hello:v1

+Example - Push repository with platform:
+  oras push --platform linux/arm/v5 localhost:5000/hello:v1


CC @FeynmanZhou for the flag naming. attach already using it so better call the new one --config-platform

It is a good point that --platform means differently in oras push and oras attach. We need a better flag name.

--config-platform does not sound like a good name. Would it be better to name it --artifact-platform? That will be consistent with --artifact-type.

This does mean pull will use --platform and push --artifact-platform though which does seem a bit inconsistent to me.

It's hard to choose a good flag name.

Some historical view on this: --platform was firstly introduced to oras manifest fetch to select an image with specific platform from a multi-arch image. Then the same UX was applied to oras discover and oras attach for selecting a platform-specific subject artifact.

cmd/oras/root/push.go

wangxiaoxuan273 · 2024-09-18T02:04:56Z

cmd/oras/root/push.go

+		desc := ocispec.Descriptor{
+			MediaType: oras.MediaTypeUnknownConfig,
+			Digest:    digest.FromBytes(blob),
+			Size:      int64(len(blob)),
+		}


nit: This part can be replaced by func NewDescriptorFromBytes(mediaType string, content []byte) ocispec.Descriptor of oras-go. Reference: NewDescriptorFromBytes

shizhMSFT · 2024-09-18T07:28:06Z

cmd/oras/root/push.go

@@ -97,6 +101,9 @@ Example - Push repository with manifest annotations:
 Example - Push repository with manifest annotation file:
  oras push --annotation-file annotation.json localhost:5000/hello:v1

+Example - Push repository with platform:
+  oras push --platform linux/arm/v5 localhost:5000/hello:v1


It is a good point that --platform means differently in oras push and oras attach. We need a better flag name.

shizhMSFT · 2024-09-18T07:29:05Z

cmd/oras/root/push.go

@@ -97,6 +101,9 @@ Example - Push repository with manifest annotations:
 Example - Push repository with manifest annotation file:
  oras push --annotation-file annotation.json localhost:5000/hello:v1

+Example - Push repository with platform:
+  oras push --platform linux/arm/v5 localhost:5000/hello:v1


--config-platform does not sound like a good name. Would it be better to name it --artifact-platform? That will be consistent with --artifact-type.

shizhMSFT · 2024-09-18T07:32:14Z

cmd/oras/root/push.go

+				if opts.Platform.Platform != nil && opts.artifactType != "" {
+					return errors.New("--artifact-type and --platform cannot both be provided for 1.0 OCI image")
+				}


They can be used together, can't they?

Indeed they can be used together. We can populate the artifactType as mediaType into the config descriptor.

shizhMSFT · 2024-09-18T07:33:36Z

cmd/oras/root/push.go

+			return err
+		}
+		desc := ocispec.Descriptor{
+			MediaType: oras.MediaTypeUnknownConfig,


This value depends on the image-spec version.

If the image-spec version is v1.0. The media type can also be the value of --artifact-type.

Wwwsylvia

LGTM with nit, but I am not a maintainer.

Wwwsylvia · 2024-09-25T06:58:18Z

cmd/oras/root/push.go

+		if opts.Flag == option.ImageSpecV1_0 {
+			if opts.artifactType != "" {


nit: how about

Suggested change

if opts.Flag == option.ImageSpecV1_0 {

if opts.artifactType != "" {

if opts.Flag == option.ImageSpecV1_0 && opts.artifactType != "" {

sabre1041

LGTM

qweeah · 2024-09-29T07:40:42Z

cmd/oras/root/push.go

+	configAndPlatform := []string{"config", "artifact-platform"}
+	if err := oerrors.CheckMutuallyExclusiveFlags(cmd.Flags(), configAndPlatform...); err != nil {
+		return err
+	}


This can be done in PreRunE before option.Parse

qweeah · 2024-09-29T07:49:17Z

cmd/oras/root/push.go

@@ -116,7 +122,7 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t
 				return err
 			}

-			if opts.manifestConfigRef != "" && opts.artifactType == "" {
+			if (opts.manifestConfigRef != "" || opts.Platform.Platform != nil) && opts.artifactType == "" {


This block handles oras push --config xxx ... without using --artifact-type xxx:

For v1.1 artifacts, this usage invalid.

Otherwise give best-effort try to pack v1.0 artifacts.

Since --artifact-platform cannot be used with --config, there is no need to change this code block.

Only need to make sure: --artifact-platform is not used for v1.0 artifact in the below opts.PackVersion switching block.

Made the change I think you requested. I do need to think about this some more though.

cmd/oras/internal/option/platform.go

Signed-off-by: Terry Howe <terrylhowe@gmail.com>

TerryHowe requested review from sajayantony, shizhMSFT, SteveLasker and qweeah as code owners September 16, 2024 17:21

TerryHowe force-pushed the feature-push-platform branch 4 times, most recently from 641cbba to 7e915b1 Compare September 17, 2024 13:56

qweeah reviewed Sep 18, 2024

View reviewed changes

cmd/oras/root/push.go Outdated Show resolved Hide resolved

qweeah reviewed Sep 18, 2024

View reviewed changes

cmd/oras/root/push.go Outdated Show resolved Hide resolved

shizhMSFT changed the title ~~feature: add platform option to push command~~ feat: add platform option to push command Sep 18, 2024

shizhMSFT mentioned this pull request Sep 18, 2024

feat: add how-to for multiple architecture oras-project/oras-www#395

Open

wangxiaoxuan273 reviewed Sep 18, 2024

View reviewed changes

shizhMSFT reviewed Sep 18, 2024

View reviewed changes

TerryHowe force-pushed the feature-push-platform branch 6 times, most recently from 3038b7b to fafd301 Compare September 23, 2024 14:28

Wwwsylvia approved these changes Sep 25, 2024

View reviewed changes

TerryHowe force-pushed the feature-push-platform branch from fafd301 to a8a6a36 Compare September 25, 2024 14:38

sabre1041 approved these changes Sep 26, 2024

View reviewed changes

qweeah requested changes Sep 29, 2024

View reviewed changes

TerryHowe force-pushed the feature-push-platform branch from bb6cfaa to 8d4062e Compare September 30, 2024 15:25

feature: add platform option to push command

35fe5b7

Signed-off-by: Terry Howe <terrylhowe@gmail.com>

TerryHowe force-pushed the feature-push-platform branch from 8d4062e to 35fe5b7 Compare September 30, 2024 16:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add platform option to push command #1500

feat: add platform option to push command #1500

TerryHowe commented Sep 16, 2024 •

edited

Loading

codecov bot commented Sep 16, 2024 •

edited

Loading

qweeah Sep 18, 2024

shizhMSFT Sep 18, 2024

shizhMSFT Sep 18, 2024

TerryHowe Sep 22, 2024

TerryHowe Sep 28, 2024

qweeah Sep 29, 2024 •

edited

Loading

wangxiaoxuan273 Sep 18, 2024

TerryHowe Sep 24, 2024

shizhMSFT Sep 18, 2024

shizhMSFT Sep 18, 2024

shizhMSFT Sep 18, 2024

Wwwsylvia Sep 18, 2024

TerryHowe Sep 22, 2024

shizhMSFT Sep 18, 2024

Wwwsylvia Sep 18, 2024

TerryHowe Sep 22, 2024

Wwwsylvia left a comment

Wwwsylvia Sep 25, 2024

TerryHowe Sep 25, 2024

sabre1041 left a comment

qweeah Sep 29, 2024

TerryHowe Sep 30, 2024

qweeah Sep 29, 2024 •

edited

Loading

qweeah Sep 29, 2024

TerryHowe Sep 30, 2024

		if opts.Flag == option.ImageSpecV1_0 {
		if opts.artifactType != "" {

	if opts.Flag == option.ImageSpecV1_0 {
	if opts.artifactType != "" {
	if opts.Flag == option.ImageSpecV1_0 && opts.artifactType != "" {

feat: add platform option to push command #1500

Are you sure you want to change the base?

feat: add platform option to push command #1500

Conversation

TerryHowe commented Sep 16, 2024 • edited Loading

codecov bot commented Sep 16, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qweeah Sep 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wwwsylvia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabre1041 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qweeah Sep 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TerryHowe commented Sep 16, 2024 •

edited

Loading

codecov bot commented Sep 16, 2024 •

edited

Loading

qweeah Sep 29, 2024 •

edited

Loading

qweeah Sep 29, 2024 •

edited

Loading