From bc4ef3db6b9b5395612f5418c1f435ed0b1e7732 Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Mon, 15 May 2023 10:59:48 +0300 Subject: [PATCH] fix: harbor regular NOT_FOUND error treated as 'broken image' internal registry error Signed-off-by: Timofey Kirillov --- pkg/docker_registry/api.go | 24 +++++++++------ pkg/docker_registry/default.go | 21 +------------ pkg/docker_registry/errors.go | 48 ++++++++++++++++++++++++++++++ pkg/docker_registry/generic_api.go | 3 +- pkg/storage/repo_stages_storage.go | 7 ++--- 5 files changed, 67 insertions(+), 36 deletions(-) create mode 100644 pkg/docker_registry/errors.go diff --git a/pkg/docker_registry/api.go b/pkg/docker_registry/api.go index a7329ee95b..ebcde05a07 100644 --- a/pkg/docker_registry/api.go +++ b/pkg/docker_registry/api.go @@ -88,27 +88,33 @@ func (api *api) IsRepoImageExists(ctx context.Context, reference string) (bool, } } -func (api *api) TryGetRepoImage(ctx context.Context, reference string) (*image.Info, error) { +func (api *api) tryGetRepoImage(ctx context.Context, reference, implementation string) (*image.Info, error) { if imgInfo, err := api.GetRepoImage(ctx, reference); err != nil { - if IsStatusNotFoundErr(err) || IsQuayTagExpiredErr(err) { - // TODO: 1. auto reject images with manifest-unknown or blob-unknown errors - // TODO: 2. why TryGetRepoImage for rejected image records gives manifest-unknown errors? - // TODO: 3. make sure werf never ever creates rejected image records for name-unknown errors. - // TODO: 4. werf-cleanup should remove broken images - + if implementation != "" { + if IsQuayTagExpiredErr(err) && implementation != QuayImplementationName { + logboek.Context(ctx).Error().LogF("WARNING: Detected error specific for quay container registry implementation!\n") + logboek.Context(ctx).Error().LogF("WARNING: Use --repo-container-registry=quay option (or WERF_CONTAINER_REGISTRY env var)\n") + logboek.Context(ctx).Error().LogF("WARNING: to instruct werf to use quay driver.\n") + } + } + if IsImageNotFoundError(err) || IsBrokenImageError(err) { + // TODO: 1. make sure werf never ever creates rejected image records for name-unknown errors. + // TODO: 2. werf-cleanup should remove broken images if os.Getenv("WERF_DOCKER_REGISTRY_DEBUG") == "1" { logboek.Context(ctx).Error().LogF("WARNING: Got an error when inspecting repo image %q: %s\n", reference, err) } - return nil, nil } - return imgInfo, err } else { return imgInfo, nil } } +func (api *api) TryGetRepoImage(ctx context.Context, reference string) (*image.Info, error) { + return api.tryGetRepoImage(ctx, reference, "") +} + func (api *api) GetRepoImage(ctx context.Context, reference string) (*image.Info, error) { desc, _, err := api.getImageDesc(reference) if err != nil { diff --git a/pkg/docker_registry/default.go b/pkg/docker_registry/default.go index 533532792f..542c398f21 100644 --- a/pkg/docker_registry/default.go +++ b/pkg/docker_registry/default.go @@ -51,18 +51,7 @@ func (r *defaultImplementation) IsTagExist(_ context.Context, _ string, _ ...Opt } func (r *defaultImplementation) TryGetRepoImage(ctx context.Context, reference string) (*image.Info, error) { - info, err := r.api.TryGetRepoImage(ctx, reference) - if err != nil { - if IsQuayTagExpiredErr(err) && r.Implementation != QuayImplementationName { - logboek.Context(ctx).Error().LogF("WARNING: Detected error specific for quay container registry implementation!\n") - logboek.Context(ctx).Error().LogF("WARNING: Use --repo-container-registry=quay option (or WERF_CONTAINER_REGISTRY env var)\n") - logboek.Context(ctx).Error().LogF("WARNING: to instruct werf to use quay driver.\n") - } - - return nil, err - } - - return info, nil + return r.tryGetRepoImage(ctx, reference, r.Implementation) } func (r *defaultImplementation) CreateRepo(_ context.Context, _ string) error { @@ -84,11 +73,3 @@ func (r *defaultImplementation) DeleteRepoImage(ctx context.Context, repoImage * func (r *defaultImplementation) String() string { return DefaultImplementationName } - -func IsManifestUnknownError(err error) bool { - return (err != nil) && strings.Contains(err.Error(), "MANIFEST_UNKNOWN") -} - -func IsNameUnknownError(err error) bool { - return (err != nil) && strings.Contains(err.Error(), "NAME_UNKNOWN") -} diff --git a/pkg/docker_registry/errors.go b/pkg/docker_registry/errors.go new file mode 100644 index 0000000000..1526911828 --- /dev/null +++ b/pkg/docker_registry/errors.go @@ -0,0 +1,48 @@ +package docker_registry + +import ( + "strings" + + "github.com/google/go-containerregistry/pkg/v1/remote/transport" +) + +var ( + BrokenImageCodes = []transport.ErrorCode{ + transport.BlobUnknownErrorCode, + transport.BlobUploadInvalidErrorCode, + transport.BlobUploadUnknownErrorCode, + transport.DigestInvalidErrorCode, + transport.ManifestBlobUnknownErrorCode, + transport.ManifestInvalidErrorCode, + transport.ManifestUnverifiedErrorCode, + transport.NameInvalidErrorCode, + } + + NotFoundImageCodes = []transport.ErrorCode{ + transport.ManifestUnknownErrorCode, + transport.NameUnknownErrorCode, + } +) + +func isErrorMatched(err error, codes []transport.ErrorCode) bool { + if err != nil { + for _, code := range codes { + if strings.Contains(err.Error(), string(code)) { + return true + } + } + } + return false +} + +func IsBrokenImageError(err error) bool { + return isErrorMatched(err, BrokenImageCodes) || IsQuayTagExpiredErr(err) +} + +func IsImageNotFoundError(err error) bool { + return isErrorMatched(err, NotFoundImageCodes) || IsHarborNotFoundError(err) +} + +func IsHarborNotFoundError(err error) bool { + return (err != nil) && strings.Contains(err.Error(), "NOT_FOUND") +} diff --git a/pkg/docker_registry/generic_api.go b/pkg/docker_registry/generic_api.go index 2039302fdf..352def090a 100644 --- a/pkg/docker_registry/generic_api.go +++ b/pkg/docker_registry/generic_api.go @@ -38,7 +38,7 @@ func (api *genericApi) GetRepoImageConfigFile(ctx context.Context, reference str for _, mirrorReference := range mirrorReferenceList { config, err := api.getRepoImageConfigFile(ctx, mirrorReference) if err != nil { - if IsStatusNotFoundErr(err) || IsQuayTagExpiredErr(err) { + if IsStatusNotFoundErr(err) || IsImageNotFoundError(err) || IsBrokenImageError(err) { continue } @@ -76,7 +76,6 @@ func (api *genericApi) GetRepoImage(ctx context.Context, reference string) (*ima if err != nil { return nil, fmt.Errorf("unable to try getting mirror repo image %q: %w", mirrorReference, err) } - if info != nil { return info, nil } diff --git a/pkg/storage/repo_stages_storage.go b/pkg/storage/repo_stages_storage.go index d30deb4b94..a69f402210 100644 --- a/pkg/storage/repo_stages_storage.go +++ b/pkg/storage/repo_stages_storage.go @@ -269,15 +269,12 @@ func (storage *RepoStagesStorage) GetStageDescription(ctx context.Context, proje logboek.Context(ctx).Debug().LogF("-- RepoStagesStorage stageImageName = %q\n", stageImageName) imgInfo, err := storage.DockerRegistry.GetRepoImage(ctx, stageImageName) - - if docker_registry.IsNameUnknownError(err) || docker_registry.IsManifestUnknownError(err) { + if docker_registry.IsImageNotFoundError(err) { return nil, nil } - - if docker_registry.IsStatusNotFoundErr(err) || docker_registry.IsQuayTagExpiredErr(err) { + if docker_registry.IsBrokenImageError(err) { return nil, ErrBrokenImage } - if err != nil { return nil, fmt.Errorf("unable to inspect repo image %s: %w", stageImageName, err) }