feat: add manifest fetch-config #540

yuehaoliang · 2022-09-07T07:45:38Z

Resolve #131

Signed-off-by: Haoliang Yue [email protected]

Signed-off-by: Haoliang Yue <[email protected]>

codecov-commenter · 2022-09-07T07:47:18Z

Codecov Report

Merging #540 (eb49aba) into main (ba2385c) will increase coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #540      +/-   ##
==========================================
+ Coverage   72.44%   72.54%   +0.10%     
==========================================
  Files          13       14       +1     
  Lines         508      510       +2     
==========================================
+ Hits          368      370       +2     
  Misses        112      112              
  Partials       28       28

Impacted Files	Coverage Δ
internal/descriptor/descriptor.go	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

cmd/oras/manifest/fetchConfig.go

…aoliang/131

Signed-off-by: Haoliang Yue <[email protected]>

cmd/oras/internal/descriptor/descriptor.go

cmd/oras/manifest/fetchConfig.go

shizhMSFT · 2022-09-14T14:37:14Z

cmd/oras/manifest/fetchConfig.go

+	}
+
+	cmd.Flags().StringVarP(&opts.outputPath, "output", "o", "", "output file path")
+	cmd.Flags().StringVarP(&opts.mediaType, "media-type", "", "", "media type of the manifest config")


Can we remove this flag as it does not have a user scenario and it confuses users? /cc @FeynmanZhou @yizha1

Can we remove this flag as it does not have a user scenario and it confuses users? /cc @FeynmanZhou @yizha1

I was tried to support the same feature as oras pull --config, but if users only use this command for an image manifest, --media-type is not required, because we can always recognize the config for an image manifest.

Removed --media-type for now. I think if only image manifest type support this manifest fetch-config command, this argument is unnecessary.

shizhMSFT · 2022-09-14T14:40:30Z

cmd/oras/manifest/fetchConfig.go

+func fetchConfigDesc(ctx context.Context, src oras.ReadOnlyTarget, reference string, configMediaType string) (ocispec.Descriptor, error) {
+	// fetch manifest descriptor
+	manifestDesc, err := oras.Resolve(ctx, src, reference, oras.ResolveOptions{})
+	if err != nil {
+		return ocispec.Descriptor{}, err
+	}
+
+	// fetch config descriptor
+	successors, err := content.Successors(ctx, src, manifestDesc)
+	if err != nil {
+		return ocispec.Descriptor{}, err
+	}
+	for i, s := range successors {
+		if s.MediaType == configMediaType || (configMediaType == "" && i == 0 && descriptor.IsImageManifest(manifestDesc.MediaType)) {
+			return s, nil
+		}
+	}
+	return ocispec.Descriptor{}, fmt.Errorf("%s does not have a config", reference)
+}


Since we know it is an image manifest, we can call oras.FetchBytes() and then unmarshal it to extract the config descriptor. The simplified operation not only has less network calls but also cleaner.

Since we know it is an image manifest, we can call oras.FetchBytes() and then unmarshal it to extract the config descriptor. The simplified operation not only has less network calls but also cleaner.

Are we sure that manifest fetch-config is only used for image manifest? I see in oras pull --config, users can provided the media type of the config file. If the manifest is not an image manifest, the command will fetch the blob who has a matched media type.

Are we sure that manifest fetch-config is only used for image manifest?

Yes, manifest fetch-config should only run against image manifests.

oras manifest fetch-config should return error if the manifest is not an image manifest.

Resolved as required.

cmd/oras/manifest/fetchConfig.go

qweeah · 2022-09-16T01:57:25Z

@yuehaoliang-microsoft Can we have a UX described in the #131 or https://hackmd.io/aLxws7mhSZukfFzx3PKq3w

Signed-off-by: Haoliang Yue <[email protected]>

yuehaoliang · 2022-09-16T04:04:42Z

@yuehaoliang-microsoft Can we have a UX described in the #131 or https://hackmd.io/aLxws7mhSZukfFzx3PKq3w

I attached a few picture about UX in #131, and also updates the HackMD documentation.

cmd/oras/manifest/fetch_config.go

internal/file/file.go

Signed-off-by: Haoliang Yue <[email protected]>

cmd/oras/manifest/fetch_config.go

cmd/oras/pull.go

Signed-off-by: Haoliang Yue <[email protected]>

cmd/oras/manifest/fetch_config.go

shizhMSFT · 2022-09-16T14:34:30Z

cmd/oras/manifest/fetch_config.go

+
+** This command is in preview and under development. **
+
+Example - Fetch the config:


There is a scenario missing:

oras manifest fetch-config --platform 'linux/arm/v5' localhost:5000/hello:latest

Supported option.Platform.

Signed-off-by: Haoliang Yue <[email protected]>

shizhMSFT · 2022-09-19T04:57:03Z

cmd/oras/manifest/fetch_config.go

+	// fetch manifest descriptor and content
+	fetchOpts := oras.DefaultFetchBytesOptions
+	fetchOpts.TargetPlatform = targetPlatform
+	manifestDesc, manifestContent, err := oras.FetchBytes(ctx, src, reference, oras.DefaultFetchBytesOptions)


Suggested change

manifestDesc, manifestContent, err := oras.FetchBytes(ctx, src, reference, oras.DefaultFetchBytesOptions)

manifestDesc, manifestContent, err := oras.FetchBytes(ctx, src, reference, fetchOpts)

Signed-off-by: Haoliang Yue <[email protected]>

…aoliang/131

shizhMSFT

LGTM

Signed-off-by: Haoliang Yue <[email protected]>

feat: add manifest fetch-config

1bf788d

Signed-off-by: Haoliang Yue <[email protected]>

lizMSFT reviewed Sep 7, 2022

View reviewed changes

cmd/oras/manifest/fetchConfig.go Outdated Show resolved Hide resolved

lizMSFT reviewed Sep 7, 2022

View reviewed changes

cmd/oras/manifest/fetchConfig.go Outdated Show resolved Hide resolved

Merge branch 'main' of https://github.com/oras-project/oras into yueh…

443fb5c

…aoliang/131

shizhMSFT added this to the v0.15.0 milestone Sep 8, 2022

yuehaoliang added 3 commits September 8, 2022 15:13

refactor

e5e7326

Signed-off-by: Haoliang Yue <[email protected]>

support --output -

baf5928

Signed-off-by: Haoliang Yue <[email protected]>

add unit test

ec29d6b

Signed-off-by: Haoliang Yue <[email protected]>

shizhMSFT requested changes Sep 14, 2022

View reviewed changes

yuehaoliang added 2 commits September 16, 2022 10:11

pull from upstream and resolve conflict

895dd31

Signed-off-by: Haoliang Yue <[email protected]>

improve the fetchConfigDesc func

ebd7de8

Signed-off-by: Haoliang Yue <[email protected]>

qweeah requested changes Sep 16, 2022

View reviewed changes

cmd/oras/manifest/fetch_config.go Show resolved Hide resolved

cmd/oras/manifest/fetch_config.go Outdated Show resolved Hide resolved

cmd/oras/manifest/fetch_config.go Outdated Show resolved Hide resolved

cmd/oras/manifest/fetch_config.go Outdated Show resolved Hide resolved

shizhMSFT reviewed Sep 16, 2022

View reviewed changes

internal/file/file.go Outdated Show resolved Hide resolved

yuehaoliang added 3 commits September 16, 2022 17:22

resolve review comments

3bc296b

Signed-off-by: Haoliang Yue <[email protected]>

resolve conflict in pull

113d026

Signed-off-by: Haoliang Yue <[email protected]>

move IsImageManifest to descriptor package

d2e2519

Signed-off-by: Haoliang Yue <[email protected]>

shizhMSFT reviewed Sep 16, 2022

View reviewed changes

cmd/oras/manifest/fetch_config.go Outdated Show resolved Hide resolved

shizhMSFT reviewed Sep 16, 2022

View reviewed changes

cmd/oras/pull.go Outdated Show resolved Hide resolved

modify IsImageManifest to take a descriptor as an input

1e9370c

Signed-off-by: Haoliang Yue <[email protected]>

shizhMSFT requested changes Sep 16, 2022

View reviewed changes

add platform option

66229b4

Signed-off-by: Haoliang Yue <[email protected]>

shizhMSFT requested changes Sep 19, 2022

View reviewed changes

yuehaoliang added 2 commits September 19, 2022 13:12

fix a bug for used platform variable

98e2ee2

Signed-off-by: Haoliang Yue <[email protected]>

Merge branch 'main' of https://github.com/oras-project/oras into yueh…

eb49aba

…aoliang/131

shizhMSFT approved these changes Sep 19, 2022

View reviewed changes

shizhMSFT merged commit 1075b3b into oras-project:main Sep 19, 2022

TerryHowe pushed a commit to TerryHowe/oras that referenced this pull request Feb 2, 2023

feat: add manifest fetch-config (oras-project#540)

9448bbc

Signed-off-by: Haoliang Yue <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add manifest fetch-config #540

feat: add manifest fetch-config #540

yuehaoliang commented Sep 7, 2022

codecov-commenter commented Sep 7, 2022 •

edited

Loading

shizhMSFT Sep 14, 2022

yuehaoliang Sep 15, 2022 •

edited

Loading

yuehaoliang Sep 16, 2022

shizhMSFT Sep 14, 2022

yuehaoliang Sep 15, 2022 •

edited

Loading

shizhMSFT Sep 15, 2022 •

edited

Loading

shizhMSFT Sep 15, 2022

yuehaoliang Sep 16, 2022

qweeah commented Sep 16, 2022

yuehaoliang commented Sep 16, 2022

shizhMSFT Sep 16, 2022

yuehaoliang Sep 18, 2022

shizhMSFT Sep 19, 2022

shizhMSFT left a comment


		This command is in preview and under development.

		Example - Fetch the config:

	manifestDesc, manifestContent, err := oras.FetchBytes(ctx, src, reference, oras.DefaultFetchBytesOptions)
	manifestDesc, manifestContent, err := oras.FetchBytes(ctx, src, reference, fetchOpts)

feat: add manifest fetch-config #540

feat: add manifest fetch-config #540

Conversation

yuehaoliang commented Sep 7, 2022

codecov-commenter commented Sep 7, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

yuehaoliang Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuehaoliang Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

shizhMSFT Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qweeah commented Sep 16, 2022

yuehaoliang commented Sep 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 7, 2022 •

edited

Loading

yuehaoliang Sep 15, 2022 •

edited

Loading

yuehaoliang Sep 15, 2022 •

edited

Loading

shizhMSFT Sep 15, 2022 •

edited

Loading