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

fix: added warning for dangling referrers index deletion #619

Merged
merged 4 commits into from
Apr 13, 2023
Merged

fix: added warning for dangling referrers index deletion #619

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1e7380a
added warning for dangling referrers index deletion
Two-Hearts Apr 6, 2023
4ae0f85
update
Two-Hearts Apr 6, 2023
ee4cb98
updated per code review
Two-Hearts Apr 7, 2023
4f7a677
update
Two-Hearts Apr 10, 2023
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
15 changes: 13 additions & 2 deletions cmd/notation/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"os"
"strings"
"time"

"github.com/notaryproject/notation-go"
Expand All @@ -22,6 +23,8 @@ const (
signatureManifestImage = "image"
)

const referrersTagSchemaDeleteError = "failed to delete dangling referrers index"
Copy link
Contributor

Choose a reason for hiding this comment

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

@shizhMSFT @Wwwsylvia Since this error is generated based on oras-go-specific procedure, should oras-go export this error so caller can implement upon?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's beyond this PR's scope, I created an issue for tracking, let's move discussion there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we need to think about how to improve oras-go for such use case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Once oras-go is updated and have a new release, we can update here. But for now, let's do a string match.


var supportedSignatureManifest = []string{signatureManifestArtifact, signatureManifestImage}

type signOpts struct {
Expand Down Expand Up @@ -112,8 +115,16 @@ func runSign(command *cobra.Command, cmdOpts *signOpts) error {
_, err = notation.Sign(ctx, signer, sigRepo, opts)
if err != nil {
var errorPushSignatureFailed notation.ErrorPushSignatureFailed
if errors.As(err, &errorPushSignatureFailed) && !ociImageManifest {
return fmt.Errorf("%v. Possible reason: target registry does not support OCI artifact manifest. Try removing the flag `--signature-manifest artifact` to store signatures using OCI image manifest", err)
if errors.As(err, &errorPushSignatureFailed) {
if !ociImageManifest {
return fmt.Errorf("%v. Possible reason: target registry does not support OCI artifact manifest. Try removing the flag `--signature-manifest artifact` to store signatures using OCI image manifest", err)
}
if strings.Contains(err.Error(), referrersTagSchemaDeleteError) {
fmt.Fprintln(os.Stderr, "Warning: Removal of outdated referrers index is not supported by the remote registry. Garbage collection may be required.")
// write out
fmt.Println("Successfully signed", ref)
return nil
}
}
return err
}
Expand Down