Skip to content

Commit ab216a9

Browse files
committed
detect if image exists or access is denied
This also adds e2e tests for edge-cases that may have been missed as well as a test-case that exposes the clusters' internal image registry (if it exists) and validates the error handling functions against it. The exposure functions were repackaged from the devex helpers for easy reuse across both contexts.
1 parent cc2f713 commit ab216a9

File tree

9 files changed

+988
-274
lines changed

9 files changed

+988
-274
lines changed

devex/cmd/mco-builder/local.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/openshift/machine-config-operator/devex/internal/pkg/utils"
1616
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
1717
"github.com/openshift/machine-config-operator/test/framework"
18+
"github.com/openshift/machine-config-operator/test/helpers"
1819
"github.com/spf13/cobra"
1920
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2021
aggerrs "k8s.io/apimachinery/pkg/util/errors"
@@ -192,7 +193,7 @@ func buildLocallyAndPushIntoCluster(cs *framework.ClientSet, buildOpts localBuil
192193
}
193194
}()
194195

195-
extHostname, err := rollout.ExposeClusterImageRegistry(cs)
196+
extHostname, err := helpers.ExposeClusterImageRegistry(context.TODO(), cs)
196197
if err != nil {
197198
return err
198199
}

devex/cmd/mco-builder/revert.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/openshift/machine-config-operator/devex/internal/pkg/rollout"
88
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
99
"github.com/openshift/machine-config-operator/test/framework"
10+
"github.com/openshift/machine-config-operator/test/helpers"
1011
"github.com/spf13/cobra"
1112
apierrs "k8s.io/apimachinery/pkg/api/errors"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -34,7 +35,7 @@ func doRevert(_ *cobra.Command, _ []string) error {
3435
return fmt.Errorf("could not revert to original MCO image: %w", err)
3536
}
3637

37-
if err := rollout.UnexposeClusterImageRegistry(cs); err != nil {
38+
if err := helpers.UnexposeClusterImageRegistry(context.TODO(), cs); err != nil {
3839
return fmt.Errorf("could not unexpose cluster image registry: %w", err)
3940
}
4041

devex/internal/pkg/rollout/external.go

Lines changed: 0 additions & 125 deletions
This file was deleted.

pkg/controller/build/imagepruner/errors.go

Lines changed: 121 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88

99
"github.com/containers/image/v5/docker"
1010
"github.com/docker/distribution/registry/api/errcode"
11-
errcodev2 "github.com/docker/distribution/registry/api/v2"
11+
v2 "github.com/docker/distribution/registry/api/v2"
1212
)
1313

1414
// IsTolerableDeleteErr determines if the returned error message during image deletion can be
@@ -24,23 +24,59 @@ func IsTolerableDeleteErr(err error) bool {
2424
return false
2525
}
2626

27-
// Any errors related to the actual image registry query are wrapped in an
28-
// ErrImage instance. This allows us to easily identify intolerable errors
29-
// such as not being able to write the authfile or certs, etc.
30-
var errImage *ErrImage
31-
if !errors.As(err, &errImage) {
27+
if IsImageNotFoundErr(err) {
28+
return true
29+
}
30+
31+
if IsAccessDeniedErr(err) {
32+
return true
33+
}
34+
35+
return false
36+
}
37+
38+
// IsImageNotFoundErr determines if the returned error message indicates that
39+
// the image is not found. This assumes that the image registry returns such an
40+
// error code, which is not always the case. Some image registries return an
41+
// unauthorized or forbidden error which this function does not take into
42+
// account. For that, use IsAccessDeniedErr below as an additional check.
43+
func IsImageNotFoundErr(err error) bool {
44+
if err == nil {
3245
return false
3346
}
3447

35-
if isTolerableErrorCode(err) {
48+
if !isErrImage(err) {
49+
return false
50+
}
51+
52+
if isMaskedHTTP404(err) {
3653
return true
3754
}
3855

39-
if isTolerableUnexpectedHTTPStatusError(err) {
56+
if isImageNotFoundErrorCode(err) {
4057
return true
4158
}
4259

43-
if isMaskedHTTP404(err) {
60+
return false
61+
}
62+
63+
// IsAccessDeniedErr determines if the returned error indicates that the image
64+
// cannot be accessed or the operation cannot be performed due to permissions
65+
// issue. Some image registries use this as a proxy for the image not existing.
66+
func IsAccessDeniedErr(err error) bool {
67+
if err == nil {
68+
return false
69+
}
70+
71+
if !isErrImage(err) {
72+
return false
73+
}
74+
75+
if isAccessDeniedErrorCode(err) {
76+
return true
77+
}
78+
79+
if isTolerableUnexpectedHTTPStatusError(err) {
4480
return true
4581
}
4682

@@ -59,37 +95,80 @@ func isMaskedHTTP404(err error) bool {
5995
return strings.Contains(err.Error(), "Image may not exist or is not stored with a v2 Schema in a v2 registry")
6096
}
6197

62-
// isTolerableErrorCode checks if the error code from the registry is tolerable for deletion.
63-
// This includes cases where the manifest is unknown, or authorization is denied.
64-
func isTolerableErrorCode(err error) bool {
98+
// isImageNotFoundErrorCode checks if the error is an ErrorCode instance
99+
// indicating that a given manifest is not found or the repo name is unknown.
100+
// This also handles the Quay.io edgecase of an image being deleted and Quay
101+
// returning an HTTP 500 indicating that.
102+
func isImageNotFoundErrorCode(err error) bool {
65103
if err == nil {
66104
return false
67105
}
68106

107+
if isManifestUnknownError(err) {
108+
return true
109+
}
110+
111+
if isNameUnknownError(err) {
112+
return true
113+
}
114+
69115
var errCode errcode.Error
70-
if !errors.As(err, &errCode) {
71-
return false
116+
if errors.As(err, &errCode) {
117+
return isQuayErrorCode(errCode)
72118
}
73119

74-
code := errCode.ErrorCode()
120+
return false
121+
}
75122

76-
if code == errcodev2.ErrorCodeManifestUnknown {
123+
// Determines if the error is due to the repo name being unknown.
124+
func isNameUnknownError(err error) bool {
125+
var ec errcode.ErrorCoder
126+
if errors.As(err, &ec) && ec.ErrorCode() == v2.ErrorCodeNameUnknown {
77127
return true
78128
}
79129

80-
// Quay.io returns this code whenever one is not authorized to delete an image.
81-
if code == errcode.ErrorCodeUnauthorized {
130+
return false
131+
}
132+
133+
// Adapted from: https://github.com/containers/image/blob/52ee4dff559a09ffa45783c50bcb7b3f7faebb04/docker/docker_client.go#L1109-L1133
134+
func isManifestUnknownError(err error) bool {
135+
// docker/distribution, and as defined in the spec
136+
var ec errcode.ErrorCoder
137+
if errors.As(err, &ec) && ec.ErrorCode() == v2.ErrorCodeManifestUnknown {
138+
return true
139+
}
140+
// registry.redhat.io as of October 2022
141+
var e errcode.Error
142+
if errors.As(err, &e) && e.ErrorCode() == errcode.ErrorCodeUnknown && e.Message == "Not Found" {
143+
return true
144+
}
145+
// Harbor v2.10.2
146+
if errors.As(err, &e) && e.ErrorCode() == errcode.ErrorCodeUnknown && strings.Contains(strings.ToLower(e.Message), "not found") {
147+
return true
148+
}
149+
// registry.access.redhat.com as of August 2025
150+
if errors.As(err, &e) && e.ErrorCode() == v2.ErrorCodeNameUnknown {
82151
return true
83152
}
84153

85-
// Docker.io returns this code whenever one is not authorized to delete an image.
86-
if code == errcode.ErrorCodeDenied {
154+
return false
155+
}
156+
157+
// isAccessDeniedErrorCode checks if the error is an ErrorCode instance and
158+
// then checks the known status codes.
159+
func isAccessDeniedErrorCode(err error) bool {
160+
if err == nil {
161+
return false
162+
}
163+
164+
var ec errcode.ErrorCoder
165+
// Quay.io returns this code whenever one is not authorized to delete an image.
166+
if errors.As(err, &ec) && ec.ErrorCode() == errcode.ErrorCodeUnauthorized {
87167
return true
88168
}
89169

90-
// Quay.io returns an HTTP 500 if an image was recently deleted and the
91-
// garbage collection has not run yet.
92-
if isQuayErrorCode(errCode) {
170+
// Docker.io returns this code whenever one is not authorized to delete an image.
171+
if errors.As(err, &ec) && ec.ErrorCode() == errcode.ErrorCodeDenied {
93172
return true
94173
}
95174

@@ -128,15 +207,16 @@ func isTolerableUnexpectedHTTPStatusError(err error) bool {
128207
}
129208

130209
var unexpectedHTTPErr docker.UnexpectedHTTPStatusError
131-
if !errors.As(err, &unexpectedHTTPErr) {
132-
return false
210+
if errors.As(err, &unexpectedHTTPErr) && unexpectedHTTPErr.StatusCode == http.StatusUnauthorized {
211+
return true
133212
}
134213

135-
if unexpectedHTTPErr.StatusCode == http.StatusUnauthorized {
214+
if errors.As(err, &unexpectedHTTPErr) && unexpectedHTTPErr.StatusCode == http.StatusForbidden {
136215
return true
137216
}
138217

139-
if unexpectedHTTPErr.StatusCode == http.StatusForbidden {
218+
var unauthedForCreds docker.ErrUnauthorizedForCredentials
219+
if errors.As(err, &unauthedForCreds) {
140220
return true
141221
}
142222

@@ -189,3 +269,17 @@ func (e *ErrImage) Error() string {
189269
func (e *ErrImage) Unwrap() error {
190270
return e.err
191271
}
272+
273+
// isErrImage determines whether the given error is an instance of the ErrImage
274+
// type defined above.
275+
func isErrImage(err error) bool {
276+
if err == nil {
277+
return false
278+
}
279+
280+
// Any errors related to the actual image registry query are wrapped in an
281+
// ErrImage instance. This allows us to easily identify intolerable errors
282+
// such as not being able to write the authfile or certs, etc.
283+
var errImage *ErrImage
284+
return errors.As(err, &errImage)
285+
}

0 commit comments

Comments
 (0)